llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: kadir çetinkaya (kadircet) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/137163.diff 4 Files Affected: - (modified) clang/include/clang/AST/Expr.h (+12) - (modified) clang/lib/AST/Expr.cpp (+9) - (modified) clang/lib/AST/ExprConstant.cpp (+25-8) - (added) clang/test/AST/static-compound-literals.cpp (+12) ``````````diff diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index a83320a7ddec2..95c0f910c22f8 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; +} `````````` </details> https://github.com/llvm/llvm-project/pull/137163 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
