[PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

Richard Biener rguenther@suse.de
Fri Oct 2 09:17:50 GMT 2020


On Thu, 1 Oct 2020, Jason Merrill wrote:

> On 10/1/20 5:26 AM, Richard Biener wrote:
> > On Wed, 30 Sep 2020, Jason Merrill wrote:
> > 
> >> On 9/28/20 3:09 PM, Jason Merrill wrote:
> >>> On 9/28/20 3:56 AM, Richard Biener wrote:
> >>>> On Fri, 25 Sep 2020, Jason Merrill wrote:
> >>>>
> >>>>> On 9/25/20 2:30 AM, Richard Biener wrote:
> >>>>>> On Thu, 24 Sep 2020, Jason Merrill wrote:
> >>>>>>
> >>>>>>> On 9/24/20 3:43 AM, Richard Biener wrote:
> >>>>>>>> On Wed, 23 Sep 2020, Jason Merrill wrote:
> >>>>>>>>
> >>>>>>>>> On 9/23/20 2:42 PM, Richard Biener wrote:
> >>>>>>>>>> On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill
> >>>>>>>>>> <jason@redhat.com>
> >>>>>>>>>> wrote:
> >>>>>>>>>>> On 9/23/20 4:14 AM, Richard Biener wrote:
> >>>>>>>>>>>> C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
> >>>>>>>>>>>> does not cause the deleted object to be escaped.? It also has no
> >>>>>>>>>>>> other interesting side-effects for PTA so skip it like we do
> >>>>>>>>>>>> for BUILT_IN_FREE.
> >>>>>>>>>>>
> >>>>>>>>>>> Hmm, this is true of the default implementation, but since the
> >>>>>>>>>>> function
> >>>>>>>>>>>
> >>>>>>>>>>> is replaceable, we don't know what a user definition might do with
> >>>>>>>>>>> the
> >>>>>>>>>>> pointer.
> >>>>>>>>>>
> >>>>>>>>>> But can the object still be 'used' after delete? Can delete fail /
> >>>>>>>>>> throw?
> >>>>>>>>>>
> >>>>>>>>>> What guarantee does the predicate give us?
> >>>>>>>>>
> >>>>>>>>> The deallocation function is called as part of a delete expression
> >>>>>>>>> in
> >>>>>>>>> order
> >>>>>>>>> to
> >>>>>>>>> release the storage for an object, ending its lifetime (if it was
> >>>>>>>>> not
> >>>>>>>>> ended
> >>>>>>>>> by
> >>>>>>>>> a destructor), so no, the object can't be used afterward.
> >>>>>>>>
> >>>>>>>> OK, but the delete operator can access the object contents if there
> >>>>>>>> wasn't a destructor ...
> >>>>>>>
> >>>>>>>>> A deallocation function that throws has undefined behavior.
> >>>>>>>>
> >>>>>>>> OK, so it seems the 'replaceable' operators are the global ones
> >>>>>>>> (for user-defined/class-specific placement variants I see arbitrary
> >>>>>>>> extra arguments that we'd possibly need to handle).
> >>>>>>>>
> >>>>>>>> I'm happy to revert but I'd like to have a testcase that FAILs
> >>>>>>>> with the patch ;)
> >>>>>>>>
> >>>>>>>> Now, the following aborts:
> >>>>>>>>
> >>>>>>>> struct X {
> >>>>>>>> ???? static struct X saved;
> >>>>>>>> ???? int *p;
> >>>>>>>> ???? X() { __builtin_memcpy (this, &saved, sizeof (X)); }
> >>>>>>>> };
> >>>>>>>> void operator delete (void *p)
> >>>>>>>> {
> >>>>>>>> ???? __builtin_memcpy (&X::saved, p, sizeof (X));
> >>>>>>>> }
> >>>>>>>> int main()
> >>>>>>>> {
> >>>>>>>> ???? int y = 1;
> >>>>>>>> ???? X *p = new X;
> >>>>>>>> ???? p->p = &y;
> >>>>>>>> ???? delete p;
> >>>>>>>> ???? X *q = new X;
> >>>>>>>> ???? *(q->p) = 2;
> >>>>>>>> ???? if (y != 2)
> >>>>>>>> ?????? __builtin_abort ();
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> and I could fix this by not making *p but what *p points to escape.
> >>>>>>>> The testcase is of course maximally awkward, but hey ... ;)
> >>>>>>>>
> >>>>>>>> Now this would all be moot if operator delete may not access
> >>>>>>>> the object (or if the object contents are undefined at that point).
> >>>>>>>>
> >>>>>>>> Oh, and the testcase segfaults when compiled with GCC 10 because
> >>>>>>>> there we elide the new X / delete p pair ... which is invalid then?
> >>>>>>>> Hmm, we emit
> >>>>>>>>
> >>>>>>>> ???? MEM[(struct X *)_8] ={v} {CLOBBER};
> >>>>>>>> ???? operator delete (_8, 8);
> >>>>>>>>
> >>>>>>>> so the object contents are undefined _before_ calling delete
> >>>>>>>> even when I do not have a DTOR?? That is, the above,
> >>>>>>>> w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.
> >>>>>>>
> >>>>>>> Yes, all classes have a destructor, even if it's trivial, so the
> >>>>>>> object's
> >>>>>>> lifetime definitely ends before the call to operator delete. This is
> >>>>>>> less
> >>>>>>> clear for scalar objects, but treating them similarly would be
> >>>>>>> consistent
> >>>>>>> with
> >>>>>>> other recent changes, so I think it's fine for us to assume that
> >>>>>>> scalar
> >>>>>>> objects are also invalidated before the call to operator delete.  But
> >>>>>>> of
> >>>>>>> course this doesn't apply to explicit calls to operator delete outside
> >>>>>>> of a
> >>>>>>> delete expression.
> >>>>>>
> >>>>>> OK, so change the testcase main slightly to
> >>>>>>
> >>>>>> int main()
> >>>>>> {
> >>>>>> ??? int y = 1;
> >>>>>> ??? X *p = new X;
> >>>>>> ??? p->p = &y;
> >>>>>> ??? ::operator delete(p);
> >>>>>> ??? X *q = new X;
> >>>>>> ??? *(q->p) = 2;
> >>>>>> ??? if (y != 2)
> >>>>>> ????? __builtin_abort ();
> >>>>>> }
> >>>>>>
> >>>>>> in this case the lifetime of *p does not end before calling
> >>>>>> ::operator delete() and delete can stash the object contents
> >>>>>> somewhere before ending its lifetime.? For the very same reason
> >>>>>> we may not elide a new/delete pair like in
> >>>>>>
> >>>>>> int main()
> >>>>>> {
> >>>>>> ??? int *p = new int;
> >>>>>> ??? *p = 1;
> >>>>>> ??? ::operator delete (p);
> >>>>>> }
> >>>>>
> >>>>> Correct; the permission to elide new/delete pairs are for the
> >>>>> expressions,
> >>>>> not
> >>>>> the functions.
> >>>>>
> >>>>>> which we before the change did not do only because calling
> >>>>>> operator delete made p escape.? Unfortunately points-to analysis
> >>>>>> cannot really reconstruct whether delete was called as part of
> >>>>>> a delete expression or directly (and thus whether object lifetime
> >>>>>> ended already), neither can DCE.? So I guess we need to mark
> >>>>>> the operator delete call in some way to make those transforms
> >>>>>> safe.? At least currently any operator delete call makes the
> >>>>>> alias guarantee of a operator new call moot by forcing the object
> >>>>>> to be aliased with all global and escaped memory ...
> >>>>>>
> >>>>>> Looks like there are some unallocated flags for CALL_EXPR we could
> >>>>>> pick but I wonder if we can recycle protected_flag which is
> >>>>>>
> >>>>>> ???????? CALL_FROM_THUNK_P and
> >>>>>> ???????? CALL_ALLOCA_FOR_VAR_P in
> >>>>>> ???????????? CALL_EXPR
> >>>>>>
> >>>>>> for calls to DECL_IS_OPERATOR_{NEW,DELETE}_P, thus whether
> >>>>>> we have CALL_FROM_THUNK_P for those operators.? Guess picking
> >>>>>> a new flag is safer.
> >>>>>
> >>>>> We won't ever call those operators from a thunk, so it should be OK to
> >>>>> reuse
> >>>>> it.
> >>>>>
> >>>>>> But, does it seem correct that we need to distinguish
> >>>>>> delete expressions from plain calls to operator delete?
> >>>>>
> >>>>> A reason for that distinction came up in the context of omitting
> >>>>> new/delete
> >>>>> pairs: we want to consider the operator first called by the new or
> >>>>> delete
> >>>>> expression, not a call from that first operator to another operator
> >>>>> new/delete
> >>>>> and exposed by inlining.
> >>>>>
> >>>>> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543404.html
> >>>>>
> >>>>>> In this context I also wonder about non-replaceable operator delete,
> >>>>>> specifically operator delete in classes - are there any semantic
> >>>>>> differences between those or why did we choose to only mark
> >>>>>> the replaceable ones?
> >>>>>
> >>>>> The standard says that for omitting a 'new' allocation, the operator new
> >>>>> has
> >>>>> to be a replaceable one, but does not say the same about 'delete'; it
> >>>>> just
> >>>>> says that if the allocation was omitted, the delete-expression does not
> >>>>> call a
> >>>>> deallocation function.? It may not be necessary to make this distinction
> >>>>> for
> >>>>> delete.? And this distinction could be local to the front end.
> >>>>>
> >>>>> In the front end, we currently have cxx_replaceable_global_alloc_fn that
> >>>>> already ignores the replaceability of operator delete.? And we have
> >>>>> CALL_FROM_NEW_OR_DELETE_P, that would just need to move into the middle
> >>>>> end.
> >>>>> And perhaps get renamed to CALL_OMITTABLE_NEW_OR_DELETE_P, and not get
> >>>>> set
> >>>>> for
> >>>>> calls to non-replaceable operator new.
> >>>>
> >>>> CALL_FROM_NEW_OR_DELETE_P indeed looks like the best fit - it's
> >>>> only evaluated when cxx_replaceable_global_alloc_fn matches in the C++
> >>>> FE so could be made to cover only replaceable variants.
> >>>>
> >>>> CALL_REPLACEABLE_NEW_OR_DELETE_P () maybe, since we already use
> >>>> REPLACEABLE for the fndecl flags?? OMITTABLE is too specific
> >>>> for the PTA case where it really matters whether the object
> >>>> lifetime ends before the delete call, not whether it can be
> >>>> omitted (hmm, guess that's not 100% overlap then either...).
> >>>
> >>> That seems like good overlap to me, if we agree that object lifetime ends
> >>> before any delete call from a delete-expression, whether or not the
> >>> operator
> >>> delete is replaceable.
> >>>
> >>>> Mind doing the C++ side of things recycling protected_flag as suggested?
> >>>
> >>> OK.
> > 
> > Find attached a patch series, your patch plus the GIMPLE side of the fix.
> > This shows that the C++ FE side is possibly incomplete with for
> > example g++.dg/pr94314-2.C now FAILing.  There we have
> > 
> >    A *a = new A (argc);
> >    delete a;
> > 
> > being expanded to
> > 
> >    <<cleanup_point <<< Unknown tree: expr_stmt
> >    (void) (a = TARGET_EXPR <D.2357, operator new (1)>;, try
> >      {
> >        A::A ((struct A *) D.2357, argc);
> >      }
> >    catch
> >      {
> >        operator delete (D.2357);
> >      }, (struct A *) D.2357;) >>>>>;
> >    <<cleanup_point <<< Unknown tree: expr_stmt
> >    if (SAVE_EXPR <a> != 0B)
> >      {
> >        try
> >          {
> >            *SAVE_EXPR <a> = {CLOBBER};
> >          }
> >        finally
> >          {
> >            operator delete ((void *) SAVE_EXPR <a>);
> >          }
> >      }
> >    else
> >      {
> >        <<< Unknown tree: void_cst >>>
> >      } >>>>>;
> >    return <retval> = 0;
> > 
> > where the operator delete call in the catch {} expression is
> > not marked as CALL_FROM_NEW_OR_DELETE_P, possibly because this
> > call is compiler generated.
> > 
> > So it's probably technically true that CALL_FROM_NEW_OR_DELETE_P is
> > false but we still expect the same guarantees to hold here, in
> > particular we expect to elide the new/delete pair?
> 
> That seems like an oversight in the standard.  This first patch sets the flag.
> The second patch in the file removes consideration of whether an operator
> delete is replaceable, as I was discussing earlier.

Thanks.  I've re-bootstrapped / tested the series and pushed it to trunk.

Richard.

>From 50125f62cbd726dda1768fc9cda44a34b434b57e Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Thu, 1 Oct 2020 10:08:58 +0200
Subject: [PATCH 1/3] c++: Move CALL_FROM_NEW_OR_DELETE_P to tree.h
To: gcc-patches@gcc.gnu.org

As discussed with richi, we should be able to use TREE_PROTECTED for this
flag, since CALL_FROM_THUNK_P will never be set on a call to an operator new
or delete.

2020-10-01  Jason Merril  <jason@redhat.com>

gcc/cp/ChangeLog:
	* lambda.c (call_from_lambda_thunk_p): New.
	* cp-gimplify.c (cp_genericize_r): Use it.
	* pt.c (tsubst_copy_and_build): Use it.
	* typeck.c (check_return_expr): Use it.
	* cp-tree.h: Declare it.
	(CALL_FROM_NEW_OR_DELETE_P): Move to gcc/tree.h.

gcc/ChangeLog:
	* tree.h (CALL_FROM_NEW_OR_DELETE_P): Move from cp-tree.h.
---
 gcc/cp/cp-gimplify.c | 2 +-
 gcc/cp/cp-tree.h     | 7 +------
 gcc/cp/lambda.c      | 7 +++++++
 gcc/cp/pt.c          | 2 +-
 gcc/cp/typeck.c      | 2 +-
 gcc/tree-core.h      | 3 ++-
 gcc/tree.h           | 9 ++++++++-
 7 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index bc8a03c7b41..07549828dc9 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -962,7 +962,7 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
     omp_cxx_notice_variable (wtd->omp_ctx, stmt);
 
   /* Don't dereference parms in a thunk, pass the references through. */
-  if ((TREE_CODE (stmt) == CALL_EXPR && CALL_FROM_THUNK_P (stmt))
+  if ((TREE_CODE (stmt) == CALL_EXPR && call_from_lambda_thunk_p (stmt))
       || (TREE_CODE (stmt) == AGGR_INIT_EXPR && AGGR_INIT_FROM_THUNK_P (stmt)))
     {
       *walk_subtrees = 0;
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3ccd54ce24b..fda5ffa4036 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -464,7 +464,6 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
       SWITCH_STMT_NO_BREAK_P (in SWITCH_STMT)
       LAMBDA_EXPR_CAPTURE_OPTIMIZED (in LAMBDA_EXPR)
       IMPLICIT_CONV_EXPR_BRACED_INIT (in IMPLICIT_CONV_EXPR)
-      CALL_FROM_NEW_OR_DELETE_P (in CALL_EXPR)
    3: IMPLICIT_RVALUE_P (in NON_LVALUE_EXPR or STATIC_CAST_EXPR)
       ICS_BAD_FLAG (in _CONV)
       FN_TRY_BLOCK_P (in TRY_BLOCK)
@@ -3839,11 +3838,6 @@ struct GTY(()) lang_decl {
    should be performed at instantiation time.  */
 #define KOENIG_LOOKUP_P(NODE) TREE_LANG_FLAG_0 (CALL_EXPR_CHECK (NODE))
 
-/* In a CALL_EXPR, true for allocator calls from new or delete
-   expressions.  */
-#define CALL_FROM_NEW_OR_DELETE_P(NODE) \
-  TREE_LANG_FLAG_2 (CALL_EXPR_CHECK (NODE))
-
 /* True if the arguments to NODE should be evaluated in left-to-right
    order regardless of PUSH_ARGS_REVERSED.  */
 #define CALL_EXPR_ORDERED_ARGS(NODE) \
@@ -7268,6 +7262,7 @@ extern bool lambda_fn_in_template_p		(tree);
 extern void maybe_add_lambda_conv_op            (tree);
 extern bool is_lambda_ignored_entity            (tree);
 extern bool lambda_static_thunk_p		(tree);
+extern bool call_from_lambda_thunk_p		(tree);
 extern tree finish_builtin_launder		(location_t, tree,
 						 tsubst_flags_t);
 extern tree cp_build_vec_convert		(tree, location_t, tree,
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 07a5401c97b..1a1647f465e 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -1325,6 +1325,13 @@ lambda_static_thunk_p (tree fn)
 	  && LAMBDA_TYPE_P (CP_DECL_CONTEXT (fn)));
 }
 
+bool
+call_from_lambda_thunk_p (tree call)
+{
+  return (CALL_FROM_THUNK_P (call)
+	  && lambda_static_thunk_p (current_function_decl));
+}
+
 /* Returns true iff VAL is a lambda-related declaration which should
    be ignored by unqualified lookup.  */
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 45b18f6a5ad..72efecff37f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -19955,7 +19955,7 @@ tsubst_copy_and_build (tree t,
 
 	/* Stripped-down processing for a call in a thunk.  Specifically, in
 	   the thunk template for a generic lambda.  */
-	if (CALL_FROM_THUNK_P (t))
+	if (call_from_lambda_thunk_p (t))
 	  {
 	    /* Now that we've expanded any packs, the number of call args
 	       might be different.  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 9166156a5d5..95b36a92491 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -10171,7 +10171,7 @@ check_return_expr (tree retval, bool *no_warning)
 
       /* The call in a (lambda) thunk needs no conversions.  */
       if (TREE_CODE (retval) == CALL_EXPR
-	  && CALL_FROM_THUNK_P (retval))
+	  && call_from_lambda_thunk_p (retval))
 	converted = true;
 
       /* First convert the value to the function's return type, then
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 0e158784d0e..752bec31c3f 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1220,7 +1220,8 @@ struct GTY(()) tree_base {
            all decls
 
        CALL_FROM_THUNK_P and
-       CALL_ALLOCA_FOR_VAR_P in
+       CALL_ALLOCA_FOR_VAR_P and
+       CALL_FROM_NEW_OR_DELETE_P in
            CALL_EXPR
 
        OMP_CLAUSE_LINEAR_VARIABLE_STRIDE in
diff --git a/gcc/tree.h b/gcc/tree.h
index 5bb6e7bc000..f27a7399a37 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -921,7 +921,8 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
   (TREE_CHECK (NODE, PARM_DECL)->decl_common.decl_nonshareable_flag)
 
 /* In a CALL_EXPR, means that the call is the jump from a thunk to the
-   thunked-to function.  */
+   thunked-to function.  Be careful to avoid using this macro when one of the
+   next two applies instead.  */
 #define CALL_FROM_THUNK_P(NODE) (CALL_EXPR_CHECK (NODE)->base.protected_flag)
 
 /* In a CALL_EXPR, if the function being called is BUILT_IN_ALLOCA, means that
@@ -931,6 +932,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
 #define CALL_ALLOCA_FOR_VAR_P(NODE) \
   (CALL_EXPR_CHECK (NODE)->base.protected_flag)
 
+/* In a CALL_EXPR, if the function being called is DECL_IS_OPERATOR_NEW_P or
+   DECL_IS_OPERATOR_DELETE_P, true for allocator calls from C++ new or delete
+   expressions.  */
+#define CALL_FROM_NEW_OR_DELETE_P(NODE) \
+  (CALL_EXPR_CHECK (NODE)->base.protected_flag)
+
 /* Used in classes in C++.  */
 #define TREE_PRIVATE(NODE) ((NODE)->base.private_flag)
 /* Used in classes in C++. */
-- 
2.26.2


>From 6b5fbcfa6437e16984953d6b3caa3561146e1971 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguenther@suse.de>
Date: Thu, 1 Oct 2020 10:44:27 +0200
Subject: [PATCH 2/3] make use of CALL_FROM_NEW_OR_DELETE_P
To: gcc-patches@gcc.gnu.org

This fixes points-to analysis and DCE to only consider new/delete
operator calls from new or delete expressions and not direct calls.

2020-10-01  Richard Biener  <rguenther@suse.de>

	* gimple.h (GF_CALL_FROM_NEW_OR_DELETE): New call flag.
	(gimple_call_set_from_new_or_delete): New.
	(gimple_call_from_new_or_delete): Likewise.
	* gimple.c (gimple_build_call_from_tree): Set
	GF_CALL_FROM_NEW_OR_DELETE appropriately.
	* ipa-icf-gimple.c (func_checker::compare_gimple_call):
	Compare gimple_call_from_new_or_delete.
	* tree-ssa-dce.c (mark_all_reaching_defs_necessary_1): Make
	sure to only consider new/delete calls from new or delete
	expressions.
	(propagate_necessity): Likewise.
	(eliminate_unnecessary_stmts): Likewise.
	* tree-ssa-structalias.c (find_func_aliases_for_call):
	Likewise.

	* g++.dg/tree-ssa/pta-delete-1.C: New testcase.
---
 gcc/gimple.c                                 |  4 +++
 gcc/gimple.h                                 | 24 +++++++++++++++
 gcc/ipa-icf-gimple.c                         |  1 +
 gcc/testsuite/g++.dg/tree-ssa/pta-delete-1.C | 24 +++++++++++++++
 gcc/tree-ssa-dce.c                           | 31 ++++++++++++--------
 gcc/tree-ssa-structalias.c                   |  8 ++++-
 6 files changed, 78 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pta-delete-1.C

diff --git a/gcc/gimple.c b/gcc/gimple.c
index fd4e0fac0d4..f07ddab7953 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -387,6 +387,10 @@ gimple_build_call_from_tree (tree t, tree fnptrtype)
       && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
       && ALLOCA_FUNCTION_CODE_P (DECL_FUNCTION_CODE (fndecl)))
     gimple_call_set_alloca_for_var (call, CALL_ALLOCA_FOR_VAR_P (t));
+  else if (fndecl
+	   && (DECL_IS_OPERATOR_NEW_P (fndecl)
+	       || DECL_IS_OPERATOR_DELETE_P (fndecl)))
+    gimple_call_set_from_new_or_delete (call, CALL_FROM_NEW_OR_DELETE_P (t));
   else
     gimple_call_set_from_thunk (call, CALL_FROM_THUNK_P (t));
   gimple_call_set_va_arg_pack (call, CALL_EXPR_VA_ARG_PACK (t));
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 6cc7e66059d..108ae846849 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -149,6 +149,7 @@ enum gf_mask {
     GF_CALL_MUST_TAIL_CALL	= 1 << 9,
     GF_CALL_BY_DESCRIPTOR	= 1 << 10,
     GF_CALL_NOCF_CHECK		= 1 << 11,
+    GF_CALL_FROM_NEW_OR_DELETE	= 1 << 12,
     GF_OMP_PARALLEL_COMBINED	= 1 << 0,
     GF_OMP_TASK_TASKLOOP	= 1 << 0,
     GF_OMP_TASK_TASKWAIT	= 1 << 1,
@@ -3387,6 +3388,29 @@ gimple_call_from_thunk_p (gcall *s)
 }
 
 
+/* If FROM_NEW_OR_DELETE_P is true, mark GIMPLE_CALL S as being a call
+   to operator new or delete created from a new or delete expression.  */
+
+static inline void
+gimple_call_set_from_new_or_delete (gcall *s, bool from_new_or_delete_p)
+{
+  if (from_new_or_delete_p)
+    s->subcode |= GF_CALL_FROM_NEW_OR_DELETE;
+  else
+    s->subcode &= ~GF_CALL_FROM_NEW_OR_DELETE;
+}
+
+
+/* Return true if GIMPLE_CALL S is a call to operator new or delete from
+   from a new or delete expression.  */
+
+static inline bool
+gimple_call_from_new_or_delete (gcall *s)
+{
+  return (s->subcode & GF_CALL_FROM_NEW_OR_DELETE) != 0;
+}
+
+
 /* If PASS_ARG_PACK_P is true, GIMPLE_CALL S is a stdarg call that needs the
    argument pack in its argument list.  */
 
diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 1cd5872c03d..d5423a7e9b2 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -556,6 +556,7 @@ func_checker::compare_gimple_call (gcall *s1, gcall *s2)
       || gimple_call_tail_p (s1) != gimple_call_tail_p (s2)
       || gimple_call_return_slot_opt_p (s1) != gimple_call_return_slot_opt_p (s2)
       || gimple_call_from_thunk_p (s1) != gimple_call_from_thunk_p (s2)
+      || gimple_call_from_new_or_delete (s1) != gimple_call_from_new_or_delete (s2)
       || gimple_call_va_arg_pack_p (s1) != gimple_call_va_arg_pack_p (s2)
       || gimple_call_alloca_for_var_p (s1) != gimple_call_alloca_for_var_p (s2))
     return false;
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pta-delete-1.C b/gcc/testsuite/g++.dg/tree-ssa/pta-delete-1.C
new file mode 100644
index 00000000000..5e1e322344a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pta-delete-1.C
@@ -0,0 +1,24 @@
+// { dg-do run }
+// { dg-options "-O2" }
+
+struct X {
+  static struct X saved;
+  int *p;
+  X() { __builtin_memcpy (this, &saved, sizeof (X)); }
+};
+X X::saved;
+void __attribute__((noinline)) operator delete (void *p)
+{
+  __builtin_memcpy (&X::saved, p, sizeof (X));
+}
+int main()
+{
+  int y = 1;
+  X *p = new X;
+  p->p = &y;
+  ::operator delete (p);
+  X *q = new X;
+  *(q->p) = 2;
+  if (y != 2)
+    __builtin_abort ();
+}
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index fae5ae72340..c9e0c8fd116 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -593,9 +593,9 @@ mark_all_reaching_defs_necessary_1 (ao_ref *ref ATTRIBUTE_UNUSED,
 
   /* We want to skip statments that do not constitute stores but have
      a virtual definition.  */
-  if (is_gimple_call (def_stmt))
+  if (gcall *call = dyn_cast <gcall *> (def_stmt))
     {
-      tree callee = gimple_call_fndecl (def_stmt);
+      tree callee = gimple_call_fndecl (call);
       if (callee != NULL_TREE
 	  && fndecl_built_in_p (callee, BUILT_IN_NORMAL))
 	switch (DECL_FUNCTION_CODE (callee))
@@ -612,7 +612,8 @@ mark_all_reaching_defs_necessary_1 (ao_ref *ref ATTRIBUTE_UNUSED,
 
       if (callee != NULL_TREE
 	  && (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)
-	      || DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (callee)))
+	      || DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (callee))
+	  && gimple_call_from_new_or_delete (call))
 	return false;
     }
 
@@ -875,23 +876,25 @@ propagate_necessity (bool aggressive)
 	     processing the argument.  */
 	  bool is_delete_operator
 	    = (is_gimple_call (stmt)
+	       && gimple_call_from_new_or_delete (as_a <gcall *> (stmt))
 	       && gimple_call_replaceable_operator_delete_p (as_a <gcall *> (stmt)));
 	  if (is_delete_operator
 	      || gimple_call_builtin_p (stmt, BUILT_IN_FREE))
 	    {
 	      tree ptr = gimple_call_arg (stmt, 0);
-	      gimple *def_stmt;
+	      gcall *def_stmt;
 	      tree def_callee;
 	      /* If the pointer we free is defined by an allocation
 		 function do not add the call to the worklist.  */
 	      if (TREE_CODE (ptr) == SSA_NAME
-		  && is_gimple_call (def_stmt = SSA_NAME_DEF_STMT (ptr))
+		  && (def_stmt = dyn_cast <gcall *> (SSA_NAME_DEF_STMT (ptr)))
 		  && (def_callee = gimple_call_fndecl (def_stmt))
 		  && ((DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
 		       && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_ALIGNED_ALLOC
 			   || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
 			   || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
-		      || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)))
+		      || (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)
+			  && gimple_call_from_new_or_delete (def_stmt))))
 		{
 		  if (is_delete_operator)
 		    {
@@ -947,9 +950,9 @@ propagate_necessity (bool aggressive)
 	     in 1).  By keeping a global visited bitmap for references
 	     we walk for 2) we avoid quadratic behavior for those.  */
 
-	  if (is_gimple_call (stmt))
+	  if (gcall *call = dyn_cast <gcall *> (stmt))
 	    {
-	      tree callee = gimple_call_fndecl (stmt);
+	      tree callee = gimple_call_fndecl (call);
 	      unsigned i;
 
 	      /* Calls to functions that are merely acting as barriers
@@ -972,22 +975,23 @@ propagate_necessity (bool aggressive)
 
 	      if (callee != NULL_TREE
 		  && (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)
-		      || DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (callee)))
+		      || DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (callee))
+		  && gimple_call_from_new_or_delete (call))
 		continue;
 
 	      /* Calls implicitly load from memory, their arguments
 	         in addition may explicitly perform memory loads.  */
-	      mark_all_reaching_defs_necessary (stmt);
-	      for (i = 0; i < gimple_call_num_args (stmt); ++i)
+	      mark_all_reaching_defs_necessary (call);
+	      for (i = 0; i < gimple_call_num_args (call); ++i)
 		{
-		  tree arg = gimple_call_arg (stmt, i);
+		  tree arg = gimple_call_arg (call, i);
 		  if (TREE_CODE (arg) == SSA_NAME
 		      || is_gimple_min_invariant (arg))
 		    continue;
 		  if (TREE_CODE (arg) == WITH_SIZE_EXPR)
 		    arg = TREE_OPERAND (arg, 0);
 		  if (!ref_may_be_aliased (arg))
-		    mark_aliased_reaching_defs_necessary (stmt, arg);
+		    mark_aliased_reaching_defs_necessary (call, arg);
 		}
 	    }
 	  else if (gimple_assign_single_p (stmt))
@@ -1397,6 +1401,7 @@ eliminate_unnecessary_stmts (void)
 	  if (gimple_plf (stmt, STMT_NECESSARY)
 	      && (gimple_call_builtin_p (stmt, BUILT_IN_FREE)
 		  || (is_gimple_call (stmt)
+		      && gimple_call_from_new_or_delete (as_a <gcall *> (stmt))
 		      && gimple_call_replaceable_operator_delete_p (as_a <gcall *> (stmt)))))
 	    {
 	      tree ptr = gimple_call_arg (stmt, 0);
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index f676bf91e95..69de932b14c 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -4857,7 +4857,13 @@ find_func_aliases_for_call (struct function *fn, gcall *t)
 	 point for reachable memory of their arguments.  */
       else if (flags & (ECF_PURE|ECF_LOOPING_CONST_OR_PURE))
 	handle_pure_call (t, &rhsc);
-      else if (fndecl && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fndecl))
+      /* If the call is to a replaceable operator delete and results
+	 from a delete expression as opposed to a direct call to
+	 such operator, then the effects for PTA (in particular
+	 the escaping of the pointer) can be ignored.  */
+      else if (fndecl
+	       && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fndecl)
+	       && gimple_call_from_new_or_delete (t))
 	;
       else
 	handle_rhs_call (t, &rhsc);
-- 
2.26.2


>From cbb43538314f38f1e7f5fa6c907d662c7c9dcc16 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Fri, 2 Oct 2020 09:00:49 +0200
Subject: [PATCH 3/3] c++: Set CALL_FROM_NEW_OR_DELETE_P on more calls.
To: gcc-patches@gcc.gnu.org

We were failing to set the flag on a delete call in a new expression, in a
deleting destructor, and in a coroutine.  Fixed by setting it in the
function that builds the call.

2020-10-02  Jason Merril  <jason@redhat.com>

gcc/cp/ChangeLog:
	* call.c (build_operator_new_call): Set CALL_FROM_NEW_OR_DELETE_P.
	(build_op_delete_call): Likewise.
	* init.c (build_new_1, build_vec_delete_1, build_delete): Not here.
	(build_delete):

gcc/testsuite/ChangeLog:
	* g++.dg/pr94314.C: new/delete no longer omitted.
---
 gcc/cp/call.c                  | 29 ++++++++++++++++++++++++-----
 gcc/cp/init.c                  | 14 --------------
 gcc/gimple.c                   |  4 ++--
 gcc/gimple.h                   |  2 +-
 gcc/testsuite/g++.dg/pr94314.C |  2 +-
 gcc/tree-ssa-dce.c             |  8 ++++----
 gcc/tree-ssa-structalias.c     |  2 +-
 gcc/tree.h                     |  3 ---
 8 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index d67e8fe2b28..bd662518958 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -4769,7 +4769,16 @@ build_operator_new_call (tree fnname, vec<tree, va_gc> **args,
      *fn = cand->fn;
 
    /* Build the CALL_EXPR.  */
-   return build_over_call (cand, LOOKUP_NORMAL, complain);
+   tree ret = build_over_call (cand, LOOKUP_NORMAL, complain);
+
+   /* Set this flag for all callers of this function.  In addition to
+      new-expressions, this is called for allocating coroutine state; treat
+      that as an implicit new-expression.  */
+   tree call = extract_call_expr (ret);
+   if (TREE_CODE (call) == CALL_EXPR)
+     CALL_FROM_NEW_OR_DELETE_P (call) = 1;
+
+   return ret;
 }
 
 /* Build a new call to operator().  This may change ARGS.  */
@@ -6146,7 +6155,7 @@ build_new_op_1 (const op_location_t &loc, enum tree_code code, int flags,
     case VEC_NEW_EXPR:
     case VEC_DELETE_EXPR:
     case DELETE_EXPR:
-      /* Use build_op_new_call and build_op_delete_call instead.  */
+      /* Use build_operator_new_call and build_op_delete_call instead.  */
       gcc_unreachable ();
 
     case CALL_EXPR:
@@ -6983,6 +6992,7 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
       if (DECL_DELETED_FN (fn) && alloc_fn)
 	return NULL_TREE;
 
+      tree ret;
       if (placement)
 	{
 	  /* The placement args might not be suitable for overload
@@ -6995,7 +7005,7 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 	    argarray[i] = CALL_EXPR_ARG (placement, i);
 	  if (!mark_used (fn, complain) && !(complain & tf_error))
 	    return error_mark_node;
-	  return build_cxx_call (fn, nargs, argarray, complain);
+	  ret = build_cxx_call (fn, nargs, argarray, complain);
 	}
       else
 	{
@@ -7013,7 +7023,6 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 						  complain);
 	    }
 
-	  tree ret;
 	  releasing_vec args;
 	  args->quick_push (addr);
 	  if (destroying)
@@ -7026,8 +7035,18 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 	      args->quick_push (al);
 	    }
 	  ret = cp_build_function_call_vec (fn, &args, complain);
-	  return ret;
 	}
+
+      /* Set this flag for all callers of this function.  In addition to
+	 delete-expressions, this is called for deallocating coroutine state;
+	 treat that as an implicit delete-expression.  This is also called for
+	 the delete if the constructor throws in a new-expression, and for a
+	 deleting destructor (which implements a delete-expression).  */
+      tree call = extract_call_expr (ret);
+      if (TREE_CODE (call) == CALL_EXPR)
+	CALL_FROM_NEW_OR_DELETE_P (call) = 1;
+
+      return ret;
     }
 
   /* [expr.new]
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index e84e985492d..00fff3f7327 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3433,10 +3433,6 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 	}
     }
 
-  tree alloc_call_expr = extract_call_expr (alloc_call);
-  if (TREE_CODE (alloc_call_expr) == CALL_EXPR)
-    CALL_FROM_NEW_OR_DELETE_P (alloc_call_expr) = 1;
-
   if (cookie_size)
     alloc_call = maybe_wrap_new_for_constexpr (alloc_call, elt_type,
 					       cookie_size);
@@ -4145,10 +4141,6 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
 					      /*placement=*/NULL_TREE,
 					      /*alloc_fn=*/NULL_TREE,
 					      complain);
-
-      tree deallocate_call_expr = extract_call_expr (deallocate_expr);
-      if (TREE_CODE (deallocate_call_expr) == CALL_EXPR)
-	CALL_FROM_NEW_OR_DELETE_P (deallocate_call_expr) = 1;
     }
 
   body = loop;
@@ -5073,12 +5065,6 @@ build_delete (location_t loc, tree otype, tree addr,
 
   if (do_delete == error_mark_node)
     return error_mark_node;
-  else if (do_delete)
-    {
-      tree do_delete_call_expr = extract_call_expr (do_delete);
-      if (TREE_CODE (do_delete_call_expr) == CALL_EXPR)
-	CALL_FROM_NEW_OR_DELETE_P (do_delete_call_expr) = 1;
-    }
 
   if (do_delete && !TREE_SIDE_EFFECTS (expr))
     expr = do_delete;
diff --git a/gcc/gimple.c b/gcc/gimple.c
index f07ddab7953..523d845de89 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2717,12 +2717,12 @@ gimple_builtin_call_types_compatible_p (const gimple *stmt, tree fndecl)
 /* Return true when STMT is operator a replaceable delete call.  */
 
 bool
-gimple_call_replaceable_operator_delete_p (const gcall *stmt)
+gimple_call_operator_delete_p (const gcall *stmt)
 {
   tree fndecl;
 
   if ((fndecl = gimple_call_fndecl (stmt)) != NULL_TREE)
-    return DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fndecl);
+    return DECL_IS_OPERATOR_DELETE_P (fndecl);
   return false;
 }
 
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 108ae846849..3c9b9965f5a 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1605,7 +1605,7 @@ extern alias_set_type gimple_get_alias_set (tree);
 extern bool gimple_ior_addresses_taken (bitmap, gimple *);
 extern bool gimple_builtin_call_types_compatible_p (const gimple *, tree);
 extern combined_fn gimple_call_combined_fn (const gimple *);
-extern bool gimple_call_replaceable_operator_delete_p (const gcall *);
+extern bool gimple_call_operator_delete_p (const gcall *);
 extern bool gimple_call_builtin_p (const gimple *);
 extern bool gimple_call_builtin_p (const gimple *, enum built_in_class);
 extern bool gimple_call_builtin_p (const gimple *, enum built_in_function);
diff --git a/gcc/testsuite/g++.dg/pr94314.C b/gcc/testsuite/g++.dg/pr94314.C
index 4e5ae122e9f..72467127fea 100644
--- a/gcc/testsuite/g++.dg/pr94314.C
+++ b/gcc/testsuite/g++.dg/pr94314.C
@@ -78,5 +78,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 c9e0c8fd116..a0466127f9c 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -612,7 +612,7 @@ mark_all_reaching_defs_necessary_1 (ao_ref *ref ATTRIBUTE_UNUSED,
 
       if (callee != NULL_TREE
 	  && (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)
-	      || DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (callee))
+	      || DECL_IS_OPERATOR_DELETE_P (callee))
 	  && gimple_call_from_new_or_delete (call))
 	return false;
     }
@@ -877,7 +877,7 @@ propagate_necessity (bool aggressive)
 	  bool is_delete_operator
 	    = (is_gimple_call (stmt)
 	       && gimple_call_from_new_or_delete (as_a <gcall *> (stmt))
-	       && gimple_call_replaceable_operator_delete_p (as_a <gcall *> (stmt)));
+	       && gimple_call_operator_delete_p (as_a <gcall *> (stmt)));
 	  if (is_delete_operator
 	      || gimple_call_builtin_p (stmt, BUILT_IN_FREE))
 	    {
@@ -975,7 +975,7 @@ propagate_necessity (bool aggressive)
 
 	      if (callee != NULL_TREE
 		  && (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)
-		      || DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (callee))
+		      || DECL_IS_OPERATOR_DELETE_P (callee))
 		  && gimple_call_from_new_or_delete (call))
 		continue;
 
@@ -1402,7 +1402,7 @@ eliminate_unnecessary_stmts (void)
 	      && (gimple_call_builtin_p (stmt, BUILT_IN_FREE)
 		  || (is_gimple_call (stmt)
 		      && gimple_call_from_new_or_delete (as_a <gcall *> (stmt))
-		      && gimple_call_replaceable_operator_delete_p (as_a <gcall *> (stmt)))))
+		      && gimple_call_operator_delete_p (as_a <gcall *> (stmt)))))
 	    {
 	      tree ptr = gimple_call_arg (stmt, 0);
 	      if (TREE_CODE (ptr) == SSA_NAME)
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 69de932b14c..30a8c93b4ff 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -4862,7 +4862,7 @@ find_func_aliases_for_call (struct function *fn, gcall *t)
 	 such operator, then the effects for PTA (in particular
 	 the escaping of the pointer) can be ignored.  */
       else if (fndecl
-	       && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fndecl)
+	       && DECL_IS_OPERATOR_DELETE_P (fndecl)
 	       && gimple_call_from_new_or_delete (t))
 	;
       else
diff --git a/gcc/tree.h b/gcc/tree.h
index f27a7399a37..c0a027a650d 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3074,9 +3074,6 @@ set_function_decl_type (tree decl, function_decl_type t, bool set)
 #define DECL_IS_OPERATOR_DELETE_P(NODE) \
   (FUNCTION_DECL_CHECK (NODE)->function_decl.decl_type == OPERATOR_DELETE)
 
-#define DECL_IS_REPLACEABLE_OPERATOR_DELETE_P(NODE) \
-  (DECL_IS_OPERATOR_DELETE_P (NODE) && DECL_IS_REPLACEABLE_OPERATOR (NODE))
-
 #define DECL_SET_IS_OPERATOR_DELETE(NODE, VAL) \
   set_function_decl_type (FUNCTION_DECL_CHECK (NODE), OPERATOR_DELETE, VAL)
 
-- 
2.26.2



More information about the Gcc-patches mailing list