Skip to content

[clang][bytecode] Check lifetime of all ptr bases in placement-new #141272

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

Merged
merged 1 commit into from
May 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions clang/lib/AST/ByteCode/Compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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 =
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/AST/ByteCode/DynamicAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
56 changes: 54 additions & 2 deletions clang/lib/AST/ByteCode/Interp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand All @@ -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))
Expand Down
18 changes: 3 additions & 15 deletions clang/lib/AST/ByteCode/Interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/AST/ByteCode/Opcodes.td
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 3 additions & 4 deletions clang/test/AST/ByteCode/new-delete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 15 additions & 1 deletion clang/test/AST/ByteCode/placement-new.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}}
}