[clang] [clang][bytecode] Mark pointers destroyed in destructors (PR #192460)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 16 07:04:07 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Timm Baeder (tbaederr)
<details>
<summary>Changes</summary>
We didn't use to do this at all, so calling the destructor explicitly twice in a row wasn't an error. Calling it and accessing the object afterwards wasn't an error either.
---
Full diff: https://github.com/llvm/llvm-project/pull/192460.diff
8 Files Affected:
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+3)
- (modified) clang/lib/AST/ByteCode/Descriptor.h (+1)
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+39-23)
- (modified) clang/lib/AST/ByteCode/Interp.h (+1)
- (modified) clang/lib/AST/ByteCode/Opcodes.td (+1)
- (modified) clang/lib/AST/ByteCode/Pointer.cpp (+11-23)
- (modified) clang/lib/AST/ByteCode/Pointer.h (+1)
- (modified) clang/test/AST/ByteCode/cxx20.cpp (+19-8)
``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 59510612d9617..46416f8e78be6 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -6910,6 +6910,9 @@ bool Compiler<Emitter>::compileDestructor(const CXXDestructorDecl *Dtor) {
return false;
}
+ if (!this->emitMarkDestroyed(Dtor))
+ return false;
+
// FIXME: Virtual bases.
return this->emitPopPtr(Dtor) && this->emitRetVoid(Dtor);
}
diff --git a/clang/lib/AST/ByteCode/Descriptor.h b/clang/lib/AST/ByteCode/Descriptor.h
index 9046801f4ebef..daf7251aa0603 100644
--- a/clang/lib/AST/ByteCode/Descriptor.h
+++ b/clang/lib/AST/ByteCode/Descriptor.h
@@ -54,6 +54,7 @@ static_assert(sizeof(GlobalInlineDescriptor) == sizeof(void *), "");
enum class Lifetime : uint8_t {
Started,
+ Destroyed,
Ended,
};
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 5bc1067518558..17d194355208d 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1506,6 +1506,22 @@ static bool checkConstructor(InterpState &S, CodePtr OpPC, const Function *Func,
return false;
}
+static bool diagnoseOutOfLifetimeDestroy(InterpState &S, CodePtr OpPC,
+ const Pointer &Ptr) {
+ assert(Ptr.getLifetime() != Lifetime::Started);
+ // Try to use the declaration for better diagnostics
+ if (const Decl *D = Ptr.getDeclDesc()->asDecl()) {
+ auto *ND = cast<NamedDecl>(D);
+ S.FFDiag(ND->getLocation(), diag::note_constexpr_destroy_out_of_lifetime)
+ << ND->getNameAsString();
+ } else {
+ S.FFDiag(Ptr.getDeclDesc()->getLocation(),
+ diag::note_constexpr_destroy_out_of_lifetime)
+ << Ptr.toDiagnosticString(S.getASTContext());
+ }
+ return false;
+}
+
bool CheckDestructor(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
if (!CheckLive(S, OpPC, Ptr, AK_Destroy))
return false;
@@ -1513,8 +1529,11 @@ bool CheckDestructor(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
return false;
if (!CheckRange(S, OpPC, Ptr, AK_Destroy))
return false;
- if (!CheckLifetime(S, OpPC, Ptr, AK_Destroy))
- return false;
+
+ if (Ptr.getLifetime() == Lifetime::Destroyed)
+ return diagnoseOutOfLifetimeDestroy(S, OpPC, Ptr);
+ if (Ptr.getLifetime() == Lifetime::Ended)
+ return CheckLifetime(S, OpPC, Ptr, AK_Destroy);
// Can't call a dtor on a global variable.
if (Ptr.block()->isStatic()) {
@@ -1954,11 +1973,11 @@ bool StartLifetime(InterpState &S, CodePtr OpPC) {
// FIXME: It might be better to the recursing as part of the generated code for
// a destructor?
-static void endLifetimeRecurse(const Pointer &Ptr) {
+static void setLifeStateRecurse(const Pointer &Ptr, Lifetime L) {
if (const Record *R = Ptr.getRecord()) {
- Ptr.endLifetime();
+ Ptr.setLifeState(L);
for (const Record::Field &Fi : R->fields())
- endLifetimeRecurse(Ptr.atField(Fi.Offset));
+ setLifeStateRecurse(Ptr.atField(Fi.Offset), L);
return;
}
@@ -1967,11 +1986,11 @@ static void endLifetimeRecurse(const Pointer &Ptr) {
// No endLifetime() for array roots.
assert(Ptr.getLifetime() == Lifetime::Started);
for (unsigned I = 0; I != FieldDesc->getNumElems(); ++I)
- endLifetimeRecurse(Ptr.atIndex(I).narrow());
+ setLifeStateRecurse(Ptr.atIndex(I).narrow(), L);
return;
}
- Ptr.endLifetime();
+ Ptr.setLifeState(L);
}
/// Ends the lifetime of the peek'd pointer.
@@ -1980,7 +1999,7 @@ bool EndLifetime(InterpState &S, CodePtr OpPC) {
if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Destroy))
return false;
- endLifetimeRecurse(Ptr.narrow());
+ setLifeStateRecurse(Ptr.narrow(), Lifetime::Ended);
return true;
}
@@ -1990,7 +2009,16 @@ bool EndLifetimePop(InterpState &S, CodePtr OpPC) {
if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Destroy))
return false;
- endLifetimeRecurse(Ptr.narrow());
+ setLifeStateRecurse(Ptr.narrow(), Lifetime::Ended);
+ return true;
+}
+
+bool MarkDestroyed(InterpState &S, CodePtr OpPC) {
+ const auto &Ptr = S.Stk.peek<Pointer>();
+ if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Destroy))
+ return false;
+
+ setLifeStateRecurse(Ptr.narrow(), Lifetime::Destroyed);
return true;
}
@@ -2489,20 +2517,8 @@ bool Destroy(InterpState &S, CodePtr OpPC, uint32_t I) {
if (!S.Current->getLocalBlock(Local.Offset)->isInitialized())
continue;
const Pointer &Ptr = S.Current->getLocalPointer(Local.Offset);
- if (Ptr.getLifetime() == Lifetime::Ended) {
- // Try to use the declaration for better diagnostics
- if (const Decl *D = Ptr.getDeclDesc()->asDecl()) {
- auto *ND = cast<NamedDecl>(D);
- S.FFDiag(ND->getLocation(),
- diag::note_constexpr_destroy_out_of_lifetime)
- << ND->getNameAsString();
- } else {
- S.FFDiag(Ptr.getDeclDesc()->getLocation(),
- diag::note_constexpr_destroy_out_of_lifetime)
- << Ptr.toDiagnosticString(S.getASTContext());
- }
- return false;
- }
+ if (Ptr.getLifetime() == Lifetime::Ended)
+ return diagnoseOutOfLifetimeDestroy(S, OpPC, Ptr);
}
S.Current->destroy(I);
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index ea94df2c90187..2bc74fcfe7b95 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -1544,6 +1544,7 @@ bool GetLocal(InterpState &S, CodePtr OpPC, uint32_t I) {
bool EndLifetime(InterpState &S, CodePtr OpPC);
bool EndLifetimePop(InterpState &S, CodePtr OpPC);
bool StartLifetime(InterpState &S, CodePtr OpPC);
+bool MarkDestroyed(InterpState &S, CodePtr OpPC);
/// 1) Pops the value from the stack.
/// 2) Writes the value to the local variable with the
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 0215cd92966ee..849dde1dd1152 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -432,6 +432,7 @@ def SetLocal : AccessOpcode { let HasCustomEval = 1; }
def EndLifetimePop : Opcode;
def EndLifetime : Opcode;
+def MarkDestroyed : Opcode;
def StartLifetime : Opcode;
def CheckDecl : Opcode {
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index a0a1c64bfd975..b1f88b2a3a316 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -504,44 +504,32 @@ bool Pointer::isElementAlive(unsigned Index) const {
return IM->isElementAlive(Index);
}
-void Pointer::startLifetime() const {
- if (!isBlockPointer())
- return;
- if (BS.Base < sizeof(InlineDescriptor))
- return;
+void Pointer::startLifetime() const { setLifeState(Lifetime::Started); }
- if (inArray()) {
- const Descriptor *Desc = getFieldDesc();
- InitMapPtr &IM = getInitMap();
- if (!IM.hasInitMap())
- IM.setInitMap(new InitMap(Desc->getNumElems(), IM.allInitialized()));
-
- IM->startElementLifetime(getIndex());
- assert(isArrayRoot() || (this->getLifetime() == Lifetime::Started));
- return;
- }
-
- getInlineDesc()->LifeState = Lifetime::Started;
-}
+void Pointer::endLifetime() const { setLifeState(Lifetime::Ended); }
-void Pointer::endLifetime() const {
+void Pointer::setLifeState(Lifetime L) const {
if (!isBlockPointer())
return;
if (BS.Base < sizeof(InlineDescriptor))
return;
- if (inArray()) {
+ if (inArray() && !isArrayRoot()) {
+ assert(L == Lifetime::Started || L == Lifetime::Ended);
const Descriptor *Desc = getFieldDesc();
InitMapPtr &IM = getInitMap();
if (!IM.hasInitMap())
IM.setInitMap(new InitMap(Desc->getNumElems(), IM.allInitialized()));
- IM->endElementLifetime(getIndex());
- assert(isArrayRoot() || (this->getLifetime() == Lifetime::Ended));
+ if (L == Lifetime::Ended)
+ IM->endElementLifetime(getIndex());
+ else if (L == Lifetime::Started)
+ IM->startElementLifetime(getIndex());
+ assert(isArrayRoot() || (this->getLifetime() == L));
return;
}
- getInlineDesc()->LifeState = Lifetime::Ended;
+ getInlineDesc()->LifeState = L;
}
void Pointer::initialize() const {
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index 6573d3b12e0e9..1c2b2f7699d17 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -789,6 +789,7 @@ class Pointer {
/// InlineDescriptor as well as primitive array elements. This function is
/// used by std::destroy_at.
void endLifetime() const;
+ void setLifeState(Lifetime L) const;
/// Strip base casts from this Pointer.
/// The result is either a root pointer or something
diff --git a/clang/test/AST/ByteCode/cxx20.cpp b/clang/test/AST/ByteCode/cxx20.cpp
index 27ba0349d634e..4c4624d7505e2 100644
--- a/clang/test/AST/ByteCode/cxx20.cpp
+++ b/clang/test/AST/ByteCode/cxx20.cpp
@@ -390,21 +390,16 @@ namespace Destructors {
}
static_assert(E() == 1, "");
-
- /// FIXME: This should be rejected, since we call the destructor
- /// twice. However, GCC doesn't care either.
constexpr int ManualDtor() {
int i = 0;
{
- Inc I(i); // ref-note {{destroying object 'I' whose lifetime has already ended}}
+ Inc I(i); // both-note {{destroying object 'I' whose lifetime has already ended}}
I.~Inc();
}
return i;
}
- static_assert(ManualDtor() == 1, ""); // expected-error {{static assertion failed}} \
- // expected-note {{evaluates to '2 == 1'}} \
- // ref-error {{not an integral constant expression}} \
- // ref-note {{in call to 'ManualDtor()'}}
+ static_assert(ManualDtor() == 1, ""); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to 'ManualDtor()'}}
constexpr void doInc(int &i) {
Inc I(i);
@@ -562,6 +557,22 @@ namespace Destructors {
constexpr Outer O;
static_assert(O.bar() == 12);
+
+ struct S {
+ int a;
+ constexpr ~S() {}
+ };
+
+ constexpr int foo() {
+ S s; // both-note {{declared here}}
+ s.a = 10;
+ s.~S();
+ s.a = 11; // both-note {{assignment to object outside its lifetime}}
+ return 20;
+ }
+ static_assert(foo() == 20); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+
}
namespace BaseAndFieldInit {
``````````
</details>
https://github.com/llvm/llvm-project/pull/192460
More information about the cfe-commits
mailing list