Author: Timm Bäder Date: 2024-07-18T15:07:29+02:00 New Revision: fc65a9603bf16ed1fe98fbee6933bca9e2083384
URL: https://github.com/llvm/llvm-project/commit/fc65a9603bf16ed1fe98fbee6933bca9e2083384 DIFF: https://github.com/llvm/llvm-project/commit/fc65a9603bf16ed1fe98fbee6933bca9e2083384.diff LOG: [clang][Interp] Run record destructors when deallocating dynamic memory Added: Modified: clang/lib/AST/Interp/Interp.cpp clang/lib/AST/Interp/Interp.h clang/test/AST/Interp/new-delete.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp index fb63228f8aea8..2be9b5360d055 100644 --- a/clang/lib/AST/Interp/Interp.cpp +++ b/clang/lib/AST/Interp/Interp.cpp @@ -819,6 +819,81 @@ bool CheckNonNullArgs(InterpState &S, CodePtr OpPC, const Function *F, return true; } +// FIXME: This is similar to code we already have in Compiler.cpp. +// I think it makes sense to instead add the field and base destruction stuff +// to the destructor Function itself. Then destroying a record would really +// _just_ be calling its destructor. That would also help with the diagnostic +// diff erence when the destructor or a field/base fails. +static bool runRecordDestructor(InterpState &S, CodePtr OpPC, + const Pointer &BasePtr, + const Descriptor *Desc) { + assert(Desc->isRecord()); + const Record *R = Desc->ElemRecord; + assert(R); + + // Fields. + for (const Record::Field &Field : llvm::reverse(R->fields())) { + const Descriptor *D = Field.Desc; + if (D->isRecord()) { + if (!runRecordDestructor(S, OpPC, BasePtr.atField(Field.Offset), D)) + return false; + } else if (D->isCompositeArray()) { + const Descriptor *ElemDesc = Desc->ElemDesc; + assert(ElemDesc->isRecord()); + for (unsigned I = 0; I != Desc->getNumElems(); ++I) { + if (!runRecordDestructor(S, OpPC, BasePtr.atIndex(I).narrow(), + ElemDesc)) + return false; + } + } + } + + // Destructor of this record. + if (const CXXDestructorDecl *Dtor = R->getDestructor(); + Dtor && !Dtor->isTrivial()) { + const Function *DtorFunc = S.getContext().getOrCreateFunction(Dtor); + if (!DtorFunc) + return false; + + S.Stk.push<Pointer>(BasePtr); + if (!Call(S, OpPC, DtorFunc, 0)) + return false; + } + + // Bases. + for (const Record::Base &Base : llvm::reverse(R->bases())) { + if (!runRecordDestructor(S, OpPC, BasePtr.atField(Base.Offset), Base.Desc)) + return false; + } + + return true; +} + +bool RunDestructors(InterpState &S, CodePtr OpPC, const Block *B) { + assert(B); + const Descriptor *Desc = B->getDescriptor(); + + if (Desc->isPrimitive() || Desc->isPrimitiveArray()) + return true; + + assert(Desc->isRecord() || Desc->isCompositeArray()); + + if (Desc->isCompositeArray()) { + const Descriptor *ElemDesc = Desc->ElemDesc; + assert(ElemDesc->isRecord()); + + Pointer RP(const_cast<Block *>(B)); + for (unsigned I = 0; I != Desc->getNumElems(); ++I) { + if (!runRecordDestructor(S, OpPC, RP.atIndex(I).narrow(), ElemDesc)) + return false; + } + return true; + } + + assert(Desc->isRecord()); + return runRecordDestructor(S, OpPC, Pointer(const_cast<Block *>(B)), Desc); +} + bool Interpret(InterpState &S, APValue &Result) { // The current stack frame when we started Interpret(). // This is being used by the ops to determine wheter diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h index b4f8c03280c85..17b3157cb40a9 100644 --- a/clang/lib/AST/Interp/Interp.h +++ b/clang/lib/AST/Interp/Interp.h @@ -2872,8 +2872,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; @@ -2904,6 +2904,10 @@ static inline bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm) { assert(Source); assert(BlockToDelete); + // Invoke destructors before deallocating the memory. + if (!RunDestructors(S, OpPC, BlockToDelete)) + return false; + DynamicAllocator &Allocator = S.getAllocator(); bool WasArrayAlloc = Allocator.isArrayAllocation(Source); const Descriptor *BlockDesc = BlockToDelete->getDescriptor(); diff --git a/clang/test/AST/Interp/new-delete.cpp b/clang/test/AST/Interp/new-delete.cpp index 04ce3ae5f6637..cb46426c0e3be 100644 --- a/clang/test/AST/Interp/new-delete.cpp +++ b/clang/test/AST/Interp/new-delete.cpp @@ -476,7 +476,80 @@ constexpr Sp ss[] = {Sp{new int{154}}}; // both-error {{must be initialized by a // both-note {{pointer to heap-allocated object}} \ // both-note {{allocation performed here}} +namespace DeleteRunsDtors { + struct InnerFoo { + int *mem; + constexpr ~InnerFoo() { + delete mem; + } + }; + + struct Foo { + int *a; + InnerFoo IF; + + constexpr Foo() { + a = new int(13); + IF.mem = new int(100); + } + constexpr ~Foo() { delete a; } + }; + + constexpr int abc() { + Foo *F = new Foo(); + int n = *F->a; + delete F; + + return n; + } + static_assert(abc() == 13); + + constexpr int abc2() { + Foo *f = new Foo[3]; + + delete[] f; + + return 1; + } + static_assert(abc2() == 1); +} + +/// FIXME: There is a slight diff erence in diagnostics here, because we don't +/// create a new frame when we delete record fields or bases at all. +namespace FaultyDtorCalledByDelete { + struct InnerFoo { + int *mem; + constexpr ~InnerFoo() { + if (mem) { + (void)(1/0); // both-warning {{division by zero is undefined}} \ + // both-note {{division by zero}} + } + delete mem; + } + }; + + struct Foo { + int *a; + InnerFoo IF; + constexpr Foo() { + a = new int(13); + IF.mem = new int(100); + } + constexpr ~Foo() { delete a; } + }; + + constexpr int abc() { + Foo *F = new Foo(); + int n = *F->a; + delete F; // both-note {{in call to}} \ + // ref-note {{in call to}} + + return n; + } + static_assert(abc() == 13); // both-error {{not an integral constant expression}} \ + // both-note {{in call to 'abc()'}} +} #else _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
