[clang] 9b7c43d - [Analyzer][StreamChecker] Report every leak, clean up state.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 20 02:46:39 PDT 2020
Author: Balázs Kéri
Date: 2020-07-20T11:49:00+02:00
New Revision: 9b7c43d341da319c69b11205ee1deb642f286e59
URL: https://github.com/llvm/llvm-project/commit/9b7c43d341da319c69b11205ee1deb642f286e59
DIFF: https://github.com/llvm/llvm-project/commit/9b7c43d341da319c69b11205ee1deb642f286e59.diff
LOG: [Analyzer][StreamChecker] Report every leak, clean up state.
Summary:
Report resource leaks with non-fatal error.
Report every resource leak.
Stream state is cleaned up at `checkDeadSymbols`.
Reviewers: Szelethus, baloghadamsoftware, NoQ
Reviewed By: Szelethus
Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D82845
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-note.c
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index f6abbe4f8f03..1e963249156f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -337,6 +337,12 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
/// 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,19 @@ void StreamChecker::reportFEofWarning(CheckerContext &C,
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.
+ // FIXME: Use BugType::SuppressOnSink instead.
+ 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 +986,7 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
// from a
diff erent 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 +996,38 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
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(
diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c
index 08927e8902be..71a5ba2315d9 100644
--- a/clang/test/Analysis/stream-note.c
+++ b/clang/test/Analysis/stream-note.c
@@ -46,3 +46,34 @@ void check_note_freopen() {
}
// expected-warning at -1 {{Opened stream never closed. Potential resource leak}}
// expected-note at -2 {{Opened stream never closed. Potential resource leak}}
+
+void check_note_leak_2(int c) {
+ FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
+ if (!F1)
+ // expected-note at -1 {{'F1' is non-null}}
+ // expected-note at -2 {{Taking false branch}}
+ // expected-note at -3 {{'F1' is non-null}}
+ // expected-note at -4 {{Taking false branch}}
+ return;
+ FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
+ if (!F2) {
+ // expected-note at -1 {{'F2' is non-null}}
+ // expected-note at -2 {{Taking false branch}}
+ // expected-note at -3 {{'F2' is non-null}}
+ // expected-note at -4 {{Taking false branch}}
+ fclose(F1);
+ return;
+ }
+ if (c)
+ // expected-note at -1 {{Assuming 'c' is not equal to 0}}
+ // expected-note at -2 {{Taking true branch}}
+ // expected-note at -3 {{Assuming 'c' is not equal to 0}}
+ // expected-note at -4 {{Taking true branch}}
+ return;
+ // expected-warning at -1 {{Opened stream never closed. Potential resource leak}}
+ // expected-note at -2 {{Opened stream never closed. Potential resource leak}}
+ // expected-warning at -3 {{Opened stream never closed. Potential resource leak}}
+ // expected-note at -4 {{Opened stream never closed. Potential resource leak}}
+ fclose(F1);
+ fclose(F2);
+}
More information about the cfe-commits
mailing list