[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.

llvm-svn: 153010

1 files changed, 27 insertions, 3 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 133482fcce6..99b84897a59 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -846,6 +846,10 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N,
   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 @@ MallocChecker::checkRegionChanges(ProgramStateRef State,
   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 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
     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 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
       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.