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 @@ -337,6 +337,12 @@ /// to ensure uniform handling. void reportFEofWarning(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 + /// were generated). Return value is non-null. + ExplodedNode *reportLeaks(const SmallVector &LeakedSyms, + CheckerContext &C, ExplodedNode *Pred) 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 { @@ -956,28 +962,18 @@ C.addTransition(State); } -void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, - CheckerContext &C) const { - ProgramStateRef State = C.getState(); - - // TODO: Clean up the state. - const StreamMapTy &Map = State->get(); - for (const auto &I : Map) { - SymbolRef Sym = I.first; - const StreamState &SS = I.second; - if (!SymReaper.isDead(Sym) || !SS.isOpened()) - continue; - - ExplodedNode *N = C.generateErrorNode(); - if (!N) - continue; +ExplodedNode * +StreamChecker::reportLeaks(const SmallVector &LeakedSyms, + CheckerContext &C, ExplodedNode *Pred) const { + // Do not warn for non-closed stream at program exit. + if (Pred && Pred->getCFGBlock() && Pred->getCFGBlock()->hasNoReturnElement()) + return Pred; - // Do not warn for non-closed stream at program exit. - ExplodedNode *Pred = C.getPredecessor(); - if (Pred && Pred->getCFGBlock() && - Pred->getCFGBlock()->hasNoReturnElement()) - continue; + ExplodedNode *Err = C.generateNonFatalErrorNode(C.getState(), Pred); + if (!Err) + return Pred; + for (SymbolRef LeakSym : LeakedSyms) { // Resource leaks can result in multiple warning that describe the same kind // of programming error: // void f() { @@ -989,8 +985,7 @@ // from a different kinds of errors), the reduction in redundant reports // makes this a worthwhile heuristic. // FIXME: Add a checker option to turn this uniqueing feature off. - - const ExplodedNode *StreamOpenNode = getAcquisitionSite(N, Sym, C); + const ExplodedNode *StreamOpenNode = getAcquisitionSite(Err, LeakSym, C); assert(StreamOpenNode && "Could not find place of stream opening."); PathDiagnosticLocation LocUsedForUniqueing = PathDiagnosticLocation::createBegin( @@ -1000,12 +995,38 @@ std::unique_ptr R = std::make_unique( BT_ResourceLeak, - "Opened stream never closed. Potential resource leak.", N, + "Opened stream never closed. Potential resource leak.", Err, LocUsedForUniqueing, StreamOpenNode->getLocationContext()->getDecl()); - R->markInteresting(Sym); + R->markInteresting(LeakSym); C.emitReport(std::move(R)); } + + return Err; +} + +void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + llvm::SmallVector LeakedSyms; + + const StreamMapTy &Map = State->get(); + for (const auto &I : Map) { + SymbolRef Sym = I.first; + const StreamState &SS = I.second; + if (!SymReaper.isDead(Sym)) + continue; + if (SS.isOpened()) + LeakedSyms.push_back(Sym); + State = State->remove(Sym); + } + + ExplodedNode *N = C.getPredecessor(); + if (!LeakedSyms.empty()) + N = reportLeaks(LeakedSyms, C, N); + + C.addTransition(State, N); } ProgramStateRef StreamChecker::checkPointerEscape( 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 @@ -134,7 +134,7 @@ fclose(p); } -void f_leak(int c) { +void f_leak_1(int c) { FILE *p = fopen("foo.c", "r"); if (!p) return; @@ -143,6 +143,23 @@ fclose(p); } +void f_leak_2(int c) { + FILE *p1 = fopen("foo1.c", "r"); + if (!p1) + return; + FILE *p2 = fopen("foo2.c", "r"); + if (!p2) { + fclose(p1); + return; + } + if (c) + return; + // expected-warning@-1 {{Opened stream never closed. Potential resource leak}} + // expected-warning@-2 {{Opened stream never closed. Potential resource leak}} + fclose(p1); + fclose(p2); +} + FILE *f_null_checked(void) { FILE *p = fopen("foo.c", "r"); if (p)