[pushed] c++: check delete access with trivial init [PR20040]

[pushed] c++: check delete access with trivial init [PR20040]

[Jason Merrill]

Apparently we need to check the accessibility of the deallocation function
even if there is no initialization.

Tested x86_64-pc-linux-gnu, applying to trunk.

	PR c++/20040

gcc/cp/ChangeLog:

	* init.c (build_new_1): Also build pointer cleanup if
	TYPE_GETS_DELETE.
	* cp-tree.h (TYPE_GETS_VEC_DELETE): New.

gcc/testsuite/ChangeLog:

	* g++.dg/init/delete4.C: New test.
---
 gcc/cp/cp-tree.h                    |   1 +
 gcc/cp/init.c                       | 142 +++++++++++++++-------------
 gcc/testsuite/g++.dg/init/delete4.C |  14 +++
 3 files changed, 89 insertions(+), 68 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/init/delete4.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index e204182da97..f8225c18a48 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -2395,6 +2395,7 @@ struct GTY(()) lang_type {
 /* Nonzero for _CLASSTYPE means that operator delete is defined.  */
 #define TYPE_GETS_DELETE(NODE) (LANG_TYPE_CLASS_CHECK (NODE)->gets_delete)
 #define TYPE_GETS_REG_DELETE(NODE) (TYPE_GETS_DELETE (NODE) & 1)
+#define TYPE_GETS_VEC_DELETE(NODE) (TYPE_GETS_DELETE (NODE) & 2)
 
 /* Nonzero if `new NODE[x]' should cause the allocation of extra
    storage to indicate how many array elements are in use.  */
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index c932699ffa6..6226812b470 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3316,6 +3316,12 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 		     ? TYPE_HAS_ARRAY_NEW_OPERATOR (elt_type)
 		     : TYPE_HAS_NEW_OPERATOR (elt_type));
 
+  bool member_delete_p = (!globally_qualified_p
+			  && CLASS_TYPE_P (elt_type)
+			  && (array_p
+			      ? TYPE_GETS_VEC_DELETE (elt_type)
+			      : TYPE_GETS_REG_DELETE (elt_type)));
+
   if (member_new_p)
     {
       /* Use a class-specific operator new.  */
@@ -3473,7 +3479,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 
   /* In the simple case, we can stop now.  */
   pointer_type = build_pointer_type (type);
-  if (!cookie_size && !is_initialized)
+  if (!cookie_size && !is_initialized && !member_delete_p)
     return build_nop (pointer_type, alloc_call);
 
   /* Store the result of the allocation call in a variable so that we can
@@ -3700,77 +3706,77 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 
       if (init_expr == error_mark_node)
 	return error_mark_node;
-
-      /* If any part of the object initialization terminates by throwing an
-	 exception and a suitable deallocation function can be found, the
-	 deallocation function is called to free the memory in which the
-	 object was being constructed, after which the exception continues
-	 to propagate in the context of the new-expression. If no
-	 unambiguous matching deallocation function can be found,
-	 propagating the exception does not cause the object's memory to be
-	 freed.  */
-      if (flag_exceptions)
-	{
-	  enum tree_code dcode = array_p ? VEC_DELETE_EXPR : DELETE_EXPR;
-	  tree cleanup;
-
-	  /* The Standard is unclear here, but the right thing to do
-	     is to use the same method for finding deallocation
-	     functions that we use for finding allocation functions.  */
-	  cleanup = (build_op_delete_call
-		     (dcode,
-		      alloc_node,
-		      size,
-		      globally_qualified_p,
-		      placement_allocation_fn_p ? alloc_call : NULL_TREE,
-		      alloc_fn,
-		      complain));
-
-	  if (cleanup && !processing_template_decl)
-	    /* Ack!  First we allocate the memory.  Then we set our sentry
-	       variable to true, and expand a cleanup that deletes the
-	       memory if sentry is true.  Then we run the constructor, and
-	       finally clear the sentry.
-
-	       We need to do this because we allocate the space first, so
-	       if there are any temporaries with cleanups in the
-	       constructor args, we need this EH region to extend until
-	       end of full-expression to preserve nesting.
-
-	       We used to try to evaluate the args first to avoid this, but
-	       since C++17 [expr.new] says that "The invocation of the
-	       allocation function is sequenced before the evaluations of
-	       expressions in the new-initializer."  */
-	    {
-	      tree end, sentry, begin;
-
-	      begin = get_target_expr (boolean_true_node);
-	      CLEANUP_EH_ONLY (begin) = 1;
-
-	      sentry = TARGET_EXPR_SLOT (begin);
-
-	      /* CLEANUP is compiler-generated, so no diagnostics.  */
-	      suppress_warning (cleanup);
-
-	      TARGET_EXPR_CLEANUP (begin)
-		= build3 (COND_EXPR, void_type_node, sentry,
-			  cleanup, void_node);
-
-	      end = build2 (MODIFY_EXPR, TREE_TYPE (sentry),
-			    sentry, boolean_false_node);
-
-	      init_expr
-		= build2 (COMPOUND_EXPR, void_type_node, begin,
-			  build2 (COMPOUND_EXPR, void_type_node, init_expr,
-				  end));
-	      /* Likewise, this is compiler-generated.  */
-	      suppress_warning (init_expr);
-	    }
-	}
     }
   else
     init_expr = NULL_TREE;
 
+  /* If any part of the object initialization terminates by throwing an
+     exception and a suitable deallocation function can be found, the
+     deallocation function is called to free the memory in which the
+     object was being constructed, after which the exception continues
+     to propagate in the context of the new-expression. If no
+     unambiguous matching deallocation function can be found,
+     propagating the exception does not cause the object's memory to be
+     freed.  */
+  if (flag_exceptions && (init_expr || member_delete_p))
+    {
+      enum tree_code dcode = array_p ? VEC_DELETE_EXPR : DELETE_EXPR;
+      tree cleanup;
+
+      /* The Standard is unclear here, but the right thing to do
+	 is to use the same method for finding deallocation
+	 functions that we use for finding allocation functions.  */
+      cleanup = (build_op_delete_call
+		 (dcode,
+		  alloc_node,
+		  size,
+		  globally_qualified_p,
+		  placement_allocation_fn_p ? alloc_call : NULL_TREE,
+		  alloc_fn,
+		  complain));
+
+      if (cleanup && init_expr && !processing_template_decl)
+	/* Ack!  First we allocate the memory.  Then we set our sentry
+	   variable to true, and expand a cleanup that deletes the
+	   memory if sentry is true.  Then we run the constructor, and
+	   finally clear the sentry.
+
+	   We need to do this because we allocate the space first, so
+	   if there are any temporaries with cleanups in the
+	   constructor args, we need this EH region to extend until
+	   end of full-expression to preserve nesting.
+
+	   We used to try to evaluate the args first to avoid this, but
+	   since C++17 [expr.new] says that "The invocation of the
+	   allocation function is sequenced before the evaluations of
+	   expressions in the new-initializer."  */
+	{
+	  tree end, sentry, begin;
+
+	  begin = get_target_expr (boolean_true_node);
+	  CLEANUP_EH_ONLY (begin) = 1;
+
+	  sentry = TARGET_EXPR_SLOT (begin);
+
+	  /* CLEANUP is compiler-generated, so no diagnostics.  */
+	  suppress_warning (cleanup);
+
+	  TARGET_EXPR_CLEANUP (begin)
+	    = build3 (COND_EXPR, void_type_node, sentry,
+		      cleanup, void_node);
+
+	  end = build2 (MODIFY_EXPR, TREE_TYPE (sentry),
+			sentry, boolean_false_node);
+
+	  init_expr
+	    = build2 (COMPOUND_EXPR, void_type_node, begin,
+		      build2 (COMPOUND_EXPR, void_type_node, init_expr,
+			      end));
+	  /* Likewise, this is compiler-generated.  */
+	  suppress_warning (init_expr);
+	}
+    }
+
   /* Now build up the return value in reverse order.  */
 
   rval = data_addr;
diff --git a/gcc/testsuite/g++.dg/init/delete4.C b/gcc/testsuite/g++.dg/init/delete4.C
new file mode 100644
index 00000000000..94932b4b82c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/delete4.C
@@ -0,0 +1,14 @@
+// PR c++/20040
+
+class X
+{
+  void operator delete(void *p) throw () {} // { dg-message "declared private" }
+};
+
+X xa;
+
+int mymain()
+{
+  X *p = new X; // { dg-error "is private" }
+  return 0;
+}

base-commit: 997130f778c56466a825627644e510960585521b
-- 
2.27.0