https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/137163
From 99fa9a1f5fb74401c790871e6eb1ce473d048d74 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/3] [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 a83320a7ddec2..ed56522d107dd 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3489,6 +3489,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)
@@ -3518,6 +3523,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 59c0e47c7c195..442e85b892a51 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -5467,3 +5467,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 7c933f47bf7f0..2379e78c1631a 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4596,10 +4596,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
@@ -4622,7 +4618,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
@@ -9125,9 +9126,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 e341332f9d825862b0f61a64944df4cff3cb5787 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/3] 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 ed56522d107dd..cc29baf69e61e 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3490,8 +3490,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:
@@ -3524,11 +3523,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 442e85b892a51..8e8cf8aa710df 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -5468,11 +5468,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 2379e78c1631a..203e7c794d308 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"
@@ -4522,6 +4523,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);
@@ -4587,45 +4620,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 &&
@@ -9130,15 +9125,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 ee8e0f88e974828ffd825881fd5c4edba045f254 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/3] 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 203e7c794d308..610f97d126f8c 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4525,14 +4525,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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
