https://github.com/akash-manna-sky updated https://github.com/llvm/llvm-project/pull/226023
>From b9e07419d5137b1738e18b55b42f014fecd1a04a Mon Sep 17 00:00:00 2001 From: Akash Manna <[email protected]> Date: Thu, 24 Sep 2026 10:56:13 +0530 Subject: [PATCH] [clang][bytecode] Mark globals pointing to temporaries as uninitialized In C++ a compound literal is a prvalue, so `&(struct s){1, 2}` only compiles as the -Waddress-of-temporary extension and wraps the literal in a temporary that dies at the end of the full-expression. The interpreter stored a pointer to that temporary in the global and left the global marked as initialized. Once the evaluation ended the dead block was freed and the stored pointer lost its Pointee, so the next read through it asserted in getDeclDesc(). The same state leaked out of the existing "initializer failed" paths as well, e.g. for a const struct global initialized with a new expression, because invokeDtor() destroys the stored pointers but leaves the inline initialized bits set. Program::markGlobalUninitialized() now destroys what has been stored, re-runs the block constructor so every subobject reads as uninitialized again and sets InitializerFailed; the two existing failure sites use it too. A new CheckGlobalInit opcode runs after the value has been returned from visitDeclAndReturn(), so the "pointer to temporary" note is still produced, and resets the global if any pointer in it (fields, bases and array elements included) refers to a block without static storage duration. Fixes #167840 --- clang/lib/AST/ByteCode/Compiler.cpp | 28 +++++------ clang/lib/AST/ByteCode/Interp.cpp | 70 ++++++++++++++++++++++++++++ clang/lib/AST/ByteCode/Interp.h | 3 ++ clang/lib/AST/ByteCode/Opcodes.td | 4 ++ clang/lib/AST/ByteCode/Program.cpp | 11 +++++ clang/lib/AST/ByteCode/Program.h | 4 ++ clang/test/AST/ByteCode/literals.cpp | 63 +++++++++++++++++++++++++ 7 files changed, 166 insertions(+), 17 deletions(-) diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index d7bdb2f217a9a..7f956d138956b 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -5503,13 +5503,8 @@ VarCreationState Compiler<Emitter>::visitDecl(const VarDecl *VD) { return true; if (!R && Context::shouldBeGloballyIndexed(VD)) { - if (auto GlobalIndex = P.getGlobal(VD)) { - Block *GlobalBlock = P.getGlobal(*GlobalIndex); - auto &GD = GlobalBlock->getBlockDesc<GlobalInlineDescriptor>(); - - GD.InitState = GlobalInitState::InitializerFailed; - GlobalBlock->invokeDtor(); - } + if (auto GlobalIndex = P.getGlobal(VD)) + P.markGlobalUninitialized(*GlobalIndex); } return R; @@ -5537,8 +5532,9 @@ bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD, const Expr *Init, OptPrimType VarT = classify(VD->getType()); bool IsReference = VD->getType()->isReferenceType(); + UnsignedOrNone GlobalIndex = std::nullopt; if (Context::shouldBeGloballyIndexed(VD)) { - auto GlobalIndex = P.getGlobal(VD); + GlobalIndex = P.getGlobal(VD); assert(GlobalIndex); // visitVarDecl() didn't return false. if (VarT) { if (!this->emitGetGlobalUnchecked(*VarT, *GlobalIndex, VD)) @@ -5566,18 +5562,16 @@ bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD, const Expr *Init, if (!this->emitRet(VarT.value_or(PT_Ptr), VD)) { // If the Ret above failed and this is a global variable, mark it as // uninitialized, even everything else succeeded. - if (Context::shouldBeGloballyIndexed(VD)) { - auto GlobalIndex = P.getGlobal(VD); - assert(GlobalIndex); - Block *GlobalBlock = P.getGlobal(*GlobalIndex); - auto &GD = GlobalBlock->getBlockDesc<GlobalInlineDescriptor>(); - - GD.InitState = GlobalInitState::InitializerFailed; - GlobalBlock->invokeDtor(); - } + if (GlobalIndex) + P.markGlobalUninitialized(*GlobalIndex); return false; } + // Returning a pointer to a local or temporary succeeds here since the caller + // diagnoses that, but the global must not stay initialized with it. + if (GlobalIndex && !this->emitCheckGlobalInit(*GlobalIndex, VD)) + return false; + return VDScope.destroyLocals() && this->emitCheckAllocations(VD); } diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index b6975f250b3c1..2f44090f6cc02 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -3183,6 +3183,76 @@ bool FinishInitGlobal(InterpState &S) { return true; } +static bool pointsToNonStaticBlock(const Pointer &Ptr) { + return Ptr.isBlockPointer() && !Ptr.isZero() && !Ptr.block()->isStatic(); +} + +/// Checks if the object \p Ptr points to, or any of its subobjects, holds a +/// pointer to a block without static storage duration. +static bool holdsPointerToNonStaticBlock(PtrView Ptr, + bool IsCompleteClass = true) { + const Descriptor *Desc = Ptr.getFieldDesc(); + + if (Desc->isPrimitive()) { + return Desc->getPrimType() == PT_Ptr && + pointsToNonStaticBlock(Ptr.deref<Pointer>()); + } + + if (Desc->isPrimitiveArray()) { + if (Desc->getPrimType() != PT_Ptr) + return false; + for (unsigned I = 0, N = Desc->getNumElems(); I != N; ++I) { + if (pointsToNonStaticBlock(Ptr.elem<Pointer>(I))) + return true; + } + return false; + } + + if (Desc->isCompositeArray()) { + for (unsigned I = 0, N = Desc->getNumElems(); I != N; ++I) { + if (holdsPointerToNonStaticBlock(Ptr.atIndex(I).narrow())) + return true; + } + return false; + } + + if (!Desc->isRecord()) + return false; + + const Record *R = Desc->ElemRecord; + for (const Record::Base &B : R->bases()) { + if (holdsPointerToNonStaticBlock(Ptr.atField(B.Offset), + /*IsCompleteClass=*/false)) + return true; + } + for (const Record::Field &F : R->fields()) { + if (holdsPointerToNonStaticBlock(Ptr.atField(F.Offset))) + return true; + } + if (!IsCompleteClass) + return false; + for (const Record::Base &B : R->virtual_bases()) { + if (holdsPointerToNonStaticBlock(Ptr.atField(B.Offset), + /*IsCompleteClass=*/false)) + return true; + } + return false; +} + +bool CheckGlobalInit(InterpState &S, uint32_t I) { + Block *B = S.P.getGlobal(I); + if (B->getBlockDesc<GlobalInlineDescriptor>().InitState != + GlobalInitState::Initialized) + return true; + + // Such blocks are gone once this evaluation ends, so the initializer isn't a + // constant expression and the pointers must not be kept around. + PtrView Root{B, B->getMetadataSize(), B->getMetadataSize()}; + if (holdsPointerToNonStaticBlock(Root)) + S.P.markGlobalUninitialized(I); + return true; +} + bool InvalidCast(InterpState &S, CodePtr OpPC, CastKind Kind, bool Fatal) { const SourceLocation &Loc = S.Current->getLocation(OpPC); diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index edbf2adcda637..4ec2c91d7343a 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -2165,6 +2165,9 @@ inline bool FinishInitActivatePop(InterpState &S) { } bool FinishInitGlobal(InterpState &S); +/// Marks the global as uninitialized if its initializer left it pointing to a +/// local variable, a temporary or a dynamic allocation. +bool CheckGlobalInit(InterpState &S, uint32_t I); inline bool Dump(InterpState &S) { S.Stk.dump(); diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td index 11ba6ddf41b74..e345ea8723c2d 100644 --- a/clang/lib/AST/ByteCode/Opcodes.td +++ b/clang/lib/AST/ByteCode/Opcodes.td @@ -518,6 +518,10 @@ def InitGlobalTempComp : Opcode { let Args = [ArgLETD]; let NeedsOpPC = 0; } +// [] -> [] +def CheckGlobalInit : SuccessOpcode { + let Args = [ArgUint32]; +} // [Value] -> [] def SetGlobal : AccessOpcode; diff --git a/clang/lib/AST/ByteCode/Program.cpp b/clang/lib/AST/ByteCode/Program.cpp index cf6b7e1682121..ce877a758e3ee 100644 --- a/clang/lib/AST/ByteCode/Program.cpp +++ b/clang/lib/AST/ByteCode/Program.cpp @@ -22,6 +22,17 @@ Pointer Program::getPtrGlobal(unsigned Idx) const { return Pointer(Globals[Idx]->block()); } +void Program::markGlobalUninitialized(unsigned Idx) { + Block *B = getGlobal(Idx); + if (B->isInitialized()) + B->invokeDtor(); + // Re-run the constructor so all subobjects are uninitialized again. This + // also zeroes the metadata, so the InitState has to be set afterwards. + B->invokeCtor(); + B->getBlockDesc<GlobalInlineDescriptor>().InitState = + GlobalInitState::InitializerFailed; +} + UnsignedOrNone Program::getGlobal(const ValueDecl *VD) { if (auto It = GlobalIndices.find(VD); It != GlobalIndices.end()) return It->second; diff --git a/clang/lib/AST/ByteCode/Program.h b/clang/lib/AST/ByteCode/Program.h index ccd6720cce738..ed5b3f2e0cc8e 100644 --- a/clang/lib/AST/ByteCode/Program.h +++ b/clang/lib/AST/ByteCode/Program.h @@ -73,6 +73,10 @@ class Program final { return getPtrGlobal(Index).isInitialized(); } + /// Marks the global as uninitialized after its initializer failed, destroying + /// everything that has been stored in it so far. + void markGlobalUninitialized(unsigned Index); + /// Finds a global's index. UnsignedOrNone getGlobal(const ValueDecl *VD); UnsignedOrNone getGlobal(const Expr *E); diff --git a/clang/test/AST/ByteCode/literals.cpp b/clang/test/AST/ByteCode/literals.cpp index 97eab655032b6..df32fcab269ad 100644 --- a/clang/test/AST/ByteCode/literals.cpp +++ b/clang/test/AST/ByteCode/literals.cpp @@ -986,6 +986,69 @@ namespace CompoundLiterals { both-note {{in call to 'f3(nullptr, nullptr)'}} } +namespace GH167840 { + /// In C++ the compound literals are temporaries, so these globals dangle. + extern void abort(void); + + struct s { + int a; + int b; + }; + + struct s *s0 = &(struct s){1, 2}; // both-error {{taking the address of a temporary object of type 'struct s'}} + struct s *s1 = &(struct s){1, 2}; // both-error {{taking the address of a temporary object of type 'struct s'}} + const struct s *s2 = &(const struct s){1, 2}; // both-error {{taking the address of a temporary object of type 'const struct s'}} \ + // both-note {{declared here}} + + void foo(void) { + if (s0->a != 1 || s0->b != 2 || s1->a != 1 || s1->b != 2 || s2->a != 1 || + s2->b != 2) + abort(); + } + + static_assert(s2->a == 1, ""); // both-error {{not an integral constant expression}} \ + // ref-note {{read of non-constexpr variable 's2' is not allowed in a constant expression}} \ + // expected-note {{initializer of 's2' is not a constant expression}} + + struct P { + const s *p; + }; + const P sub = { &(const s){5} }; // both-error {{taking the address of a temporary object of type 'const s'}} \ + // both-note {{declared here}} + void bar(void) { + if (sub.p->a != 5) + abort(); + } + static_assert(sub.p->a == 5, ""); // both-error {{not an integral constant expression}} \ + // both-note {{initializer of 'sub' is not a constant expression}} + + /// Reads through the lifetime-extended temporary are rejected by CheckTemporary. + const P &ref = P{ &(const s){5, 6} }; // both-error {{taking the address of a temporary object of type 'const s'}} \ + // ref-note {{declared here}} \ + // expected-note {{temporary created here}} + void qux(void) { + if (ref.p->a != 5) + abort(); + } + static_assert(ref.p->a == 5, ""); // both-error {{not an integral constant expression}} \ + // ref-note {{initializer of 'ref' is not a constant expression}} \ + // expected-note {{read of temporary is not allowed in a constant expression outside the expression that created the temporary}} + +#if __cplusplus >= 202002L + /// Same with a pointer to a heap allocation. + struct H { + s *p; + }; + const H heap = { new s{1, 2} }; // both-note {{declared here}} + void baz(void) { + if (heap.p->a != 1) + abort(); + } + static_assert(heap.p->a == 1, ""); // both-error {{not an integral constant expression}} \ + // both-note {{initializer of 'heap' is not a constant expression}} +#endif +} + namespace TypeTraits { static_assert(__is_trivial(int), ""); static_assert(__is_trivial(float), ""); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
