[analyzer] Detect leak of a stack address through output arguments 2/3

clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp

@@ -288,6 +288,23 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
   EmitStackError(C, R, RetE);
 }
 
+static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) {
+  assert(R);
+  if (const auto *MemSpace = R->getMemorySpace()) {
+    if (const auto *SSR = MemSpace->getAs<StackSpaceRegion>())
+      return SSR;
+    if (const auto *GSR = MemSpace->getAs<GlobalsSpaceRegion>())
+      return GSR;
+  }
+  // If R describes a lambda capture, it will be a symbolic region
+  // referring to a field region of another symbolic region.
+  if (const auto *SymReg = R->getBaseRegion()->getAs<SymbolicRegion>()) {
+    if (const auto *OriginReg = SymReg->getSymbol()->getOriginRegion())
+      return getStackOrGlobalSpaceRegion(OriginReg);
+  }
+  return nullptr;
+}
+
 std::optional<std::string> printReferrer(const MemRegion *Referrer) {
   assert(Referrer);
   const StringRef ReferrerMemorySpace = [](const MemSpaceRegion *Space) {
@@ -297,20 +314,31 @@ std::optional<std::string> printReferrer(const MemRegion *Referrer) {
       return "global";
     assert(isa<StackSpaceRegion>(Space));
     return "stack";
-  }(Referrer->getMemorySpace());
-
-  // We should really only have VarRegions here.
-  // Anything else is really surprising, and we should get notified if such
-  // ever happens.
-  const auto *ReferrerVar = dyn_cast<VarRegion>(Referrer);
-  if (!ReferrerVar) {
-    assert(false && "We should have a VarRegion here");
-    return std::nullopt; // Defensively skip this one.
+  }(getStackOrGlobalSpaceRegion(Referrer));
+
+  while (!Referrer->canPrintPretty()) {
+    if (const auto *SymReg = dyn_cast<SymbolicRegion>(Referrer);
+        SymReg && SymReg->getSymbol()->getOriginRegion()) {
+      Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
+    } else if (isa<CXXThisRegion>(Referrer)) {
+      // Skip members of a class, it is handled in CheckExprLifetime.cpp as
+      // warn_bind_ref_member_to_parameter or
+      // warn_init_ptr_member_to_parameter_addr
+      return std::nullopt;
+    } else {
+      Referrer->dump();
+      assert(false && "Unexpected referrer region type.");
+      return std::nullopt;
+    }
   }
-  const std::string ReferrerVarName =
-      ReferrerVar->getDecl()->getDeclName().getAsString();
+  assert(Referrer);
+  assert(Referrer->canPrintPretty());
 
-  return (ReferrerMemorySpace + " variable '" + ReferrerVarName + "'").str();
+  std::string buf;
+  llvm::raw_string_ostream os(buf);
+  os << ReferrerMemorySpace << " variable ";
+  Referrer->printPretty(os);
+  return buf;
 }
 
 void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
@@ -332,16 +360,20 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
     /// referred by an other stack variable from different stack frame.
     bool checkForDanglingStackVariable(const MemRegion *Referrer,
                                        const MemRegion *Referred) {
-      const auto *ReferrerMemSpace =
-          Referrer->getMemorySpace()->getAs<StackSpaceRegion>();
+      const auto *ReferrerMemSpace = getStackOrGlobalSpaceRegion(Referrer);
       const auto *ReferredMemSpace =
           Referred->getMemorySpace()->getAs<StackSpaceRegion>();
 
       if (!ReferrerMemSpace || !ReferredMemSpace)
         return false;
 
+      const auto *ReferrerStackSpace =
+          ReferrerMemSpace->getAs<StackSpaceRegion>();
+      if (!ReferrerStackSpace)
+        return false;
+
       if (ReferredMemSpace->getStackFrame() == PoppedFrame &&
-          ReferrerMemSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+          ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
         V.emplace_back(Referrer, Referred);
         return true;
       }
@@ -387,7 +419,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
   if (!BT_stackleak)
     BT_stackleak =
         std::make_unique<BugType>(CheckNames[CK_StackAddrEscapeChecker],
-                                  "Stack address stored into global variable");
+                                  "Stack address leaks outside of stack frame");
 
   for (const auto &P : Cb.V) {
     const MemRegion *Referrer = P.first->getBaseRegion();

clang/test/Analysis/stack-addr-ps.cpp

@@ -1,7 +1,10 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s -Wno-undefined-bool-conversion
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s -Wno-undefined-bool-conversion
 
 typedef __INTPTR_TYPE__ intptr_t;
 
+template <typename T>
+void clang_analyzer_dump(T x);
+
 const int& g() {
   int s;
   return s; // expected-warning{{Address of stack memory associated with local variable 's' returned}} expected-warning{{reference to stack memory associated with local variable 's' returned}}
@@ -321,7 +324,7 @@ void param_ptr_to_ptr_to_ptr_top(void*** ppp) {
 
 void param_ptr_to_ptr_to_ptr_callee(void*** ppp) {
   int local = 42;
-  **ppp = &local; // no-warning FIXME
+  **ppp = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'pp'}}
 }
 
 void param_ptr_to_ptr_to_ptr_caller(void** pp) {
@@ -331,7 +334,7 @@ void param_ptr_to_ptr_to_ptr_caller(void** pp) {
 void lambda_to_context_ptr_to_ptr(int **pp) {
   auto lambda = [&] {
     int local = 42;
-    *pp = &local; // no-warning FIXME
+    *pp = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'pp'}}
   };
   lambda();
   (void)*pp;
@@ -734,7 +737,7 @@ void param_nested_and_transitive_top(NestedAndTransitive* nat) {
 
 void param_nested_and_transitive_callee(NestedAndTransitive* nat) {
   int local = 42;
-  *nat->next[2]->next[1]->p = &local; // no-warning FIXME
+  *nat->next[2]->next[1]->p = &local;  // expected-warning{{local variable 'local' is still referred to by the stack variable 'natCaller'}}
 }
 
 void param_nested_and_transitive_caller(NestedAndTransitive natCaller) {
@@ -757,3 +760,23 @@ class CPtr {
   }
 };
 } // namespace leaking_as_member
+
+namespace origin_region_limitation {
+void leaker(int ***leakerArg) {
+    int local;
+    clang_analyzer_dump(*leakerArg); // expected-warning{{&SymRegion{reg_$0<int ** arg>}}}
+    // Incorrect message: 'arg', after it is reinitialized with value returned by 'tweak'
+    // is no longer relevant.
+    // The message must refer to 'original_arg' instead, but there is no easy way to
+    // connect the SymRegion stored in 'original_arg' and 'original_arg' as variable.
+    **leakerArg = &local; // expected-warning{{ 'local' is still referred to by the stack variable 'arg'}}
+}
+
+int **tweak();
+
+void foo(int **arg) {
+    int **original_arg = arg;
+    arg = tweak();
+    leaker(&original_arg);
+}
+} // namespace origin_region_limitation