https://github.com/akash-manna-sky created
https://github.com/llvm/llvm-project/pull/226023
Fixes #167840
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 (Sema's implicit-conversion analysis of
the `if` condition, which skips the null check) asserted in `getDeclDesc()`.
Nothing to do with there being three literals or the `const` mix, by the way:
the reduced case in the issue with a single global crashes the same way. The
existing "initializer failed" paths leaked the same state, e.g. for a `const`
struct global initialized with a `new` expression, since `invokeDtor()`
destroys the stored pointers but leaves the inline initialized bits set, so a
later field read still loaded them.
After analyzing the suggestion in the issue, such a global is now marked
uninitialized. `Program::markGlobalUninitialized()` 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 `CheckLValueConstantExpression` still gets to say
"pointer to temporary is not a constant expression", and resets the global if
any pointer in it (fields, bases and array elements included) refers to a block
without static storage duration, i.e. a local, a temporary or a heap
allocation. Later reads then report that the initializer is not a constant
expression instead of crashing.
>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