[clang] b55dd8f - [clang][analyzer] Correctly handle structured bindings captured by lambda (#132579)

[via cfe-commits]

Author: flovent
Date: 2025-03-26T16:03:43+01:00
New Revision: b55dd8f607dab5b122e09836022a37ef10c8c653

URL: https://github.com/llvm/llvm-project/commit/b55dd8f607dab5b122e09836022a37ef10c8c653
DIFF: https://github.com/llvm/llvm-project/commit/b55dd8f607dab5b122e09836022a37ef10c8c653.diff

LOG: [clang][analyzer] Correctly handle structured bindings captured by lambda (#132579)

this PR fixes #91835.

For `DeclRefExpr` in lambda's function body, it will references to
original variable declaration in AST rather than `FieldDecl` for lambda
class, so it's needed to find the corresponding `FieldDecl` and bind
`DeclRefExpr`'s value to it.

This is already implemented for variables that are not in a structured
binding structure, so I extracted that part of the code so that it can
be used in the structured binding case.

Added: 
    clang/test/Analysis/issue-91835.cpp
    clang/test/Analysis/lambda-capture-structured-binding.cpp

Modified: 
    clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 12a5b248c843f..86e2e8f634bfd 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3129,16 +3129,10 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
   ProgramStateRef state = Pred->getState();
   const LocationContext *LCtx = Pred->getLocationContext();
 
-  if (const auto *VD = dyn_cast<VarDecl>(D)) {
-    // C permits "extern void v", and if you cast the address to a valid type,
-    // you can even do things with it. We simply pretend
-    assert(Ex->isGLValue() || VD->getType()->isVoidType());
-    const LocationContext *LocCtxt = Pred->getLocationContext();
-    const Decl *D = LocCtxt->getDecl();
-    const auto *MD = dyn_cast_or_null<CXXMethodDecl>(D);
+  auto resolveAsLambdaCapturedVar =
+      [&](const ValueDecl *VD) -> std::optional<std::pair<SVal, QualType>> {
+    const auto *MD = dyn_cast<CXXMethodDecl>(LCtx->getDecl());
     const auto *DeclRefEx = dyn_cast<DeclRefExpr>(Ex);
-    std::optional<std::pair<SVal, QualType>> VInfo;
-
     if (AMgr.options.ShouldInlineLambdas && DeclRefEx &&
         DeclRefEx->refersToEnclosingVariableOrCapture() && MD &&
         MD->getParent()->isLambda()) {
@@ -3151,13 +3145,23 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
       // Sema follows a sequence of complex rules to determine whether the
       // variable should be captured.
       if (const FieldDecl *FD = LambdaCaptureFields[VD]) {
-        Loc CXXThis =
-            svalBuilder.getCXXThis(MD, LocCtxt->getStackFrame());
+        Loc CXXThis = svalBuilder.getCXXThis(MD, LCtx->getStackFrame());
         SVal CXXThisVal = state->getSVal(CXXThis);
-        VInfo = std::make_pair(state->getLValue(FD, CXXThisVal), FD->getType());
+        return std::make_pair(state->getLValue(FD, CXXThisVal), FD->getType());
       }
     }
 
+    return std::nullopt;
+  };
+
+  if (const auto *VD = dyn_cast<VarDecl>(D)) {
+    // C permits "extern void v", and if you cast the address to a valid type,
+    // you can even do things with it. We simply pretend
+    assert(Ex->isGLValue() || VD->getType()->isVoidType());
+    const LocationContext *LocCtxt = Pred->getLocationContext();
+    std::optional<std::pair<SVal, QualType>> VInfo =
+        resolveAsLambdaCapturedVar(VD);
+
     if (!VInfo)
       VInfo = std::make_pair(state->getLValue(VD, LocCtxt), VD->getType());
 
@@ -3195,6 +3199,23 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
     return;
   }
   if (const auto *BD = dyn_cast<BindingDecl>(D)) {
+    // Handle structured bindings captured by lambda.
+    if (std::optional<std::pair<SVal, QualType>> VInfo =
+            resolveAsLambdaCapturedVar(BD)) {
+      auto [V, T] = VInfo.value();
+
+      if (T->isReferenceType()) {
+        if (const MemRegion *R = V.getAsRegion())
+          V = state->getSVal(R);
+        else
+          V = UnknownVal();
+      }
+
+      Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
+                        ProgramPoint::PostLValueKind);
+      return;
+    }
+
     const auto *DD = cast<DecompositionDecl>(BD->getDecomposedDecl());
 
     SVal Base = state->getLValue(DD, LCtx);

diff  --git a/clang/test/Analysis/issue-91835.cpp b/clang/test/Analysis/issue-91835.cpp
new file mode 100644
index 0000000000000..9581bd86156b0
--- /dev/null
+++ b/clang/test/Analysis/issue-91835.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_analyze_cc1 -std=c++20 %s -analyzer-checker=core.NullDereference -analyzer-output=text -verify
+
+// expected-no-diagnostics
+
+struct S { int x; };
+
+void f(int x) { (void)x; }
+
+int main()
+{
+    S s{42};
+    auto& [x] = s;
+    auto g = [x](){ f(x); }; // no warning
+    g();
+}

diff  --git a/clang/test/Analysis/lambda-capture-structured-binding.cpp b/clang/test/Analysis/lambda-capture-structured-binding.cpp
new file mode 100644
index 0000000000000..7ab8720c22480
--- /dev/null
+++ b/clang/test/Analysis/lambda-capture-structured-binding.cpp
@@ -0,0 +1,127 @@
+// RUN: %clang_analyze_cc1 -std=c++20 -analyzer-checker=core,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+void clang_analyzer_warnIfReached();
+void clang_analyzer_eval(int);
+
+void capture_structured_binding_to_array_byref() {
+  int arr[] {5};
+  auto& [i] = arr;
+  [i]() mutable {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    ++i;
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    i++;
+  }();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+}
+
+void capture_structured_binding_to_array_byvalue() {
+  int arr[] {5};
+  auto [i] = arr;
+  [i]() mutable {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    ++i;
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    i++;
+  }();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+}
+
+void capture_structured_binding_to_tuple_like_byref() {
+  std::pair<int, int> p {5, 6};
+  auto& [i, _] = p;
+  [i]() mutable {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    ++i;
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    i++;
+  }();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+}
+
+void capture_structured_binding_to_tuple_like_byvalue() {
+  std::pair<int, int> p {5, 6};
+  auto [i, _] = p;
+  [i]() mutable {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    ++i;
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    i++;
+  }();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+}
+
+struct S { int x; };
+
+void capture_structured_binding_to_data_member_byref() {
+  S s{5};
+  auto& [i] = s;
+  [i]() mutable {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    ++i;
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    i++;
+  }();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+}
+
+void capture_structured_binding_to_data_member_byvalue() {
+  S s{5};
+  auto [i] = s;
+  [i]() mutable {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    ++i;
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+  }();
+  [&i] {
+    if (i != 5)
+      clang_analyzer_warnIfReached();
+    i++;
+  }();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+}