[clang] [clang][bytecode] Diagnose class-specific operator delete calls (PR #111700)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 9 08:46:15 PDT 2024
https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/111700
None
>From e4ec0569603e78f82939d179b06839d2bc3deed3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Wed, 9 Oct 2024 17:30:44 +0200
Subject: [PATCH] [clang][bytecode] Diagnose class-specific operator delete
calls
---
clang/lib/AST/ByteCode/Compiler.cpp | 2 +-
clang/lib/AST/ByteCode/Interp.cpp | 85 +++++++++++++++++++++++++-
clang/lib/AST/ByteCode/Interp.h | 57 +----------------
clang/lib/AST/ByteCode/Opcodes.td | 2 +-
clang/test/AST/ByteCode/new-delete.cpp | 29 +++++++--
5 files changed, 112 insertions(+), 63 deletions(-)
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 050de67c2e77dd..9f26f54c1abdc1 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -963,7 +963,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();
@@ -988,6 +988,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 2c5538d221bf0b..3ebf73213b8cf3 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -3007,61 +3007,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}}
More information about the cfe-commits
mailing list