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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: None (flovent)

<details>
<summary>Changes</summary>

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
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

``````````

</details>


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