commit 393f98b5b7f7c950d2b0a7d84501b5dfd00ad780
author Jordy Rose <jediknil@belkadan.com> Sun Mar 18 07:43:35 2012 +0000
committer Jordy Rose <jediknil@belkadan.com> Sun Mar 18 07:43:35 2012 +0000
tree 391c1596283c8fcac53d9d144870f05d64a87c48
parent 4207edaf03da28fa917a94b31c5c1eef0e6417dc

[analyzer] Mark a failed-realloc's result as an interesting symbol between the realloc call and the null check, so we get nicer path notes. Fixes a regression introduced by the diagnostic pruning added in r152361.

This is accomplished by calling markInteresting /during/ path diagnostic generation, and as such relies on deterministic ordering of BugReporterVisitors -- namely, that BugReporterVisitors are run in /reverse/ order from how they are added. (Right now that's a consequence of storing visitors in an ImmutableList, where new items are added to the front.) It's a little hacky, but it works for now.

I think this is the best we can do without storing the relation between the old and new symbols, and that would be a hit whether or not there ends up being an error.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@153010 91177308-0d34-0410-b5e6-96231b3b80d8

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 133482f..99b8489 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -846,6 +846,10 @@
   BugReport *R = new BugReport(*BT_Leak,
     "Memory is never released; potential memory leak", N, LocUsedForUniqueing);
   R->markInteresting(Sym);
+  // FIXME: This is a hack to make sure the MallocBugVisitor gets to look at
+  // the ExplodedNode chain first, in order to mark any failed realloc symbols
+  // as interesting for ConditionBRVisitor.
+  R->addVisitor(new ConditionBRVisitor());
   R->addVisitor(new MallocBugVisitor(Sym));
   C.EmitReport(R);
 }
@@ -1260,13 +1264,31 @@
   return State;
 }
 
+static SymbolRef findFailedReallocSymbol(ProgramStateRef currState,
+                                         ProgramStateRef prevState) {
+  ReallocMap currMap = currState->get<ReallocPairs>();
+  ReallocMap prevMap = prevState->get<ReallocPairs>();
+
+  for (ReallocMap::iterator I = prevMap.begin(), E = prevMap.end();
+       I != E; ++I) {
+    SymbolRef sym = I.getKey();
+    if (!currMap.lookup(sym))
+      return sym;
+  }
+
+  return NULL;
+}
+
 PathDiagnosticPiece *
 MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
                                            const ExplodedNode *PrevN,
                                            BugReporterContext &BRC,
                                            BugReport &BR) {
-  const RefState *RS = N->getState()->get<RegionState>(Sym);
-  const RefState *RSPrev = PrevN->getState()->get<RegionState>(Sym);
+  ProgramStateRef state = N->getState();
+  ProgramStateRef statePrev = PrevN->getState();
+
+  const RefState *RS = state->get<RegionState>(Sym);
+  const RefState *RSPrev = statePrev->get<RegionState>(Sym);
   if (!RS && !RSPrev)
     return 0;
 
@@ -1288,7 +1310,6 @@
     return 0;
 
   // Find out if this is an interesting point and what is the kind.
-  // TODO: Replace 'callee' by the function name.
   if (Mode == Normal) {
     if (isAllocated(RS, RSPrev, S)) {
       Msg = "Memory is allocated";
@@ -1303,6 +1324,9 @@
       Msg = "Reallocation failed";
       StackHint = new StackHintGeneratorForReallocationFailed(Sym,
                                                        "Reallocation failed");
+
+      if (SymbolRef sym = findFailedReallocSymbol(state, statePrev))
+        BR.markInteresting(sym);
     }
 
   // We are in a special mode if a reallocation failed later in the path.