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

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

[Richard Biener]

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.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

Richard.

2020-09-23  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/97151
	* tree-ssa-structalias.c (find_func_aliases_for_call):
	DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
	arguments.

	* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
---
 gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
 gcc/tree-ssa-structalias.c        | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C b/gcc/testsuite/g++.dg/cpp1y/new1.C
index aa5f647d535..fec0088cb40 100644
--- a/gcc/testsuite/g++.dg/cpp1y/new1.C
+++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
@@ -69,5 +69,5 @@ test_unused() {
   delete p;
 }
 
-/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5 "cddce1"} } */
-/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 7 "cddce1"} } */
+/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6 "cddce1"} } */
+/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 8 "cddce1"} } */
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 44fe52e0f65..f676bf91e95 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -4857,6 +4857,8 @@ 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))
+	;
       else
 	handle_rhs_call (t, &rhsc);
       if (gimple_call_lhs (t))
-- 
2.26.2

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

[Jason Merrill]

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.

> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
> 
> Richard.
> 
> 2020-09-23  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/97151
> 	* tree-ssa-structalias.c (find_func_aliases_for_call):
> 	DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
> 	arguments.
> 
> 	* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
> ---
>   gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
>   gcc/tree-ssa-structalias.c        | 2 ++
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C b/gcc/testsuite/g++.dg/cpp1y/new1.C
> index aa5f647d535..fec0088cb40 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/new1.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
> @@ -69,5 +69,5 @@ test_unused() {
>     delete p;
>   }
>   
> -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5 "cddce1"} } */
> -/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 7 "cddce1"} } */
> +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6 "cddce1"} } */
> +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 8 "cddce1"} } */
> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> index 44fe52e0f65..f676bf91e95 100644
> --- a/gcc/tree-ssa-structalias.c
> +++ b/gcc/tree-ssa-structalias.c
> @@ -4857,6 +4857,8 @@ 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))
> +	;
>         else
>   	handle_rhs_call (t, &rhsc);
>         if (gimple_call_lhs (t))
> 

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

[Richard Biener]

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? 

>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>> 
>> Richard.
>> 
>> 2020-09-23  Richard Biener  <rguenther@suse.de>
>> 
>> 	PR tree-optimization/97151
>> 	* tree-ssa-structalias.c (find_func_aliases_for_call):
>> 	DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
>> 	arguments.
>> 
>> 	* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
>> ---
>>   gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
>>   gcc/tree-ssa-structalias.c        | 2 ++
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C
>b/gcc/testsuite/g++.dg/cpp1y/new1.C
>> index aa5f647d535..fec0088cb40 100644
>> --- a/gcc/testsuite/g++.dg/cpp1y/new1.C
>> +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
>> @@ -69,5 +69,5 @@ test_unused() {
>>     delete p;
>>   }
>>   
>> -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5
>"cddce1"} } */
>> -/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
>new" 7 "cddce1"} } */
>> +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6
>"cddce1"} } */
>> +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
>new" 8 "cddce1"} } */
>> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
>> index 44fe52e0f65..f676bf91e95 100644
>> --- a/gcc/tree-ssa-structalias.c
>> +++ b/gcc/tree-ssa-structalias.c
>> @@ -4857,6 +4857,8 @@ 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))
>> +	;
>>         else
>>   	handle_rhs_call (t, &rhsc);
>>         if (gimple_call_lhs (t))
>> 

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

[Jason Merrill]

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.

A deallocation function that throws has undefined behavior.

>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>>>
>>> Richard.
>>>
>>> 2020-09-23  Richard Biener  <rguenther@suse.de>
>>>
>>> 	PR tree-optimization/97151
>>> 	* tree-ssa-structalias.c (find_func_aliases_for_call):
>>> 	DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
>>> 	arguments.
>>>
>>> 	* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
>>> ---
>>>    gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
>>>    gcc/tree-ssa-structalias.c        | 2 ++
>>>    2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C
>> b/gcc/testsuite/g++.dg/cpp1y/new1.C
>>> index aa5f647d535..fec0088cb40 100644
>>> --- a/gcc/testsuite/g++.dg/cpp1y/new1.C
>>> +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
>>> @@ -69,5 +69,5 @@ test_unused() {
>>>      delete p;
>>>    }
>>>    
>>> -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5
>> "cddce1"} } */
>>> -/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
>> new" 7 "cddce1"} } */
>>> +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6
>> "cddce1"} } */
>>> +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
>> new" 8 "cddce1"} } */
>>> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
>>> index 44fe52e0f65..f676bf91e95 100644
>>> --- a/gcc/tree-ssa-structalias.c
>>> +++ b/gcc/tree-ssa-structalias.c
>>> @@ -4857,6 +4857,8 @@ 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))
>>> +	;
>>>          else
>>>    	handle_rhs_call (t, &rhsc);
>>>          if (gimple_call_lhs (t))
>>>
> 

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

[Richard Biener]

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.

Richard.

> >>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
> >>>
> >>> Richard.
> >>>
> >>> 2020-09-23  Richard Biener  <rguenther@suse.de>
> >>>
> >>>  PR tree-optimization/97151
> >>>  * tree-ssa-structalias.c (find_func_aliases_for_call):
> >>>  DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
> >>>  arguments.
> >>>
> >>> 	* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
> >>> ---
> >>>    gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
> >>>    gcc/tree-ssa-structalias.c        | 2 ++
> >>>    2 files changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C
> >> b/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>> index aa5f647d535..fec0088cb40 100644
> >>> --- a/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>> +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>> @@ -69,5 +69,5 @@ test_unused() {
> >>>      delete p;
> >>>    }
> >>>    
> >>> -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5
> >> "cddce1"} } */
> >>> -/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
> >> new" 7 "cddce1"} } */
> >>> +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6
> >> "cddce1"} } */
> >>> +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
> >> new" 8 "cddce1"} } */
> >>> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> >>> index 44fe52e0f65..f676bf91e95 100644
> >>> --- a/gcc/tree-ssa-structalias.c
> >>> +++ b/gcc/tree-ssa-structalias.c
> >>> @@ -4857,6 +4857,8 @@ 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))
> >>> +	;
> >>>           else
> >>>     handle_rhs_call (t, &rhsc);
> >>>          if (gimple_call_lhs (t))
> >>>
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

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

[Jason Merrill]

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.

> Richard.
> 
>>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>>>>>
>>>>> Richard.
>>>>>
>>>>> 2020-09-23  Richard Biener  <rguenther@suse.de>
>>>>>
>>>>>   PR tree-optimization/97151
>>>>>   * tree-ssa-structalias.c (find_func_aliases_for_call):
>>>>>   DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
>>>>>   arguments.
>>>>>
>>>>> 	* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
>>>>> ---
>>>>>     gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
>>>>>     gcc/tree-ssa-structalias.c        | 2 ++
>>>>>     2 files changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C
>>>> b/gcc/testsuite/g++.dg/cpp1y/new1.C
>>>>> index aa5f647d535..fec0088cb40 100644
>>>>> --- a/gcc/testsuite/g++.dg/cpp1y/new1.C
>>>>> +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
>>>>> @@ -69,5 +69,5 @@ test_unused() {
>>>>>       delete p;
>>>>>     }
>>>>>     
>>>>> -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5
>>>> "cddce1"} } */
>>>>> -/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
>>>> new" 7 "cddce1"} } */
>>>>> +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6
>>>> "cddce1"} } */
>>>>> +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
>>>> new" 8 "cddce1"} } */
>>>>> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
>>>>> index 44fe52e0f65..f676bf91e95 100644
>>>>> --- a/gcc/tree-ssa-structalias.c
>>>>> +++ b/gcc/tree-ssa-structalias.c
>>>>> @@ -4857,6 +4857,8 @@ 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))
>>>>> +	;
>>>>>            else
>>>>>      handle_rhs_call (t, &rhsc);
>>>>>           if (gimple_call_lhs (t))
>>>>>
>>>
>>
>>
> 

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

[Richard Biener]

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);
}

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.

But, does it seem correct that we need to distinguish
delete expressions from plain calls to operator delete?
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?

Thanks,
Richard.

> > Richard.
> > 
> >>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
> >>>>>
> >>>>> Richard.
> >>>>>
> >>>>> 2020-09-23  Richard Biener  <rguenther@suse.de>
> >>>>>
> >>>>>   PR tree-optimization/97151
> >>>>>   * tree-ssa-structalias.c (find_func_aliases_for_call):
> >>>>>   DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
> >>>>>   arguments.
> >>>>>
> >>>>> 	* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
> >>>>> ---
> >>>>>     gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
> >>>>>     gcc/tree-ssa-structalias.c        | 2 ++
> >>>>>     2 files changed, 4 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>>> b/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>>>> index aa5f647d535..fec0088cb40 100644
> >>>>> --- a/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>>>> +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>>>> @@ -69,5 +69,5 @@ test_unused() {
> >>>>>       delete p;
> >>>>>     }
> >>>>>     
> >>>>> -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5
> >>>> "cddce1"} } */
> >>>>> -/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
> >>>> new" 7 "cddce1"} } */
> >>>>> +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6
> >>>> "cddce1"} } */
> >>>>> +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
> >>>> new" 8 "cddce1"} } */
> >>>>> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> >>>>> index 44fe52e0f65..f676bf91e95 100644
> >>>>> --- a/gcc/tree-ssa-structalias.c
> >>>>> +++ b/gcc/tree-ssa-structalias.c
> >>>>> @@ -4857,6 +4857,8 @@ 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))
> >>>>> +	;
> >>>>>            else
> >>>>>      handle_rhs_call (t, &rhsc);
> >>>>>           if (gimple_call_lhs (t))
> >>>>>
> >>>
> >>
> >>
> > 
> 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

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

[Jason Merrill]

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.

Jason

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

[Richard Biener]

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...).

Mind doing the C++ side of things recycling protected_flag as suggested?

Thanks,
Richard.

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

[Jason Merrill]

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.

Jason

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

[Jason Merrill]

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

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.



[-- Attachment #2: call-from-new.diff --]
[-- Type: text/x-patch, Size: 5956 bytes --]

commit d4a8ce72c51e32c7577ca93a488028995fd45ebe
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Sep 28 15:33:54 2020 -0400

    c++: Move CALL_FROM_NEW_OR_DELETE_P to tree.h.
    
    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.
    
    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.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 42d0d76bf21..272ea88960b 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)
@@ -3840,11 +3839,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) \
@@ -7286,6 +7280,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/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++. */
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/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 a09633751ca..b0c61cf31c6 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -19951,7 +19951,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

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

[Richard Biener]

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

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?

Thanks,
Richard.

[-- Attachment #2: Type: text/plain, Size: 16804 bytes --]

From 602853335c5f01adb8594b6b1dc49c365c5cd731 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/2] 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 48a4074b370..cf6e02c3bc5 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) \
@@ -7279,6 +7273,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 869477f2c2e..f0b6414b0e2 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 14e187bd7a161103fc4c87869a3d50116e51fd32 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/2] 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

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

[Jason Merrill]

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

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.


[-- Attachment #2: delete-fix.patch --]
[-- Type: text/x-patch, Size: 10161 bytes --]

commit 592714921200fa07e1a9de230522804a82bdb5c9
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Oct 1 11:21:57 2020 -0400

    c++: Set CALL_FROM_NEW_OR_DELETE_P on more calls.
    
    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.
    
    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.

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/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"} } */

commit 379fc0feec0b8051a8ef12d7f4d636ec0a990ec0
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Oct 1 16:39:03 2020 -0400

    tree-ssa-dce: Ignore whether an operator delete is replaceable.
    
    Now that we check CALL_FROM_NEW_OR_DELETE_P, we don't need to consider
    whether an operator delete is replaceable, that consideration only applies
    to operator new.
    
    gcc/ChangeLog:
    
            * tree.h (DECL_IS_REPLACEABLE_OPERATOR_DELETE_P): Remove.
            * gimple.c (gimple_call_replaceable_operator_delete_p):
            Rename to gimple_call_operator_delete_p.
            * gimple.h: Adjust.
            * tree-ssa-dce.c: Adjust.
            * tree-ssa-structalias.c: Adjust.

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/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)
 
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/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

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

[Richard Biener]

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