diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 7e10b2aa435680..8fe097826fad08 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -216,8 +216,8 @@ class StreamChecker : public CheckergetState(); + // 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(StreamSym)) + N = N->getFirstPred(); + + const ExplodedNode *Pred = N; + while (N) { + State = N->getState(); + if (!State->get(StreamSym)) + 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 +469,8 @@ void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call, StateNull = StateNull->set(RetSym, StreamState::getOpenFailed(Desc)); - C.addTransition(StateNotNull); + C.addTransition(StateNotNull, + constructNoteTag(C, RetSym, "Stream opened here")); C.addTransition(StateNull); } @@ -476,7 +525,8 @@ void StreamChecker::evalFreopen(const FnDescription *Desc, StateRetNull = StateRetNull->set(StreamSym, StreamState::getOpenFailed(Desc)); - C.addTransition(StateRetNotNull); + C.addTransition(StateRetNotNull, + constructNoteTag(C, StreamSym, "Stream reopened here")); C.addTransition(StateRetNull); } @@ -921,8 +971,38 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, if (!N) continue; - C.emitReport(std::make_unique( - BT_ResourceLeak, BT_ResourceLeak.getDescription(), N)); + // Do not warn for non-closed stream at program exit. + ExplodedNode *Pred = C.getPredecessor(); + if (Pred && Pred->getCFGBlock() && + Pred->getCFGBlock()->hasNoReturnElement()) + continue; + + // Resource leaks can result in multiple warning that describe the same kind + // of programming error: + // void f() { + // FILE *F = fopen("a.txt"); + // if (rand()) // state split + // return; // warning + // } // warning + // While this isn't necessarily true (leaking the same stream could result + // 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); + assert(StreamOpenNode && "Could not find place of stream opening."); + PathDiagnosticLocation LocUsedForUniqueing = + PathDiagnosticLocation::createBegin( + StreamOpenNode->getStmtForDiagnostics(), C.getSourceManager(), + StreamOpenNode->getLocationContext()); + + std::unique_ptr R = + std::make_unique( + BT_ResourceLeak, BT_ResourceLeak.getDescription(), N, + LocUsedForUniqueing, + StreamOpenNode->getLocationContext()->getDecl()); + R->markInteresting(Sym); + C.emitReport(std::move(R)); } } diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c new file mode 100644 index 00000000000000..08927e8902be4b --- /dev/null +++ b/clang/test/Analysis/stream-note.c @@ -0,0 +1,48 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -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}} diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index dbbcc8715e0d28..cd5d28ae845546 100644 --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -verify %s #include "Inputs/system-header-simulator.h" @@ -139,7 +139,7 @@ void f_leak(int c) { 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); } @@ -240,3 +240,28 @@ void check_escape4() { fwrite("1", 1, 1, F); // expected-warning {{might be 'indeterminate'}} fclose(F); } + +int Test; +_Noreturn void handle_error(); + +void check_leak_noreturn_1() { + FILE *F1 = tmpfile(); + if (!F1) + return; + if (Test == 1) { + handle_error(); // no warning + } + rewind(F1); +} // expected-warning {{Opened stream never closed. Potential resource leak}} + +// Check that "location uniqueing" works. +// This results in reporting only one occurence of resource leak for a stream. +void check_leak_noreturn_2() { + FILE *F1 = tmpfile(); + if (!F1) + return; + if (Test == 1) { + return; // expected-warning {{Opened stream never closed. Potential resource leak}} + } + rewind(F1); +} // no warning diff --git a/clang/test/Analysis/stream.cpp b/clang/test/Analysis/stream.cpp index c76b9d7498d092..7eca505bcaf5d9 100644 --- a/clang/test/Analysis/stream.cpp +++ b/clang/test/Analysis/stream.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -verify %s typedef struct _IO_FILE FILE; extern FILE *fopen(const char *path, const char *mode); @@ -19,4 +19,4 @@ void f1() { void f2() { FILE *f = fopen("file", "r"); -} // expected-warning {{Opened File never closed. Potential Resource leak}} +} // expected-warning {{Opened stream never closed. Potential resource leak}}