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 @@ -16,6 +16,7 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" @@ -29,15 +30,23 @@ struct StreamState { enum Kind { Opened, Closed, OpenFailed, Escaped } K; + bool ShouldCallFeof{false}; + bool ShouldCallFerror{false}; - StreamState(Kind k) : K(k) {} + StreamState(Kind k, bool Feof, bool Ferror) + : K(k), ShouldCallFeof{Feof}, ShouldCallFerror{Ferror} {} + StreamState(Kind k, bool MakeError = false) + : K(k), ShouldCallFeof{MakeError}, ShouldCallFerror{MakeError} {} bool isOpened() const { return K == Opened; } bool isClosed() const { return K == Closed; } //bool isOpenFailed() const { return K == OpenFailed; } //bool isEscaped() const { return K == Escaped; } - bool operator==(const StreamState &X) const { return K == X.K; } + bool operator==(const StreamState &X) const { + return K == X.K && ShouldCallFeof == X.ShouldCallFeof && + ShouldCallFerror == X.ShouldCallFerror; + } static StreamState getOpened() { return StreamState(Opened); } static StreamState getClosed() { return StreamState(Closed); } @@ -46,13 +55,17 @@ void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); + ID.AddBoolean(ShouldCallFeof); + ID.AddBoolean(ShouldCallFerror); } }; class StreamChecker : public Checker { mutable std::unique_ptr BT_nullfp, BT_illegalwhence, - BT_doubleclose, BT_ResourceLeak; + BT_doubleclose, BT_ResourceLeak, BT_missingerrorcheck; + + mutable int EOFv{0}; public: bool evalCall(const CallEvent &Call, CheckerContext &C) const; @@ -67,36 +80,35 @@ {{"freopen", 3}, &StreamChecker::evalFreopen}, {{"tmpfile"}, &StreamChecker::evalFopen}, {{"fclose", 1}, &StreamChecker::evalFclose}, - {{"fread", 4}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)}, - {{"fwrite", 4}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)}, + {{"fread", 4}, std::bind(&StreamChecker::evalDefault, _1, _2, _3, 3)}, + {{"fwrite", 4}, std::bind(&StreamChecker::evalDefault, _1, _2, _3, 3)}, + {{"fgetc", 1}, &StreamChecker::evalGetc}, {{"fseek", 3}, &StreamChecker::evalFseek}, - {{"ftell", 1}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}, - {{"rewind", 1}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}, - {{"fgetpos", 2}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}, - {{"fsetpos", 2}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}, - {{"clearerr", 1}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}, - {{"feof", 1}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}, - {{"ferror", 1}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}, - {{"fileno", 1}, - std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}, + {{"ftell", 1}, std::bind(&StreamChecker::evalDefault, _1, _2, _3, 0)}, + {{"rewind", 1}, std::bind(&StreamChecker::evalDefault, _1, _2, _3, 0)}, + {{"fgetpos", 2}, std::bind(&StreamChecker::evalDefault, _1, _2, _3, 0)}, + {{"fsetpos", 2}, std::bind(&StreamChecker::evalDefault, _1, _2, _3, 0)}, + {{"clearerr", 1}, std::bind(&StreamChecker::evalDefault, _1, _2, _3, 0)}, + {{"feof", 1}, &StreamChecker::evalFeof}, + {{"ferror", 1}, &StreamChecker::evalFerror}, + {{"fileno", 1}, std::bind(&StreamChecker::evalDefault, _1, _2, _3, 0)}, }; void evalFopen(const CallEvent &Call, CheckerContext &C) const; void evalFreopen(const CallEvent &Call, CheckerContext &C) const; void evalFclose(const CallEvent &Call, CheckerContext &C) const; void evalFseek(const CallEvent &Call, CheckerContext &C) const; - void checkArgNullStream(const CallEvent &Call, CheckerContext &C, - unsigned ArgI) const; - + void evalGetc(const CallEvent &Call, CheckerContext &C) const; + void evalFeof(const CallEvent &Call, CheckerContext &C) const; + void evalFerror(const CallEvent &Call, CheckerContext &C) const; + void evalDefault(const CallEvent &Call, CheckerContext &C, + unsigned StreamArgI) const; + + ProgramStateRef resetErrorState(SVal SV, CheckerContext &C, + ProgramStateRef State, bool FeofCalled, + bool FerrorCalled) const; + ExplodedNode *checkErrorState(SVal SV, CheckerContext &C, + ExplodedNode *N) const; ProgramStateRef checkNullStream(SVal SV, CheckerContext &C, ProgramStateRef State) const; ProgramStateRef checkFseekWhence(SVal SV, CheckerContext &C, @@ -185,6 +197,10 @@ if (!StreamSym) return; + ExplodedNode *N = C.addTransition(State); + N = checkErrorState(*StreamVal, C, N); + State = N->getState(); + // Generate state for non-failed case. // Return value is the passed stream pointer. // According to the documentations, the stream is closed first @@ -201,23 +217,27 @@ StateRetNull = StateRetNull->set(StreamSym, StreamState::getOpenFailed()); - C.addTransition(StateRetNotNull); - C.addTransition(StateRetNull); + C.addTransition(StateRetNotNull, N); + C.addTransition(StateRetNull, N); } void StreamChecker::evalFclose(const CallEvent &Call, CheckerContext &C) const { - ProgramStateRef State = C.getState(); + ExplodedNode *N = checkErrorState(Call.getArgSVal(0), C, C.getPredecessor()); + + ProgramStateRef State = N->getState(); State = checkDoubleClose(Call, C, State); if (State) - C.addTransition(State); + C.addTransition(State, N); } void StreamChecker::evalFseek(const CallEvent &Call, CheckerContext &C) const { + ExplodedNode *N = checkErrorState(Call.getArgSVal(0), C, C.getPredecessor()); + const Expr *AE2 = Call.getArgExpr(2); if (!AE2) return; - ProgramStateRef State = C.getState(); + ProgramStateRef State = N->getState(); State = checkNullStream(Call.getArgSVal(0), C, State); if (!State) @@ -228,15 +248,144 @@ if (!State) return; + C.addTransition(State, N); +} + +void StreamChecker::evalGetc(const CallEvent &Call, CheckerContext &C) const { + if (EOFv == 0) { + if (const llvm::Optional OptInt = + tryExpandAsInteger("EOF", C.getPreprocessor())) + EOFv = *OptInt; + else + EOFv = -1; + } + + ExplodedNode *N = checkErrorState(Call.getArgSVal(0), C, C.getPredecessor()); + + auto *CE = dyn_cast_or_null(Call.getOriginExpr()); + if (!CE) + return; + + SymbolRef StreamSym = Call.getArgSVal(0).getAsSymbol(); + if (!StreamSym) + return; + + ProgramStateRef State = N->getState(); + State = checkNullStream(Call.getArgSVal(0), C, State); + if (!State) + return; + + const StreamState *SS = State->get(StreamSym); + if (!SS) + return; + + SValBuilder &SVB = C.getSValBuilder(); + BasicValueFactory &BVF = SVB.getBasicValueFactory(); + + DefinedSVal RetVal = + SVB.conjureSymbolVal(nullptr, CE, C.getLocationContext(), C.blockCount()) + .castAs(); + State = State->BindExpr(CE, C.getLocationContext(), RetVal); + + ProgramStateRef FailedState, NonFailedState; + const llvm::APSInt &EOFVal = BVF.getValue(EOFv, Call.getResultType()); + std::tie(FailedState, NonFailedState) = + State->assumeInclusiveRange(RetVal, EOFVal, EOFVal); + if (FailedState) { + FailedState = + FailedState->set(StreamSym, StreamState{SS->K, true}); + C.addTransition(FailedState, N); + } + if (NonFailedState) { + C.addTransition(NonFailedState, N); + } +} + +void StreamChecker::evalFeof(const CallEvent &Call, CheckerContext &C) const { + SVal StreamVal = Call.getArgSVal(0); + ProgramStateRef State = C.getState(); + State = checkNullStream(StreamVal, C, State); + if (!State) + return; + + State = resetErrorState(StreamVal, C, State, true, false); + C.addTransition(State); } -void StreamChecker::checkArgNullStream(const CallEvent &Call, CheckerContext &C, - unsigned ArgI) const { +void StreamChecker::evalFerror(const CallEvent &Call, CheckerContext &C) const { + SVal StreamVal = Call.getArgSVal(0); ProgramStateRef State = C.getState(); - State = checkNullStream(Call.getArgSVal(ArgI), C, State); - if (State) - C.addTransition(State); + State = checkNullStream(StreamVal, C, State); + if (!State) + return; + + State = resetErrorState(StreamVal, C, State, false, true); + + C.addTransition(State); +} + +void StreamChecker::evalDefault(const CallEvent &Call, CheckerContext &C, + unsigned StreamArgI) const { + ExplodedNode *N = + checkErrorState(Call.getArgSVal(StreamArgI), C, C.getPredecessor()); + ProgramStateRef State = N->getState(); + State = checkNullStream(Call.getArgSVal(StreamArgI), C, State); + if (!State) + return; + + C.addTransition(State, N); +} + +ProgramStateRef StreamChecker::resetErrorState(SVal SV, CheckerContext &C, + ProgramStateRef State, + bool FeofCalled, + bool FerrorCalled) const { + SymbolRef StreamSym = SV.getAsSymbol(); + if (!StreamSym) + return State; + const StreamState *SS = State->get(StreamSym); + if (!SS || !SS->isOpened()) + return State; + if ((!SS->ShouldCallFeof || !FeofCalled) && + (!SS->ShouldCallFerror || !FerrorCalled)) + return State; + + if (FeofCalled) { + State = State->set( + StreamSym, StreamState{SS->K, false, SS->ShouldCallFerror}); + } + if (FerrorCalled) { + State = State->set( + StreamSym, StreamState{SS->K, SS->ShouldCallFeof, false}); + } + + return State; +} + +ExplodedNode *StreamChecker::checkErrorState(SVal SV, CheckerContext &C, + ExplodedNode *N) const { + SymbolRef StreamSym = SV.getAsSymbol(); + if (!StreamSym) + return N; + ProgramStateRef State = N->getState(); + const StreamState *SS = State->get(StreamSym); + if (!SS || !SS->isOpened()) + return N; + + if (SS->ShouldCallFeof || SS->ShouldCallFerror) { + State = State->set(StreamSym, StreamState{SS->K, false}); + if (ExplodedNode *N1 = C.generateNonFatalErrorNode(State)) { + if (!BT_missingerrorcheck) + BT_missingerrorcheck.reset(new BuiltinBug( + this, "Missing error check", + "Should call 'feof' and 'ferror' after failed character read.")); + C.emitReport(std::make_unique( + *BT_missingerrorcheck, BT_missingerrorcheck->getDescription(), N1)); + return N1; + } + } + return N; } ProgramStateRef StreamChecker::checkNullStream(SVal SV, CheckerContext &C, diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -6,11 +6,13 @@ #define SEEK_SET 0 /* Seek from beginning of file. */ #define SEEK_CUR 1 /* Seek from current position. */ #define SEEK_END 2 /* Seek from end of file. */ +#define EOF (-1) extern FILE *fopen(const char *path, const char *mode); extern FILE *tmpfile(void); extern int fclose(FILE *fp); extern size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream); extern size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream); +extern int fgetc(FILE *stream); extern int fseek (FILE *__stream, long int __off, int __whence); extern long int ftell (FILE *__stream); extern void rewind (FILE *__stream); @@ -176,3 +178,74 @@ fclose(f1); } } + +void check_fgetc_error() { + FILE *fp = fopen("foo.c", "r"); + if (fp) { + int C; + fpos_t pos; + C = fgetc(fp); + fread(0, 0, 0, fp); // expected-warning {{Should call}} + fread(0, 0, 0, fp); + C = fgetc(fp); + fwrite(0, 0, 0, fp); // expected-warning {{Should call}} + C = fgetc(fp); + fseek(fp, 1, SEEK_SET); // expected-warning {{Should call}} + C = fgetc(fp); + ftell(fp); // expected-warning {{Should call}} + C = fgetc(fp); + rewind(fp); // expected-warning {{Should call}} + C = fgetc(fp); + fgetpos(fp, &pos); // expected-warning {{Should call}} + C = fgetc(fp); + fsetpos(fp, 0); // expected-warning {{Should call}} + C = fgetc(fp); + clearerr(fp); // expected-warning {{Should call}} + C = fgetc(fp); + C = fgetc(fp); // expected-warning {{Should call}} + fp = freopen(0, "w", fp); // expected-warning {{Should call}} + C = fgetc(fp); + fclose(fp); // expected-warning {{Should call}} + } +} + +void check_fgetc_error1() { + FILE *fp = fopen("foo.c", "r"); + if (fp) { + int C; + C = fgetc(fp); + feof(fp); + fclose(fp); // expected-warning {{Should call}} + } + fp = fopen("foo.c", "r"); + if (fp) { + int C; + C = fgetc(fp); + ferror(fp); + fclose(fp); // expected-warning {{Should call}} + } +} + +void check_fgetc_noerror() { + FILE *fp = fopen("foo.c", "r"); + if (fp) { + int C; + C = fgetc(fp); + feof(fp); + ferror(fp); + fclose(fp); + } +} + +void check_fgetc_checked() { + FILE *fp = fopen("foo.c", "r"); + if (fp) { + int C; + C = fgetc(fp); + if (C == EOF) { + feof(fp); + ferror(fp); + } + fclose(fp); + } +}