balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso,
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
balazske added a comment.
balazske added reviewers: baloghadamsoftware, NoQ.
Herald added a subscriber: rnkovacs.
Why does this not work?
I get only warning for the first resource leak (in the test `f_leak_2`). How to
fix this?
Report resource leaks with non-fatal error.
Report every resource leak.
Stream state is cleaned up at `checkDeadSymbols`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D82845
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream.c
Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ 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)
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ 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<SymbolRef, 2> &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<StreamMap>();
- 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<SymbolRef, 2> &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<PathSensitiveBugReport> R =
std::make_unique<PathSensitiveBugReport>(
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<SymbolRef, 2> LeakedSyms;
+
+ const StreamMapTy &Map = State->get<StreamMap>();
+ 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<StreamMap>(Sym);
+ }
+
+ ExplodedNode *N = C.getPredecessor();
+ if (!LeakedSyms.empty())
+ N = reportLeaks(LeakedSyms, C, N);
+
+ C.addTransition(State, N);
}
ProgramStateRef StreamChecker::checkPointerEscape(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
