https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/140441
We need a place to destroy the temporaries created for call arguments. >From 47a04ed14736a66d6f437e704322397ca3b2d255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Fri, 16 May 2025 12:47:37 +0200 Subject: [PATCH] [clang][bytecode] Add a scope to function calls We need a place to destroy the temporaries created for call arguments. --- clang/lib/AST/ByteCode/Compiler.cpp | 117 +++++++++++++++----------- clang/lib/AST/ByteCode/Compiler.h | 59 +++++++------ clang/test/AST/ByteCode/lifetimes.cpp | 18 ++++ 3 files changed, 121 insertions(+), 73 deletions(-) diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 5017c9b76e6d1..aa8f009f58bb9 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -45,10 +45,6 @@ template <class Emitter> class DeclScope final : public LocalScope<Emitter> { Ctx->InitStack.push_back(InitLink::Decl(VD)); } - void addExtended(const Scope::Local &Local) override { - return this->addLocal(Local); - } - ~DeclScope() { this->Ctx->InitializingDecl = OldInitializingDecl; this->Ctx->InitStack.pop_back(); @@ -2021,6 +2017,45 @@ bool Compiler<Emitter>::visitArrayElemInit(unsigned ElemIndex, const Expr *Init, return this->emitFinishInitPop(Init); } +template <class Emitter> +bool Compiler<Emitter>::visitCallArgs(ArrayRef<const Expr *> Args, + const FunctionDecl *FuncDecl) { + assert(VarScope->getKind() == ScopeKind::Call); + llvm::BitVector NonNullArgs = collectNonNullArgs(FuncDecl, Args); + + unsigned ArgIndex = 0; + for (const Expr *Arg : Args) { + if (std::optional<PrimType> T = classify(Arg)) { + if (!this->visit(Arg)) + return false; + } else { + + std::optional<unsigned> LocalIndex = allocateLocal( + Arg, Arg->getType(), /*ExtendingDecl=*/nullptr, ScopeKind::Call); + if (!LocalIndex) + return false; + + if (!this->emitGetPtrLocal(*LocalIndex, Arg)) + return false; + InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex)); + if (!this->visitInitializer(Arg)) + return false; + } + + if (FuncDecl && NonNullArgs[ArgIndex]) { + PrimType ArgT = classify(Arg).value_or(PT_Ptr); + if (ArgT == PT_Ptr) { + if (!this->emitCheckNonNullArg(ArgT, Arg)) + return false; + } + } + + ++ArgIndex; + } + + return true; +} + template <class Emitter> bool Compiler<Emitter>::VisitInitListExpr(const InitListExpr *E) { return this->visitInitList(E->inits(), E->getArrayFiller(), E); @@ -4343,7 +4378,7 @@ bool Compiler<Emitter>::emitConst(const APSInt &Value, const Expr *E) { template <class Emitter> unsigned Compiler<Emitter>::allocateLocalPrimitive( DeclTy &&Src, PrimType Ty, bool IsConst, const ValueDecl *ExtendingDecl, - bool IsConstexprUnknown) { + ScopeKind SC, bool IsConstexprUnknown) { // Make sure we don't accidentally register the same decl twice. if (const auto *VD = dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) { @@ -4364,14 +4399,14 @@ unsigned Compiler<Emitter>::allocateLocalPrimitive( if (ExtendingDecl) VarScope->addExtended(Local, ExtendingDecl); else - VarScope->add(Local, false); + VarScope->addForScopeKind(Local, SC); return Local.Offset; } template <class Emitter> std::optional<unsigned> Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty, - const ValueDecl *ExtendingDecl, + const ValueDecl *ExtendingDecl, ScopeKind SC, bool IsConstexprUnknown) { // Make sure we don't accidentally register the same decl twice. if ([[maybe_unused]] const auto *VD = @@ -4409,7 +4444,7 @@ Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty, if (ExtendingDecl) VarScope->addExtended(Local, ExtendingDecl); else - VarScope->add(Local, false); + VarScope->addForScopeKind(Local, SC); return Local.Offset; } @@ -4676,7 +4711,7 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD, if (VarT) { unsigned Offset = this->allocateLocalPrimitive( VD, *VarT, VD->getType().isConstQualified(), nullptr, - IsConstexprUnknown); + ScopeKind::Block, IsConstexprUnknown); if (Init) { // If this is a toplevel declaration, create a scope for the // initializer. @@ -4692,8 +4727,9 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD, } } } else { - if (std::optional<unsigned> Offset = this->allocateLocal( - VD, VD->getType(), nullptr, IsConstexprUnknown)) { + if (std::optional<unsigned> Offset = + this->allocateLocal(VD, VD->getType(), nullptr, ScopeKind::Block, + IsConstexprUnknown)) { if (!Init) return true; @@ -4881,26 +4917,28 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) { if (FuncDecl) { if (unsigned BuiltinID = FuncDecl->getBuiltinID()) return VisitBuiltinCallExpr(E, BuiltinID); - } - // Calls to replaceable operator new/operator delete. - if (FuncDecl && - FuncDecl->isUsableAsGlobalAllocationFunctionInConstantEvaluation()) { - if (FuncDecl->getDeclName().isAnyOperatorNew()) { - return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_new); - } else { - assert(FuncDecl->getDeclName().getCXXOverloadedOperator() == OO_Delete); - return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_delete); + // Calls to replaceable operator new/operator delete. + if (FuncDecl->isUsableAsGlobalAllocationFunctionInConstantEvaluation()) { + if (FuncDecl->getDeclName().isAnyOperatorNew()) { + return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_new); + } else { + assert(FuncDecl->getDeclName().getCXXOverloadedOperator() == OO_Delete); + return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_delete); + } + } + + // Explicit calls to trivial destructors + if (const auto *DD = dyn_cast<CXXDestructorDecl>(FuncDecl); + DD && DD->isTrivial()) { + const auto *MemberCall = cast<CXXMemberCallExpr>(E); + if (!this->visit(MemberCall->getImplicitObjectArgument())) + return false; + return this->emitCheckDestruction(E) && this->emitPopPtr(E); } } - // Explicit calls to trivial destructors - if (const auto *DD = dyn_cast_if_present<CXXDestructorDecl>(FuncDecl); - DD && DD->isTrivial()) { - const auto *MemberCall = cast<CXXMemberCallExpr>(E); - if (!this->visit(MemberCall->getImplicitObjectArgument())) - return false; - return this->emitCheckDestruction(E) && this->emitPopPtr(E); - } + + BlockScope<Emitter> CallScope(this, ScopeKind::Call); QualType ReturnType = E->getCallReturnType(Ctx.getASTContext()); std::optional<PrimType> T = classify(ReturnType); @@ -4996,23 +5034,8 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) { return false; } - llvm::BitVector NonNullArgs = collectNonNullArgs(FuncDecl, Args); - // Put arguments on the stack. - unsigned ArgIndex = 0; - for (const auto *Arg : Args) { - if (!this->visit(Arg)) - return false; - - // If we know the callee already, check the known parametrs for nullability. - if (FuncDecl && NonNullArgs[ArgIndex]) { - PrimType ArgT = classify(Arg).value_or(PT_Ptr); - if (ArgT == PT_Ptr) { - if (!this->emitCheckNonNullArg(ArgT, Arg)) - return false; - } - } - ++ArgIndex; - } + if (!this->visitCallArgs(Args, FuncDecl)) + return false; // Undo the argument reversal we did earlier. if (IsAssignmentOperatorCall) { @@ -5088,9 +5111,9 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) { // Cleanup for discarded return values. if (DiscardResult && !ReturnType->isVoidType() && T) - return this->emitPop(*T, E); + return this->emitPop(*T, E) && CallScope.destroyLocals(); - return true; + return CallScope.destroyLocals(); } template <class Emitter> diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index 56a972f452af9..ac3ad84766dc6 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -102,6 +102,8 @@ struct VarCreationState { bool notCreated() const { return !S; } }; +enum class ScopeKind { Call, Block }; + /// Compilation context for expressions. template <class Emitter> class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, @@ -305,17 +307,19 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, const Expr *E); bool visitArrayElemInit(unsigned ElemIndex, const Expr *Init, std::optional<PrimType> InitT); + bool visitCallArgs(ArrayRef<const Expr *> Args, const FunctionDecl *FuncDecl); /// Creates a local primitive value. unsigned allocateLocalPrimitive(DeclTy &&Decl, PrimType Ty, bool IsConst, const ValueDecl *ExtendingDecl = nullptr, + ScopeKind SC = ScopeKind::Block, bool IsConstexprUnknown = false); /// Allocates a space storing a local given its type. std::optional<unsigned> allocateLocal(DeclTy &&Decl, QualType Ty = QualType(), const ValueDecl *ExtendingDecl = nullptr, - bool IsConstexprUnknown = false); + ScopeKind = ScopeKind::Block, bool IsConstexprUnknown = false); std::optional<unsigned> allocateTemporary(const Expr *E); private: @@ -451,28 +455,16 @@ extern template class Compiler<EvalEmitter>; /// Scope chain managing the variable lifetimes. template <class Emitter> class VariableScope { public: - VariableScope(Compiler<Emitter> *Ctx, const ValueDecl *VD) - : Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD) { + VariableScope(Compiler<Emitter> *Ctx, const ValueDecl *VD, + ScopeKind Kind = ScopeKind::Block) + : Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD), Kind(Kind) { Ctx->VarScope = this; } virtual ~VariableScope() { Ctx->VarScope = this->Parent; } - void add(const Scope::Local &Local, bool IsExtended) { - if (IsExtended) - this->addExtended(Local); - else - this->addLocal(Local); - } - virtual void addLocal(const Scope::Local &Local) { - if (this->Parent) - this->Parent->addLocal(Local); - } - - virtual void addExtended(const Scope::Local &Local) { - if (this->Parent) - this->Parent->addExtended(Local); + llvm_unreachable("Shouldn't be called"); } void addExtended(const Scope::Local &Local, const ValueDecl *ExtendingDecl) { @@ -496,10 +488,28 @@ template <class Emitter> class VariableScope { this->addLocal(Local); } + /// Like addExtended, but adds to the nearest scope of the given kind. + void addForScopeKind(const Scope::Local &Local, ScopeKind Kind) { + VariableScope *P = this; + while (P) { + if (P->Kind == Kind) { + P->addLocal(Local); + return; + } + P = P->Parent; + if (!P) + break; + } + + // Add to this scope. + this->addLocal(Local); + } + virtual void emitDestruction() {} virtual bool emitDestructors(const Expr *E = nullptr) { return true; } virtual bool destroyLocals(const Expr *E = nullptr) { return true; } VariableScope *getParent() const { return Parent; } + ScopeKind getKind() const { return Kind; } protected: /// Compiler instance. @@ -507,12 +517,14 @@ template <class Emitter> class VariableScope { /// Link to the parent scope. VariableScope *Parent; const ValueDecl *ValDecl = nullptr; + ScopeKind Kind; }; /// Generic scope for local variables. template <class Emitter> class LocalScope : public VariableScope<Emitter> { public: - LocalScope(Compiler<Emitter> *Ctx) : VariableScope<Emitter>(Ctx, nullptr) {} + LocalScope(Compiler<Emitter> *Ctx, ScopeKind Kind = ScopeKind::Block) + : VariableScope<Emitter>(Ctx, nullptr, Kind) {} LocalScope(Compiler<Emitter> *Ctx, const ValueDecl *VD) : VariableScope<Emitter>(Ctx, VD) {} @@ -599,16 +611,11 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { }; /// Scope for storage declared in a compound statement. +// FIXME: Remove? template <class Emitter> class BlockScope final : public LocalScope<Emitter> { public: - BlockScope(Compiler<Emitter> *Ctx) : LocalScope<Emitter>(Ctx) {} - - void addExtended(const Scope::Local &Local) override { - // If we to this point, just add the variable as a normal local - // variable. It will be destroyed at the end of the block just - // like all others. - this->addLocal(Local); - } + BlockScope(Compiler<Emitter> *Ctx, ScopeKind Kind = ScopeKind::Block) + : LocalScope<Emitter>(Ctx, Kind) {} }; template <class Emitter> class ArrayIndexScope final { diff --git a/clang/test/AST/ByteCode/lifetimes.cpp b/clang/test/AST/ByteCode/lifetimes.cpp index 658f696cad2e8..5e021387348b5 100644 --- a/clang/test/AST/ByteCode/lifetimes.cpp +++ b/clang/test/AST/ByteCode/lifetimes.cpp @@ -88,3 +88,21 @@ namespace PseudoDtor { // both-note {{visible outside that expression}} }; } + +/// Diagnostic differences +namespace CallScope { + struct Q { + int n = 0; + constexpr int f() const { return 0; } + }; + constexpr Q *out_of_lifetime(Q q) { return &q; } // both-warning {{address of stack}} + constexpr int k3 = out_of_lifetime({})->n; // both-error {{must be initialized by a constant expression}} \ + // expected-note {{read of temporary whose lifetime has ended}} \ + // expected-note {{temporary created here}} \ + // ref-note {{read of object outside its lifetime}} + + constexpr int k4 = out_of_lifetime({})->f(); // both-error {{must be initialized by a constant expression}} \ + // expected-note {{member call on temporary whose lifetime has ended}} \ + // expected-note {{temporary created here}} \ + // ref-note {{member call on object outside its lifetime}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits