Skip to content

[clang] Crash when referencing capture in static lambda #74661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

puremourning
Copy link
Contributor

The constant evaluator could try to reference a lambda capture in a static lambda call operator. Static lambdas can't have captures, so we simply abort. Either the lambda needs to be made non-static, or the capture (and reference to it) need to be removed.

Fixes: #74608

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-clang

Author: Ben Jackson (puremourning)

Changes

The constant evaluator could try to reference a lambda capture in a static lambda call operator. Static lambdas can't have captures, so we simply abort. Either the lambda needs to be made non-static, or the capture (and reference to it) need to be removed.

Fixes: #74608


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

2 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+12-2)
  • (modified) clang/test/Parser/cxx2b-lambdas.cpp (+12)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 986302e1fd225f..e806318efd8a5e 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8492,14 +8492,24 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
       return false;
 
     if (auto *FD = Info.CurrentCall->LambdaCaptureFields.lookup(VD)) {
+      auto *MD = cast<CXXMethodDecl>(Info.CurrentCall->Callee);
+
+      // Static lambda function call operators can't have captures. We already
+      // diagnosed this, so bail out here.
+      if (MD->isStatic()) {
+        assert(Info.CurrentCall->This == nullptr &&
+               "This should not be set for a static call operator");
+        return false;
+      }
+
       // Start with 'Result' referring to the complete closure object...
-      if (auto *MD = cast<CXXMethodDecl>(Info.CurrentCall->Callee);
-          MD->isExplicitObjectMemberFunction()) {
+      if (MD->isExplicitObjectMemberFunction()) {
         APValue *RefValue =
             Info.getParamSlot(Info.CurrentCall->Arguments, MD->getParamDecl(0));
         Result.setFrom(Info.Ctx, *RefValue);
       } else
         Result = *Info.CurrentCall->This;
+
       // ... then update it to refer to the field of the closure object
       // that represents the capture.
       if (!HandleLValueMember(Info, E, Result, FD))
diff --git a/clang/test/Parser/cxx2b-lambdas.cpp b/clang/test/Parser/cxx2b-lambdas.cpp
index bb9ed226afffaf..ad975a17b6e476 100644
--- a/clang/test/Parser/cxx2b-lambdas.cpp
+++ b/clang/test/Parser/cxx2b-lambdas.cpp
@@ -66,3 +66,15 @@ void static_captures() {
     }
   };
 }
+
+constexpr auto static_capture_constexpr() {
+  char n = 'n';
+  return [n] static { return n; }(); // expected-error {{a static lambda cannot have any captures}}
+}
+static_assert(static_capture_constexpr()); // expected-error {{static assertion expression is not an integral constant expression}}
+
+constexpr auto capture_constexpr() {
+  char n = 'n';
+  return [n] { return n; }();
+}
+static_assert(capture_constexpr());

@puremourning
Copy link
Contributor Author

I guess should ping @royjacobson on this one?

@puremourning puremourning changed the title Crash when referencing capture in static lambda [clang] Crash when referencing capture in static lambda Dec 6, 2023
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense but @cor3ntin should look at it.

@cor3ntin
Copy link
Contributor

cor3ntin commented Dec 7, 2023

Can you add a release note? Otherwise it looks good to me

@puremourning puremourning force-pushed the fix-static-lambda-capture-ref-consteval branch from a523d23 to 8a72385 Compare December 7, 2023 10:01
@puremourning
Copy link
Contributor Author

Can you add a release note? Otherwise it looks good to me

Yep, sure thing. Done.

@puremourning puremourning force-pushed the fix-static-lambda-capture-ref-consteval branch from 8a72385 to 13afd0e Compare December 7, 2023 14:46
The constant evaluator could try to reference a lambda capture in a
static lambda call operator. Static lambdas can't have captures, so we
simply abort. Either the lambda needs to be made non-static, or the
capture (and reference to it) need to be removed.
@puremourning puremourning force-pushed the fix-static-lambda-capture-ref-consteval branch from 13afd0e to cdd9b65 Compare December 7, 2023 14:48
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@tbaederr tbaederr merged commit c88d731 into llvm:main Dec 12, 2023
tbaederr added a commit to tbaederr/llvm-project that referenced this pull request Dec 12, 2023
A version of llvm#74661 for the new interpreter. It didn't crash before,
but we did emit a few non-sensical diagnostics.
tbaederr added a commit that referenced this pull request Dec 12, 2023
Static lambdas cannot have captures. They may still end up in the
constant evaluator though. They've been diagnosted appropriately before,
so just reject them here.

This is similar to #74661, but for the new constant expression
interpreter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segfault with lambda expression with a static function call operator
6 participants