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 @@ -51,6 +51,8 @@ return NoError == ES.NoError && FEof == ES.FEof && FError == ES.FError; } + bool operator!=(const StreamErrorState &ES) const { return !(*this == ES); } + StreamErrorState operator|(const StreamErrorState &E) const { return {NoError || E.NoError, FEof || E.FEof, FError || E.FError}; } @@ -171,7 +173,7 @@ class StreamChecker : public Checker { mutable std::unique_ptr BT_nullfp, BT_illegalwhence, - BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak; + BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof; public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const; @@ -189,8 +191,12 @@ {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}}, {{"fclose", 1}, {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}}, - {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}}, - {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}}, + {{"fread", 4}, + {&StreamChecker::preFread, + std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}}, + {{"fwrite", 4}, + {&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}}, @@ -232,6 +238,15 @@ void evalFclose(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; + void preFread(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + + void preFwrite(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + + void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C, bool IsFread) const; + void preFseek(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; void evalFseek(const FnDescription *Desc, const CallEvent &Call, @@ -271,6 +286,12 @@ ProgramStateRef ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C, ProgramStateRef State) const; + /// Generate warning about stream in EOF state. + /// There will be always a state transition into the passed State, + /// by the new non-fatal error node or (if failed) a normal transition, + /// to ensure uniform handling. + void reportFEofWarning(CheckerContext &C, ProgramStateRef State) const; + /// Find the description data of the function called by a call event. /// Returns nullptr if no function is recognized. const FnDescription *lookupFn(const CallEvent &Call) const { @@ -418,6 +439,120 @@ C.addTransition(State); } +void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + SVal StreamVal = getStreamArg(Desc, Call); + State = ensureStreamNonNull(StreamVal, C, State); + if (!State) + return; + State = ensureStreamOpened(StreamVal, C, State); + if (!State) + return; + + SymbolRef Sym = StreamVal.getAsSymbol(); + if (Sym && State->get(Sym)) { + const StreamState *SS = State->get(Sym); + if (SS->ErrorState & ErrorFEof) + reportFEofWarning(C, State); + } else { + C.addTransition(State); + } +} + +void StreamChecker::preFwrite(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + SVal StreamVal = getStreamArg(Desc, Call); + State = ensureStreamNonNull(StreamVal, C, State); + if (!State) + return; + State = ensureStreamOpened(StreamVal, C, State); + if (!State) + return; + + C.addTransition(State); +} + +void StreamChecker::evalFreadFwrite(const FnDescription *Desc, + const CallEvent &Call, CheckerContext &C, + bool IsFread) const { + ProgramStateRef State = C.getState(); + SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol(); + if (!StreamSym) + return; + + const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr()); + if (!CE) + return; + + Optional SizeVal = Call.getArgSVal(1).getAs(); + if (!SizeVal) + return; + Optional NMembVal = Call.getArgSVal(2).getAs(); + if (!NMembVal) + return; + + const StreamState *SS = State->get(StreamSym); + if (!SS) + return; + + assertStreamStateOpened(SS); + + // C'99 standard, ยง7.19.8.1.3, the return value of fread: + // The fread function returns the number of elements successfully read, which + // may be less than nmemb if a read error or end-of-file is encountered. If + // size or nmemb is zero, fread returns zero and the contents of the array and + // the state of the stream remain unchanged. + + if (State->isNull(*SizeVal).isConstrainedTrue() || + State->isNull(*NMembVal).isConstrainedTrue()) { + // This is the "size or nmemb is zero" case. + // Just return 0, do nothing more (not clear the error flags). + State = bindInt(0, State, C, CE); + C.addTransition(State); + return; + } + + // Generate a transition for the success state. + // If we know the state to be FEOF at fread, do not add a success state. + if (!IsFread || (SS->ErrorState != ErrorFEof)) { + ProgramStateRef StateNotFailed = + State->BindExpr(CE, C.getLocationContext(), *NMembVal); + if (StateNotFailed) { + StateNotFailed = StateNotFailed->set( + StreamSym, StreamState::getOpened(Desc)); + C.addTransition(StateNotFailed); + } + } + + // Add transition for the failed state. + Optional RetVal = makeRetVal(C, CE).castAs(); + assert(RetVal && "Value should be NonLoc."); + ProgramStateRef StateFailed = + State->BindExpr(CE, C.getLocationContext(), *RetVal); + if (!StateFailed) + return; + auto Cond = C.getSValBuilder() + .evalBinOpNN(State, BO_LT, *RetVal, *NMembVal, + C.getASTContext().IntTy) + .getAs(); + if (!Cond) + return; + StateFailed = StateFailed->assume(*Cond, true); + if (!StateFailed) + return; + + StreamErrorState NewES; + if (IsFread) + NewES = (SS->ErrorState == ErrorFEof) ? ErrorFEof : ErrorFEof | ErrorFError; + else + NewES = ErrorFError; + StreamState NewState = StreamState::getOpened(Desc, NewES); + StateFailed = StateFailed->set(StreamSym, NewState); + C.addTransition(StateFailed); +} + void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); @@ -657,6 +792,21 @@ return State; } +void StreamChecker::reportFEofWarning(CheckerContext &C, + ProgramStateRef State) const { + if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) { + if (!BT_StreamEof) + BT_StreamEof.reset( + new BuiltinBug(this, "Stream already in EOF", + "Read function called when stream is in EOF state. " + "Function has no effect.")); + C.emitReport(std::make_unique( + *BT_StreamEof, BT_StreamEof->getDescription(), N)); + return; + } + C.addTransition(State); +} + void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const { ProgramStateRef State = C.getState(); 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 @@ -7,6 +7,7 @@ #include "Inputs/system-header-simulator.h" void clang_analyzer_eval(int); +void clang_analyzer_warnIfReached(); void StreamTesterChecker_make_feof_stream(FILE *); void StreamTesterChecker_make_ferror_stream(FILE *); @@ -57,6 +58,84 @@ fclose(F); } +void error_fread() { + FILE *F = tmpfile(); + if (!F) + return; + char Buf[10]; + int Ret = fread(Buf, 1, 10, F); + if (Ret == 10) { + clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} + } else { + clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{TRUE}} + if (feof(F)) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + fread(Buf, 1, 10, F); // expected-warning {{Read function called when stream is in EOF state}} + clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + } + if (ferror(F)) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + fread(Buf, 1, 10, F); // no warning + } + } + fclose(F); + Ret = fread(Buf, 1, 10, F); // expected-warning {{Stream might be already closed}} +} + +void error_fwrite() { + FILE *F = tmpfile(); + if (!F) + return; + const char *Buf = "123456789"; + int Ret = fwrite(Buf, 1, 10, F); + if (Ret == 10) { + clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} + } else { + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} + fwrite(0, 1, 10, F); // no warning + } + fclose(F); + Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}} +} + +void freadwrite_zerosize(FILE *F) { + fwrite(0, 1, 0, F); + fwrite(0, 0, 1, F); + fread(0, 1, 0, F); + fread(0, 0, 1, F); +} + +void freadwrite_zerosize_eofstate(FILE *F) { + fwrite(0, 1, 0, F); + fwrite(0, 0, 1, F); + fread(0, 1, 0, F); // expected-warning {{Read function called when stream is in EOF state}} + fread(0, 0, 1, F); // expected-warning {{Read function called when stream is in EOF state}} +} + +void error_fread_fwrite_zerosize() { + FILE *F = fopen("file", "r"); + if (!F) + return; + + freadwrite_zerosize(F); + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + + StreamTesterChecker_make_ferror_stream(F); + freadwrite_zerosize(F); + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} + + StreamTesterChecker_make_feof_stream(F); + freadwrite_zerosize_eofstate(F); + clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + + fclose(F); +} + void error_fseek() { FILE *F = fopen("file", "r"); if (!F)