https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/137163
From 55976eaeac5bba9c36b92d7aa99f8f589d6c9ef5 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadir...@google.com>
Date: Thu, 24 Apr 2025 11:12:00 +0200
Subject: [PATCH 1/4] [clang][CompundLiteralExpr] Don't defer evaluation for
CLEs
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.
---
clang/include/clang/AST/Expr.h | 12 ++++++++
clang/lib/AST/Expr.cpp | 9 ++++++
clang/lib/AST/ExprConstant.cpp | 33 ++++++++++++++++-----
clang/test/AST/static-compound-literals.cpp | 12 ++++++++
4 files changed, 58 insertions(+), 8 deletions(-)
create mode 100644 clang/test/AST/static-compound-literals.cpp
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index d95396fd59b95..f800a803d4774 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3534,6 +3534,11 @@ class CompoundLiteralExpr : public Expr {
/// The int part of the pair stores whether this expr is file scope.
llvm::PointerIntPair<TypeSourceInfo *, 1, bool> TInfoAndScope;
Stmt *Init;
+
+ /// Value of constant literals with static storage duration. Used only for
+ /// constant folding as CompoundLiteralExpr is not an ICE.
+ mutable APValue *StaticValue = nullptr;
+
public:
CompoundLiteralExpr(SourceLocation lparenloc, TypeSourceInfo *tinfo,
QualType T, ExprValueKind VK, Expr *init, bool fileScope)
@@ -3563,6 +3568,13 @@ class CompoundLiteralExpr : public Expr {
TInfoAndScope.setPointer(tinfo);
}
+ bool hasStaticStorage() const { return isFileScope() && isGLValue(); }
+ APValue *getOrCreateStaticValue(ASTContext &Ctx) const;
+ APValue &getStaticValue() const {
+ assert(StaticValue);
+ return *StaticValue;
+ }
+
SourceLocation getBeginLoc() const LLVM_READONLY {
// FIXME: Init should never be null.
if (!Init)
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 51d83c480c2f1..209558b8218f3 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -5446,3 +5446,12 @@ ConvertVectorExpr *ConvertVectorExpr::Create(
return new (Mem) ConvertVectorExpr(SrcExpr, TI, DstType, VK, OK, BuiltinLoc,
RParenLoc, FPFeatures);
}
+
+APValue *CompoundLiteralExpr::getOrCreateStaticValue(ASTContext &Ctx) const {
+ assert(hasStaticStorage());
+ if (!StaticValue) {
+ StaticValue = new (Ctx) APValue;
+ Ctx.addDestruction(StaticValue);
+ }
+ return StaticValue;
+}
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index bf9208763b1ab..399d3e17cc6ab 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4618,10 +4618,6 @@ handleLValueToRValueConversion(EvalInfo &Info, const
Expr *Conv, QualType Type,
return false;
}
- APValue Lit;
- if (!Evaluate(Lit, Info, CLE->getInitializer()))
- return false;
-
// According to GCC info page:
//
// 6.28 Compound Literals
@@ -4644,7 +4640,12 @@ handleLValueToRValueConversion(EvalInfo &Info, const
Expr *Conv, QualType Type,
}
}
- CompleteObject LitObj(LVal.Base, &Lit, Base->getType());
+ APValue *Lit =
+ CLE->hasStaticStorage()
+ ? &CLE->getStaticValue()
+ : Info.CurrentCall->getTemporary(Base, LVal.Base.getVersion());
+
+ CompleteObject LitObj(LVal.Base, Lit, Base->getType());
return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal, AK);
} else if (isa<StringLiteral>(Base) || isa<PredefinedExpr>(Base)) {
// Special-case character extraction so we don't have to construct an
@@ -9159,9 +9160,25 @@ bool
LValueExprEvaluator::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
assert((!Info.getLangOpts().CPlusPlus || E->isFileScope()) &&
"lvalue compound literal in c++?");
- // Defer visiting the literal until the lvalue-to-rvalue conversion. We can
- // only see this when folding in C, so there's no standard to follow here.
- return Success(E);
+ APValue *Lit;
+ // If CompountLiteral has static storage, its value can be used outside
+ // this expression. So evaluate it once and store it in ASTContext.
+ if (E->hasStaticStorage()) {
+ Lit = E->getOrCreateStaticValue(Info.Ctx);
+ Result.set(E);
+ // Reset any previously evaluated state, otherwise evaluation below might
+ // fail.
+ // FIXME: Should we just re-use the previously evaluated value instead?
+ *Lit = APValue();
+ } else {
+ Lit = &Info.CurrentCall->createTemporary(E, E->getInitializer()->getType(),
+ ScopeKind::FullExpression,
Result);
+ }
+ if (!EvaluateInPlace(*Lit, Info, Result, E->getInitializer())) {
+ *Lit = APValue();
+ return false;
+ }
+ return true;
}
bool LValueExprEvaluator::VisitCXXTypeidExpr(const CXXTypeidExpr *E) {
diff --git a/clang/test/AST/static-compound-literals.cpp
b/clang/test/AST/static-compound-literals.cpp
new file mode 100644
index 0000000000000..ceb8b985ab9dc
--- /dev/null
+++ b/clang/test/AST/static-compound-literals.cpp
@@ -0,0 +1,12 @@
+// Test that we can successfully compile this code, especially under ASAN.
+// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+// expected-no-diagnostics
+struct Foo {
+ Foo* f;
+ operator bool() const { return true; }
+};
+constexpr Foo f((Foo[]){});
+int foo() {
+ if (Foo(*f.f)) return 1;
+ return 0;
+}
From a3157ac44084d14087670c295350bd87e02e22f3 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadir...@google.com>
Date: Fri, 25 Apr 2025 12:39:33 +0200
Subject: [PATCH 2/4] Address comments
---
clang/include/clang/AST/Expr.h | 10 ++---
clang/lib/AST/Expr.cpp | 9 +++-
clang/lib/AST/ExprConstant.cpp | 78 ++++++++++++++++------------------
3 files changed, 47 insertions(+), 50 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index f800a803d4774..523c0326d47ef 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3535,8 +3535,7 @@ class CompoundLiteralExpr : public Expr {
llvm::PointerIntPair<TypeSourceInfo *, 1, bool> TInfoAndScope;
Stmt *Init;
- /// Value of constant literals with static storage duration. Used only for
- /// constant folding as CompoundLiteralExpr is not an ICE.
+ /// Value of constant literals with static storage duration.
mutable APValue *StaticValue = nullptr;
public:
@@ -3569,11 +3568,8 @@ class CompoundLiteralExpr : public Expr {
}
bool hasStaticStorage() const { return isFileScope() && isGLValue(); }
- APValue *getOrCreateStaticValue(ASTContext &Ctx) const;
- APValue &getStaticValue() const {
- assert(StaticValue);
- return *StaticValue;
- }
+ APValue &getOrCreateStaticValue(ASTContext &Ctx) const;
+ APValue &getStaticValue() const;
SourceLocation getBeginLoc() const LLVM_READONLY {
// FIXME: Init should never be null.
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 209558b8218f3..f9053a31075f3 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -5447,11 +5447,16 @@ ConvertVectorExpr *ConvertVectorExpr::Create(
RParenLoc, FPFeatures);
}
-APValue *CompoundLiteralExpr::getOrCreateStaticValue(ASTContext &Ctx) const {
+APValue &CompoundLiteralExpr::getOrCreateStaticValue(ASTContext &Ctx) const {
assert(hasStaticStorage());
if (!StaticValue) {
StaticValue = new (Ctx) APValue;
Ctx.addDestruction(StaticValue);
}
- return StaticValue;
+ return *StaticValue;
+}
+
+APValue &CompoundLiteralExpr::getStaticValue() const {
+ assert(StaticValue);
+ return *StaticValue;
}
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 399d3e17cc6ab..1874a44aaebfa 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -48,6 +48,7 @@
#include "clang/AST/OptionalDiagnostic.h"
#include "clang/AST/RecordLayout.h"
#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/Type.h"
#include "clang/AST/TypeLoc.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/DiagnosticSema.h"
@@ -4544,6 +4545,38 @@ static CompleteObject findCompleteObject(EvalInfo &Info,
const Expr *E,
BaseVal = MTE->getOrCreateValue(false);
assert(BaseVal && "got reference to unevaluated temporary");
+ } else if (const CompoundLiteralExpr *CLE =
+ dyn_cast_or_null<CompoundLiteralExpr>(Base)) {
+ // In C99, a CompoundLiteralExpr is an lvalue, and we defer evaluating
+ // the initializer until now for such expressions. Such an expression
+ // can't be an ICE in C, so this only matters for fold.
+ if (LValType.isVolatileQualified()) {
+ Info.FFDiag(E);
+ return CompleteObject();
+ }
+
+ // According to GCC info page:
+ //
+ // 6.28 Compound Literals
+ //
+ // As an optimization, G++ sometimes gives array compound literals
+ // longer lifetimes: when the array either appears outside a function
or
+ // has a const-qualified type. If foo and its initializer had elements
+ // of type char *const rather than char *, or if foo were a global
+ // variable, the array would have static storage duration. But it is
+ // probably safest just to avoid the use of array compound literals in
+ // C++ code.
+ //
+ // Obey that rule by checking constness for converted array types.
+ if (QualType CLETy = CLE->getType(); CLETy->isArrayType() &&
+ !LValType->isArrayType() &&
+ !CLETy.isConstant(Info.Ctx)) {
+ Info.FFDiag(E);
+ Info.Note(CLE->getExprLoc(), diag::note_declared_at);
+ return CompleteObject();
+ }
+
+ BaseVal = &CLE->getStaticValue();
} else {
if (!IsAccess)
return CompleteObject(LVal.getLValueBase(), nullptr, BaseType);
@@ -4609,45 +4642,7 @@ handleLValueToRValueConversion(EvalInfo &Info, const
Expr *Conv, QualType Type,
WantObjectRepresentation ? AK_ReadObjectRepresentation : AK_Read;
if (Base && !LVal.getLValueCallIndex() && !Type.isVolatileQualified()) {
- if (const CompoundLiteralExpr *CLE = dyn_cast<CompoundLiteralExpr>(Base)) {
- // In C99, a CompoundLiteralExpr is an lvalue, and we defer evaluating
the
- // initializer until now for such expressions. Such an expression can't
be
- // an ICE in C, so this only matters for fold.
- if (Type.isVolatileQualified()) {
- Info.FFDiag(Conv);
- return false;
- }
-
- // According to GCC info page:
- //
- // 6.28 Compound Literals
- //
- // As an optimization, G++ sometimes gives array compound literals longer
- // lifetimes: when the array either appears outside a function or has a
- // const-qualified type. If foo and its initializer had elements of type
- // char *const rather than char *, or if foo were a global variable, the
- // array would have static storage duration. But it is probably safest
- // just to avoid the use of array compound literals in C++ code.
- //
- // Obey that rule by checking constness for converted array types.
-
- QualType CLETy = CLE->getType();
- if (CLETy->isArrayType() && !Type->isArrayType()) {
- if (!CLETy.isConstant(Info.Ctx)) {
- Info.FFDiag(Conv);
- Info.Note(CLE->getExprLoc(), diag::note_declared_at);
- return false;
- }
- }
-
- APValue *Lit =
- CLE->hasStaticStorage()
- ? &CLE->getStaticValue()
- : Info.CurrentCall->getTemporary(Base, LVal.Base.getVersion());
-
- CompleteObject LitObj(LVal.Base, Lit, Base->getType());
- return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal, AK);
- } else if (isa<StringLiteral>(Base) || isa<PredefinedExpr>(Base)) {
+ if (isa<StringLiteral>(Base) || isa<PredefinedExpr>(Base)) {
// Special-case character extraction so we don't have to construct an
// APValue for the whole string.
assert(LVal.Designator.Entries.size() <= 1 &&
@@ -9164,15 +9159,16 @@ LValueExprEvaluator::VisitCompoundLiteralExpr(const
CompoundLiteralExpr *E) {
// If CompountLiteral has static storage, its value can be used outside
// this expression. So evaluate it once and store it in ASTContext.
if (E->hasStaticStorage()) {
- Lit = E->getOrCreateStaticValue(Info.Ctx);
+ Lit = &E->getOrCreateStaticValue(Info.Ctx);
Result.set(E);
// Reset any previously evaluated state, otherwise evaluation below might
// fail.
// FIXME: Should we just re-use the previously evaluated value instead?
*Lit = APValue();
} else {
+ assert(!Info.getLangOpts().CPlusPlus);
Lit = &Info.CurrentCall->createTemporary(E, E->getInitializer()->getType(),
- ScopeKind::FullExpression,
Result);
+ ScopeKind::Block, Result);
}
if (!EvaluateInPlace(*Lit, Info, Result, E->getInitializer())) {
*Lit = APValue();
From bb08584a6101257bb586222dbadf2d0546d87063 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadir...@google.com>
Date: Mon, 28 Apr 2025 16:45:44 +0200
Subject: [PATCH 3/4] Get rid of redundant volatile check
---
clang/lib/AST/ExprConstant.cpp | 8 --------
1 file changed, 8 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 1874a44aaebfa..2dda6ac9c8e98 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4547,14 +4547,6 @@ static CompleteObject findCompleteObject(EvalInfo &Info,
const Expr *E,
assert(BaseVal && "got reference to unevaluated temporary");
} else if (const CompoundLiteralExpr *CLE =
dyn_cast_or_null<CompoundLiteralExpr>(Base)) {
- // In C99, a CompoundLiteralExpr is an lvalue, and we defer evaluating
- // the initializer until now for such expressions. Such an expression
- // can't be an ICE in C, so this only matters for fold.
- if (LValType.isVolatileQualified()) {
- Info.FFDiag(E);
- return CompleteObject();
- }
-
// According to GCC info page:
//
// 6.28 Compound Literals
From 6ca1c586d9e1bcc477ef345c968f0528cdbf2a7e Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadir...@google.com>
Date: Fri, 27 Jun 2025 11:20:16 +0200
Subject: [PATCH 4/4] add new test cases
---
.../test/AST/static-compound-literals-crash.cpp | 17 +++++++++++++++++
.../AST/static-compound-literals-reeval.cpp | 9 +++++++++
2 files changed, 26 insertions(+)
create mode 100644 clang/test/AST/static-compound-literals-crash.cpp
create mode 100644 clang/test/AST/static-compound-literals-reeval.cpp
diff --git a/clang/test/AST/static-compound-literals-crash.cpp
b/clang/test/AST/static-compound-literals-crash.cpp
new file mode 100644
index 0000000000000..1b37bfe8fad7b
--- /dev/null
+++ b/clang/test/AST/static-compound-literals-crash.cpp
@@ -0,0 +1,17 @@
+// FIXME: These test cases currently crash during codegen, despite initializers
+// for CLEs being constant.
+// RUN: not --crash %clang_cc1 -verify -std=c++20 -emit-llvm %s
+// expected-no-diagnostics
+namespace case1 {
+struct RR { int&& r; };
+struct Z { RR* x; };
+constinit Z z = { (RR[1]){1} };
+}
+
+
+namespace case2 {
+struct RR { int r; };
+struct Z { int x; const RR* y; int z; };
+inline int f() { return 0; }
+Z z2 = { 10, (const RR[1]){__builtin_constant_p(z2.x)}, z2.y->r+f() };
+}
diff --git a/clang/test/AST/static-compound-literals-reeval.cpp
b/clang/test/AST/static-compound-literals-reeval.cpp
new file mode 100644
index 0000000000000..7798f213b1be4
--- /dev/null
+++ b/clang/test/AST/static-compound-literals-reeval.cpp
@@ -0,0 +1,9 @@
+// Test that we can successfully compile this code, especially under ASAN.
+// RUN: %clang_cc1 -emit-llvm -std=c++20 %s -o- | FileCheck %s
+struct RR { int r; };
+struct Z { int x; const RR* y; int z; };
+constinit Z z = { 10, (const RR[1]){__builtin_constant_p(z.x)}, z.y->r };
+// Check that we zero-initialize z.y->r.
+// CHECK: @.compoundliteral = internal constant [1 x %struct.RR]
zeroinitializer
+// FIXME: Despite of z.y->r being 0, we evaluate z.z to 1.
+// CHECK: @z = global %struct.Z { i32 10, ptr @.compoundliteral, i32 1 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
