[clang] [analyzer] Fix StackAddrEscapeChecker crash on temporary object fields (PR #66493)

[Balazs Benics via cfe-commits]

https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/66493

>From cfdbc40487481b341d42f0472e196ff46666bd33 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Fri, 15 Sep 2023 12:42:39 +0200
Subject: [PATCH 1/2] [analyzer] Fix StackAddrEscapeChecker crash on temporary
 object fields

Basically, the issue was that we should have unwrap the base region
before we special handle temp object regions.

Fixes https://github.com/llvm/llvm-project/issues/66221
---
 .../Checkers/StackAddrEscapeChecker.cpp       |  6 +++--
 clang/test/Analysis/stackaddrleak.cpp         | 24 +++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Analysis/stackaddrleak.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 19ff8c8e2a171ae..23a774931b21dec 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -369,7 +369,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
                                   "Stack address stored into global variable");
 
   for (const auto &P : Cb.V) {
-    const MemRegion *Referrer = P.first;
+    const MemRegion *Referrer = P.first->getBaseRegion();
     const MemRegion *Referred = P.second;
 
     // Generate a report for this bug.
@@ -384,6 +384,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
           << CommonSuffix;
       auto Report =
           std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N);
+      if (Range.isValid())
+        Report->addRange(Range);
       Ctx.emitReport(std::move(Report));
       return;
     }
@@ -398,7 +400,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
     }(Referrer->getMemorySpace());
 
     // This cast supposed to succeed.
-    const VarRegion *ReferrerVar = cast<VarRegion>(Referrer->getBaseRegion());
+    const auto *ReferrerVar = cast<VarRegion>(Referrer);
     const std::string ReferrerVarName =
         ReferrerVar->getDecl()->getDeclName().getAsString();
 
diff --git a/clang/test/Analysis/stackaddrleak.cpp b/clang/test/Analysis/stackaddrleak.cpp
new file mode 100644
index 000000000000000..5828f2ac6e78c8d
--- /dev/null
+++ b/clang/test/Analysis/stackaddrleak.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+void *operator new(unsigned long, void *p) { return p; }
+
+struct myfunction {
+  union storage_t {
+    char buffer[100];
+    unsigned long long max_align;
+  } storage;
+
+  template <typename Func> myfunction(Func fn) {
+    new (&storage.buffer) Func(fn);
+  }
+  void operator()();
+};
+
+myfunction create_func() {
+  int n;
+  auto c = [&n] {};
+  return c; // expected-warning {{Address of stack memory associated with local variable 'n' is still referred to by a temporary object on the stack upon returning to the caller.  This will be a dangling reference}}
+}
+void gh_66221() {
+  create_func()();
+}

>From d569f78eb0cf3abbac13c7d8518173c4c08f4789 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Mon, 18 Sep 2023 08:43:03 +0200
Subject: [PATCH 2/2] Assert if asserted build, continue otherwise

---
 .../StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 23a774931b21dec..ea09c43cc5ce90d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -399,8 +399,14 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
       return "stack";
     }(Referrer->getMemorySpace());
 
-    // This cast supposed to succeed.
-    const auto *ReferrerVar = cast<VarRegion>(Referrer);
+    // 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");
+      continue; // Defensively skip this one.
+    }
     const std::string ReferrerVarName =
         ReferrerVar->getDecl()->getDeclName().getAsString();