Re: [PATCH] Allow new/delete operator deletion only for replaceable.

["Martin Liška" <mliska@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

On 4/9/20 8:45 AM, Richard Biener wrote:
> On Thu, Apr 9, 2020 at 7:06 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> Hi.
>>
>> We've got one another sneaky test-case (thank you Marc ;) ):
>>
>> $ cat pr94314-array.C
>> #include <stdio.h>
>> #include <new>
>>
>> int count = 0;
>>
>> __attribute__((malloc, noinline)) void* operator new[](unsigned long sz) {
>>     ++count;
>>     return ::operator new(sz);
>> }
>>
>> void operator delete[](void* ptr) noexcept {
>>     --count;
>>     ::operator delete(ptr);
>> }
>>
>> void operator delete[](void* ptr, std::size_t sz) noexcept {
>>     --count;
>>     ::operator delete(ptr, sz);
>> }
>>
>> int main() {
>>     delete[] new int[1];
>>     if (count != 0)
>>       __builtin_abort ();
>> }
>>
>> I bet we need to include the Honza's fix for inline stacks.
>> Or it the test-case invalid?
> 
> I don't see how inline stacking helps here when you consider
> 
> void *foo(unsigned long sz) { return ::operator new(sz); }
> void operator delete[](void* ptr) noexcept {
>      --count;
>      ::operator delete(ptr);
> }
> 
> thus regular functions inlining where definitely the inline
> stack depth does not need to match.

I was considering quite strict rules:
- inline stack can contain only up to 1 replaceable operator new (or delete)
- no non-replaceable operators are allowed
- number of repl. operator much match.

> 
> I guess the testcase asks for us to match the exact
> operator form (in the testcase we match ::delete and ::new[]),
> for example by instead of looking at the decl flags
> simply match the assembler names (the mangled names)
> of the operator?

What do you mean by 'decl flags'. We can't compare ASM names as one is ctor
and the second one is dtor. It's about argument types that much match, right?

Thanks,
Martin

> 
> Richard.
> 
>> Thanks,
>> Martin


[-- Attachment #2: 0001-Compare-inline-stacks-for-new-delete-repl.-operators.patch --]
[-- Type: text/x-patch, Size: 6250 bytes --]

From 16c77b6b65e31be657f91dcf0308f1638a784d7a Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 9 Apr 2020 07:36:56 +0200
Subject: [PATCH] Compare inline stacks for new/delete repl. operators.

gcc/ChangeLog:

2020-04-09  Martin Liska  <mliska@suse.cz>

	PR c++/94314
	* tree-ssa-dce.c (compare_new_delete_inline_contexts):
	(propagate_necessity):
	* cgraphclones.c (set_new_clone_decl_and_node_flags):
	Drop DECL_IS_REPLACEABLE_OPERATOR to false.

gcc/testsuite/ChangeLog:

2020-04-09  Martin Liska  <mliska@suse.cz>

	PR c++/94314
	* g++.dg/pr94314-4.C: New test.
	* g++.dg/pr94314.C: Do not expect a dtor deletion.
---
 gcc/cgraphclones.c               |  2 +
 gcc/testsuite/g++.dg/pr94314-4.C | 33 ++++++++++++++
 gcc/testsuite/g++.dg/pr94314.C   |  2 +-
 gcc/tree-ssa-dce.c               | 76 +++++++++++++++++++++++++++-----
 4 files changed, 102 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr94314-4.C

diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index c73b8f810f0..8f541a28b6e 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -165,6 +165,7 @@ set_new_clone_decl_and_node_flags (cgraph_node *new_node)
   DECL_STATIC_DESTRUCTOR (new_node->decl) = 0;
   DECL_SET_IS_OPERATOR_NEW (new_node->decl, 0);
   DECL_SET_IS_OPERATOR_DELETE (new_node->decl, 0);
+  DECL_IS_REPLACEABLE_OPERATOR (new_node->decl) = 0;
 
   new_node->externally_visible = 0;
   new_node->local = 1;
@@ -1030,6 +1031,7 @@ cgraph_node::create_version_clone_with_body
   DECL_STATIC_DESTRUCTOR (new_decl) = 0;
   DECL_SET_IS_OPERATOR_NEW (new_decl, 0);
   DECL_SET_IS_OPERATOR_DELETE (new_decl, 0);
+  DECL_IS_REPLACEABLE_OPERATOR (new_decl) = 0;
 
   /* Create the new version's call-graph node.
      and update the edges of the new node. */
diff --git a/gcc/testsuite/g++.dg/pr94314-4.C b/gcc/testsuite/g++.dg/pr94314-4.C
new file mode 100644
index 00000000000..afa2a443dc4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr94314-4.C
@@ -0,0 +1,33 @@
+/* PR c++/94314.  */
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-cddce-details -std=c++14" } */
+/* { dg-additional-options "-fdelete-null-pointer-checks" } */
+
+#include <stdio.h>
+
+int count = 0;
+
+__attribute__((malloc, noinline)) void* operator new[](__SIZE_TYPE__ sz) {
+  ++count;
+  return ::operator new(sz);
+}
+
+void operator delete[](void* ptr) noexcept {
+  --count;
+  ::operator delete(ptr);
+}
+
+void operator delete[](void* ptr, __SIZE_TYPE__ sz) noexcept {
+  --count;
+  ::operator delete(ptr, sz);
+}
+
+int main() {
+  delete[] new int[1];
+  if (count != 0)
+    __builtin_abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "Deleting : operator delete" "cddce1"} } */
diff --git a/gcc/testsuite/g++.dg/pr94314.C b/gcc/testsuite/g++.dg/pr94314.C
index 86e651d10ba..da293a7dd9b 100644
--- a/gcc/testsuite/g++.dg/pr94314.C
+++ b/gcc/testsuite/g++.dg/pr94314.C
@@ -81,5 +81,5 @@ int main(){
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 1 "cddce1"} } */
+/* { dg-final { scan-tree-dump-not "Deleting : operator delete" "cddce1"} } */
 /* { dg-final { scan-tree-dump-not "Deleting : B::operator delete" "cddce1"} } */
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index fd5f24c746c..e1085d6b049 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -646,6 +646,53 @@ degenerate_phi_p (gimple *phi)
   return true;
 }
 
+/* Compare new/delete inline contexts and return false if
+   either number of replaceable operators is different or we meet
+  a non-replaceable operator.  */
+
+static bool
+compare_new_delete_inline_contexts (gimple *new_call, gimple *delete_call)
+{
+  unsigned int replaceable_new_ops = 0;
+  unsigned int replaceable_delete_ops = 0;
+
+  for (tree block = gimple_block (new_call);
+       block && TREE_CODE (block) == BLOCK; block = BLOCK_SUPERCONTEXT (block))
+    {
+      tree fn = block_ultimate_origin (block);
+      if (fn != NULL && TREE_CODE (fn) == FUNCTION_DECL)
+	{
+	  if (DECL_IS_OPERATOR_NEW_P (fn))
+	    {
+	      if (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fn))
+		++replaceable_new_ops;
+	      else
+		return false;
+	    }
+	}
+    }
+
+  for (tree block = gimple_block (delete_call);
+       block && TREE_CODE (block) == BLOCK; block = BLOCK_SUPERCONTEXT (block))
+    {
+      tree fn = block_ultimate_origin (block);
+      if (fn != NULL && TREE_CODE (fn) == FUNCTION_DECL)
+	{
+	  if (DECL_IS_OPERATOR_DELETE_P (fn))
+	    {
+	      if (DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fn))
+		++replaceable_delete_ops;
+	      else
+		return false;
+	    }
+	}
+    }
+
+  return (replaceable_new_ops <= 1
+	  && replaceable_delete_ops <= 1
+	  && replaceable_new_ops == replaceable_delete_ops);
+}
+
 /* Propagate necessity using the operands of necessary statements.
    Process the uses on each statement in the worklist, and add all
    feeding statements which contribute to the calculation of this
@@ -824,16 +871,25 @@ propagate_necessity (bool aggressive)
 			   || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
 		      || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)))
 		{
-		  /* Delete operators can have alignment and (or) size as next
-		     arguments.  When being a SSA_NAME, they must be marked
-		     as necessary.  */
-		  if (is_delete_operator && gimple_call_num_args (stmt) >= 2)
-		    for (unsigned i = 1; i < gimple_call_num_args (stmt); i++)
-		      {
-			tree arg = gimple_call_arg (stmt, i);
-			if (TREE_CODE (arg) == SSA_NAME)
-			  mark_operand_necessary (arg);
-		      }
+		  if (is_delete_operator)
+		    {
+		      /* Verify that new and delete operators have the same
+			 context.  */
+		      if (!compare_new_delete_inline_contexts (def_stmt, stmt))
+			mark_operand_necessary (gimple_call_arg (stmt, 0));
+
+		      /* Delete operators can have alignment and (or) size
+			 as next arguments.  When being a SSA_NAME, they
+			 must be marked as necessary.  */
+		      if (gimple_call_num_args (stmt) >= 2)
+			for (unsigned i = 1; i < gimple_call_num_args (stmt);
+			     i++)
+			  {
+			    tree arg = gimple_call_arg (stmt, i);
+			    if (TREE_CODE (arg) == SSA_NAME)
+			      mark_operand_necessary (arg);
+			  }
+		    }
 
 		  continue;
 		}
-- 
2.26.0