[pushed,07/11] c++: keep destroying array after one dtor throws [PR66451]

Message ID 20220107002156.2992278-7-jason@redhat.com
State Committed
Commit 2fbc45486e13facfeb05bd6ddf70ff9973a30a3c
Headers
Series [pushed,01/11] c++: don't preevaluate new-initializer |

Commit Message

Jason Merrill Jan. 7, 2022, 12:21 a.m. UTC
  When we're cleaning up an array, if one destructor throws, we should still
try to clean up the rest of the array.  We can use TRY_CATCH_EXPR for this,
instead of a TARGET_EXPR like my other recent patches, because a destructor
call can't involve any temporaries that need to live longer.

I thought about only doing this when we call build_vec_delete_1 from
build_vec_init, but it seems appropriate for delete-expressions as well;
we've said that the array's lifetime is over, it makes sense to keep trying
to destroy it.  The standard isn't clear, but clang seems to agree with me.

	PR c++/66451

gcc/cp/ChangeLog:

	* init.c (build_vec_delete_1): Handle throwing dtor.
	(build_vec_init): Tell it we're in a cleanup already.

gcc/testsuite/ChangeLog:

	* g++.dg/eh/array3.C: New test.
---
 gcc/cp/init.c                        | 18 +++++++++++--
 gcc/testsuite/g++.dg/eh/array1.C     |  8 +++++-
 gcc/testsuite/g++.dg/eh/array3.C     | 40 ++++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/eh/delete1.C    |  2 +-
 gcc/testsuite/g++.dg/ipa/devirt-40.C | 10 ++++++-
 gcc/testsuite/g++.dg/warn/pr83054.C  |  9 ++++++-
 6 files changed, 81 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/eh/array3.C
  

Patch

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 7c7b8104026..df63e618394 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4006,7 +4006,8 @@  build_new (location_t loc, vec<tree, va_gc> **placement, tree type,
 static tree
 build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
 		    special_function_kind auto_delete_vec,
-		    int use_global_delete, tsubst_flags_t complain)
+		    int use_global_delete, tsubst_flags_t complain,
+		    bool in_cleanup = false)
 {
   tree virtual_size;
   tree ptype = build_pointer_type (type = complete_type (type));
@@ -4109,6 +4110,18 @@  build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
   body = build_compound_expr (loc, body, tmp);
 
   loop = build1 (LOOP_EXPR, void_type_node, body);
+
+  /* If one destructor throws, keep trying to clean up the rest, unless we're
+     already in a build_vec_init cleanup.  */
+  if (flag_exceptions && !in_cleanup && !expr_noexcept_p (tmp, tf_none))
+    {
+      loop = build2 (TRY_CATCH_EXPR, void_type_node, loop,
+		     unshare_expr (loop));
+      /* Tell honor_protect_cleanup_actions to discard this on the
+	 exceptional path.  */
+      TRY_CATCH_IS_CLEANUP (loop) = true;
+    }
+
   loop = build_compound_expr (loc, tbase_init, loop);
 
  no_destructor:
@@ -4490,7 +4503,8 @@  build_vec_init (tree base, tree maxindex, tree init,
 
       e = build_vec_delete_1 (input_location, rval, m,
 			      inner_elt_type, sfk_complete_destructor,
-			      /*use_global_delete=*/0, complain);
+			      /*use_global_delete=*/0, complain,
+			      /*in_cleanup*/true);
       if (e == error_mark_node)
 	errors = true;
       TARGET_EXPR_CLEANUP (iterator_targ) = e;
diff --git a/gcc/testsuite/g++.dg/eh/array1.C b/gcc/testsuite/g++.dg/eh/array1.C
index 30b035cfc52..79d62ad5058 100644
--- a/gcc/testsuite/g++.dg/eh/array1.C
+++ b/gcc/testsuite/g++.dg/eh/array1.C
@@ -2,10 +2,16 @@ 
 // rather than one for each element.
 // { dg-options "-fdump-tree-gimple" }
 
+#if __cplusplus < 201100L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept
+#endif
+
 struct A
 {
   A();
-  ~A();
+  ~A() NOTHROW;
 };
 
 void f()
diff --git a/gcc/testsuite/g++.dg/eh/array3.C b/gcc/testsuite/g++.dg/eh/array3.C
new file mode 100644
index 00000000000..547541b5dc3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/array3.C
@@ -0,0 +1,40 @@ 
+// PR c++/66451
+// { dg-do run }
+
+#if __cplusplus > 201100L
+#define THROWING noexcept(false)
+#else
+#define THROWING
+#endif
+
+extern "C" void abort();
+
+int c;
+struct A
+{
+  int a;
+
+  A(int new_a) : a(new_a) { ++c; }
+  A(const A&); // not defined
+  ~A() THROWING
+  {
+    --c;
+    if(a==4)
+      throw a;
+  }
+};
+
+struct B
+{
+  A a[2];
+  ~B() { }
+};
+
+int sink;
+int main()
+{
+  try {
+    B b = {3,4};
+  } catch(...) { }
+  if (c != 0) abort();
+}
diff --git a/gcc/testsuite/g++.dg/eh/delete1.C b/gcc/testsuite/g++.dg/eh/delete1.C
index 1727a74ff36..92ed646ef2c 100644
--- a/gcc/testsuite/g++.dg/eh/delete1.C
+++ b/gcc/testsuite/g++.dg/eh/delete1.C
@@ -69,7 +69,7 @@  int ary ()
 {
   deleted = 0;
 
-  Baz *p = new Baz[5];
+  Baz *p = new Baz[1];
   try { delete[] p; }
   catch (...) { return deleted != 1;}
   return 1;
diff --git a/gcc/testsuite/g++.dg/ipa/devirt-40.C b/gcc/testsuite/g++.dg/ipa/devirt-40.C
index 32e0d22c0e7..31fe1504cb8 100644
--- a/gcc/testsuite/g++.dg/ipa/devirt-40.C
+++ b/gcc/testsuite/g++.dg/ipa/devirt-40.C
@@ -1,4 +1,12 @@ 
 /* { dg-options "-O2 -fdump-tree-fre3-details"  } */
+
+// A throwing dtor in C++98 mode changes the results.
+#if __cplusplus < 201100L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept
+#endif
+
 typedef enum
 {
 } UErrorCode;
@@ -6,7 +14,7 @@  class UnicodeString
 {
 public:
   UnicodeString ();
-  virtual ~UnicodeString ();
+  virtual ~UnicodeString () NOTHROW;
 };
 class A
 {
diff --git a/gcc/testsuite/g++.dg/warn/pr83054.C b/gcc/testsuite/g++.dg/warn/pr83054.C
index 506c9609b90..5285f94acee 100644
--- a/gcc/testsuite/g++.dg/warn/pr83054.C
+++ b/gcc/testsuite/g++.dg/warn/pr83054.C
@@ -2,6 +2,13 @@ 
 // { dg-options "-O3 -Wsuggest-final-types" }
 // { dg-do compile }
 
+// A throwing dtor in C++98 mode changes the warning.
+#if __cplusplus < 201100L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept
+#endif
+
 extern "C" int printf (const char *, ...);
 struct foo // { dg-warning "final would enable devirtualization of 5 calls" }
 {
@@ -12,7 +19,7 @@  struct foo // { dg-warning "final would enable devirtualization of 5 calls" }
     x = count++;
     printf("this %d = %x\n", x, (void *)this);
   }
-  virtual ~foo () {
+  virtual ~foo () NOTHROW {
     printf("this %d = %x\n", x, (void *)this);
     --count;
   }