balazske updated this revision to Diff 355594. balazske added a comment. Using NoteTag. Removed the EOF sequence number. Renamed test functions.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104925/new/ https://reviews.llvm.org/D104925 Files: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp clang/test/Analysis/stream-note.c
Index: clang/test/Analysis/stream-note.c =================================================================== --- clang/test/Analysis/stream-note.c +++ 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); +} Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ 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<void(const StreamChecker *, const FnDescription *, const CallEvent &, CheckerContext &)>; @@ -203,8 +218,8 @@ "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: @@ -337,7 +352,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 @@ -370,6 +386,7 @@ std::string operator()(PathSensitiveBugReport &BR) const { if (BR.isInteresting(StreamSym) && + BR.getBugType().getDescription() == Desc_ResourceLeak && CheckerName == BR.getBugType().getCheckerName()) return Message; @@ -391,8 +408,38 @@ } // 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) +namespace { + +struct SetEofNoteFn { + const CheckerNameRef CheckerName; + SymbolRef StreamSym; + const ExplodedNode *NotePosition; + + std::string operator()(PathSensitiveBugReport &BR) const { + if (!BR.isInteresting(StreamSym) || + BR.getBugType().getDescription() != Desc_StreamEof || + CheckerName != BR.getBugType().getCheckerName()) + return ""; + + const ExplodedNode *N = BR.getErrorNode(); + while (N && N != NotePosition) { + const StreamState *StreamS = N->getState()->get<StreamMap>(StreamSym); + if (!StreamS || !StreamS->ErrorState.FEof) + return ""; + N = N->getFirstPred(); + } + + return "Assuming that stream reaches end-of-file here"; + } +}; + +} // namespace + inline void assertStreamStateOpened(const StreamState *SS) { assert(SS->isOpened() && "Previous create of error node for non-opened stream failed?"); @@ -419,6 +466,10 @@ return nullptr; } +//===----------------------------------------------------------------------===// +// Methods of StreamChecker. +//===----------------------------------------------------------------------===// + void StreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { const FnDescription *Desc = lookupFn(Call); @@ -566,7 +617,7 @@ if (Sym && State->get<StreamMap>(Sym)) { const StreamState *SS = State->get<StreamMap>(Sym); if (SS->ErrorState & ErrorFEof) - reportFEofWarning(C, State); + reportFEofWarning(Sym, C, State); } else { C.addTransition(State); } @@ -668,7 +719,12 @@ // indicator for the stream is indeterminate. StreamState NewState = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState); - C.addTransition(StateFailed); + if (IsFread && SS->ErrorState != ErrorFEof) + C.addTransition(StateFailed, + C.getNoteTag(SetEofNoteFn{getCheckerName(), StreamSym, + C.getPredecessor()})); + else + C.addTransition(StateFailed); } void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, @@ -727,7 +783,9 @@ StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true)); C.addTransition(StateNotFailed); - C.addTransition(StateFailed); + C.addTransition(StateFailed, + C.getNoteTag(SetEofNoteFn{getCheckerName(), StreamSym, + C.getPredecessor()})); } void StreamChecker::evalClearerr(const FnDescription *Desc, @@ -960,14 +1018,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<PathSensitiveBugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( 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 +1118,10 @@ return State; } +//===----------------------------------------------------------------------===// +// Checker registration. +//===----------------------------------------------------------------------===// + void ento::registerStreamChecker(CheckerManager &Mgr) { Mgr.registerChecker<StreamChecker>(); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits