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,8 +25,15 @@ using namespace ento; using namespace std::placeholders; +//===----------------------------------------------------------------------===// +// Definition of state data structures. +//===----------------------------------------------------------------------===// + namespace { +const char *Desc_StreamEof = "Stream already in EOF"; +const char *Desc_ResourceLeak = "Resource leak"; + struct FnDescription; /// State of the stream error flags. @@ -146,6 +153,14 @@ } }; +} // namespace + +//===----------------------------------------------------------------------===// +// StreamChecker class and utility functions. +//===----------------------------------------------------------------------===// + +namespace { + class StreamChecker; using FnCheck = std::function; @@ -203,11 +218,18 @@ "Stream handling error"}; BugType BT_IllegalWhence{this, "Illegal whence argument", "Stream handling error"}; - BugType BT_StreamEof{this, "Stream already in EOF", "Stream handling error"}; - BugType BT_ResourceLeak{this, "Resource leak", "Stream handling error", + BugType BT_StreamEof{this, Desc_StreamEof, "Stream handling error"}; + BugType BT_ResourceLeak{this, Desc_ResourceLeak, "Stream handling error", /*SuppressOnSink =*/true}; public: + static const StreamChecker *Instance; + + static const BugType *getBT_StreamEof() { return &Instance->BT_StreamEof; } + static const BugType *getBT_ResourceLeak() { + return &Instance->BT_ResourceLeak; + } + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; bool evalCall(const CallEvent &Call, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; @@ -337,7 +359,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 @@ -364,13 +387,12 @@ /// Generate a message for BugReporterVisitor if the stored symbol is /// marked as interesting by the actual bug report. struct NoteFn { - const CheckerNameRef CheckerName; SymbolRef StreamSym; std::string Message; std::string operator()(PathSensitiveBugReport &BR) const { if (BR.isInteresting(StreamSym) && - CheckerName == BR.getBugType().getCheckerName()) + &BR.getBugType() == StreamChecker::getBT_ResourceLeak()) return Message; return ""; @@ -379,7 +401,20 @@ const NoteTag *constructNoteTag(CheckerContext &C, SymbolRef StreamSym, const std::string &Message) const { - return C.getNoteTag(NoteFn{getCheckerName(), StreamSym, Message}); + return C.getNoteTag(NoteFn{StreamSym, Message}); + } + + const NoteTag *constructSetEofNoteTag(CheckerContext &C, + SymbolRef StreamSym) const { + return C.getNoteTag([StreamSym](PathSensitiveBugReport &BR) { + if (!BR.isInteresting(StreamSym) || + &BR.getBugType() != StreamChecker::getBT_StreamEof()) + return ""; + + BR.markNotInteresting(StreamSym); + + return "Assuming that stream reaches end-of-file here"; + }); } /// Searches for the ExplodedNode where the file descriptor was acquired for @@ -389,8 +424,13 @@ CheckerContext &C); }; +const StreamChecker *StreamChecker::Instance; + } // 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 +459,10 @@ return nullptr; } +//===----------------------------------------------------------------------===// +// Methods of StreamChecker. +//===----------------------------------------------------------------------===// + void StreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { const FnDescription *Desc = lookupFn(Call); @@ -566,7 +610,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); } @@ -668,7 +712,10 @@ // indicator for the stream is indeterminate. StreamState NewState = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); StateFailed = StateFailed->set(StreamSym, NewState); - C.addTransition(StateFailed); + if (IsFread && SS->ErrorState != ErrorFEof) + C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); + else + C.addTransition(StateFailed); } void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, @@ -727,7 +774,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 +1007,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,8 +1107,13 @@ return State; } +//===----------------------------------------------------------------------===// +// Checker registration. +//===----------------------------------------------------------------------===// + void ento::registerStreamChecker(CheckerManager &Mgr) { - Mgr.registerChecker(); + auto *Checker = Mgr.registerChecker(); + StreamChecker::Instance = Checker; } bool ento::shouldRegisterStreamChecker(const CheckerManager &Mgr) { @@ -1073,4 +1127,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 that 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 that 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 that 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); +}