Author: Timm Baeder Date: 2024-10-10T08:40:14+02:00 New Revision: f93258e4aca518cba3d48db59ed6143ca19ca99b
URL: https://github.com/llvm/llvm-project/commit/f93258e4aca518cba3d48db59ed6143ca19ca99b DIFF: https://github.com/llvm/llvm-project/commit/f93258e4aca518cba3d48db59ed6143ca19ca99b.diff LOG: [clang][bytecode] Diagnose class-specific operator delete calls (#111700) Added: Modified: clang/lib/AST/ByteCode/Compiler.cpp clang/lib/AST/ByteCode/Interp.cpp clang/lib/AST/ByteCode/Interp.h clang/lib/AST/ByteCode/Opcodes.td clang/test/AST/ByteCode/new-delete.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index f270de1054c61b..fe44238ea11869 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -3406,7 +3406,7 @@ bool Compiler<Emitter>::VisitCXXDeleteExpr(const CXXDeleteExpr *E) { if (!this->visit(Arg)) return false; - return this->emitFree(E->isArrayForm(), E); + return this->emitFree(E->isArrayForm(), E->isGlobalDelete(), E); } template <class Emitter> diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 82e11743cc5296..95715655cc9bbd 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -964,7 +964,7 @@ static bool runRecordDestructor(InterpState &S, CodePtr OpPC, return true; } -bool RunDestructors(InterpState &S, CodePtr OpPC, const Block *B) { +static bool RunDestructors(InterpState &S, CodePtr OpPC, const Block *B) { assert(B); const Descriptor *Desc = B->getDescriptor(); @@ -989,6 +989,89 @@ bool RunDestructors(InterpState &S, CodePtr OpPC, const Block *B) { return runRecordDestructor(S, OpPC, Pointer(const_cast<Block *>(B)), Desc); } +bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm, + bool IsGlobalDelete) { + if (!CheckDynamicMemoryAllocation(S, OpPC)) + return false; + + const Expr *Source = nullptr; + const Block *BlockToDelete = nullptr; + { + // Extra scope for this so the block doesn't have this pointer + // pointing to it when we destroy it. + Pointer Ptr = S.Stk.pop<Pointer>(); + + // Deleteing nullptr is always fine. + if (Ptr.isZero()) + return true; + + // Remove base casts. + while (Ptr.isBaseClass()) + Ptr = Ptr.getBase(); + + if (!Ptr.isRoot() || Ptr.isOnePastEnd() || Ptr.isArrayElement()) { + const SourceInfo &Loc = S.Current->getSource(OpPC); + S.FFDiag(Loc, diag::note_constexpr_delete_subobject) + << Ptr.toDiagnosticString(S.getASTContext()) << Ptr.isOnePastEnd(); + return false; + } + + Source = Ptr.getDeclDesc()->asExpr(); + BlockToDelete = Ptr.block(); + + if (!CheckDeleteSource(S, OpPC, Source, Ptr)) + return false; + + // For a class type with a virtual destructor, the selected operator delete + // is the one looked up when building the destructor. + QualType AllocType = Ptr.getType(); + if (!DeleteIsArrayForm && !IsGlobalDelete) { + auto getVirtualOperatorDelete = [](QualType T) -> const FunctionDecl * { + if (const CXXRecordDecl *RD = T->getAsCXXRecordDecl()) + if (const CXXDestructorDecl *DD = RD->getDestructor()) + return DD->isVirtual() ? DD->getOperatorDelete() : nullptr; + return nullptr; + }; + + AllocType->dump(); + if (const FunctionDecl *VirtualDelete = + getVirtualOperatorDelete(AllocType); + VirtualDelete && + !VirtualDelete->isReplaceableGlobalAllocationFunction()) { + S.FFDiag(S.Current->getSource(OpPC), + diag::note_constexpr_new_non_replaceable) + << isa<CXXMethodDecl>(VirtualDelete) << VirtualDelete; + return false; + } + } + } + assert(Source); + assert(BlockToDelete); + + // Invoke destructors before deallocating the memory. + if (!RunDestructors(S, OpPC, BlockToDelete)) + return false; + + DynamicAllocator &Allocator = S.getAllocator(); + const Descriptor *BlockDesc = BlockToDelete->getDescriptor(); + std::optional<DynamicAllocator::Form> AllocForm = + Allocator.getAllocationForm(Source); + + if (!Allocator.deallocate(Source, BlockToDelete, S)) { + // Nothing has been deallocated, this must be a double-delete. + const SourceInfo &Loc = S.Current->getSource(OpPC); + S.FFDiag(Loc, diag::note_constexpr_double_delete); + return false; + } + + assert(AllocForm); + DynamicAllocator::Form DeleteForm = DeleteIsArrayForm + ? DynamicAllocator::Form::Array + : DynamicAllocator::Form::NonArray; + return CheckNewDeleteForms(S, OpPC, *AllocForm, DeleteForm, BlockDesc, + Source); +} + void diagnoseEnumValue(InterpState &S, CodePtr OpPC, const EnumDecl *ED, const APSInt &Value) { llvm::APInt Min; diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 41708910024476..dece95971b7617 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -2976,61 +2976,8 @@ inline bool AllocCN(InterpState &S, CodePtr OpPC, const Descriptor *ElementDesc, return true; } -bool RunDestructors(InterpState &S, CodePtr OpPC, const Block *B); -static inline bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm) { - if (!CheckDynamicMemoryAllocation(S, OpPC)) - return false; - - const Expr *Source = nullptr; - const Block *BlockToDelete = nullptr; - { - // Extra scope for this so the block doesn't have this pointer - // pointing to it when we destroy it. - const Pointer &Ptr = S.Stk.pop<Pointer>(); - - // Deleteing nullptr is always fine. - if (Ptr.isZero()) - return true; - - if (!Ptr.isRoot() || Ptr.isOnePastEnd() || Ptr.isArrayElement()) { - const SourceInfo &Loc = S.Current->getSource(OpPC); - S.FFDiag(Loc, diag::note_constexpr_delete_subobject) - << Ptr.toDiagnosticString(S.getASTContext()) << Ptr.isOnePastEnd(); - return false; - } - - Source = Ptr.getDeclDesc()->asExpr(); - BlockToDelete = Ptr.block(); - - if (!CheckDeleteSource(S, OpPC, Source, Ptr)) - return false; - } - assert(Source); - assert(BlockToDelete); - - // Invoke destructors before deallocating the memory. - if (!RunDestructors(S, OpPC, BlockToDelete)) - return false; - - DynamicAllocator &Allocator = S.getAllocator(); - const Descriptor *BlockDesc = BlockToDelete->getDescriptor(); - std::optional<DynamicAllocator::Form> AllocForm = - Allocator.getAllocationForm(Source); - - if (!Allocator.deallocate(Source, BlockToDelete, S)) { - // Nothing has been deallocated, this must be a double-delete. - const SourceInfo &Loc = S.Current->getSource(OpPC); - S.FFDiag(Loc, diag::note_constexpr_double_delete); - return false; - } - - assert(AllocForm); - DynamicAllocator::Form DeleteForm = DeleteIsArrayForm - ? DynamicAllocator::Form::Array - : DynamicAllocator::Form::NonArray; - return CheckNewDeleteForms(S, OpPC, *AllocForm, DeleteForm, BlockDesc, - Source); -} +bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm, + bool IsGlobalDelete); static inline bool IsConstantContext(InterpState &S, CodePtr OpPC) { S.Stk.push<Boolean>(Boolean::from(S.inConstantContext())); diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td index 7b65138e5a3c94..4fa9b6d61d5ab9 100644 --- a/clang/lib/AST/ByteCode/Opcodes.td +++ b/clang/lib/AST/ByteCode/Opcodes.td @@ -818,7 +818,7 @@ def AllocCN : Opcode { } def Free : Opcode { - let Args = [ArgBool]; + let Args = [ArgBool, ArgBool]; } def CheckNewTypeMismatch : Opcode { diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp index 8c9d5d9c9b1d7c..8bcbed1aba21e9 100644 --- a/clang/test/AST/ByteCode/new-delete.cpp +++ b/clang/test/AST/ByteCode/new-delete.cpp @@ -552,8 +552,6 @@ namespace DeleteThis { // both-note {{in call to 'super_secret_double_delete()'}} } -/// FIXME: This is currently diagnosed, but should work. -/// If the destructor for S is _not_ virtual however, it should fail. namespace CastedDelete { struct S { constexpr S(int *p) : p(p) {} @@ -567,11 +565,10 @@ namespace CastedDelete { constexpr int vdtor_1() { int a; - delete (S*)new T(&a); // expected-note {{delete of pointer to subobject}} + delete (S*)new T(&a); return a; } - static_assert(vdtor_1() == 1); // expected-error {{not an integral constant expression}} \ - // expected-note {{in call to}} + static_assert(vdtor_1() == 1); } constexpr void use_after_free_2() { // both-error {{never produces a constant expression}} @@ -778,6 +775,28 @@ namespace Placement { static_assert(ok2()== 0); } +constexpr bool virt_delete(bool global) { + struct A { + virtual constexpr ~A() {} + }; + struct B : A { + void operator delete(void *); + constexpr ~B() {} + }; + + A *p = new B; + if (global) + ::delete p; + else + delete p; // both-note {{call to class-specific 'operator delete'}} + return true; +} +static_assert(virt_delete(true)); +static_assert(virt_delete(false)); // both-error {{not an integral constant expression}} \ + // both-note {{in call to}} + + + #else /// Make sure we reject this prior to C++20 constexpr int a() { // both-error {{never produces a constant expression}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
