Tweak stack address checker to report multiple cases where globals may reference stack memory.

Also refactor the diagnostics so that we print out the kind of stack memory returned.

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

1 files changed, 65 insertions, 48 deletions

diff --git a/lib/Checker/StackAddrLeakChecker.cpp b/lib/Checker/StackAddrLeakChecker.cpp
index 34a06fb500..f4a9db62df 100644
--- a/lib/Checker/StackAddrLeakChecker.cpp
+++ b/lib/Checker/StackAddrLeakChecker.cpp
@@ -35,65 +35,73 @@ public:
   void EvalEndPath(GREndPathNodeBuilder &B, void *tag, GRExprEngine &Eng);
 private:
   void EmitStackError(CheckerContext &C, const MemRegion *R, const Expr *RetE);
+  SourceRange GenName(llvm::raw_ostream &os, const MemRegion *R,
+                      SourceManager &SM);
 };
 }
 
 void clang::RegisterStackAddrLeakChecker(GRExprEngine &Eng) {
   Eng.registerCheck(new StackAddrLeakChecker());
 }
-void StackAddrLeakChecker::EmitStackError(CheckerContext &C, const MemRegion *R,
-                                          const Expr *RetE) {
-  ExplodedNode *N = C.GenerateSink();
-
-  if (!N)
-    return;
 
-  if (!BT_returnstack)
-   BT_returnstack=new BuiltinBug("Return of address to stack-allocated memory");
-
-  // Generate a report for this bug.
-  llvm::SmallString<512> buf;
-  llvm::raw_svector_ostream os(buf);
-  SourceRange range;
-
-  // Get the base region, stripping away fields and elements.
+SourceRange StackAddrLeakChecker::GenName(llvm::raw_ostream &os,
+                                          const MemRegion *R,
+                                          SourceManager &SM) {
+    // Get the base region, stripping away fields and elements.
   R = R->getBaseRegion();
-
+  SourceRange range;
+  os << "Address of ";
+  
   // Check if the region is a compound literal.
   if (const CompoundLiteralRegion* CR = dyn_cast<CompoundLiteralRegion>(R)) { 
     const CompoundLiteralExpr* CL = CR->getLiteralExpr();
-    os << "Address of stack memory associated with a compound literal "
-      "declared on line "
-       << C.getSourceManager().getInstantiationLineNumber(CL->getLocStart())
-       << " returned to caller";    
+    os << "stack memory associated with a compound literal "
+          "declared on line "
+        << SM.getInstantiationLineNumber(CL->getLocStart())
+        << " returned to caller";    
     range = CL->getSourceRange();
   }
   else if (const AllocaRegion* AR = dyn_cast<AllocaRegion>(R)) {
     const Expr* ARE = AR->getExpr();
     SourceLocation L = ARE->getLocStart();
     range = ARE->getSourceRange();    
-    os << "Address of stack memory allocated by call to alloca() on line "
-       << C.getSourceManager().getInstantiationLineNumber(L)
-       << " returned to caller";
+    os << "stack memory allocated by call to alloca() on line "
+       << SM.getInstantiationLineNumber(L);
   }
   else if (const BlockDataRegion *BR = dyn_cast<BlockDataRegion>(R)) {
     const BlockDecl *BD = BR->getCodeRegion()->getDecl();
     SourceLocation L = BD->getLocStart();
     range = BD->getSourceRange();
-    os << "Address of stack-allocated block declared on line "
-       << C.getSourceManager().getInstantiationLineNumber(L)
-       << " returned to caller";
+    os << "stack-allocated block declared on line "
+       << SM.getInstantiationLineNumber(L);
   }
   else if (const VarRegion *VR = dyn_cast<VarRegion>(R)) {
-    os << "Address of stack memory associated with local variable '"
-       << VR->getString() << "' returned";
+    os << "stack memory associated with local variable '"
+       << VR->getString() << '\'';
     range = VR->getDecl()->getSourceRange();
   }
   else {
     assert(false && "Invalid region in ReturnStackAddressChecker.");
+  } 
+  
+  return range;
+}
+
+void StackAddrLeakChecker::EmitStackError(CheckerContext &C, const MemRegion *R,
+                                          const Expr *RetE) {
+  ExplodedNode *N = C.GenerateSink();
+
+  if (!N)
     return;
-  }
 
+  if (!BT_returnstack)
+   BT_returnstack=new BuiltinBug("Return of address to stack-allocated memory");
+
+  // Generate a report for this bug.
+  llvm::SmallString<512> buf;
+  llvm::raw_svector_ostream os(buf);
+  SourceRange range = GenName(os, R, C.getSourceManager());
+  os << " returned to caller";
   RangedBugReport *report = new RangedBugReport(*BT_returnstack, os.str(), N);
   report->addRange(RetE->getSourceRange());
   if (range.isValid())
@@ -132,10 +140,10 @@ void StackAddrLeakChecker::EvalEndPath(GREndPathNodeBuilder &B, void *tag,
   private:
     const StackFrameContext *CurSFC;
   public:
-    const MemRegion *src, *dst;    
+    llvm::SmallVector<std::pair<const MemRegion*, const MemRegion*>, 10> V;
 
     CallBack(const LocationContext *LCtx)
-      : CurSFC(LCtx->getCurrentStackFrame()), src(0), dst(0) {}
+      : CurSFC(LCtx->getCurrentStackFrame()) {}
     
     bool HandleBinding(StoreManager &SMgr, Store store,
                        const MemRegion *region, SVal val) {
@@ -151,11 +159,8 @@ void StackAddrLeakChecker::EvalEndPath(GREndPathNodeBuilder &B, void *tag,
           dyn_cast<StackSpaceRegion>(vR->getMemorySpace())) {
         // If the global variable holds a location in the current stack frame,
         // record the binding to emit a warning.
-        if (SSR->getStackFrame() == CurSFC) {        
-          src = region;
-          dst = vR;
-          return false;
-        }
+        if (SSR->getStackFrame() == CurSFC)
+          V.push_back(std::make_pair(region, vR));
       }
       
       return true;
@@ -164,24 +169,36 @@ void StackAddrLeakChecker::EvalEndPath(GREndPathNodeBuilder &B, void *tag,
     
   CallBack cb(B.getPredecessor()->getLocationContext());
   state->getStateManager().getStoreManager().iterBindings(state->getStore(),cb);
-  
-  // Did we find any globals referencing stack memory?
-  if (!cb.src)
+
+  if (cb.V.empty())
     return;
 
   // Generate an error node.
   ExplodedNode *N = B.generateNode(state, tag, B.getPredecessor());
   if (!N)
     return;
-  
+
   if (!BT_stackleak)
-    BT_stackleak = new BuiltinBug("Stack address stored into global variable",
-                      "Stack address was saved into a global variable. "
-                      "is dangerous because the address will become invalid "
-                      "after returning from the function");
+    BT_stackleak =
+      new BuiltinBug("Stack address stored into global variable",
+                     "Stack address was saved into a global variable. "
+                     "This is dangerous because the address will become "
+                     "invalid after returning from the function");
   
-  BugReport *R =
-    new BugReport(*BT_stackleak,  BT_stackleak->getDescription(), N);
-
-  Eng.getBugReporter().EmitReport(R);
+  for (unsigned i = 0, e = cb.V.size(); i != e; ++i) {
+    // Generate a report for this bug.
+    llvm::SmallString<512> buf;
+    llvm::raw_svector_ostream os(buf);
+    SourceRange range = GenName(os, cb.V[i].second,
+                                Eng.getContext().getSourceManager());
+    os << " is still referred to by the global variable '";
+    const VarRegion *VR = cast<VarRegion>(cb.V[i].first->getBaseRegion());
+    os << VR->getDecl()->getNameAsString() 
+       << "' upon returning to the caller.  This will be a dangling reference";
+    RangedBugReport *report = new RangedBugReport(*BT_stackleak, os.str(), N);
+    if (range.isValid())
+      report->addRange(range);
+
+    Eng.getBugReporter().EmitReport(report);
+  }
 }