[clang][analyzer] Correctly handle structured bindings captured by lambda

[flovent]

this PR fixs #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.

[llvmbot]

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: None (flovent)

Changes

this PR fixs #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.

---

Full diff: https://github.com/llvm/llvm-project/pull/132579.diff

3 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Core/ExprEngine.cpp (+33-12)
• (added) clang/test/Analysis/issue-91835.cpp (+15)
• (added) clang/test/Analysis/lambda-capture-structured-binding.cpp (+127)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 318fa3c1caf06..07540af50a288 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3120,16 +3120,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()) {
@@ -3142,13 +3136,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());
 
@@ -3186,6 +3190,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..fb136ebb6f437
--- /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();
+}
\ No newline at end of file
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..22ca3ee791da7
--- /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}}
+}
\ No newline at end of file

[isuckatcs]

IIUC, the real issue is that we could only handle VarDecl that was captured by a lambda, and nothing else, including the BindingDecl.

In this case the proper fix would be to make sure, every declaration is correctly looked up from a lambda.

[flovent]

In this case the proper fix would be to make sure, every declaration is correctly looked up from a lambda.

As i can know, lambdas can only capture variables in program, which seems can only come from VarDecl and BindingDecl, so i think we don't need to handle other declarations for captured situation.

And for other types of declaration handled in VisitCommonDeclRefExpr(EnumConstantDecl, FunctionDecl, FieldDecl, IndirectFieldDecl), they will not be variables obviously.

[flovent]

@isuckatcs Thank you for approvel! Is this PR also need to be reviewed by other people? If not can you help me merge it? since I don't have write access.

[steakhal]

LGTM, thank you.