[c++/55635] operator delete and throwing dtors

[Nathan Sidwell <nathan at acm dot org>]

this fixes bug 55635 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55635

Somewhat surprisingly, a delete expression should always call the operator 
delete function, regardless of whether the object destructor(s) throw.  See this 
note at the end of 5.3.5/7:

[Note: The deallocation function is called regardless of whether the destructor 
for the object or some element of the array throws an exception. â end note ]

The fix is relatively simple -- use a TRY_FINALLY_EXPR, rather than a 
COMPOUND_EXPR to glue the destruction and operator delete calls together.

While there I noticed that build_delete_destructor_body was unnecessarily 
emitting cdtor_label -- that looks like a bit of copy&paste when that 
functionality was broken out of finish_destructor_body (from  whence I removed 
the no longer active operator delete call yesterday).

Mostly, destructors are of course nothrow, particularly now that DR1123 is 
implemented.  Thus usually the exception path gets DCE'd.

Fixed thusly, ok?

nathan

2016-04-01  Nathan Sidwell  <nathan@acm.org>

	PR c++/55635
	* init.c (build_vec_delete_1): Protect operator delete call in try
	finally.
	(build_delete): Likewise.
	* optimize.c (build_delete_destructor_body): Likewise.

	PR c++/55635
	* g++.dg/eh/delete1.C: New.

Index: cp/init.c
===================================================================
--- cp/init.c	(revision 234668)
+++ cp/init.c	(working copy)
@@ -3671,7 +3671,9 @@ build_vec_delete_1 (tree base, tree maxi
   else if (!body)
     body = deallocate_expr;
   else
-    body = build_compound_expr (input_location, body, deallocate_expr);
+    /* The delete operator mist be called, even if a destructor
+       throws.  */
+    body = build2 (TRY_FINALLY_EXPR, void_type_node, body, deallocate_expr);
 
   if (!body)
     body = integer_zero_node;
@@ -4508,7 +4510,13 @@ build_delete (tree otype, tree addr, spe
       if (expr == error_mark_node)
 	return error_mark_node;
       if (do_delete)
-	expr = build2 (COMPOUND_EXPR, void_type_node, expr, do_delete);
+	/* The delete operator must be called, regardless of whether
+	   the destructor throws.
+
+	   [expr.delete]/7 The deallocation function is called
+	   regardless of whether the destructor for the object or some
+	   element of the array throws an exception.  */
+	expr = build2 (TRY_FINALLY_EXPR, void_type_node, expr, do_delete);
 
       /* We need to calculate this before the dtor changes the vptr.  */
       if (head)
Index: cp/optimize.c
===================================================================
--- cp/optimize.c	(revision 234668)
+++ cp/optimize.c	(working copy)
@@ -112,26 +112,24 @@ clone_body (tree clone, tree fn, void *a
 static void
 build_delete_destructor_body (tree delete_dtor, tree complete_dtor)
 {
-  tree call_dtor, call_delete;
   tree parm = DECL_ARGUMENTS (delete_dtor);
   tree virtual_size = cxx_sizeof (current_class_type);
 
   /* Call the corresponding complete destructor.  */
   gcc_assert (complete_dtor);
-  call_dtor = build_cxx_call (complete_dtor, 1, &parm,
-			      tf_warning_or_error);
-  add_stmt (call_dtor);
-
-  add_stmt (build_stmt (0, LABEL_EXPR, cdtor_label));
+  tree call_dtor = build_cxx_call (complete_dtor, 1, &parm,
+				   tf_warning_or_error);
 
   /* Call the delete function.  */
-  call_delete = build_op_delete_call (DELETE_EXPR, current_class_ptr,
-                                      virtual_size,
-                                      /*global_p=*/false,
-                                      /*placement=*/NULL_TREE,
-                                      /*alloc_fn=*/NULL_TREE,
-				      tf_warning_or_error);
-  add_stmt (call_delete);
+  tree call_delete = build_op_delete_call (DELETE_EXPR, current_class_ptr,
+					   virtual_size,
+					   /*global_p=*/false,
+					   /*placement=*/NULL_TREE,
+					   /*alloc_fn=*/NULL_TREE,
+					   tf_warning_or_error);
+
+  /* Operator delete must be called, whether or not the dtor throws.  */
+  add_stmt (build2 (TRY_FINALLY_EXPR, void_type_node, call_dtor, call_delete));
 
   /* Return the address of the object.  */
   if (targetm.cxx.cdtor_returns_this ())
Index: testsuite/g++.dg/eh/delete1.C
===================================================================
--- testsuite/g++.dg/eh/delete1.C	(nonexistent)
+++ testsuite/g++.dg/eh/delete1.C	(working copy)
@@ -0,0 +1,79 @@
+// { dg-do run }
+// pr 55635, the delete operator must be called, regardless of whether
+// the dtor throws
+
+static int deleted;
+
+void operator delete (void *) throw ()
+{
+  deleted = 1;
+}
+
+struct Foo {
+  ~Foo() throw(int) {throw 1;}
+};
+
+struct Baz {
+  void operator delete (void *) throw ()
+  {
+    deleted = 2;
+  }
+  virtual ~Baz() throw(int) {throw 1;}
+};
+
+int non_virt ()
+{
+  deleted = 0;
+  
+  Foo *p = new Foo;
+  try { delete p; }
+  catch (...) { return deleted != 1;}
+  return 1;
+}
+
+int virt_glob ()
+{
+  deleted = 0;
+  
+  Baz *p = ::new Baz;
+  try { ::delete p; }
+  catch (...) { return deleted != 1;}
+  return 1;
+}
+
+int virt_del ()
+{
+  deleted = 0;
+  
+  Baz *p = new Baz;
+  try { delete p; }
+  catch (...) { return deleted != 2;}
+  return 1;
+}
+
+int ary ()
+{
+  deleted = 0;
+
+  Baz *p = new Baz[5];
+  try { delete[] p; }
+  catch (...) { return deleted != 1;}
+  return 1;
+}
+
+int main ()
+{
+  if (non_virt ())
+    return 1;
+
+  if (virt_glob ())
+    return 2;
+
+  if (virt_del ())
+    return 3;
+
+  if (ary ())
+    return 4;
+  
+  return 0;
+}