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.
Bug reports of resource leak are now improved.
If there are multiple resource leak paths for the same stream,
only one wil be reported.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81407
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-note.c
clang/test/Analysis/stream.c
Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -139,7 +139,7 @@
if (!p)
return;
if(c)
- return; // expected-warning {{Opened File never closed. Potential Resource leak}}
+ return; // expected-warning {{Opened stream never closed. Potential resource leak}}
fclose(p);
}
Index: clang/test/Analysis/stream-note.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/stream-note.c
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+void check_note_at_correct_open() {
+ FILE *F1 = tmpfile(); // expected-note {{Stream opened here}}
+ if (!F1)
+ // expected-note@-1 {{'F1' is non-null}}
+ // expected-note@-2 {{Taking false branch}}
+ return;
+ FILE *F2 = tmpfile();
+ if (!F2) {
+ // expected-note@-1 {{'F2' is non-null}}
+ // expected-note@-2 {{Taking false branch}}
+ fclose(F1);
+ return;
+ }
+ rewind(F2);
+ fclose(F2);
+ rewind(F1);
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+
+void check_note_fopen() {
+ FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+ if (!F)
+ // expected-note@-1 {{'F' is non-null}}
+ // expected-note@-2 {{Taking false branch}}
+ return;
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+
+void check_note_freopen() {
+ FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+ if (!F)
+ // expected-note@-1 {{'F' is non-null}}
+ // expected-note@-2 {{Taking false branch}}
+ return;
+ F = freopen(0, "w", F); // expected-note {{Stream reopened here}}
+ if (!F)
+ // expected-note@-1 {{'F' is non-null}}
+ // expected-note@-2 {{Taking false branch}}
+ return;
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -216,8 +216,8 @@
"Read function called when stream is in EOF state. "
"Function has no effect."};
BuiltinBug BT_ResourceLeak{
- this, "Resource Leak",
- "Opened File never closed. Potential Resource leak."};
+ this, "Resource leak",
+ "Opened stream never closed. Potential resource leak."};
public:
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -365,6 +365,21 @@
return FnDescriptions.lookup(Call);
}
+
+ static const ExplodedNode *getAcquireSite(const ExplodedNode *N,
+ SymbolRef Sym, CheckerContext &Ctx);
+};
+
+struct NoteFn {
+ SymbolRef StreamSym;
+ std::string Message;
+
+ std::string operator()(PathSensitiveBugReport &BR) const {
+ if (BR.isInteresting(StreamSym))
+ return Message;
+
+ return "";
+ }
};
} // end anonymous namespace
@@ -376,6 +391,26 @@
"Previous create of error node for non-opened stream failed?");
}
+const ExplodedNode *StreamChecker::getAcquireSite(const ExplodedNode *N,
+ SymbolRef Sym,
+ CheckerContext &Ctx) {
+ ProgramStateRef State = N->getState();
+ // When bug type is resource leak, exploded node N may not have state info
+ // for leaked file descriptor, but predecessor should have it.
+ if (!State->get<StreamMap>(Sym))
+ N = N->getFirstPred();
+
+ const ExplodedNode *Pred = N;
+ while (N) {
+ State = N->getState();
+ if (!State->get<StreamMap>(Sym))
+ return Pred;
+ Pred = N;
+ N = N->getFirstPred();
+ }
+ return nullptr;
+}
+
void StreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
const FnDescription *Desc = lookupFn(Call);
@@ -421,7 +456,8 @@
StateNull =
StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
- C.addTransition(StateNotNull);
+ const NoteTag *T = C.getNoteTag(NoteFn{RetSym, "Stream opened here"});
+ C.addTransition(StateNotNull, T);
C.addTransition(StateNull);
}
@@ -476,7 +512,8 @@
StateRetNull =
StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc));
- C.addTransition(StateRetNotNull);
+ const NoteTag *T = C.getNoteTag(NoteFn{StreamSym, "Stream reopened here"});
+ C.addTransition(StateRetNotNull, T);
C.addTransition(StateRetNull);
}
@@ -921,8 +958,20 @@
if (!N)
continue;
- C.emitReport(std::make_unique<PathSensitiveBugReport>(
- BT_ResourceLeak, BT_ResourceLeak.getDescription(), N));
+ ;
+ const ExplodedNode *AcquireNode = getAcquireSite(N, Sym, C);
+ assert(AcquireNode && "Could not find place of stream opening.");
+ PathDiagnosticLocation LocUsedForUniqueing =
+ PathDiagnosticLocation::createBegin(
+ AcquireNode->getStmtForDiagnostics(), C.getSourceManager(),
+ AcquireNode->getLocationContext());
+
+ std::unique_ptr<PathSensitiveBugReport> R =
+ std::make_unique<PathSensitiveBugReport>(
+ BT_ResourceLeak, BT_ResourceLeak.getDescription(), N,
+ LocUsedForUniqueing, AcquireNode->getLocationContext()->getDecl());
+ R->markInteresting(Sym);
+ C.emitReport(std::move(R));
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
