-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[Clang] Only remove lambda scope after computing evaluation context #154106
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
Conversation
The immediate evaluation context needs the lambda scope info to propagate some flags, however that LSI was removed in ActOnFinishFunctionBody which happened before rebuilding a lambda expression. This also converts the wrapper function to default arguments as a drive-by fix.
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesThe immediate evaluation context needs the lambda scope info to propagate some flags, however that LSI was removed in ActOnFinishFunctionBody which happened before rebuilding a lambda expression. This also converts the wrapper function to default arguments as a drive-by fix. Fixes #145776 Full diff: https://github.com/llvm/llvm-project/pull/154106.diff 6 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9231f2cae9bfa..ca6fc663c9525 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -158,6 +158,7 @@ Bug Fixes to C++ Support
- Diagnose binding a reference to ``*nullptr`` during constant evaluation. (#GH48665)
- Suppress ``-Wdeprecated-declarations`` in implicitly generated functions. (#GH147293)
- Fix a crash when deleting a pointer to an incomplete array (#GH150359).
+- Fixed a mismatched lambda scope bug when propagating up ``consteval`` within nested lambdas. (#GH145776)
- Fix an assertion failure when expression in assumption attribute
(``[[assume(expr)]]``) creates temporary objects.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5211373367677..160b0739b5aa2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4179,8 +4179,9 @@ class Sema final : public SemaBase {
/// return statement in the scope of a variable has the same NRVO candidate,
/// that candidate is an NRVO variable.
void computeNRVO(Stmt *Body, sema::FunctionScopeInfo *Scope);
- Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body);
- Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body, bool IsInstantiation);
+ Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body,
+ bool IsInstantiation = false,
+ bool RetainFunctionScopeInfo = false);
Decl *ActOnSkippedFunctionBody(Decl *Decl);
void ActOnFinishInlineFunctionDef(FunctionDecl *D);
@@ -6873,23 +6874,23 @@ class Sema final : public SemaBase {
assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
return ExprEvalContexts.back();
- };
+ }
ExpressionEvaluationContextRecord ¤tEvaluationContext() {
assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
return ExprEvalContexts.back();
- };
+ }
ExpressionEvaluationContextRecord &parentEvaluationContext() {
assert(ExprEvalContexts.size() >= 2 &&
"Must be in an expression evaluation context");
return ExprEvalContexts[ExprEvalContexts.size() - 2];
- };
+ }
const ExpressionEvaluationContextRecord &parentEvaluationContext() const {
return const_cast<Sema *>(this)->parentEvaluationContext();
- };
+ }
bool isAttrContext() const {
return ExprEvalContexts.back().ExprContext ==
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index b5eb825eb52cc..4c8974b2e6c9a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16149,10 +16149,6 @@ Decl *Sema::ActOnSkippedFunctionBody(Decl *Decl) {
return Decl;
}
-Decl *Sema::ActOnFinishFunctionBody(Decl *D, Stmt *BodyArg) {
- return ActOnFinishFunctionBody(D, BodyArg, /*IsInstantiation=*/false);
-}
-
/// RAII object that pops an ExpressionEvaluationContext when exiting a function
/// body.
class ExitFunctionBodyRAII {
@@ -16223,8 +16219,8 @@ void Sema::CheckCoroutineWrapper(FunctionDecl *FD) {
Diag(FD->getLocation(), diag::err_coroutine_return_type) << RD;
}
-Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
- bool IsInstantiation) {
+Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, bool IsInstantiation,
+ bool RetainFunctionScopeInfo) {
FunctionScopeInfo *FSI = getCurFunction();
FunctionDecl *FD = dcl ? dcl->getAsFunction() : nullptr;
@@ -16681,7 +16677,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
if (!IsInstantiation)
PopDeclContext();
- PopFunctionScopeInfo(ActivePolicy, dcl);
+ if (!RetainFunctionScopeInfo)
+ PopFunctionScopeInfo(ActivePolicy, dcl);
// If any errors have occurred, clear out any temporaries that may have
// been leftover. This ensures that these temporaries won't be picked up for
// deletion in some later function.
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index bc3c4b0addeba..eb6bc83cf861a 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1968,12 +1968,13 @@ ExprResult Sema::BuildCaptureInit(const Capture &Cap,
}
ExprResult Sema::ActOnLambdaExpr(SourceLocation StartLoc, Stmt *Body) {
- LambdaScopeInfo LSI = *cast<LambdaScopeInfo>(FunctionScopes.back());
+ LambdaScopeInfo &LSI = *cast<LambdaScopeInfo>(FunctionScopes.back());
if (LSI.CallOperator->hasAttr<SYCLKernelEntryPointAttr>())
SYCL().CheckSYCLEntryPointFunctionDecl(LSI.CallOperator);
- ActOnFinishFunctionBody(LSI.CallOperator, Body);
+ ActOnFinishFunctionBody(LSI.CallOperator, Body, /*IsInstantiation=*/false,
+ /*RetainFunctionScopeInfo=*/true);
return BuildLambdaExpr(StartLoc, Body->getEndLoc(), &LSI);
}
@@ -2134,6 +2135,11 @@ ConstructFixItRangeForUnusedCapture(Sema &S, SourceRange CaptureRange,
ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
LambdaScopeInfo *LSI) {
+ // Copy the LSI before PopFunctionScopeInfo removes it.
+ // FIXME: This is dumb. Store the lambda information somewhere that outlives
+ // the call operator.
+ LambdaScopeInfo LSICopy = *LSI;
+ LSI = &LSICopy;
// Collect information from the lambda scope.
SmallVector<LambdaCapture, 4> Captures;
SmallVector<Expr *, 4> CaptureInits;
@@ -2170,6 +2176,10 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
PopExpressionEvaluationContext();
+ sema::AnalysisBasedWarnings::Policy WP =
+ AnalysisWarnings.getPolicyInEffectAt(EndLoc);
+ PopFunctionScopeInfo(&WP, LSI->CallOperator);
+
// True if the current capture has a used capture or default before it.
bool CurHasPreviousCapture = CaptureDefault != LCD_None;
SourceLocation PrevCaptureLoc = CurHasPreviousCapture ?
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 5ff5fbf52ae25..c3893959c85bf 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -15791,12 +15791,9 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
return ExprError();
}
- // Copy the LSI before ActOnFinishFunctionBody removes it.
- // FIXME: This is dumb. Store the lambda information somewhere that outlives
- // the call operator.
- auto LSICopy = *LSI;
getSema().ActOnFinishFunctionBody(NewCallOperator, Body.get(),
- /*IsInstantiation*/ true);
+ /*IsInstantiation=*/true,
+ /*RetainFunctionScopeInfo=*/true);
SavedContext.pop();
// Recompute the dependency of the lambda so that we can defer the lambda call
@@ -15832,7 +15829,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
// *after* the substitution in case we can't decide the dependency
// so early, e.g. because we want to see if any of the *substituted*
// parameters are dependent.
- DependencyKind = getDerived().ComputeLambdaDependency(&LSICopy);
+ DependencyKind = getDerived().ComputeLambdaDependency(LSI);
Class->setLambdaDependencyKind(DependencyKind);
// Clean up the type cache created previously. Then, we re-create a type for
// such Decl with the new DependencyKind.
@@ -15840,7 +15837,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
getSema().Context.getTypeDeclType(Class);
return getDerived().RebuildLambdaExpr(E->getBeginLoc(),
- Body.get()->getEndLoc(), &LSICopy);
+ Body.get()->getEndLoc(), LSI);
}
template<typename Derived>
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index c4cfd9398920a..6cf0e0251ab62 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -610,3 +610,19 @@ namespace GH135281 {
void (*ff)() = f2<B>; // expected-note {{instantiation of function template specialization}}
}
#endif
+
+namespace GH145776 {
+
+void runtime_only() {}
+consteval void comptime_only() {}
+
+void fn() {
+ []() {
+ runtime_only();
+ []() {
+ &comptime_only;
+ }();
+ }();
+}
+
+}
|
AnalysisWarnings.getPolicyInEffectAt(EndLoc); | ||
// We cannot release LSI until we finish computing captures, which | ||
// requires the scope to be popped. | ||
PoppedFunctionScopePtr _ = PopFunctionScopeInfo(&WP, LSI->CallOperator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is causing a use-after-free that CI is hitting:
#154342
#154347
because LSI
is used outside of this block:
llvm-project/clang/lib/Sema/SemaLambda.cpp
Line 2315 in 5cc8c92
DiagnoseShadowingLambdaDecls(LSI); |
llvm-project/clang/lib/Sema/SemaLambda.cpp
Line 2347 in 5cc8c92
maybeAddDeclWithEffects(LSI->CallOperator); |
Perhaps we should revert the changes until that's fixed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I reverted in #154382 :(
…ontext" (#154382) Revert due to breakage as reported in #154106 (comment) Reverts #154106
…valuation context" (#154382) Revert due to breakage as reported in llvm/llvm-project#154106 (comment) Reverts llvm/llvm-project#154106
We started seeing a segmentation fault when compiling
I realized that a revert has landed, but I wanted to mention this issue before relanding. |
The immediate evaluation context needs the lambda scope info to propagate some flags, however that LSI was removed in ActOnFinishFunctionBody which happened before rebuilding a lambda expression.
This also converts the wrapper function to default arguments as a drive-by fix.
Fixes #145776