[analyzer] Allow checkers to supply call stack diagnostic hints for the

BugVisitor DiagnosticPieces.

When checkers create a DiagnosticPieceEvent, they can supply an extra
string, which will be concatenated with the call exit message for every
call on the stack between the diagnostic event and the final bug report.
(This is a simple version, which could be/will be further enhanced.)

For example, this is used in Malloc checker to produce the ",
which allocated memory" in the following example:

static char *malloc_wrapper() { // 2. Entered call from 'use'
    return malloc(12);    // 3. Memory is allocated
}

void use() {
    char *v;
    v = malloc_wrapper(); // 1. Calling 'malloc_wrappers'
        // 4. Returning from 'malloc_wrapper', which allocated memory
}                         // 5. Memory is never released; potential
memory leak

llvm-svn: 152837

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

@@ -359,10 +359,21 @@ public:
 
 class PathDiagnosticEventPiece : public PathDiagnosticSpotPiece {
   llvm::Optional<bool> IsPrunable;
+
+  /// If the event occurs in a different frame than the final diagnostic,
+  /// supply a message that will be used to construct an extra hint on the
+  /// returns from all the calls on the stack from this event to the final
+  /// diagnostic.
+  /// TODO: This should be a callback that constructs a string given the
+  /// ExplodedNode, which would allow the checkers to refer to the expression.
+  std::string CallStackMessage;
+
 public:
   PathDiagnosticEventPiece(const PathDiagnosticLocation &pos,
-                           StringRef s, bool addPosRange = true)
-    : PathDiagnosticSpotPiece(pos, s, Event, addPosRange) {}
+                           StringRef s, bool addPosRange = true,
+                           StringRef callStackMsg = "")
+    : PathDiagnosticSpotPiece(pos, s, Event, addPosRange),
+      CallStackMessage(callStackMsg) {}
 
   ~PathDiagnosticEventPiece();
 
@@ -380,6 +391,13 @@ public:
     return IsPrunable.hasValue() ? IsPrunable.getValue() : false;
   }
   
+  StringRef getCallStackMessage() {
+    if (!CallStackMessage.empty())
+      return CallStackMessage;
+    else
+      return StringRef();
+  }
+
   static inline bool classof(const PathDiagnosticPiece *P) {
     return P->getKind() == Event;
   }
@@ -402,6 +420,10 @@ class PathDiagnosticCallPiece : public PathDiagnosticPiece {
   // call exit.
   bool NoExit;
 
+  // The custom string, which should appear after the call Return Diagnostic.
+  // TODO: Should we allow multiple diagnostics?
+  std::string CallStackMessage;
+
 public:
   PathDiagnosticLocation callEnter;
   PathDiagnosticLocation callEnterWithin;
@@ -415,6 +437,11 @@ public:
   const Decl *getCallee() const { return Callee; }
   void setCallee(const CallEnter &CE, const SourceManager &SM);
   
+  bool hasCallStackMessage() { return !CallStackMessage.empty(); }
+  void setCallStackMessage(StringRef st) {
+    CallStackMessage = st;
+  }
+
   virtual PathDiagnosticLocation getLocation() const {
     return callEnter;
   }

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -1249,6 +1249,7 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
 
   const Stmt *S = 0;
   const char *Msg = 0;
+  const char *StackMsg = 0;
 
   // Retrieve the associated statement.
   ProgramPoint ProgLoc = N->getLocation();
@@ -1264,14 +1265,18 @@ 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))
+    if (isAllocated(RS, RSPrev, S)) {
       Msg = "Memory is allocated";
-    else if (isReleased(RS, RSPrev, S))
+      StackMsg = ", which allocated memory";
+    } else if (isReleased(RS, RSPrev, S)) {
       Msg = "Memory is released";
-    else if (isReallocFailedCheck(RS, RSPrev, S)) {
+      StackMsg = ", which released memory";
+    } else if (isReallocFailedCheck(RS, RSPrev, S)) {
       Mode = ReallocationFailed;
       Msg = "Reallocation failed";
+      StackMsg = ", where reallocation failed";
     }
 
   // We are in a special mode if a reallocation failed later in the path.
@@ -1291,16 +1296,18 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
     if (!(FunName.equals("realloc") || FunName.equals("reallocf")))
       return 0;
     Msg = "Attempt to reallocate memory";
+    StackMsg = ", which attempted to reallocate memory";
     Mode = Normal;
   }
 
   if (!Msg)
     return 0;
+  assert(StackMsg);
 
   // Generate the extra diagnostic.
   PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
                              N->getLocationContext());
-  return new PathDiagnosticEventPiece(Pos, Msg);
+  return new PathDiagnosticEventPiece(Pos, Msg, true, StackMsg);
 }
 
 

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -380,6 +380,24 @@ PathDiagnosticBuilder::getEnclosingStmtLocation(const Stmt *S) {
 //===----------------------------------------------------------------------===//
 // "Minimal" path diagnostic generation algorithm.
 //===----------------------------------------------------------------------===//
+static void updateStackPiecesWithMessage(PathDiagnosticPiece *P,
+                   llvm::SmallVector<PathDiagnosticCallPiece*, 6> &CallStack) {
+  // If the piece contains a special message, add it to all the call
+  // pieces on the active stack.
+  if (PathDiagnosticEventPiece *ep =
+        dyn_cast<PathDiagnosticEventPiece>(P)) {
+    StringRef stackMsg = ep->getCallStackMessage();
+
+    if (!stackMsg.empty())
+      for (llvm::SmallVector<PathDiagnosticCallPiece*, 6>::iterator
+             I = CallStack.begin(), E = CallStack.end(); I != E; ++I)
+        // The last message on the path to final bug is the most important
+        // one. Since we traverse the path backwards, do not add the message
+        // if one has been previously added.
+        if  (!(*I)->hasCallStackMessage())
+          (*I)->setCallStackMessage(stackMsg);
+  }
+}
 
 static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM);
 
@@ -391,6 +409,9 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
   const LocationContext *LC = PDB.LC;
   const ExplodedNode *NextNode = N->pred_empty()
                                         ? NULL : *(N->pred_begin());
+
+  llvm::SmallVector<PathDiagnosticCallPiece*, 6> CallStack;
+
   while (NextNode) {
     N = NextNode;
     PDB.LC = N->getLocationContext();
@@ -403,6 +424,7 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
         PathDiagnosticCallPiece::construct(N, *CE, SMgr);
       PD.getActivePath().push_front(C);
       PD.pushActivePath(&C->path);
+      CallStack.push_back(C);
       continue;      
     }
     
@@ -423,6 +445,10 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
         C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller);
       }
       C->setCallee(*CE, SMgr);
+      if (!CallStack.empty()) {
+        assert(CallStack.back() == C);
+        CallStack.pop_back();
+      }
       continue;
     }
 
@@ -681,8 +707,10 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
       BugReport *R = PDB.getBugReport();
       for (BugReport::visitor_iterator I = R->visitor_begin(),
            E = R->visitor_end(); I!=E; ++I) {
-        if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB, *R))
+        if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB, *R)) {
           PD.getActivePath().push_front(p);
+          updateStackPiecesWithMessage(p, CallStack);
+        }
       }
     }
   }
@@ -1019,6 +1047,7 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
                                             const ExplodedNode *N) {
   EdgeBuilder EB(PD, PDB);
   const SourceManager& SM = PDB.getSourceManager();
+  llvm::SmallVector<PathDiagnosticCallPiece*, 6> CallStack;
 
   const ExplodedNode *NextNode = N->pred_empty() ? NULL : *(N->pred_begin());
   while (NextNode) {
@@ -1039,6 +1068,7 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
           PathDiagnosticCallPiece::construct(N, *CE, SM);
         PD.getActivePath().push_front(C);
         PD.pushActivePath(&C->path);
+        CallStack.push_back(C);
         break;
       }
       
@@ -1072,6 +1102,11 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
         }
         C->setCallee(*CE, SM);
         EB.addContext(CE->getCallExpr());
+
+        if (!CallStack.empty()) {
+          assert(CallStack.back() == C);
+          CallStack.pop_back();
+        }
         break;
       }
       
@@ -1147,6 +1182,8 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
         const PathDiagnosticLocation &Loc = p->getLocation();
         EB.addEdge(Loc, true);
         PD.getActivePath().push_front(p);
+        updateStackPiecesWithMessage(p, CallStack);
+
         if (const Stmt *S = Loc.asStmt())
           EB.addExtendedContext(PDB.getEnclosingStmtLocation(S).asStmt());
       }

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

@@ -572,6 +572,8 @@ PathDiagnosticCallPiece::getCallExitEvent() const {
     Out << "Returning from '" << *ND << "'";
   else
     Out << "Returning to caller";
+  if (!CallStackMessage.empty())
+    Out << CallStackMessage;
   return new PathDiagnosticEventPiece(callReturn, Out.str());
 }