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
> >>> <[email protected]>
> >>> 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 <[email protected]>
> >>>>>
> >>>>> 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 <[email protected]>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
