balazske updated this revision to Diff 473659.
balazske added a comment.
Added more functions to StdLibraryFunctionsChecker.
This is redundant with the StreamChecker for now but
makes possible to remove the NULL stream check from
StreamChecker and reduce overlap of the checkers.
This way no dependency should be needed between these checkers.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135360/new/
https://reviews.llvm.org/D135360
Files:
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-errno-note.c
clang/test/Analysis/stream-errno.c
clang/test/Analysis/stream-noopen.c
Index: clang/test/Analysis/stream-noopen.c
===================================================================
--- clang/test/Analysis/stream-noopen.c
+++ clang/test/Analysis/stream-noopen.c
@@ -77,6 +77,49 @@
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
}
+void check_fgetpos(FILE *F) {
+ errno = 0;
+ fpos_t Pos;
+ int Ret = fgetpos(F, &Pos);
+ if (Ret)
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ else
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+ if (errno) {} // no-warning
+ clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_fsetpos(FILE *F) {
+ errno = 0;
+ fpos_t Pos;
+ int Ret = fsetpos(F, &Pos);
+ if (Ret)
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ else
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+ if (errno) {} // no-warning
+ clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_ftell(FILE *F) {
+ errno = 0;
+ long Ret = ftell(F);
+ if (Ret == -1) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ } else {
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+ clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+ }
+ if (errno) {} // no-warning
+ clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
void freadwrite_zerosize(FILE *F) {
fwrite(WBuf, 1, 0, F);
clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
Index: clang/test/Analysis/stream-errno.c
===================================================================
--- clang/test/Analysis/stream-errno.c
+++ clang/test/Analysis/stream-errno.c
@@ -123,6 +123,64 @@
fclose(F);
}
+void check_fgetpos(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ errno = 0;
+ fpos_t Pos;
+ int Ret = fgetpos(F, &Pos);
+ if (Ret)
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ else
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ if (errno) {} // no-warning
+ fclose(F);
+}
+
+void check_fsetpos(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ errno = 0;
+ fpos_t Pos;
+ int Ret = fsetpos(F, &Pos);
+ if (Ret)
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ else
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ if (errno) {} // no-warning
+ fclose(F);
+}
+
+void check_ftell(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ errno = 0;
+ long Ret = ftell(F);
+ if (Ret == -1) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ } else {
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+ }
+ if (errno) {} // no-warning
+ fclose(F);
+}
+
+void check_rewind(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ errno = 0;
+ rewind(F);
+ clang_analyzer_eval(errno == 0);
+ // expected-warning@-1{{FALSE}}
+ // expected-warning@-2{{TRUE}}
+ fclose(F);
+}
+
void check_fileno(void) {
FILE *F = tmpfile();
if (!F)
Index: clang/test/Analysis/stream-errno-note.c
===================================================================
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -98,6 +98,18 @@
(void)fclose(F);
}
+void check_rewind_errnocheck(void) {
+ FILE *F = tmpfile();
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ errno = 0;
+ rewind(F); // expected-note{{Function 'rewind' indicates failure only by setting of 'errno'}}
+ fclose(F); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'fclose' [alpha.unix.Errno]}}
+ // expected-note@-1{{Value of 'errno' was not checked and may be overwritten by function 'fclose'}}
+}
+
void check_fileno(void) {
FILE *F = tmpfile();
// expected-note@+2{{'F' is non-null}}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -251,10 +251,14 @@
{&StreamChecker::preFwrite,
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
{{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
- {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+ {{"ftell", 1},
+ {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
+ {{"rewind", 1},
+ {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
+ {{"fgetpos", 2},
+ {&StreamChecker::preDefault, &StreamChecker::evalFgetpos, 0}},
+ {{"fsetpos", 2},
+ {&StreamChecker::preDefault, &StreamChecker::evalFsetpos, 0}},
{{"clearerr", 1},
{&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
{{"feof", 1},
@@ -307,6 +311,18 @@
void evalFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalFgetpos(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void evalFsetpos(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void evalFtell(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void evalRewind(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
void preDefault(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -797,6 +813,131 @@
C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
}
+void StreamChecker::evalFgetpos(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!Sym)
+ return;
+
+ // Do not evaluate if stream is not found.
+ if (!State->get<StreamMap>(Sym))
+ return;
+
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ ProgramStateRef StateNotFailed, StateFailed;
+ std::tie(StateFailed, StateNotFailed) =
+ C.getConstraintManager().assumeDual(State, RetVal);
+
+ // This function does not affect the stream state.
+ // Still we add success and failure state with the appropriate return value.
+ // StdLibraryFunctionsChecker can change these states (with 'errno').
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalFsetpos(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ if (!SS)
+ return;
+
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ ProgramStateRef StateNotFailed, StateFailed;
+ std::tie(StateFailed, StateNotFailed) =
+ C.getConstraintManager().assumeDual(State, RetVal);
+
+ StateNotFailed = StateNotFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, ErrorNone, false));
+
+ // At failure ferror could set.
+ // The standards do not tell what happens with the file position at failure.
+ // But we can assume that it is dangerous to make a next I/O operation after
+ // the position was not set correctly (similar to 'fseek').
+ StateFailed = StateFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
+
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!Sym)
+ return;
+
+ if (!State->get<StreamMap>(Sym))
+ return;
+
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ SValBuilder &SVB = C.getSValBuilder();
+ NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(CE, C.getLocationContext(), RetVal);
+ auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
+ SVB.makeZeroVal(C.getASTContext().LongTy),
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ if (!Cond)
+ return;
+ StateNotFailed = StateNotFailed->assume(*Cond, true);
+ if (!StateNotFailed)
+ return;
+
+ ProgramStateRef StateFailed = State->BindExpr(
+ CE, C.getLocationContext(), SVB.makeIntVal(-1, C.getASTContext().LongTy));
+
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalRewind(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ if (!SS)
+ return;
+
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ State = State->set<StreamMap>(StreamSym,
+ StreamState::getOpened(Desc, ErrorNone, false));
+
+ C.addTransition(State);
+}
+
void StreamChecker::evalClearerr(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -512,6 +512,21 @@
}
};
+ class ErrnoMustBeCheckedConstraint : public ErrnoConstraintBase {
+ public:
+ ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+ const Summary &Summary,
+ CheckerContext &C) const override {
+ return errno_modeling::setErrnoStdMustBeChecked(State, C,
+ Call.getOriginExpr());
+ }
+
+ const NoteTag *describe(CheckerContext &C,
+ StringRef FunctionName) const override {
+ return errno_modeling::getNoteTagForStdMustBeChecked(C, FunctionName);
+ }
+ };
+
/// A single branch of a function summary.
///
/// A branch is defined by a series of constraints - "assumptions" -
@@ -773,6 +788,7 @@
/// constraints (and different return value constraints).
const NoErrnoConstraint ErrnoUnchanged{};
const ResetErrnoConstraint ErrnoIrrelevant{};
+ const ErrnoMustBeCheckedConstraint ErrnoMustBeChecked{};
const SuccessErrnoConstraint ErrnoMustNotBeChecked{};
const FailureErrnoConstraint ErrnoNEZeroIrrelevant{};
};
@@ -1399,11 +1415,22 @@
auto IsNull = [&](ArgNo ArgN) {
return std::make_shared<NotNullConstraint>(ArgN, false);
};
+ auto NotZero = [&](ArgNo ArgN) {
+ return std::make_shared<NotZeroConstraint>(ArgN);
+ };
+ auto IsZero = [&](ArgNo ArgN) {
+ return std::make_shared<NotZeroConstraint>(ArgN, false);
+ };
Optional<QualType> FileTy = lookupTy("FILE");
Optional<QualType> FilePtrTy = getPointerTy(FileTy);
Optional<QualType> FilePtrRestrictTy = getRestrictTy(FilePtrTy);
+ Optional<QualType> FPosTTy = lookupTy("fpos_t");
+ Optional<QualType> FPosTPtrTy = getPointerTy(FPosTTy);
+ Optional<QualType> ConstFPosTPtrTy = getPointerTy(getConstTy(FPosTTy));
+ Optional<QualType> FPosTPtrRestrictTy = getRestrictTy(FPosTPtrTy);
+
// We are finally ready to define specifications for all supported functions.
//
// Argument ranges should always cover all variants. If return value
@@ -1788,6 +1815,46 @@
.ArgConstraint(NotNull(ArgNo(0)))
.ArgConstraint(ArgumentCondition(2, WithinRange, {{0, 2}})));
+ // int fgetpos(FILE *restrict stream, fpos_t *restrict pos);
+ // From 'The Open Group Base Specifications Issue 7, 2018 edition':
+ // "The fgetpos() function shall not change the setting of errno if
+ // successful."
+ addToFunctionSummaryMap(
+ "fgetpos",
+ Signature(ArgTypes{FilePtrRestrictTy, FPosTPtrRestrictTy},
+ RetType{IntTy}),
+ Summary(NoEvalCall)
+ .Case({IsZero(ArgNo(Ret))}, ErrnoUnchanged)
+ .Case({NotZero(ArgNo(Ret))}, ErrnoNEZeroIrrelevant)
+ .ArgConstraint(NotNull(ArgNo(0)))
+ .ArgConstraint(NotNull(ArgNo(1))));
+
+ // int fsetpos(FILE *stream, const fpos_t *pos);
+ // From 'The Open Group Base Specifications Issue 7, 2018 edition':
+ // "The fsetpos() function shall not change the setting of errno if
+ // successful."
+ addToFunctionSummaryMap(
+ "fsetpos",
+ Signature(ArgTypes{FilePtrTy, ConstFPosTPtrTy}, RetType{IntTy}),
+ Summary(NoEvalCall)
+ .Case({IsZero(ArgNo(Ret))}, ErrnoUnchanged)
+ .Case({NotZero(ArgNo(Ret))}, ErrnoNEZeroIrrelevant)
+ .ArgConstraint(NotNull(ArgNo(0)))
+ .ArgConstraint(NotNull(ArgNo(1))));
+
+ // long ftell(FILE *stream);
+ // From 'The Open Group Base Specifications Issue 7, 2018 edition':
+ // "The ftell() function shall not change the setting of errno if
+ // successful."
+ addToFunctionSummaryMap(
+ "ftell", Signature(ArgTypes{FilePtrTy}, RetType{LongTy}),
+ Summary(NoEvalCall)
+ .Case({ReturnValueCondition(WithinRange, Range(1, LongMax))},
+ ErrnoUnchanged)
+ .Case({ReturnValueCondition(WithinRange, SingleValue(-1))},
+ ErrnoNEZeroIrrelevant)
+ .ArgConstraint(NotNull(ArgNo(0))));
+
// int fileno(FILE *stream);
addToFunctionSummaryMap(
"fileno", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
@@ -1796,6 +1863,29 @@
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(NotNull(ArgNo(0))));
+ // void rewind(FILE *stream);
+ // This function indicates error only by setting of 'errno'.
+ addToFunctionSummaryMap("rewind",
+ Signature(ArgTypes{FilePtrTy}, RetType{VoidTy}),
+ Summary(NoEvalCall)
+ .Case({}, ErrnoMustBeChecked)
+ .ArgConstraint(NotNull(ArgNo(0))));
+
+ // void clearerr(FILE *stream);
+ addToFunctionSummaryMap(
+ "clearerr", Signature(ArgTypes{FilePtrTy}, RetType{VoidTy}),
+ Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
+
+ // int feof(FILE *stream);
+ addToFunctionSummaryMap(
+ "feof", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+ Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
+
+ // int ferror(FILE *stream);
+ addToFunctionSummaryMap(
+ "ferror", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+ Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
+
// long a64l(const char *str64);
addToFunctionSummaryMap(
"a64l", Signature(ArgTypes{ConstCharPtrTy}, RetType{LongTy}),
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -104,11 +104,26 @@
ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
NonLoc ErrnoSym);
+/// Set errno state for the common case when a standard function indicates
+/// failure only by \c errno. Sets \c ErrnoCheckState to \c MustBeChecked, and
+/// invalidates the errno region (clear of previous value).
+/// At the state transition a note tag created by
+/// \c getNoteTagForStdMustBeChecked can be used.
+/// \arg \c InvalE Expression that causes invalidation of \c errno.
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+ CheckerContext &C, const Expr *InvalE);
+
/// Generate the note tag that can be applied at the state generated by
/// \c setErrnoForStdSuccess .
/// \arg \c Fn Name of the (standard) function that is modeled.
const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
+/// Generate the note tag that can be applied at the state generated by
+/// \c setErrnoStdMustBeChecked .
+/// \arg \c Fn Name of the (standard) function that is modeled.
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+ llvm::StringRef Fn);
+
} // namespace errno_modeling
} // namespace ento
} // namespace clang
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -303,6 +303,19 @@
return setErrnoValue(State, C.getLocationContext(), ErrnoSym, Irrelevant);
}
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+ CheckerContext &C,
+ const Expr *InvalE) {
+ const MemRegion *ErrnoR = State->get<ErrnoRegion>();
+ if (!ErrnoR)
+ return State;
+ State = State->invalidateRegions(ErrnoR, InvalE, C.blockCount(),
+ C.getLocationContext(), false);
+ if (!State)
+ return nullptr;
+ return setErrnoState(State, MustBeChecked);
+}
+
const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
return getErrnoNoteTag(
C, (Twine("Assuming that function '") + Twine(Fn) +
@@ -311,6 +324,14 @@
.str());
}
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+ llvm::StringRef Fn) {
+ return getErrnoNoteTag(
+ C, (Twine("Function '") + Twine(Fn) +
+ Twine("' indicates failure only by setting of 'errno'"))
+ .str());
+}
+
} // namespace errno_modeling
} // namespace ento
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
