[clang][bytecode] Handle CXXPseudoDestructorExprs #125835
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make lifetime management more explicit. We're only using this for CXXPseudoDestructorExprs for now but we need this to handle std::construct_at/placement-new after destructor calls later anyway.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesMake lifetime management more explicit. We're only using this for CXXPseudoDestructorExprs for now but we need this to handle std::construct_at/placement-new after destructor calls later anyway. Full diff: https://github.com/llvm/llvm-project/pull/125835.diff 7 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index c1408379b4c1b8..cf89cdc667acc2 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -4715,6 +4715,14 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
} else if (!this->visit(MC->getImplicitObjectArgument())) {
return false;
}
+ } else if (const auto *PD =
+ dyn_cast<CXXPseudoDestructorExpr>(E->getCallee())) {
+ const Expr *Base = PD->getBase();
+ if (!Base->isGLValue())
+ return this->discard(Base);
+ if (!this->visit(Base))
+ return false;
+ return this->emitKill(E);
} else if (!FuncDecl) {
const Expr *Callee = E->getCallee();
CalleeOffset = this->allocateLocalPrimitive(Callee, PT_FnPtr, true, false);
diff --git a/clang/lib/AST/ByteCode/Descriptor.h b/clang/lib/AST/ByteCode/Descriptor.h
index a73e28d2e600e8..96c82a18913e02 100644
--- a/clang/lib/AST/ByteCode/Descriptor.h
+++ b/clang/lib/AST/ByteCode/Descriptor.h
@@ -61,6 +61,11 @@ struct alignas(void *) GlobalInlineDescriptor {
};
static_assert(sizeof(GlobalInlineDescriptor) == sizeof(void *), "");
+enum class Lifetime : uint8_t {
+ Started,
+ Ended,
+};
+
/// Inline descriptor embedded in structures and arrays.
///
/// Such descriptors precede all composite array elements and structure fields.
@@ -100,12 +105,14 @@ struct InlineDescriptor {
LLVM_PREFERRED_TYPE(bool)
unsigned IsArrayElement : 1;
+ Lifetime LifeState;
+
const Descriptor *Desc;
InlineDescriptor(const Descriptor *D)
: Offset(sizeof(InlineDescriptor)), IsConst(false), IsInitialized(false),
IsBase(false), IsActive(false), IsFieldMutable(false),
- IsArrayElement(false), Desc(D) {}
+ IsArrayElement(false), LifeState(Lifetime::Started), Desc(D) {}
void dump() const { dump(llvm::errs()); }
void dump(llvm::raw_ostream &OS) const;
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 1123ced99eb069..bf48139f57c0f0 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -561,6 +561,18 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
return false;
}
+static bool CheckLifetime(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+ AccessKinds AK) {
+ if (Ptr.getLifetime() == Lifetime::Started)
+ return true;
+
+ if (!S.checkingPotentialConstantExpression()) {
+ S.FFDiag(S.Current->getSource(OpPC), diag::note_constexpr_access_uninit)
+ << AK << /*uninitialized=*/false << S.Current->getRange(OpPC);
+ }
+ return false;
+}
+
bool CheckGlobalInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
if (Ptr.isInitialized())
return true;
@@ -605,6 +617,8 @@ bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
return false;
if (!CheckActive(S, OpPC, Ptr, AK))
return false;
+ if (!CheckLifetime(S, OpPC, Ptr, AK))
+ return false;
if (!CheckInitialized(S, OpPC, Ptr, AK))
return false;
if (!CheckTemporary(S, OpPC, Ptr, AK))
@@ -634,6 +648,8 @@ bool CheckFinalLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
return false;
if (!CheckActive(S, OpPC, Ptr, AK_Read))
return false;
+ if (!CheckLifetime(S, OpPC, Ptr, AK_Read))
+ return false;
if (!CheckInitialized(S, OpPC, Ptr, AK_Read))
return false;
if (!CheckTemporary(S, OpPC, Ptr, AK_Read))
@@ -650,6 +666,8 @@ bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
return false;
if (!CheckDummy(S, OpPC, Ptr, AK_Assign))
return false;
+ if (!CheckLifetime(S, OpPC, Ptr, AK_Assign))
+ return false;
if (!CheckExtern(S, OpPC, Ptr))
return false;
if (!CheckRange(S, OpPC, Ptr, AK_Assign))
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 9f029adc708390..66fd31feb24f4e 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -1254,6 +1254,12 @@ bool GetLocal(InterpState &S, CodePtr OpPC, uint32_t I) {
return true;
}
+static inline bool Kill(InterpState &S, CodePtr OpPC) {
+ const auto &Ptr = S.Stk.pop<Pointer>();
+ Ptr.endLifetime();
+ return true;
+}
+
/// 1) Pops the value from the stack.
/// 2) Writes the value to the local variable with the
/// given offset.
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 4b0c902ab29268..088a3e40fe2a7b 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -394,6 +394,11 @@ def GetLocal : AccessOpcode { let HasCustomEval = 1; }
// [] -> [Pointer]
def SetLocal : AccessOpcode { let HasCustomEval = 1; }
+def Kill : Opcode {
+ let Types = [];
+ let Args = [];
+}
+
def CheckDecl : Opcode {
let Args = [ArgVarDecl];
}
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index 971b0d5e14cf8c..3970d5833fcdc1 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -687,6 +687,22 @@ class Pointer {
/// Deactivates an entire strurcutre.
void deactivate() const;
+ Lifetime getLifetime() const {
+ if (!isBlockPointer())
+ return Lifetime::Started;
+ if (asBlockPointer().Base < sizeof(InlineDescriptor))
+ return Lifetime::Started;
+ return getInlineDesc()->LifeState;
+ }
+
+ void endLifetime() const {
+ if (!isBlockPointer())
+ return;
+ if (asBlockPointer().Base < sizeof(InlineDescriptor))
+ return;
+ getInlineDesc()->LifeState = Lifetime::Ended;
+ }
+
/// Compare two pointers.
ComparisonCategoryResult compare(const Pointer &Other) const {
if (!hasSameBase(*this, Other))
diff --git a/clang/test/AST/ByteCode/cxx20.cpp b/clang/test/AST/ByteCode/cxx20.cpp
index a63aea1ea5548b..6f65fa5c7cfd3e 100644
--- a/clang/test/AST/ByteCode/cxx20.cpp
+++ b/clang/test/AST/ByteCode/cxx20.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexperimental-new-constant-interpreter -std=c++20 -verify=both,expected -fcxx-exceptions %s
+// RUN: %clang_cc1 -fcxx-exceptions -fexperimental-new-constant-interpreter -std=c++20 -verify=both,expected -fcxx-exceptions %s -DNEW_INTERP
// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=both,ref -fcxx-exceptions %s
void test_alignas_operand() {
@@ -931,3 +931,34 @@ namespace LocalDestroy {
}
static_assert(f() == 1);
}
+
+namespace PseudoDtor {
+ constexpr int f1() {
+ using T = int;
+ int a = 0;
+ a.~T();
+ return a; // both-note {{read of object outside its lifetime}}
+ }
+ static_assert(f1() == 0); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+
+ constexpr int f2() {
+ using T = int;
+ int a = 0;
+ a.~T();
+ a = 0; // both-note {{assignment to object outside its lifetime}}
+ return a;
+ }
+ static_assert(f2() == 0); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+
+#ifdef NEW_INTERP
+ /// FIXME: Currently crashes with the current interpreter, see https://github.com/llvm/llvm-project/issues/53741
+ constexpr int f3() {
+ using T = int;
+ 0 .~T();
+ return 0;
+ }
+ static_assert(f3() == 0);
+#endif
+}
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/15280 Here is the relevant piece of the build log for the reference |
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
Make lifetime management more explicit. We're only using this for CXXPseudoDestructorExprs for now but we need this to handle std::construct_at/placement-new after destructor calls later anyway.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make lifetime management more explicit. We're only using this for CXXPseudoDestructorExprs for now but we need this to handle std::construct_at/placement-new after destructor calls later anyway.