diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h --- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h +++ b/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 diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp --- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp +++ b/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(); + 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 diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/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(ArgN, false); }; + auto NotZero = [&](ArgNo ArgN) { + return std::make_shared(ArgN); + }; + auto IsZero = [&](ArgNo ArgN) { + return std::make_shared(ArgN, false); + }; Optional FileTy = lookupTy("FILE"); Optional FilePtrTy = getPointerTy(FileTy); Optional FilePtrRestrictTy = getRestrictTy(FilePtrTy); + Optional FPosTTy = lookupTy("fpos_t"); + Optional FPosTPtrTy = getPointerTy(FPosTTy); + Optional ConstFPosTPtrTy = getPointerTy(getConstTy(FPosTTy)); + Optional 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}), diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/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(Sym)) + return; + + auto *CE = dyn_cast_or_null(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(StreamSym); + if (!SS) + return; + + auto *CE = dyn_cast_or_null(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( + 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( + 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(Sym)) + return; + + auto *CE = dyn_cast_or_null(Call.getOriginExpr()); + if (!CE) + return; + + SValBuilder &SVB = C.getSValBuilder(); + NonLoc RetVal = makeRetVal(C, CE).castAs(); + ProgramStateRef StateNotFailed = + State->BindExpr(CE, C.getLocationContext(), RetVal); + auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, + SVB.makeZeroVal(C.getASTContext().LongTy), + SVB.getConditionType()) + .getAs(); + 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(StreamSym); + if (!SS) + return; + + auto *CE = dyn_cast_or_null(Call.getOriginExpr()); + if (!CE) + return; + + assertStreamStateOpened(SS); + + State = State->set(StreamSym, + StreamState::getOpened(Desc, ErrorNone, false)); + + C.addTransition(State); +} + void StreamChecker::evalClearerr(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { diff --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c --- a/clang/test/Analysis/stream-errno-note.c +++ b/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}} diff --git a/clang/test/Analysis/stream-errno.c b/clang/test/Analysis/stream-errno.c --- a/clang/test/Analysis/stream-errno.c +++ b/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) diff --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c --- a/clang/test/Analysis/stream-noopen.c +++ b/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}}