[clang] [Clang] [Sema] Don't crash on unexpanded pack in invalid block literal (PR #110762)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 1 16:24:40 PDT 2024
https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/110762
Consider #109148:
```c++
template <typename ...Ts>
void f() {
[] {
(^Ts);
};
}
```
When we encounter `^Ts`, we try to parse a block and subsequently call `DiagnoseUnexpandedParameterPack()` (in `ActOnBlockArguments()`), which sees `Ts` and sets `ContainsUnexpandedParameterPack` to `true` in the `LambdaScopeInfo` of the enclosing lambda. However, the entire block is subsequently discarded entirely because it isn’t even syntactically well-formed. As a result, `ContainsUnexpandedParameterPack` is `true` despite the lambda’s body no longer containing any unexpanded packs, which causes an assertion the next time `DiagnoseUnexpandedParameterPack()` is called.
Side note: lambdas don’t suffer from this problem because they push a new `LambdaScopeInfo`, so if the lambda is discarded, the ‘enclosing lambda’ (which is just that very same lambda) is discarded along with it. I’m not sure making blocks work the same way would work (i.e. allowing unexpanded parameter packs if there is an enclosing lambda *or block*, but I’m assuming there is a reason why we don’t support that...), because currently, e.g. this doesn’t work at all:
```c++
template<typename... Ts>
void f(Ts...ts) {
((^ void (Ts) {} (ts)), ...); // error: block contains unexpanded parameter pack 'Ts'
};
```
I spent some time thinking of a few ways of fixing this (and the author of the issue for this has also proposed some approaches, including the one I decided on); ultimately, I decided on having `ActOnBlockError` (which is always called when the block doesn’t make it into the AST) reset the enclosing lambda’s `ContainsUnexpandedParameterPack` member by storing its previous state in the block’s `BlockScopeInfo`.
This feels like a bit of a hack, but the alternatives I’ve seen so far or managed to think of myself are either
1. removing the assertion from every overload of `DiagnoseUnexpandedParameterPack()`;
2. *somehow* including the malformed block in the AST (by wrapping it in a `RecoveryExpr` perhaps?), but I’m not sure how well that would work because imo at least something like `(^Ts);` is just ‘too invalid’ to have a sensible representation in the AST; or
3. Making blocks work like lambdas (as mentioned above).
Option 1 is dubious at best imo; I have no idea how 2 would work; and 3 seems like it might end up being a lot of work if there’s a reason why blocks don’t act like lambdas wrt unexpanded packs.
This fixes #109148.
>From a471d32c94bfcbd10291053bb6ce0541ea2e626c Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 2 Oct 2024 00:59:37 +0200
Subject: [PATCH 1/3] [Clang] [Sema] Don't crash on unexpanded pack in invalid
block literal
---
clang/docs/ReleaseNotes.rst | 2 +
clang/include/clang/Sema/ScopeInfo.h | 10 ++++
clang/lib/Sema/Sema.cpp | 8 +++-
clang/lib/Sema/SemaExpr.cpp | 11 +++++
clang/test/SemaCXX/block-unexpanded-pack.cpp | 48 ++++++++++++++++++++
5 files changed, 77 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/block-unexpanded-pack.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 34d2b584274a5f..ed4a7affcf6919 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -457,6 +457,8 @@ Bug Fixes to C++ Support
containing outer unexpanded parameters were not correctly expanded. (#GH101754)
- Fixed a bug in constraint expression comparison where the ``sizeof...`` expression was not handled properly
in certain friend declarations. (#GH93099)
+- Clang no longer crashes when a lambda contains an invalid block declaration that contains an unexpanded
+ parameter pack. (#GH109148)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h
index 700e361ef83f13..a943f2edc51fba 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -793,6 +793,16 @@ class BlockScopeInfo final : public CapturingScopeInfo {
/// Its return type may be BuiltinType::Dependent.
QualType FunctionType;
+ /// We sometimes diagnose unexpanded parameter packs in block literals,
+ /// but an error while the block is parsed causes it to be discarded, in
+ /// which case we need to reset the enclosing lambda's
+ /// ContainsUnexpandedParameterPackFlag.
+ ///
+ /// Note: This issue does not exist with lambdas because they push a new
+ /// LambdaScopeInfo, so if the expression is discarded, the 'enclosing
+ /// lambda' is discarded along with it.
+ bool EnclosingLambdaContainsUnexpandedParameterPack = false;
+
BlockScopeInfo(DiagnosticsEngine &Diag, Scope *BlockScope, BlockDecl *Block)
: CapturingScopeInfo(Diag, ImpCap_Block), TheDecl(Block),
TheScope(BlockScope) {
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 4be7dfbc293927..1e72910da27d5e 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2187,9 +2187,13 @@ void Sema::PushFunctionScope() {
}
void Sema::PushBlockScope(Scope *BlockScope, BlockDecl *Block) {
- FunctionScopes.push_back(new BlockScopeInfo(getDiagnostics(),
- BlockScope, Block));
+ auto *BSI = new BlockScopeInfo(getDiagnostics(), BlockScope, Block);
+ FunctionScopes.push_back(BSI);
CapturingFunctionScopes++;
+
+ LambdaScopeInfo *Enclosing = getEnclosingLambda();
+ BSI->EnclosingLambdaContainsUnexpandedParameterPack =
+ Enclosing && Enclosing->ContainsUnexpandedParameterPack;
}
LambdaScopeInfo *Sema::PushLambdaScope() {
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2db9d1fc69ed1e..29745b1f3edff1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16187,6 +16187,15 @@ void Sema::ActOnBlockArguments(SourceLocation CaretLoc, Declarator &ParamInfo,
}
void Sema::ActOnBlockError(SourceLocation CaretLoc, Scope *CurScope) {
+ // If the enclosing lambda did not contain any unexpanded parameter
+ // packs before this block, then reset the unexpanded parameter pack
+ // flag, otherwise, we might end up crashing trying to find a pack
+ // that we are about to discard along with the rest of the block.
+ if (!getCurBlock()->EnclosingLambdaContainsUnexpandedParameterPack) {
+ LambdaScopeInfo *L = getEnclosingLambda();
+ if (L) L->ContainsUnexpandedParameterPack = false;
+ }
+
// Leave the expression-evaluation context.
DiscardCleanupsInEvaluationContext();
PopExpressionEvaluationContext();
@@ -16379,6 +16388,8 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
if (getCurFunction())
getCurFunction()->addBlock(BD);
+ // This can happen if the block's return type is deduced, but
+ // the return expression is invalid.
if (BD->isInvalidDecl())
return CreateRecoveryExpr(Result->getBeginLoc(), Result->getEndLoc(),
{Result}, Result->getType());
diff --git a/clang/test/SemaCXX/block-unexpanded-pack.cpp b/clang/test/SemaCXX/block-unexpanded-pack.cpp
new file mode 100644
index 00000000000000..c1435b8e314a81
--- /dev/null
+++ b/clang/test/SemaCXX/block-unexpanded-pack.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fblocks -triple x86_64-apple-darwin -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fblocks -triple x86_64-apple-darwin -fsyntax-only -verify %s -frecovery-ast -frecovery-ast-type
+
+// This checks that when a block is discarded, the enclosing lambda’s
+// unexpanded parameter pack flag is reset to what it was before the
+// block is parsed so we don't crash when trying to diagnose unexpanded
+// parameter packs in the lambda.
+
+template <typename ...Ts>
+void gh109148() {
+ (^Ts); // expected-error {{expected expression}} expected-error {{unexpanded parameter pack 'Ts'}}
+
+ [] {
+ (^Ts); // expected-error {{expected expression}}
+ ^Ts; // expected-error {{expected expression}}
+ ^(Ts); // expected-error {{expected expression}}
+ ^ Ts); // expected-error {{expected expression}}
+ };
+
+ ([] {
+ (^Ts); // expected-error {{expected expression}}
+ ^Ts; // expected-error {{expected expression}}
+ ^(Ts); // expected-error {{expected expression}}
+ ^ Ts); // expected-error {{expected expression}}
+ }, ...); // expected-error {{pack expansion does not contain any unexpanded parameter packs}}
+
+ [] { // expected-error {{unexpanded parameter pack 'Ts'}}
+ ^ (Ts) {};
+ };
+
+ [] { // expected-error {{unexpanded parameter pack 'Ts'}}
+ (void) ^ { Ts x; };
+ };
+
+ [] { // expected-error {{unexpanded parameter pack 'Ts'}}
+ Ts s;
+ (^Ts); // expected-error {{expected expression}}
+ };
+
+ ([] {
+ Ts s;
+ (^Ts); // expected-error {{expected expression}}
+ }, ...)
+
+ [] { // expected-error {{unexpanded parameter pack 'Ts'}}
+ ^ { Ts s; return not_defined; }; // expected-error {{use of undeclared identifier 'not_defined'}}
+ };
+}
>From 69ce3193bfd0a507f6eac6336bef599c2d6d53e9 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 2 Oct 2024 01:21:56 +0200
Subject: [PATCH 2/3] clang-format
---
clang/lib/Sema/SemaExpr.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 29745b1f3edff1..d1c91296a9d468 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16193,7 +16193,8 @@ void Sema::ActOnBlockError(SourceLocation CaretLoc, Scope *CurScope) {
// that we are about to discard along with the rest of the block.
if (!getCurBlock()->EnclosingLambdaContainsUnexpandedParameterPack) {
LambdaScopeInfo *L = getEnclosingLambda();
- if (L) L->ContainsUnexpandedParameterPack = false;
+ if (L)
+ L->ContainsUnexpandedParameterPack = false;
}
// Leave the expression-evaluation context.
>From 48ca9d63084310087b3d86878b9ae100ccdc3f52 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 2 Oct 2024 01:22:59 +0200
Subject: [PATCH 3/3] Fix typo in comment
---
clang/include/clang/Sema/ScopeInfo.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h
index a943f2edc51fba..3bad1e3bad4312 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -794,9 +794,9 @@ class BlockScopeInfo final : public CapturingScopeInfo {
QualType FunctionType;
/// We sometimes diagnose unexpanded parameter packs in block literals,
- /// but an error while the block is parsed causes it to be discarded, in
- /// which case we need to reset the enclosing lambda's
- /// ContainsUnexpandedParameterPackFlag.
+ /// but an error while the block is parsed can cause it to be discarded,
+ /// in which case we need to reset the enclosing lambda's
+ /// ContainsUnexpandedParameterPack flag.
///
/// Note: This issue does not exist with lambdas because they push a new
/// LambdaScopeInfo, so if the expression is discarded, the 'enclosing
More information about the cfe-commits
mailing list