[clang] Fix a crash when a variable is captured by a block nested inside a lambda

[ahatanak]

Eval->Value.get returns a null pointer when the variable doesn't have an initializer.

This fixes #93625.

rdar://128482541

[llvmbot]

@llvm/pr-subscribers-clang

Author: Akira Hatanaka (ahatanak)

Changes

Eval->Value.get returns a null pointer when the variable doesn't have an initializer.

This fixes #93625.

rdar://128482541

---

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

2 Files Affected:

• (modified) clang/lib/AST/Decl.cpp (+5-3)
• (modified) clang/test/SemaObjCXX/block-capture.mm (+18)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 41fbfe281ef65..4940a787e7d30 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2408,9 +2408,11 @@ Expr *VarDecl::getInit() {
     return cast<Expr>(S);
 
   auto *Eval = getEvaluatedStmt();
-  return cast<Expr>(Eval->Value.isOffset()
-                        ? Eval->Value.get(getASTContext().getExternalSource())
-                        : Eval->Value.get(nullptr));
+
+  return cast_or_null<Expr>(
+      Eval->Value.isOffset()
+          ? Eval->Value.get(getASTContext().getExternalSource())
+          : Eval->Value.get(nullptr));
 }
 
 Stmt **VarDecl::getInitAddress() {
diff --git a/clang/test/SemaObjCXX/block-capture.mm b/clang/test/SemaObjCXX/block-capture.mm
index 8ba02f919e015..231aef33f2c7e 100644
--- a/clang/test/SemaObjCXX/block-capture.mm
+++ b/clang/test/SemaObjCXX/block-capture.mm
@@ -83,3 +83,21 @@
   SubMove(SubSubMove &&);
 };
 TEST(SubMove);
+
+
+#if __cplusplus >= 202302L
+// clang used to crash compiling this code.
+namespace BlockInLambda {
+  struct S {
+    constexpr ~S();
+  };
+
+  void func(S const &a) {
+    [a](auto b) {
+      ^{
+        (void)a;
+      }();
+    }(12);
+  }
+}
+#endif

[ahatanak]

Note that the call to hasInit at the beginning of function VarDecl::getInit returns true even when the variable doesn't have an initializer if VarDecl::ensureEvaluatedStmt is called before that.

[fahadnayyar]

LGTM!

[zyn0217]

nit: Prefer cast_if_present.

[cor3ntin]

Can you provide a better description for the PR (such as "[clang] fix a crash with block captures inside a lambda" ) and an entry in clang/docs/ReleaseNotes.rst referencing the fixed issue?

Thanks!

The change itself looks good except modulo @zyn0217's feedback

[cor3ntin]

LGTM, thanks for the fix!

[mizvekov]

I see it stops crashing, but is it actually working now?

Why hasInit returns true, but the variable doesn't actually have an initializer?
Shouldn't the fix go there?

[ahatanak]

hasInit checks whether VarDecl::Init's pointer is null. But VarDecl::ensureEvaluatedStmt creates an EvaluatedStmt if there isn't one already, after which hasInit returns true.

llvm-project/clang/lib/AST/Decl.cpp

Line 2541 in 3af717d

Eval->Value = Init.get<Stmt *>();

Init.get<Stmt *>() returns null when there's no initializer.

[mizvekov]

hasInit checks whether VarDecl::Init's pointer is null. But VarDecl::ensureEvaluatedStmt creates an
Init.get<Stmt *>() returns null when there's no initializer.

I see. It sounds like it would be less confusing if hasInit would check that as well, or if we need the current behavior elsewhere, we should rename it.

[ahatanak]

I can see if we can make hasInit check that too or rename the function after committing this patch.

[mizvekov]

LGTM, we can finish in post.