balazske updated this revision to Diff 474558.
balazske added a comment.
Rebase to main and D137722 <https://reviews.llvm.org/D137722>.
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
@@ -521,6 +521,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" -
@@ -785,6 +800,7 @@
/// constraints (and different return value constraints).
const NoErrnoConstraint ErrnoUnchanged{};
const ResetErrnoConstraint ErrnoIrrelevant{};
+ const ErrnoMustBeCheckedConstraint ErrnoMustBeChecked{};
const SuccessErrnoConstraint ErrnoMustNotBeChecked{};
const FailureErrnoConstraint ErrnoNEZeroIrrelevant{};
};
@@ -1428,11 +1444,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
@@ -1817,6 +1844,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}),
@@ -1825,6 +1892,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
