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 @@ -25,6 +25,10 @@ using namespace ento; using namespace std::placeholders; +//===----------------------------------------------------------------------===// +// Definition of state data structures. +//===----------------------------------------------------------------------===// + namespace { struct FnDescription; @@ -146,6 +150,14 @@ } }; +} // namespace + +//===----------------------------------------------------------------------===// +// StreamChecker class and utility functions. +//===----------------------------------------------------------------------===// + +namespace { + class StreamChecker; using FnCheck = std::function; @@ -219,6 +231,8 @@ /// If true, evaluate special testing stream functions. bool TestMode = false; + const BugType *getBT_StreamEof() const { return &BT_StreamEof; } + private: CallDescriptionMap FnDescriptions = { {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}}, @@ -337,7 +351,8 @@ /// 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; + void reportFEofWarning(SymbolRef StreamSym, CheckerContext &C, + ProgramStateRef State) const; /// Emit resource leak warnings for the given symbols. /// Createn a non-fatal error node for these, and returns it (if any warnings @@ -363,14 +378,14 @@ /// Generate a message for BugReporterVisitor if the stored symbol is /// marked as interesting by the actual bug report. + // FIXME: Use lambda instead. struct NoteFn { - const CheckerNameRef CheckerName; + const BugType *BT_ResourceLeak; SymbolRef StreamSym; std::string Message; std::string operator()(PathSensitiveBugReport &BR) const { - if (BR.isInteresting(StreamSym) && - CheckerName == BR.getBugType().getCheckerName()) + if (BR.isInteresting(StreamSym) && &BR.getBugType() == BT_ResourceLeak) return Message; return ""; @@ -379,7 +394,20 @@ const NoteTag *constructNoteTag(CheckerContext &C, SymbolRef StreamSym, const std::string &Message) const { - return C.getNoteTag(NoteFn{getCheckerName(), StreamSym, Message}); + return C.getNoteTag(NoteFn{&BT_ResourceLeak, StreamSym, Message}); + } + + const NoteTag *constructSetEofNoteTag(CheckerContext &C, + SymbolRef StreamSym) const { + return C.getNoteTag([this, StreamSym](PathSensitiveBugReport &BR) { + if (!BR.isInteresting(StreamSym) || + &BR.getBugType() != this->getBT_StreamEof()) + return ""; + + BR.markNotInteresting(StreamSym); + + return "Assuming stream reaches end-of-file here"; + }); } /// Searches for the ExplodedNode where the file descriptor was acquired for @@ -391,6 +419,9 @@ } // end anonymous namespace +// This map holds the state of a stream. +// The stream is identified with a SymbolRef that is created when a stream +// opening function is modeled by the checker. REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) inline void assertStreamStateOpened(const StreamState *SS) { @@ -419,6 +450,10 @@ return nullptr; } +//===----------------------------------------------------------------------===// +// Methods of StreamChecker. +//===----------------------------------------------------------------------===// + void StreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { const FnDescription *Desc = lookupFn(Call); @@ -566,7 +601,7 @@ if (Sym && State->get(Sym)) { const StreamState *SS = State->get(Sym); if (SS->ErrorState & ErrorFEof) - reportFEofWarning(C, State); + reportFEofWarning(Sym, C, State); } else { C.addTransition(State); } @@ -609,11 +644,11 @@ if (!NMembVal) return; - const StreamState *SS = State->get(StreamSym); - if (!SS) + const StreamState *OldSS = State->get(StreamSym); + if (!OldSS) return; - assertStreamStateOpened(SS); + assertStreamStateOpened(OldSS); // C'99 standard, ยง7.19.8.1.3, the return value of fread: // The fread function returns the number of elements successfully read, which @@ -632,7 +667,7 @@ // 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)) { + if (!IsFread || (OldSS->ErrorState != ErrorFEof)) { ProgramStateRef StateNotFailed = State->BindExpr(CE, C.getLocationContext(), *NMembVal); if (StateNotFailed) { @@ -661,14 +696,18 @@ StreamErrorState NewES; if (IsFread) - NewES = (SS->ErrorState == ErrorFEof) ? ErrorFEof : ErrorFEof | ErrorFError; + NewES = + (OldSS->ErrorState == ErrorFEof) ? ErrorFEof : ErrorFEof | ErrorFError; else NewES = ErrorFError; // If a (non-EOF) error occurs, the resulting value of the file position // indicator for the stream is indeterminate. - StreamState NewState = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); - StateFailed = StateFailed->set(StreamSym, NewState); - C.addTransition(StateFailed); + StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); + StateFailed = StateFailed->set(StreamSym, NewSS); + if (IsFread && OldSS->ErrorState != ErrorFEof) + C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); + else + C.addTransition(StateFailed); } void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, @@ -727,7 +766,7 @@ StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true)); C.addTransition(StateNotFailed); - C.addTransition(StateFailed); + C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); } void StreamChecker::evalClearerr(const FnDescription *Desc, @@ -960,14 +999,16 @@ return State; } -void StreamChecker::reportFEofWarning(CheckerContext &C, +void StreamChecker::reportFEofWarning(SymbolRef StreamSym, CheckerContext &C, ProgramStateRef State) const { if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) { - C.emitReport(std::make_unique( + auto R = std::make_unique( BT_StreamEof, "Read function called when stream is in EOF state. " "Function has no effect.", - N)); + N); + R->markInteresting(StreamSym); + C.emitReport(std::move(R)); return; } C.addTransition(State); @@ -1058,6 +1099,10 @@ return State; } +//===----------------------------------------------------------------------===// +// Checker registration. +//===----------------------------------------------------------------------===// + void ento::registerStreamChecker(CheckerManager &Mgr) { Mgr.registerChecker(); } @@ -1073,4 +1118,4 @@ bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) { return true; -} +} \ No newline at end of file diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c --- a/clang/test/Analysis/stream-note.c +++ b/clang/test/Analysis/stream-note.c @@ -88,3 +88,60 @@ fclose(F); // expected-warning {{Stream pointer might be NULL}} // expected-note@-1 {{Stream pointer might be NULL}} } + +void check_eof_notes_feof_after_feof() { + FILE *F; + char Buf[10]; + F = fopen("foo1.c", "r"); + if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}} + return; + } + fread(Buf, 1, 1, F); + if (feof(F)) { // expected-note {{Taking true branch}} + clearerr(F); + fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}} + if (feof(F)) { // expected-note {{Taking true branch}} + fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}} + // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}} + } + } + fclose(F); +} + +void check_eof_notes_feof_after_no_feof() { + FILE *F; + char Buf[10]; + F = fopen("foo1.c", "r"); + if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}} + return; + } + fread(Buf, 1, 1, F); + if (feof(F)) { // expected-note {{Taking false branch}} + fclose(F); + return; + } else if (ferror(F)) { // expected-note {{Taking false branch}} + fclose(F); + return; + } + fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}} + if (feof(F)) { // expected-note {{Taking true branch}} + fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}} + // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}} + } + fclose(F); +} + +void check_eof_notes_feof_or_no_error() { + FILE *F; + char Buf[10]; + F = fopen("foo1.c", "r"); + if (F == NULL) // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}} + return; + int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}} + if (ferror(F)) { // expected-note {{Taking false branch}} + } else { + fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}} + // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}} + } + fclose(F); +}