[clang] fc65a96 - [clang][Interp] Run record destructors when deallocating dynamic memory
Timm Bäder via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 18 06:08:08 PDT 2024
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
More information about the cfe-commits
mailing list