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 @@ -101,11 +101,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 @@ -299,6 +299,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) + @@ -307,6 +320,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/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}, @@ -265,6 +269,8 @@ {&StreamChecker::preDefault, std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError), 0}}, + // 'fileno' is checked sufficiently by StdLibraryFunctionsChecker. + // This check additionally checks if the file is in opened state. {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}}, }; @@ -307,6 +313,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; @@ -835,6 +853,153 @@ 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; + + const StreamState *SS = State->get(Sym); + 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); + + // In the success case, do not change 'errno' and errno state. + StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C, + getErrnoSVal(C, Call)); + if (!StateFailed) + return; + + 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); + + // In the success case, do not change 'errno' and errno state. + StateNotFailed = StateNotFailed->set( + StreamSym, StreamState::getOpened(Desc, ErrorNone, false)); + + // At failure it may happen that ferror is set but not always. + // 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)); + StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C, + getErrnoSVal(C, Call)); + if (!StateFailed) + return; + + 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; + + const StreamState *SS = State->get(Sym); + if (!SS) + return; + + auto *CE = dyn_cast_or_null(Call.getOriginExpr()); + if (!CE) + return; + + assertStreamStateOpened(SS); + + 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)); + + StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C, + getErrnoSVal(C, Call)); + if (!StateFailed) + return; + + 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)); + + State = + errno_modeling::setErrnoStdMustBeChecked(State, C, Call.getOriginExpr()); + + C.addTransition(State, + errno_modeling::getNoteTagForStdMustBeChecked(C, "rewind")); +} + 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 @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions \ -// RUN: -analyzer-output text -verify %s +// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -analyzer-output text -verify %s #include "Inputs/system-header-simulator.h" #include "Inputs/errno_func.h" @@ -97,3 +97,29 @@ // expected-note@-1{{An undefined value may be read from 'errno'}} (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'}} +} + +// 'fileno' is modeled by StdCLibraryFunctions checker (the errno related parts) +void check_fileno(void) { + FILE *F = tmpfile(); + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) + return; + fileno(F); + // expected-note@-1{{Assuming that function 'fileno' is successful}} + if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} + // expected-note@-1{{An undefined value may be read from 'errno'}} + (void)fclose(F); +} 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 @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions,debug.ExprInspection \ -// RUN: -verify %s +// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify %s #include "Inputs/system-header-simulator.h" #include "Inputs/errno_func.h" @@ -121,3 +121,75 @@ clang_analyzer_eval(errno == 1); // expected-warning{{TRUE}} 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) + return; + int N = fileno(F); + if (N == -1) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} + if (errno) {} // no-warning + fclose(F); + return; + } + if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} +} diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -11,6 +11,9 @@ void StreamTesterChecker_make_feof_stream(FILE *); void StreamTesterChecker_make_ferror_stream(FILE *); +const char *WBuf = "123456789"; +char RBuf[10]; + void error_fopen(void) { FILE *F = fopen("file", "r"); if (!F) @@ -233,3 +236,37 @@ } fclose(F); } + +void error_fsetpos(void) { + FILE *F = fopen("file", "w"); + if (!F) + return; + fpos_t Pos; + int rc = fsetpos(F, &Pos); + clang_analyzer_eval(feof(F)); // expected-warning{{FALSE}} + if (rc) { + if (ferror(F)) + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + else + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + } else { + clang_analyzer_eval(ferror(F)); // expected-warning{{FALSE}} + } + fwrite(WBuf, 1, 10, F); // expected-warning {{might be 'indeterminate'}} + fclose(F); +} + +void error_rewind(void) { + FILE *F = fopen("file", "r"); + if (!F) + return; + fread(RBuf, 1, 10, F); + if (feof(F)) { + rewind(F); + clang_analyzer_eval(feof(F)); // expected-warning{{FALSE}} + } else if (ferror(F)) { + rewind(F); + clang_analyzer_eval(ferror(F)); // expected-warning{{FALSE}} + } + fclose(F); +}