llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> placement-new'ing an object with a dead base object is not allowed, so we need to check all the pointer bases. --- Full diff: https://github.com/llvm/llvm-project/pull/141272.diff 7 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+3-2) - (modified) clang/lib/AST/ByteCode/DynamicAllocator.cpp (+4) - (modified) clang/lib/AST/ByteCode/Interp.cpp (+54-2) - (modified) clang/lib/AST/ByteCode/Interp.h (+3-15) - (modified) clang/lib/AST/ByteCode/Opcodes.td (+2-1) - (modified) clang/test/AST/ByteCode/new-delete.cpp (+3-4) - (modified) clang/test/AST/ByteCode/placement-new.cpp (+15-1) ``````````diff 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}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/141272 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
