[clang] [clang][bytecode] Check lifetime of all ptr bases in placement-new (PR #141272)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Fri May 23 12:15:21 PDT 2025
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/141272
>From 9a8f1f46b1a4e9610b1c3e6d1124ba6321896da9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Fri, 23 May 2025 19:41:59 +0200
Subject: [PATCH] [clang][bytecode] Check lifetime of all ptr bases in
placement-new
placement-new'ing an object with a dead base object is not allowed,
so we need to check all the pointer bases.
---
clang/lib/AST/ByteCode/Compiler.cpp | 5 +-
clang/lib/AST/ByteCode/DynamicAllocator.cpp | 4 ++
clang/lib/AST/ByteCode/Interp.cpp | 56 ++++++++++++++++++++-
clang/lib/AST/ByteCode/Interp.h | 18 ++-----
clang/lib/AST/ByteCode/Opcodes.td | 3 +-
clang/test/AST/ByteCode/new-delete.cpp | 7 ++-
clang/test/AST/ByteCode/placement-new.cpp | 16 +++++-
7 files changed, 84 insertions(+), 25 deletions(-)
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 54a4647a604fb..bf38b2e5d537d 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -4927,7 +4927,8 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
const auto *MemberCall = cast<CXXMemberCallExpr>(E);
if (!this->visit(MemberCall->getImplicitObjectArgument()))
return false;
- return this->emitCheckDestruction(E) && this->emitPopPtr(E);
+ return this->emitCheckDestruction(E) && this->emitEndLifetime(E) &&
+ this->emitPopPtr(E);
}
}
@@ -5016,7 +5017,7 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
return this->discard(Base);
if (!this->visit(Base))
return false;
- return this->emitKill(E);
+ return this->emitEndLifetimePop(E);
} else if (!FuncDecl) {
const Expr *Callee = E->getCallee();
CalleeOffset =
diff --git a/clang/lib/AST/ByteCode/DynamicAllocator.cpp b/clang/lib/AST/ByteCode/DynamicAllocator.cpp
index 945f35cea017e..733984508ed79 100644
--- a/clang/lib/AST/ByteCode/DynamicAllocator.cpp
+++ b/clang/lib/AST/ByteCode/DynamicAllocator.cpp
@@ -86,6 +86,10 @@ Block *DynamicAllocator::allocate(const Descriptor *D, unsigned EvalID,
ID->IsInitialized = false;
ID->IsVolatile = false;
+ ID->LifeState =
+ AllocForm == Form::Operator ? Lifetime::Ended : Lifetime::Started;
+ ;
+
B->IsDynamic = true;
if (auto It = AllocationSites.find(D->asExpr()); It != AllocationSites.end())
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 6d6beef73a726..e454d9e3bc218 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1699,6 +1699,50 @@ bool CallPtr(InterpState &S, CodePtr OpPC, uint32_t ArgSize,
return Call(S, OpPC, F, VarArgSize);
}
+bool StartLifetime(InterpState &S, CodePtr OpPC) {
+ const auto &Ptr = S.Stk.peek<Pointer>();
+ if (!CheckDummy(S, OpPC, Ptr, AK_Destroy))
+ return false;
+
+ Ptr.startLifetime();
+ return true;
+}
+
+// FIXME: It might be better to the recursing as part of the generated code for
+// a destructor?
+static void endLifetimeRecurse(const Pointer &Ptr) {
+ Ptr.endLifetime();
+ if (const Record *R = Ptr.getRecord()) {
+ for (const Record::Field &Fi : R->fields())
+ endLifetimeRecurse(Ptr.atField(Fi.Offset));
+ return;
+ }
+
+ if (const Descriptor *FieldDesc = Ptr.getFieldDesc();
+ FieldDesc->isCompositeArray()) {
+ for (unsigned I = 0; I != FieldDesc->getNumElems(); ++I)
+ endLifetimeRecurse(Ptr.atIndex(I).narrow());
+ }
+}
+
+/// Ends the lifetime of the peek'd pointer.
+bool EndLifetime(InterpState &S, CodePtr OpPC) {
+ const auto &Ptr = S.Stk.peek<Pointer>();
+ if (!CheckDummy(S, OpPC, Ptr, AK_Destroy))
+ return false;
+ endLifetimeRecurse(Ptr);
+ return true;
+}
+
+/// Ends the lifetime of the pop'd pointer.
+bool EndLifetimePop(InterpState &S, CodePtr OpPC) {
+ const auto &Ptr = S.Stk.pop<Pointer>();
+ if (!CheckDummy(S, OpPC, Ptr, AK_Destroy))
+ return false;
+ endLifetimeRecurse(Ptr);
+ return true;
+}
+
bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
std::optional<uint64_t> ArraySize) {
const Pointer &Ptr = S.Stk.peek<Pointer>();
@@ -1711,8 +1755,16 @@ bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
return false;
if (!CheckDummy(S, OpPC, Ptr, AK_Construct))
return false;
- if (!CheckLifetime(S, OpPC, Ptr, AK_Construct))
- return false;
+
+ // CheckLifetime for this and all base pointers.
+ for (Pointer P = Ptr;;) {
+ if (!CheckLifetime(S, OpPC, P, AK_Construct)) {
+ return false;
+ }
+ if (P.isRoot())
+ break;
+ P = P.getBase();
+ }
if (!CheckExtern(S, OpPC, Ptr))
return false;
if (!CheckRange(S, OpPC, Ptr, AK_Construct))
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 881a8b12c2626..5473733578d7e 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -1326,21 +1326,9 @@ 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>();
- if (!CheckDummy(S, OpPC, Ptr, AK_Destroy))
- return false;
- Ptr.endLifetime();
- return true;
-}
-
-static inline bool StartLifetime(InterpState &S, CodePtr OpPC) {
- const auto &Ptr = S.Stk.peek<Pointer>();
- if (!CheckDummy(S, OpPC, Ptr, AK_Destroy))
- return false;
- Ptr.startLifetime();
- return true;
-}
+bool EndLifetime(InterpState &S, CodePtr OpPC);
+bool EndLifetimePop(InterpState &S, CodePtr OpPC);
+bool StartLifetime(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 c8db8da113bd4..7031d7026fac4 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -395,7 +395,8 @@ def GetLocal : AccessOpcode { let HasCustomEval = 1; }
// [] -> [Pointer]
def SetLocal : AccessOpcode { let HasCustomEval = 1; }
-def Kill : Opcode;
+def EndLifetimePop : Opcode;
+def EndLifetime : Opcode;
def StartLifetime : Opcode;
def CheckDecl : Opcode {
diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp
index 6726659d49e71..1ee41a98e13bb 100644
--- a/clang/test/AST/ByteCode/new-delete.cpp
+++ b/clang/test/AST/ByteCode/new-delete.cpp
@@ -682,17 +682,16 @@ namespace OperatorNewDelete {
}
static_assert(zeroAlloc());
- /// FIXME: This is broken in the current interpreter.
constexpr int arrayAlloc() {
int *F = std::allocator<int>().allocate(2);
- F[0] = 10; // ref-note {{assignment to object outside its lifetime is not allowed in a constant expression}}
+ F[0] = 10; // both-note {{assignment to object outside its lifetime is not allowed in a constant expression}}
F[1] = 13;
int Res = F[1] + F[0];
std::allocator<int>().deallocate(F);
return Res;
}
- static_assert(arrayAlloc() == 23); // ref-error {{not an integral constant expression}} \
- // ref-note {{in call to}}
+ static_assert(arrayAlloc() == 23); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
struct S {
int i;
diff --git a/clang/test/AST/ByteCode/placement-new.cpp b/clang/test/AST/ByteCode/placement-new.cpp
index 81f7782c6f252..a301c96739c83 100644
--- a/clang/test/AST/ByteCode/placement-new.cpp
+++ b/clang/test/AST/ByteCode/placement-new.cpp
@@ -17,13 +17,16 @@ namespace std {
// both-note {{placement new would change type of storage from 'int' to 'float'}} \
// both-note {{construction of subobject of member 'x' of union with active member 'a' is not allowed in a constant expression}} \
// both-note {{construction of temporary is not allowed}} \
- // both-note {{construction of heap allocated object that has been deleted}}
+ // both-note {{construction of heap allocated object that has been deleted}} \
+ // both-note {{construction of subobject of object outside its lifetime is not allowed in a constant expression}}
}
}
void *operator new(std::size_t, void *p) { return p; }
void* operator new[] (std::size_t, void* p) {return p;}
+constexpr int no_lifetime_start = (*std::allocator<int>().allocate(1) = 1); // both-error {{constant expression}} \
+ // both-note {{assignment to object outside its lifetime}}
consteval auto ok1() {
bool b;
@@ -409,3 +412,14 @@ namespace PlacementNewAfterDelete {
static_assert(construct_after_lifetime()); // both-error {{}} \
// both-note {{in call}}
}
+
+namespace SubObj {
+ constexpr bool construct_after_lifetime_2() {
+ struct A { struct B {} b; };
+ A a;
+ a.~A();
+ std::construct_at<A::B>(&a.b); // both-note {{in call}}
+ return true;
+ }
+ static_assert(construct_after_lifetime_2()); // both-error {{}} both-note {{in call}}
+}
More information about the cfe-commits
mailing list