[C++] PR 88969 ("[9 Regression] ICE in build_op_delete_call, at cp/call.c:6509")

Message ID 7e951363-a1a6-ff72-ae93-ba08ab796616@oracle.com
State New
Headers show
Series
  • [C++] PR 88969 ("[9 Regression] ICE in build_op_delete_call, at cp/call.c:6509")
Related show

Commit Message

Paolo Carlini Jan. 24, 2019, 7:53 p.m.
Hi,

as far as I can see this ICE on invalid points to a substantive, if 
minor, weakness of our implementation of the destroying operator delete 
facility: we aren't implementing the bits, per 7.6.2.5/(10.1), about 
destroying operator delete having precedence over any other operator 
delete. Thus the below, which is the most straightforward implementation 
I have been able to figure out given the current infrastructure. Tested 
x86_64-linux.

Thanks, Paolo.

////////////////////
/cp
2019-01-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/88969
	* call.c (build_op_delete_call): Implement 7.6.2.5/(10.1).

/testsuite
2019-01-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/88969
	* g++.dg/cpp2a/destroying-delete2.C: New.

Comments

Jason Merrill Jan. 24, 2019, 10:21 p.m. | #1
On 1/24/19 2:53 PM, Paolo Carlini wrote:
> Hi,

> 

> as far as I can see this ICE on invalid points to a substantive, if 

> minor, weakness of our implementation of the destroying operator delete 

> facility: we aren't implementing the bits, per 7.6.2.5/(10.1), about 

> destroying operator delete having precedence over any other operator 

> delete. Thus the below, which is the most straightforward implementation 

> I have been able to figure out given the current infrastructure. Tested 

> x86_64-linux.


OK, thanks.

Jason
Paolo Carlini Jan. 25, 2019, 11:20 a.m. | #2
Hi,

On 24/01/19 23:21, Jason Merrill wrote:
> On 1/24/19 2:53 PM, Paolo Carlini wrote:

>> Hi,

>>

>> as far as I can see this ICE on invalid points to a substantive, if 

>> minor, weakness of our implementation of the destroying operator 

>> delete facility: we aren't implementing the bits, per 7.6.2.5/(10.1), 

>> about destroying operator delete having precedence over any other 

>> operator delete. Thus the below, which is the most straightforward 

>> implementation I have been able to figure out given the current 

>> infrastructure. Tested x86_64-linux.

>

> OK, thanks.


Thanks you.

Yesterday I didn't notice that the bug report includes another testcase, 
for an unrelated buglet: if the destroying operator delete is wrongly 
specified as not-taking a pointer to the class type as first argument, 
TYPE_POINTER_TO may not be set yet in coerce_delete_type and we crash 
later on. Thus the below, which changes it to build_pointer_type.

Thanks, Paolo.

//////////////////////////
/cp
2019-01-25  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/88969
	* call.c (build_op_delete_call): Implement 7.6.2.5/(10.1).
	* decl2.c (coerce_delete_type): Use build_pointer_type instead
	of TYPE_POINTER_TO.

/testsuite
2019-01-25  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/88969
	* g++.dg/cpp2a/destroying-delete2.C: New.
	* g++.dg/cpp2a/destroying-delete3.C: Likewise.
Index: cp/call.c
===================================================================
--- cp/call.c	(revision 268257)
+++ cp/call.c	(working copy)
@@ -6461,6 +6461,19 @@ build_op_delete_call (enum tree_code code, tree ad
 		continue;
 	      }
 
+	    /* -- If any of the deallocation functions is a destroying
+	       operator delete, all deallocation functions that are not
+	       destroying operator deletes are eliminated from further
+	       consideration.  */
+	    bool fn_destroying = destroying_delete_p (fn);
+	    bool elt_destroying = destroying_delete_p (elt);
+	    if (elt_destroying != fn_destroying)
+	      {
+		if (elt_destroying)
+		  fn = elt;
+		continue;
+	      }
+
 	    /* -- If the type has new-extended alignment, a function with a
 	       parameter of type std::align_val_t is preferred; otherwise a
 	       function without such a parameter is preferred. If exactly one
Index: cp/decl2.c
===================================================================
--- cp/decl2.c	(revision 268257)
+++ cp/decl2.c	(working copy)
@@ -1757,9 +1757,9 @@ coerce_delete_type (tree decl, location_t loc)
   if (destroying_delete_p (decl))
     {
       if (DECL_CLASS_SCOPE_P (decl))
-	/* If the function is a destroying operator delete declared in class type
-	   C, the type of its first parameter shall be C*.  */
-	ptrtype = TYPE_POINTER_TO (DECL_CONTEXT (decl));
+	/* If the function is a destroying operator delete declared in class
+	   type C, the type of its first parameter shall be C*.  */
+	ptrtype = build_pointer_type (DECL_CONTEXT (decl));
       else
 	/* A destroying operator delete shall be a class member function named
 	   operator delete.  */
Index: testsuite/g++.dg/cpp2a/destroying-delete2.C
===================================================================
--- testsuite/g++.dg/cpp2a/destroying-delete2.C	(nonexistent)
+++ testsuite/g++.dg/cpp2a/destroying-delete2.C	(working copy)
@@ -0,0 +1,20 @@
+// PR c++/88969
+// { dg-do compile { target c++2a } }
+
+#include <new>
+
+namespace delete_selection_d {
+  struct B {
+    void operator delete(void*) = delete;
+    void operator delete(B *, std::destroying_delete_t) = delete;  // { dg-message "declared here" }
+  };
+  void delete_B(B *b) { delete b; }  // { dg-error "use of deleted function" }
+}
+
+namespace delete_selection_r {
+  struct B {
+    void operator delete(B *, std::destroying_delete_t) = delete;  // { dg-message "declared here" }
+    void operator delete(void*) = delete;
+  };
+  void delete_B(B *b) { delete b; }  // { dg-error "use of deleted function" }
+}
Index: testsuite/g++.dg/cpp2a/destroying-delete3.C
===================================================================
--- testsuite/g++.dg/cpp2a/destroying-delete3.C	(nonexistent)
+++ testsuite/g++.dg/cpp2a/destroying-delete3.C	(working copy)
@@ -0,0 +1,8 @@
+// PR c++/88969
+// { dg-do compile { target c++2a } }
+
+#include <new>
+
+struct B {
+  void operator delete(void*, std::destroying_delete_t);  // { dg-error ".operator delete. takes type .B*." }
+};
Jason Merrill Jan. 25, 2019, 2:30 p.m. | #3
On 1/25/19 6:20 AM, Paolo Carlini wrote:
> Hi,

> 

> On 24/01/19 23:21, Jason Merrill wrote:

>> On 1/24/19 2:53 PM, Paolo Carlini wrote:

>>> Hi,

>>>

>>> as far as I can see this ICE on invalid points to a substantive, if 

>>> minor, weakness of our implementation of the destroying operator 

>>> delete facility: we aren't implementing the bits, per 7.6.2.5/(10.1), 

>>> about destroying operator delete having precedence over any other 

>>> operator delete. Thus the below, which is the most straightforward 

>>> implementation I have been able to figure out given the current 

>>> infrastructure. Tested x86_64-linux.

>>

>> OK, thanks.

> 

> Thanks you.

> 

> Yesterday I didn't notice that the bug report includes another testcase, 

> for an unrelated buglet: if the destroying operator delete is wrongly 

> specified as not-taking a pointer to the class type as first argument, 

> TYPE_POINTER_TO may not be set yet in coerce_delete_type and we crash 

> later on. Thus the below, which changes it to build_pointer_type.


Also OK. :)

Jason

Patch

Index: cp/call.c
===================================================================
--- cp/call.c	(revision 268234)
+++ cp/call.c	(working copy)
@@ -6461,6 +6461,19 @@  build_op_delete_call (enum tree_code code, tree ad
 		continue;
 	      }
 
+	    /* -- If any of the deallocation functions is a destroying
+	       operator delete, all deallocation functions that are not
+	       destroying operator deletes are eliminated from further
+	       consideration.  */
+	    bool fn_destroying = destroying_delete_p (fn);
+	    bool elt_destroying = destroying_delete_p (elt);
+	    if (elt_destroying != fn_destroying)
+	      {
+		if (elt_destroying)
+		  fn = elt;
+		continue;
+	      }
+
 	    /* -- If the type has new-extended alignment, a function with a
 	       parameter of type std::align_val_t is preferred; otherwise a
 	       function without such a parameter is preferred. If exactly one
Index: testsuite/g++.dg/cpp2a/destroying-delete2.C
===================================================================
--- testsuite/g++.dg/cpp2a/destroying-delete2.C	(nonexistent)
+++ testsuite/g++.dg/cpp2a/destroying-delete2.C	(working copy)
@@ -0,0 +1,20 @@ 
+// PR c++/88969
+// { dg-do compile { target c++2a } }
+
+#include <new>
+
+namespace delete_selection_d {
+  struct B {
+    void operator delete(void*) = delete;
+    void operator delete(B *, std::destroying_delete_t) = delete;  // { dg-message "declared here" }
+  };
+  void delete_B(B *b) { delete b; }  // { dg-error "use of deleted function" }
+}
+
+namespace delete_selection_r {
+  struct B {
+    void operator delete(B *, std::destroying_delete_t) = delete;  // { dg-message "declared here" }
+    void operator delete(void*) = delete;
+  };
+  void delete_B(B *b) { delete b; }  // { dg-error "use of deleted function" }
+}