[clang] aab25fa - Never call a destroying operator delete when cleaning up from an

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 8 16:51:59 PST 2021


Author: Richard Smith
Date: 2021-01-08T16:51:47-08:00
New Revision: aab25fa7d853d6da960607310e2cd3e3a843d5a9

URL: https://github.com/llvm/llvm-project/commit/aab25fa7d853d6da960607310e2cd3e3a843d5a9
DIFF: https://github.com/llvm/llvm-project/commit/aab25fa7d853d6da960607310e2cd3e3a843d5a9.diff

LOG: Never call a destroying operator delete when cleaning up from an
exception thrown during construction in a new-expression.

Instead, when performing deallocation function lookup for a
new-expression, ignore all destroying operator delete candidates, and
fall back to global operator delete if there is no member operator
delete other than a destroying operator delete.

Use of destroying operator delete only makes sense when there is an
object to destroy, which there isn't in this case. The language wording
doesn't cover this case; this oversight has been reported to WG21, with
the approach in this patch as the proposed fix.

Added: 
    

Modified: 
    clang/lib/Sema/SemaExprCXX.cpp
    clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
    clang/test/SemaCXX/cxx2a-destroying-delete.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 05b28c11e5a5..1ee52107c3da 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2630,8 +2630,24 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
   if (FoundDelete.isAmbiguous())
     return true; // FIXME: clean up expressions?
 
+  // Filter out any destroying operator deletes. We can't possibly call such a
+  // function in this context, because we're handling the case where the object
+  // was not successfully constructed.
+  // FIXME: This is not covered by the language rules yet.
+  {
+    LookupResult::Filter Filter = FoundDelete.makeFilter();
+    while (Filter.hasNext()) {
+      auto *FD = dyn_cast<FunctionDecl>(Filter.next()->getUnderlyingDecl());
+      if (FD && FD->isDestroyingOperatorDelete())
+        Filter.erase();
+    }
+    Filter.done();
+  }
+
   bool FoundGlobalDelete = FoundDelete.empty();
   if (FoundDelete.empty()) {
+    FoundDelete.clear(LookupOrdinaryName);
+
     if (DeleteScope == AFS_Class)
       return true;
 

diff  --git a/clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp b/clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
index ac0407d37e45..2fe489efdd90 100644
--- a/clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
+++ b/clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
@@ -1,11 +1,11 @@
-// RUN: %clang_cc1 -std=c++2a -emit-llvm %s -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ITANIUM,CHECK-64BIT
-// RUN: %clang_cc1 -std=c++2a -emit-llvm %s -triple x86_64-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI64,CHECK-64BIT
-// RUN: %clang_cc1 -std=c++2a -emit-llvm %s -triple i386-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI32,CHECK-32BIT
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ITANIUM,CHECK-64BIT
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple x86_64-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI64,CHECK-64BIT
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple i386-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI32,CHECK-32BIT
 
 // PR46908: ensure the IR passes the verifier with optimizations enabled.
-// RUN: %clang_cc1 -std=c++2a -emit-llvm-only %s -triple x86_64-linux-gnu -O2
-// RUN: %clang_cc1 -std=c++2a -emit-llvm-only %s -triple x86_64-windows -O2
-// RUN: %clang_cc1 -std=c++2a -emit-llvm-only %s -triple i386-windows -O2
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple x86_64-linux-gnu -O2
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple x86_64-windows -O2
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple i386-windows -O2
 
 namespace std {
   using size_t = decltype(sizeof(0));
@@ -102,6 +102,41 @@ void delete_D(D *d) { delete d; }
 // CHECK-NOT: call
 // CHECK: }
 
+struct J {
+  J(); // might throw
+  void operator delete(J *, std::destroying_delete_t);
+};
+
+// CHECK-ITANIUM-LABEL: define {{.*}}@_Z1j
+// CHECK-MSABI-LABEL: define {{.*}}@"?j@@
+J *j() {
+  // CHECK-ITANIUM: invoke {{.*}}@_ZN1JC1Ev(
+  // CHECK-ITANIUM: call {{.*}}@_ZdlPv(
+  // CHECK-NOT: }
+  // CHECK-MSABI: invoke {{.*}}@"??0J@@Q{{AE|EAA}}@XZ"(
+  // CHECK-MSABI: call {{.*}}@"??3 at YAXP{{E?}}AX at Z"(
+  return new J;
+  // CHECK: }
+}
+
+struct K {
+  K(); // might throw
+  void operator delete(void *);
+  void operator delete(K *, std::destroying_delete_t);
+};
+
+// CHECK-ITANIUM-LABEL: define {{.*}}@_Z1k
+// CHECK-MSABI-LABEL: define {{.*}}@"?k@@
+K *k() {
+  // CHECK-ITANIUM: invoke {{.*}}@_ZN1KC1Ev(
+  // CHECK-ITANIUM: call {{.*}}@_ZN1KdlEPv(
+  // CHECK-NOT: }
+  // CHECK-MSABI: invoke {{.*}}@"??0K@@Q{{AE|EAA}}@XZ"(
+  // CHECK-MSABI: call {{.*}}@"??3K@@SAXP{{E?}}AX at Z"(
+  return new K;
+  // CHECK: }
+}
+
 struct E { void *data; };
 struct F { void operator delete(F *, std::destroying_delete_t, std::size_t, std::align_val_t); void *data; };
 struct alignas(16) G : E, F { void *data; };
@@ -127,8 +162,8 @@ H::~H() { call_in_dtor(); }
 // CHECK-ITANIUM-NOT: call
 // CHECK-ITANIUM: }
 
-// CHECK-MSABI64: define {{.*}} @"??_GH@@UEAAPEAXI at Z"(
-// CHECK-MSABI32: define {{.*}} @"??_GH@@UAEPAXI at Z"(
+// CHECK-MSABI64-LABEL: define {{.*}} @"??_GH@@UEAAPEAXI at Z"(
+// CHECK-MSABI32-LABEL: define {{.*}} @"??_GH@@UAEPAXI at Z"(
 // CHECK-MSABI-NOT: call{{ }}
 // CHECK-MSABI: load i32
 // CHECK-MSABI: icmp eq i32 {{.*}}, 0
@@ -158,8 +193,8 @@ I::~I() { call_in_dtor(); }
 // CHECK-ITANIUM-NOT: call
 // CHECK-ITANIUM: }
 
-// CHECK-MSABI64: define {{.*}} @"??_GI@@UEAAPEAXI at Z"(
-// CHECK-MSABI32: define {{.*}} @"??_GI@@UAEPAXI at Z"(
+// CHECK-MSABI64-LABEL: define {{.*}} @"??_GI@@UEAAPEAXI at Z"(
+// CHECK-MSABI32-LABEL: define {{.*}} @"??_GI@@UAEPAXI at Z"(
 // CHECK-MSABI-NOT: call{{ }}
 // CHECK-MSABI: load i32
 // CHECK-MSABI: icmp eq i32 {{.*}}, 0

diff  --git a/clang/test/SemaCXX/cxx2a-destroying-delete.cpp b/clang/test/SemaCXX/cxx2a-destroying-delete.cpp
index 46e5228ec6be..1416fa24ff52 100644
--- a/clang/test/SemaCXX/cxx2a-destroying-delete.cpp
+++ b/clang/test/SemaCXX/cxx2a-destroying-delete.cpp
@@ -58,8 +58,8 @@ namespace delete_selection {
   struct C {
     C();
     void *operator new(std::size_t);
-    void operator delete(void*) = delete;
-    void operator delete(C *, std::destroying_delete_t) = delete; // expected-note 0-1 {{deleted here}}
+    void operator delete(void*) = delete; // expected-note 0-1 {{deleted here}}
+    void operator delete(C *, std::destroying_delete_t) = delete;
   };
   // TODO: We only diagnose the use of a deleted operator delete when exceptions
   // are enabled. Otherwise we don't bother doing the lookup.
@@ -167,3 +167,23 @@ namespace dtor_access {
 
   void g() { delete (T *)new U; } // expected-error {{calling a protected destructor}}
 }
+
+namespace delete_from_new {
+  struct A {
+    A(); // might throw
+    void operator delete(A *, std::destroying_delete_t) = delete;
+  };
+  struct B {
+    B(); // might throw
+    void operator delete(void *) = delete; // #member-delete-from-new
+    void operator delete(B *, std::destroying_delete_t) = delete;
+  };
+  void f() {
+    new A; // calls ::operator delete
+    new B; // calls B::operator delete
+#ifdef __EXCEPTIONS
+  // expected-error at -2 {{attempt to use a deleted function}}
+  // expected-note@#member-delete-from-new {{deleted here}}
+#endif
+  }
+}


        


More information about the cfe-commits mailing list