Skip to content
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

[Clang] [Sema] Don't crash on unexpanded pack in invalid block literal #110762

Merged
merged 6 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/AST/ComputeDependence.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ ExprDependence computeDependence(ArrayInitLoopExpr *E);
ExprDependence computeDependence(ImplicitValueInitExpr *E);
ExprDependence computeDependence(InitListExpr *E);
ExprDependence computeDependence(ExtVectorElementExpr *E);
ExprDependence computeDependence(BlockExpr *E);
ExprDependence computeDependence(BlockExpr *E,
bool ContainsUnexpandedParameterPack);
ExprDependence computeDependence(AsTypeExpr *E);
ExprDependence computeDependence(DeclRefExpr *E, const ASTContext &Ctx);
ExprDependence computeDependence(RecoveryExpr *E);
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -6369,9 +6369,9 @@ class BlockExpr : public Expr {
protected:
BlockDecl *TheBlock;
public:
BlockExpr(BlockDecl *BD, QualType ty)
BlockExpr(BlockDecl *BD, QualType ty, bool ContainsUnexpandedParameterPack)
: Expr(BlockExprClass, ty, VK_PRValue, OK_Ordinary), TheBlock(BD) {
setDependence(computeDependence(this));
setDependence(computeDependence(this, ContainsUnexpandedParameterPack));
}

/// Build an empty block expression.
Expand Down
12 changes: 6 additions & 6 deletions clang/include/clang/Sema/ScopeInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -724,10 +724,16 @@ class CapturingScopeInfo : public FunctionScopeInfo {
/// is deduced (e.g. a lambda or block with omitted return type).
bool HasImplicitReturnType = false;

/// Whether this contains an unexpanded parameter pack.
bool ContainsUnexpandedParameterPack = false;

/// ReturnType - The target type of return statements in this context,
/// or null if unknown.
QualType ReturnType;

/// Packs introduced by this, if any.
SmallVector<NamedDecl *, 4> LocalPacks;

void addCapture(ValueDecl *Var, bool isBlock, bool isByref, bool isNested,
SourceLocation Loc, SourceLocation EllipsisLoc,
QualType CaptureType, bool Invalid) {
Expand Down Expand Up @@ -895,12 +901,6 @@ class LambdaScopeInfo final :
/// Whether any of the capture expressions requires cleanups.
CleanupInfo Cleanup;

/// Whether the lambda contains an unexpanded parameter pack.
bool ContainsUnexpandedParameterPack = false;

/// Packs introduced by this lambda, if any.
SmallVector<NamedDecl*, 4> LocalPacks;

/// Source range covering the explicit template parameter list (if it exists).
SourceRange ExplicitTemplateParamsRange;

Expand Down
8 changes: 4 additions & 4 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -750,10 +750,10 @@ class Sema final : public SemaBase {
/// Retrieve the current block, if any.
sema::BlockScopeInfo *getCurBlock();

/// Get the innermost lambda enclosing the current location, if any. This
/// looks through intervening non-lambda scopes such as local functions and
/// blocks.
sema::LambdaScopeInfo *getEnclosingLambda() const;
/// Get the innermost lambda or block enclosing the current location, if any.
/// This looks through intervening non-lambda, non-block scopes such as local
/// functions.
sema::CapturingScopeInfo *getEnclosingLambdaOrBlock() const;

/// Retrieve the current lambda scope info, if any.
/// \param IgnoreNonLambdaCapturingScope true if should find the top-most
Expand Down
16 changes: 8 additions & 8 deletions clang/include/clang/Sema/Template.h
Original file line number Diff line number Diff line change
Expand Up @@ -411,10 +411,10 @@ enum class TemplateSubstitutionKind : char {
/// lookup will search our outer scope.
bool CombineWithOuterScope;

/// Whether this scope is being used to instantiate a lambda expression,
/// in which case it should be reused for instantiating the lambda's
/// FunctionProtoType.
bool InstantiatingLambda = false;
/// Whether this scope is being used to instantiate a lambda or block
/// expression, in which case it should be reused for instantiating the
/// lambda's FunctionProtoType.
bool InstantiatingLambdaOrBlock = false;

/// If non-NULL, the template parameter pack that has been
/// partially substituted per C++0x [temp.arg.explicit]p9.
Expand All @@ -431,10 +431,10 @@ enum class TemplateSubstitutionKind : char {

public:
LocalInstantiationScope(Sema &SemaRef, bool CombineWithOuterScope = false,
bool InstantiatingLambda = false)
bool InstantiatingLambdaOrBlock = false)
: SemaRef(SemaRef), Outer(SemaRef.CurrentInstantiationScope),
CombineWithOuterScope(CombineWithOuterScope),
InstantiatingLambda(InstantiatingLambda) {
InstantiatingLambdaOrBlock(InstantiatingLambdaOrBlock) {
SemaRef.CurrentInstantiationScope = this;
}

Expand Down Expand Up @@ -561,8 +561,8 @@ enum class TemplateSubstitutionKind : char {
/// Determine whether D is a pack expansion created in this scope.
bool isLocalPackExpansion(const Decl *D);

/// Determine whether this scope is for instantiating a lambda.
bool isLambda() const { return InstantiatingLambda; }
/// Determine whether this scope is for instantiating a lambda or block.
bool isLambdaOrBlock() const { return InstantiatingLambdaOrBlock; }
};

class TemplateDeclInstantiator
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/AST/ComputeDependence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,13 @@ ExprDependence clang::computeDependence(ExtVectorElementExpr *E) {
return E->getBase()->getDependence();
}

ExprDependence clang::computeDependence(BlockExpr *E) {
ExprDependence clang::computeDependence(BlockExpr *E,
bool ContainsUnexpandedParameterPack) {
auto D = toExprDependenceForImpliedType(E->getType()->getDependence());
if (E->getBlockDecl()->isDependentContext())
D |= ExprDependence::Instantiation;
if (ContainsUnexpandedParameterPack)
D |= ExprDependence::UnexpandedPack;
return D;
}

Expand Down
9 changes: 5 additions & 4 deletions clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2382,18 +2382,19 @@ FunctionScopeInfo *Sema::getEnclosingFunction() const {
return nullptr;
}

LambdaScopeInfo *Sema::getEnclosingLambda() const {
CapturingScopeInfo *Sema::getEnclosingLambdaOrBlock() const {
for (auto *Scope : llvm::reverse(FunctionScopes)) {
if (auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope)) {
if (LSI->Lambda && !LSI->Lambda->Encloses(CurContext) &&
if (auto *CSI = dyn_cast<CapturingScopeInfo>(Scope)) {
auto *LSI = dyn_cast<LambdaScopeInfo>(CSI);
if (LSI && LSI->Lambda && !LSI->Lambda->Encloses(CurContext) &&
LSI->AfterParameterList) {
// We have switched contexts due to template instantiation.
// FIXME: We should swap out the FunctionScopes during code synthesis
// so that we don't need to check for this.
assert(!CodeSynthesisContexts.empty());
return nullptr;
}
return LSI;
return CSI;
}
}
return nullptr;
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15139,8 +15139,8 @@ ParmVarDecl *Sema::CheckParameter(DeclContext *DC, SourceLocation StartLoc,
// we know that references to that pack must also be expanded within the
// lambda scope.
if (New->isParameterPack())
if (auto *LSI = getEnclosingLambda())
LSI->LocalPacks.push_back(New);
if (auto *CSI = getEnclosingLambdaOrBlock())
CSI->LocalPacks.push_back(New);

if (New->getType().hasNonTrivialToPrimitiveDestructCUnion() ||
New->getType().hasNonTrivialToPrimitiveCopyCUnion())
Expand Down
17 changes: 5 additions & 12 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16076,17 +16076,7 @@ void Sema::ActOnBlockArguments(SourceLocation CaretLoc, Declarator &ParamInfo,

TypeSourceInfo *Sig = GetTypeForDeclarator(ParamInfo);
QualType T = Sig->getType();

// FIXME: We should allow unexpanded parameter packs here, but that would,
// in turn, make the block expression contain unexpanded parameter packs.
if (DiagnoseUnexpandedParameterPack(CaretLoc, Sig, UPPC_Block)) {
// Drop the parameters.
FunctionProtoType::ExtProtoInfo EPI;
EPI.HasTrailingReturn = false;
EPI.TypeQuals.addConst();
T = Context.getFunctionType(Context.DependentTy, std::nullopt, EPI);
Sig = Context.getTrivialTypeSourceInfo(T);
}
DiagnoseUnexpandedParameterPack(CaretLoc, Sig, UPPC_Block);

// GetTypeForDeclarator always produces a function type for a block
// literal signature. Furthermore, it is always a FunctionProtoType
Expand Down Expand Up @@ -16356,7 +16346,8 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy);

BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy);
BlockExpr *Result = new (Context)
BlockExpr(BD, BlockTy, BSI->ContainsUnexpandedParameterPack);

// If the block isn't obviously global, i.e. it captures anything at
// all, then we need to do a few things in the surrounding context:
Expand All @@ -16379,6 +16370,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());
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Sema/SemaLambda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2350,7 +2350,10 @@ ExprResult Sema::BuildBlockForLambdaConversion(SourceLocation CurrentLocation,
Block->setBody(new (Context) CompoundStmt(ConvLocation));

// Create the block literal expression.
Expr *BuildBlock = new (Context) BlockExpr(Block, Conv->getConversionType());
// TODO: Do we ever get here if we have unexpanded packs in the lambda???
Expr *BuildBlock =
new (Context) BlockExpr(Block, Conv->getConversionType(),
/*ContainsUnexpandedParameterPack=*/false);
ExprCleanupObjects.push_back(Block);
Cleanup.setExprNeedsCleanups(true);

Expand Down
10 changes: 5 additions & 5 deletions clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1010,8 +1010,8 @@ NamedDecl *Sema::ActOnTypeParameter(Scope *S, bool Typename,
Param->setAccess(AS_public);

if (Param->isParameterPack())
if (auto *LSI = getEnclosingLambda())
LSI->LocalPacks.push_back(Param);
if (auto *CSI = getEnclosingLambdaOrBlock())
CSI->LocalPacks.push_back(Param);

if (ParamName) {
maybeDiagnoseTemplateParameterShadow(*this, S, ParamNameLoc, ParamName);
Expand Down Expand Up @@ -1542,8 +1542,8 @@ NamedDecl *Sema::ActOnNonTypeTemplateParameter(Scope *S, Declarator &D,
Param->setInvalidDecl();

if (Param->isParameterPack())
if (auto *LSI = getEnclosingLambda())
LSI->LocalPacks.push_back(Param);
if (auto *CSI = getEnclosingLambdaOrBlock())
CSI->LocalPacks.push_back(Param);

if (ParamName) {
maybeDiagnoseTemplateParameterShadow(*this, S, D.getIdentifierLoc(),
Expand Down Expand Up @@ -1593,7 +1593,7 @@ NamedDecl *Sema::ActOnTemplateTemplateParameter(
Param->setAccess(AS_public);

if (Param->isParameterPack())
if (auto *LSI = getEnclosingLambda())
if (auto *LSI = getEnclosingLambdaOrBlock())
LSI->LocalPacks.push_back(Param);

// If the template template parameter has a name, then link the identifier
Expand Down
12 changes: 9 additions & 3 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1693,12 +1693,18 @@ namespace {
if (SemaRef.RebuildingImmediateInvocation)
return E;
LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true,
/*InstantiatingLambda=*/true);
/*InstantiatingLambdaOrBlock=*/true);
Sema::ConstraintEvalRAII<TemplateInstantiator> RAII(*this);

return inherited::TransformLambdaExpr(E);
}

ExprResult TransformBlockExpr(BlockExpr *E) {
LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true,
/*InstantiatingLambdaOrBlock=*/true);
return inherited::TransformBlockExpr(E);
}

ExprResult RebuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
LambdaScopeInfo *LSI) {
CXXMethodDecl *MD = LSI->CallOperator;
Expand Down Expand Up @@ -2475,7 +2481,7 @@ QualType TemplateInstantiator::TransformFunctionProtoType(TypeLocBuilder &TLB,
CXXRecordDecl *ThisContext,
Qualifiers ThisTypeQuals,
Fn TransformExceptionSpec) {
// If this is a lambda, the transformation MUST be done in the
// If this is a lambda or block, the transformation MUST be done in the
// CurrentInstantiationScope since it introduces a mapping of
// the original to the newly created transformed parameters.
//
Expand All @@ -2484,7 +2490,7 @@ QualType TemplateInstantiator::TransformFunctionProtoType(TypeLocBuilder &TLB,
// a second one.
LocalInstantiationScope *Current = getSema().CurrentInstantiationScope;
std::optional<LocalInstantiationScope> Scope;
if (!Current || !Current->isLambda())
if (!Current || !Current->isLambdaOrBlock())
Scope.emplace(SemaRef, /*CombineWithOuterScope=*/true);

return inherited::TransformFunctionProtoType(
Expand Down
Loading
Loading