-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[clang][bytecode] Handle CXXPseudoDestructorExprs #125835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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.
@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
|
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.
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.