Skip to content

Commit f72e53f

Browse files
authored
[clang][CompundLiteralExpr] Don't defer evaluation for CLEs (#137163)
Previously we would defer evaluation of CLEs until LValue to RValue conversions, which would result in creating values within wrong scope and triggering use-after-frees. This patch instead eagerly evaluates CLEs, within the scope requiring them. This requires storing an extra pointer for CLE expressions with static storage. Fixes #137165
1 parent 39bc052 commit f72e53f

File tree

6 files changed

+109
-41
lines changed

6 files changed

+109
-41
lines changed

clang/include/clang/AST/Expr.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3534,6 +3534,10 @@ class CompoundLiteralExpr : public Expr {
35343534
/// The int part of the pair stores whether this expr is file scope.
35353535
llvm::PointerIntPair<TypeSourceInfo *, 1, bool> TInfoAndScope;
35363536
Stmt *Init;
3537+
3538+
/// Value of constant literals with static storage duration.
3539+
mutable APValue *StaticValue = nullptr;
3540+
35373541
public:
35383542
CompoundLiteralExpr(SourceLocation lparenloc, TypeSourceInfo *tinfo,
35393543
QualType T, ExprValueKind VK, Expr *init, bool fileScope)
@@ -3563,6 +3567,10 @@ class CompoundLiteralExpr : public Expr {
35633567
TInfoAndScope.setPointer(tinfo);
35643568
}
35653569

3570+
bool hasStaticStorage() const { return isFileScope() && isGLValue(); }
3571+
APValue &getOrCreateStaticValue(ASTContext &Ctx) const;
3572+
APValue &getStaticValue() const;
3573+
35663574
SourceLocation getBeginLoc() const LLVM_READONLY {
35673575
// FIXME: Init should never be null.
35683576
if (!Init)

clang/lib/AST/Expr.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5446,3 +5446,17 @@ ConvertVectorExpr *ConvertVectorExpr::Create(
54465446
return new (Mem) ConvertVectorExpr(SrcExpr, TI, DstType, VK, OK, BuiltinLoc,
54475447
RParenLoc, FPFeatures);
54485448
}
5449+
5450+
APValue &CompoundLiteralExpr::getOrCreateStaticValue(ASTContext &Ctx) const {
5451+
assert(hasStaticStorage());
5452+
if (!StaticValue) {
5453+
StaticValue = new (Ctx) APValue;
5454+
Ctx.addDestruction(StaticValue);
5455+
}
5456+
return *StaticValue;
5457+
}
5458+
5459+
APValue &CompoundLiteralExpr::getStaticValue() const {
5460+
assert(StaticValue);
5461+
return *StaticValue;
5462+
}

clang/lib/AST/ExprConstant.cpp

Lines changed: 49 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#include "clang/AST/OptionalDiagnostic.h"
4949
#include "clang/AST/RecordLayout.h"
5050
#include "clang/AST/StmtVisitor.h"
51+
#include "clang/AST/Type.h"
5152
#include "clang/AST/TypeLoc.h"
5253
#include "clang/Basic/Builtins.h"
5354
#include "clang/Basic/DiagnosticSema.h"
@@ -4534,6 +4535,30 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
45344535

45354536
BaseVal = MTE->getOrCreateValue(false);
45364537
assert(BaseVal && "got reference to unevaluated temporary");
4538+
} else if (const CompoundLiteralExpr *CLE =
4539+
dyn_cast_or_null<CompoundLiteralExpr>(Base)) {
4540+
// According to GCC info page:
4541+
//
4542+
// 6.28 Compound Literals
4543+
//
4544+
// As an optimization, G++ sometimes gives array compound literals
4545+
// longer lifetimes: when the array either appears outside a function or
4546+
// has a const-qualified type. If foo and its initializer had elements
4547+
// of type char *const rather than char *, or if foo were a global
4548+
// variable, the array would have static storage duration. But it is
4549+
// probably safest just to avoid the use of array compound literals in
4550+
// C++ code.
4551+
//
4552+
// Obey that rule by checking constness for converted array types.
4553+
if (QualType CLETy = CLE->getType(); CLETy->isArrayType() &&
4554+
!LValType->isArrayType() &&
4555+
!CLETy.isConstant(Info.Ctx)) {
4556+
Info.FFDiag(E);
4557+
Info.Note(CLE->getExprLoc(), diag::note_declared_at);
4558+
return CompleteObject();
4559+
}
4560+
4561+
BaseVal = &CLE->getStaticValue();
45374562
} else {
45384563
if (!IsAccess)
45394564
return CompleteObject(LVal.getLValueBase(), nullptr, BaseType);
@@ -4599,44 +4624,7 @@ handleLValueToRValueConversion(EvalInfo &Info, const Expr *Conv, QualType Type,
45994624
WantObjectRepresentation ? AK_ReadObjectRepresentation : AK_Read;
46004625

46014626
if (Base && !LVal.getLValueCallIndex() && !Type.isVolatileQualified()) {
4602-
if (const CompoundLiteralExpr *CLE = dyn_cast<CompoundLiteralExpr>(Base)) {
4603-
// In C99, a CompoundLiteralExpr is an lvalue, and we defer evaluating the
4604-
// initializer until now for such expressions. Such an expression can't be
4605-
// an ICE in C, so this only matters for fold.
4606-
if (Type.isVolatileQualified()) {
4607-
Info.FFDiag(Conv);
4608-
return false;
4609-
}
4610-
4611-
APValue Lit;
4612-
if (!Evaluate(Lit, Info, CLE->getInitializer()))
4613-
return false;
4614-
4615-
// According to GCC info page:
4616-
//
4617-
// 6.28 Compound Literals
4618-
//
4619-
// As an optimization, G++ sometimes gives array compound literals longer
4620-
// lifetimes: when the array either appears outside a function or has a
4621-
// const-qualified type. If foo and its initializer had elements of type
4622-
// char *const rather than char *, or if foo were a global variable, the
4623-
// array would have static storage duration. But it is probably safest
4624-
// just to avoid the use of array compound literals in C++ code.
4625-
//
4626-
// Obey that rule by checking constness for converted array types.
4627-
4628-
QualType CLETy = CLE->getType();
4629-
if (CLETy->isArrayType() && !Type->isArrayType()) {
4630-
if (!CLETy.isConstant(Info.Ctx)) {
4631-
Info.FFDiag(Conv);
4632-
Info.Note(CLE->getExprLoc(), diag::note_declared_at);
4633-
return false;
4634-
}
4635-
}
4636-
4637-
CompleteObject LitObj(LVal.Base, &Lit, Base->getType());
4638-
return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal, AK);
4639-
} else if (isa<StringLiteral>(Base) || isa<PredefinedExpr>(Base)) {
4627+
if (isa<StringLiteral>(Base) || isa<PredefinedExpr>(Base)) {
46404628
// Special-case character extraction so we don't have to construct an
46414629
// APValue for the whole string.
46424630
assert(LVal.Designator.Entries.size() <= 1 &&
@@ -9144,9 +9132,29 @@ bool
91449132
LValueExprEvaluator::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
91459133
assert((!Info.getLangOpts().CPlusPlus || E->isFileScope()) &&
91469134
"lvalue compound literal in c++?");
9147-
// Defer visiting the literal until the lvalue-to-rvalue conversion. We can
9148-
// only see this when folding in C, so there's no standard to follow here.
9149-
return Success(E);
9135+
APValue *Lit;
9136+
// If CompountLiteral has static storage, its value can be used outside
9137+
// this expression. So evaluate it once and store it in ASTContext.
9138+
if (E->hasStaticStorage()) {
9139+
Lit = &E->getOrCreateStaticValue(Info.Ctx);
9140+
Result.set(E);
9141+
// Reset any previously evaluated state, otherwise evaluation below might
9142+
// fail.
9143+
// FIXME: Should we just re-use the previously evaluated value instead?
9144+
*Lit = APValue();
9145+
} else {
9146+
assert(!Info.getLangOpts().CPlusPlus);
9147+
Lit = &Info.CurrentCall->createTemporary(E, E->getInitializer()->getType(),
9148+
ScopeKind::Block, Result);
9149+
}
9150+
// FIXME: Evaluating in place isn't always right. We should figure out how to
9151+
// use appropriate evaluation context here, see
9152+
// clang/test/AST/static-compound-literals-reeval.cpp for a failure.
9153+
if (!EvaluateInPlace(*Lit, Info, Result, E->getInitializer())) {
9154+
*Lit = APValue();
9155+
return false;
9156+
}
9157+
return true;
91509158
}
91519159

91529160
bool LValueExprEvaluator::VisitCXXTypeidExpr(const CXXTypeidExpr *E) {
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// FIXME: These test cases currently crash during codegen, despite initializers
2+
// for CLEs being constant.
3+
// RUN: not --crash %clang_cc1 -verify -std=c++20 -emit-llvm %s
4+
// expected-no-diagnostics
5+
namespace case1 {
6+
struct RR { int&& r; };
7+
struct Z { RR* x; };
8+
constinit Z z = { (RR[1]){1} };
9+
}
10+
11+
12+
namespace case2 {
13+
struct RR { int r; };
14+
struct Z { int x; const RR* y; int z; };
15+
inline int f() { return 0; }
16+
Z z2 = { 10, (const RR[1]){__builtin_constant_p(z2.x)}, z2.y->r+f() };
17+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Test that we can successfully compile this code, especially under ASAN.
2+
// RUN: %clang_cc1 -emit-llvm -std=c++20 %s -o- | FileCheck %s
3+
struct RR { int r; };
4+
struct Z { int x; const RR* y; int z; };
5+
constinit Z z = { 10, (const RR[1]){__builtin_constant_p(z.x)}, z.y->r };
6+
// Check that we zero-initialize z.y->r.
7+
// CHECK: @.compoundliteral = internal constant [1 x %struct.RR] zeroinitializer
8+
// FIXME: Despite of z.y->r being 0, we evaluate z.z to 1.
9+
// CHECK: global %struct.Z { i32 10, ptr @.compoundliteral, i32 1 }
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Test that we can successfully compile this code, especially under ASAN.
2+
// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
3+
// expected-no-diagnostics
4+
struct Foo {
5+
Foo* f;
6+
operator bool() const { return true; }
7+
};
8+
constexpr Foo f((Foo[]){});
9+
int foo() {
10+
if (Foo(*f.f)) return 1;
11+
return 0;
12+
}

0 commit comments

Comments
 (0)