[clang] [clang] Fix a crash from nested ArrayInitLoopExpr (PR #67722)

via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 28 11:52:34 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

<details>
<summary>Changes</summary>

For the following snippets clang performs the same steps:
```
  S s[2][2];

  auto [a1,a2] = s;
```

```
void crash() {
  S s[2][2];

  int arr[4];

  arr[0] = [s] { return s[0][0].i; }();
}
```

>From the two however the latter crashes. The source of the crash is that we create a temporary variable for the source expression, the `ArrayInitLoop` is performed on.

```
-ArrayInitLoopExpr ... 'S[2][2]'
 |-OpaqueValueExpr ...
 | `-DeclRefExpr ... 'S[2][2]' lvalue Var ... 's' 'S[2][2]'
 `-ArrayInitLoopExpr ... 'S[2]' <-- a temporary is created for this during every iteration
   |-...
   | `...
   `-...
```
The problem is that we are unable to differentiate between the temporaries that are created in the different iterations, as we only use the pointer to the node as the key, which stays the same.

As for why only the second snippet crashes, the reason is that after evaluating the first iteration, we note a failure and would return, however during analyzing the second snippet, the `CheckingForUndefinedBehavior` flag is set, probably because we assign to an array member and clang wants us to keep going.

This patch changes the described behaviour, as if we keep going the exact same expression would be evaluated again and it would fail again, so there's no point of doing that. A different solution would be to wrap the temporary in a `FullExpressionRAII` so that the temporary is cleaned up before it is created once again.

Fixes #<!-- -->57135



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


2 Files Affected:

- (modified) clang/lib/AST/ExprConstant.cpp (+4-4) 
- (added) clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp (+13) 


``````````diff
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fea06b97259fe31..3a4ef81672911ba 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10967,19 +10967,19 @@ bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) {
   LValue Subobject = This;
   Subobject.addArray(Info, E, CAT);
 
-  bool Success = true;
   for (EvalInfo::ArrayInitLoopIndex Index(Info); Index != Elements; ++Index) {
     if (!EvaluateInPlace(Result.getArrayInitializedElt(Index),
                          Info, Subobject, E->getSubExpr()) ||
         !HandleLValueArrayAdjustment(Info, E, Subobject,
                                      CAT->getElementType(), 1)) {
-      if (!Info.noteFailure())
+      
+      // There's no need to try and evaluate the rest, as those are the exact same expressions.
+      std::ignore = Info.noteFailure();
         return false;
-      Success = false;
     }
   }
 
-  return Success;
+  return true;
 }
 
 bool ArrayExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E) {
diff --git a/clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp b/clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp
new file mode 100644
index 000000000000000..82d27380b637d03
--- /dev/null
+++ b/clang/test/AST/nested-array-init-loop-in-lambda-capture.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify=ref %s
+
+// ref-no-diagnostics
+// expected-no-diagnostics
+
+void used_to_crash() {
+  int s[2][2];
+
+  int arr[4];
+
+  arr[0] = [s] { return s[0][0]; }();
+}

``````````

</details>


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


More information about the cfe-commits mailing list