From ca8e36eb637e232475ef31c3f22d5da907390917 Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Thu, 23 Feb 2012 21:38:21 +0000 Subject: [PATCH] [analyzer] Malloc: unique leak reports by allocation site. When we find two leak reports with the same allocation site, report only one of them. Provide a helper method to BugReporter to facilitate this. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@151287 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/BugReporter/BugReporter.h | 13 ++++++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 40 +++++++++++++++++-- lib/StaticAnalyzer/Core/BugReporter.cpp | 4 +- test/Analysis/malloc.c | 4 +- 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index 633085527b..0978d86de4 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -71,6 +71,7 @@ protected: std::string ShortDescription; std::string Description; PathDiagnosticLocation Location; + PathDiagnosticLocation UniqueingLocation; const ExplodedNode *ErrorNode; SmallVector Ranges; ExtraTextList ExtraText; @@ -95,6 +96,18 @@ public: : BT(bt), Description(desc), Location(l), ErrorNode(0), Callbacks(F.getEmptyList()) {} + /// \brief Create a BugReport with a custom uniqueing location. + /// + /// The reports that have the same report location, description, bug type, and + /// ranges are uniqued - only one of the equivalent reports will be presented + /// to the user. This method allows to rest the location which should be used + /// for uniquing reports. For example, memory leaks checker, could set this to + /// the allocation site, rather then the location where the bug is reported. + BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode, + PathDiagnosticLocation LocationToUnique) + : BT(bt), Description(desc), UniqueingLocation(LocationToUnique), + ErrorNode(errornode), Callbacks(F.getEmptyList()) {} + virtual ~BugReport(); const BugType& getBugType() const { return BT; } diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 4ae1dd81ef..d6dc97a82c 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -186,6 +186,11 @@ private: static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR); void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range) const; + /// Find the location of the allocation for Sym on the path leading to the + /// exploded node N. + const Stmt *getAllocationSite(const ExplodedNode *N, SymbolRef Sym, + CheckerContext &C) const; + void reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const; /// The bug visitor which allows us to print extra diagnostics along the @@ -766,6 +771,24 @@ ProgramStateRef MallocChecker::CallocMem(CheckerContext &C, const CallExpr *CE){ return MallocMemAux(C, CE, TotalSize, zeroVal, state); } +const Stmt * +MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym, + CheckerContext &C) const { + // Walk the ExplodedGraph backwards and find the first node that referred to + // the tracked symbol. + const ExplodedNode *AllocNode = N; + + while (N) { + if (!N->getState()->get(Sym)) + break; + AllocNode = N; + N = N->pred_empty() ? NULL : *(N->pred_begin()); + } + + ProgramPoint P = AllocNode->getLocation(); + return cast(P).getStmt(); +} + void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const { assert(N); @@ -779,8 +802,16 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, BT_Leak->setSuppressOnSink(true); } + // Most bug reports are cached at the location where they occurred. + // With leaks, we want to unique them by the location where they were + // allocated, and only report a single path. + const Stmt *AllocStmt = getAllocationSite(N, Sym, C); + PathDiagnosticLocation LocUsedForUniqueing = + PathDiagnosticLocation::createBegin(AllocStmt, C.getSourceManager(), + N->getLocationContext()); + BugReport *R = new BugReport(*BT_Leak, - "Memory is never released; potential memory leak", N); + "Memory is never released; potential memory leak", N, LocUsedForUniqueing); R->addVisitor(new MallocBugVisitor(Sym)); C.EmitReport(R); } @@ -818,14 +849,17 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, } } - ExplodedNode *N = C.addTransition(state->set(RS)); + // Generate leak node. + static SimpleProgramPointTag Tag("MallocChecker : DeadSymbolsLeak"); + ExplodedNode *N = C.addTransition(C.getState(), C.getPredecessor(), &Tag); - if (N && generateReport) { + if (generateReport) { for (llvm::SmallVector::iterator I = Errors.begin(), E = Errors.end(); I != E; ++I) { reportLeak(*I, N, C); } } + C.addTransition(state->set(RS), N); } void MallocChecker::checkEndPath(CheckerContext &C) const { diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 0df71f21fb..0dbc7f115d 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -1270,7 +1270,9 @@ BugReport::~BugReport() { void BugReport::Profile(llvm::FoldingSetNodeID& hash) const { hash.AddPointer(&BT); hash.AddString(Description); - if (Location.isValid()) { + if (UniqueingLocation.isValid()) { + UniqueingLocation.Profile(hash); + } else if (Location.isValid()) { Location.Profile(hash); } else { assert(ErrorNode); diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index 4e42657c19..f9e3663494 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -379,7 +379,7 @@ void mallocEscapeMalloc() { void mallocMalloc() { int *p = malloc(12); - p = malloc(12); // expected-warning{{Memory is never released; potential memory leak}} + p = malloc(12); // expected-warning 2 {{Memory is never released; potential memory leak}} } void mallocFreeMalloc() { @@ -666,7 +666,7 @@ int testStrndup(const char *s, unsigned validIndex, unsigned size) { char *s2 = strndup(s, size); s2 [validIndex + 1] = 'b'; if (s2[validIndex] != 'a') - return 0;// expected-warning {{Memory is never released; potential memory leak}} + return 0; else return 1;// expected-warning {{Memory is never released; potential memory leak}} } -- 2.40.0