[Bug c++/101480] New: Miscompiled code involving operator new

[Bug c++/101480] New: Miscompiled code involving operator new

[jens.maurer at gmx dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

            Bug ID: 101480
           Summary: Miscompiled code involving operator new
           Product: gcc
           Version: 11.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jens.maurer at gmx dot net
  Target Milestone: ---

The following test case works correctly with gcc 10.3 (with any of -O0, -O1, or
-O3) and works with gcc 11.1 with -O0, but the assertion at #2 fires with gcc
11.1 with -O1 (and above).

The problem is that setting the flag at #1 (inlined into "f" just before
calling "new") is not performed in the generated machine code, and the
assertion in "operator new" then fails.


#include <stdlib.h>
#include <assert.h>

static bool flag = false;

class C
{
  bool prev;

public:
  C() : prev(flag)
  {
    flag = true; // #1
  }

  ~C() {
    flag = prev;
  }
};

void* operator new(unsigned long size)
{
  assert(flag);  // #2
  return malloc(size);
}

void operator delete(void *p)
{
  free(p);
}

void g(int* p)
{
  delete p;
}

void f()
{
  int* p;
  {
    C c;
    p = new int;
  }
  g(p);
}

int main(int, char**)
{
  f();
}

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |10.3.0
           Keywords|                            |wrong-code
      Known to fail|                            |11.1.0, 12.0
            Summary|Miscompiled code involving  |[11/12 Regression]
                   |operator new                |Miscompiled code involving
                   |                            |operator new
   Target Milestone|---                         |11.2
                 CC|                            |hubicka at gcc dot gnu.org

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Started with r11-4745

    Add fnspecs for C++ new and delete operators

    gcc/ChangeLog:

            * gimple.c (gimple_call_fnspec): Handle C++ new and delete.
            * gimple.h (gimple_call_from_new_or_delete): Constify parameter.

    gcc/testsuite/ChangeLog:

            * g++.dg/ipa/devirt-24.C: Update template.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-07-18
     Ever confirmed|0                           |1

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
           Priority|P3                          |P2

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
We treat the global operator new as not reading from global memory but here it
does.  I'm not sure if this overload is "valid".  There's also the distinction
made between replaceable and non-replaceable(?) operators.

If a replaced global operator new can have arbitrary side-effects we cannot do
the optimization that was added.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[jens.maurer at gmx dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #3 from Jens Maurer <jens.maurer at gmx dot net> ---
"We treat the global operator new as not reading from global memory"

If I implement my own global "operator new" afresh, certainly it'll need to
access global memory, e.g. to read a global pointer to the heap or so.

I have carefully reviewed [expr.new] before posting this bug report. The
compiler can omit a call to an allocation function entirely (and e.g. provide
the memory on the stack, if the lifetime fits), but I haven't found any wording
that would allow making assumptions about the global memory behavior of the
allocation function when it is, in fact, called.

"we cannot do the optimization that was added"

I understand this might be an important optimization to offer (similar to
-ffast-float), but it would be good to have a separate command-line flag to
enable or disable it.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
It is extremely important optimization and is done only if new/delete is used,
not when one calls these operators.  While when one calls the operator by hand,
the C++ standard doesn't give the possibility of optimizing away e.g. a pair of
operator delete (operator new (23)), when delete new int; is used, it can be
optimized away.  With reading and modification of the global state, sure, under
the hood the function will need to read and store some global state, but the
point is that typically that state doesn't include anything that the callers
care about.  It is similar to malloc, if one defines in C or C++ malloc and
uses variables the callers of malloc set or read in the malloc implementation,
it will not work properly.  We have a -fno-builtin-malloc or -fno-builtin-free
for that case though, so people that want to do weird things can at the expense
of massive code penalization get what they want (better is to use some compiler
barriers for such state).  For the operator new/delete I think we don't have a
switch, maybe it should be added.  But it certainly should default to the
current state.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[volker.schmidt at factset dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Volker Schmidt <volker.schmidt at factset dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |volker.schmidt at factset dot com

--- Comment #5 from Volker Schmidt <volker.schmidt at factset dot com> ---
I would simply add, that we have breaking change to gcc here, that does do
something different, without any warning or documentation, simply based on the
idea (which I find no source to point too, that new should not use globals).

Without digging to deep, I find a number of areas (inter process communication,
inter processor communication, numa optimizations, architectures with special
function memory areas), where calling new is depended on a global status.

So I find this assumption, not very intuitive.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
As Jakub said the behavior is the same for malloc() since years.

When you split the testcase like

> cat t.C
#include <stdlib.h>
#include <assert.h>

bool flag = false;

class C
{
  bool prev;

public:
  C() : prev(flag)
  {
    flag = true; // #1
  }

  ~C() {
    flag = prev;
  }
};

void g(int* p)
{
  delete p;
}

void f()
{
  int* p;
  {
    C c;
    p = new int;
  }
  g(p);
}

int main(int, char**)
{
  f();
}


> cat t2.C
#include <stdlib.h>
#include <assert.h>

extern bool flag;

void* operator new(unsigned long size)
{
  assert(flag);  // #2
  return malloc(size);
}

void operator delete(void *p)
{
  free(p);
}


it works as 'flag' is then global and no longer local to the TU.

So it might work to disable the new/delete/malloc/free optimization when
we see a definition of those functions.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #6)
> As Jakub said the behavior is the same for malloc() since years.
> 
... 
> So it might work to disable the new/delete/malloc/free optimization when
> we see a definition of those functions.

But it's surely not enough to disable transforms like

 x = 0;
 ... = new ...;
 if (x != 0)
   ..

to elide the load from x after the 'new'.  Like with malloc we assume
that state of the runtime (where malloc / new is defined) is only
accessible via API calls and not visible to the compiler at the same
time its users are.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
But operator new is not defined in the runtime here, it's a replaceable global
allocation function. The assumption seems unsafe for replaceable functions that
users can define in their own code.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=59894

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I don't think this optimization should be on by default. A switch that says
"this program does not replace operator new and delete" would make it OK. That
is the subject of PR 59894.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[jens.maurer at gmx dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #10 from Jens Maurer <jens.maurer at gmx dot net> ---
I agree with Jonathan here: The difference is that "malloc" comes with the
compiler/library and cannot be replaced (within the scope of the C or C++
standards), but "operator new" is expressly specified to be replaceable by the
C++ standard. There is text in the standard that restricts what "operator new"
can and cannot do, but otherwise it is specified as-if a regular function call.

As-is, gcc 11 is non-conforming with this optimization turned on by default.

I'm all for providing an extra command-line switch so that the user can assert
that he's not replacing "operator new" anywhere in the program, but that switch
should be off by default.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 19 Jul 2021, redi at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480
> 
> Jonathan Wakely <redi at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>            See Also|                            |https://gcc.gnu.org/bugzill
>                    |                            |a/show_bug.cgi?id=59894
> 
> --- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> I don't think this optimization should be on by default. A switch that says
> "this program does not replace operator new and delete" would make it OK. That
> is the subject of PR 59894.

The odd thing is we do

  /* 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 we can treat it as free.  */
  if (fndecl
      && DECL_IS_OPERATOR_DELETE_P (fndecl)
      && DECL_IS_REPLACEABLE_OPERATOR (fndecl)
      && gimple_call_from_new_or_delete (stmt))
    return ".co ";
  /* Similarly operator new can be treated as malloc.  */
  if (fndecl
      && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
      && gimple_call_from_new_or_delete (stmt))
    return "mC";

so explicitely asking for the replaceable variants only (well, not sure
if ones that don't have DECL_IS_REPLACEABLE_OPERATOR set are any
"better" or worse here).  Then we have valid_new_delete_pair_p used by
new/delete pair removal which explicitely checks mangled symbol names.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
That was because of PR98130 it seems.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
In PR98130 I suggested "..o " for delete and it would be "m " for new (does
a C++ new expression really set/clobber errno?).

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 863bc0d17f1..e085d9de49a 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -1516,12 +1516,12 @@ gimple_call_fnspec (const gcall *stmt)
       && DECL_IS_OPERATOR_DELETE_P (fndecl)
       && DECL_IS_REPLACEABLE_OPERATOR (fndecl)
       && gimple_call_from_new_or_delete (stmt))
-    return ".co ";
+    return ". o ";
   /* Similarly operator new can be treated as malloc.  */
   if (fndecl
       && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
       && gimple_call_from_new_or_delete (stmt))
-    return "mC";
+    return "m ";
   return "";
 }


regresses

FAIL: g++.dg/warn/Warray-bounds-16.C  -std=gnu++14  scan-tree-dump-not
optimized "goto"
FAIL: g++.dg/warn/Warray-bounds-16.C  -std=gnu++14 (test for excess errors)

where we now diagnose

/home/rguenther/src/trunk/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C:22:7:
warning: array subscript 0 is outside array bounds of 'void [0]'
[-Warray-bounds]
/home/rguenther/src/trunk/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C:22:7:
warning: 'void* __builtin_memset(void*, int, long unsigned int)' offset [0, 3]
is out of the bounds [0, 0] [-Warray-bounds]

the testcase does

    m = i;
    p = (int*) new unsigned char [sizeof (int) * m];

    for (int i = 0; i < m; i++)
      new (p + i) int ();

and we likely have to assume that 'new' changes 'm'.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.2                        |11.3

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 11.2 is being released, retargeting bugs to GCC 11.3

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #16 from Martin Sebor <msebor at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #14)
...
> the testcase does
> 
>     m = i;
>     p = (int*) new unsigned char [sizeof (int) * m];
> 
>     for (int i = 0; i < m; i++)
>       new (p + i) int ();
> 
> and we likely have to assume that 'new' changes 'm'.

Why?  Because of the flow-insensitivity of the alias analysis?

m is a member of the class whose ctor has the loop above.  Neither the
enclosing object nor the member actually escapes (the operator new to which p
is passed in the loop is the nonreplaceable placement new), so there is no way
it can be changed.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
I'm re-testing the patch and will investigate the failure more if it still
happens.

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amacleod at redhat dot com

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #16)
> (In reply to Richard Biener from comment #14)
> ...
> > the testcase does
> > 
> >     m = i;
> >     p = (int*) new unsigned char [sizeof (int) * m];
> > 
> >     for (int i = 0; i < m; i++)
> >       new (p + i) int ();
> > 
> > and we likely have to assume that 'new' changes 'm'.
> 
> Why?  Because of the flow-insensitivity of the alias analysis?
> 
> m is a member of the class whose ctor has the loop above.  Neither the
> enclosing object nor the member actually escapes (the operator new to which
> p is passed in the loop is the nonreplaceable placement new), so there is no
> way it can be changed.

What we see is the global variable construction function which accesses
just 'a', and yes, the call to 'new' is considered clobbering global
variables (including 'a'):

  <bb 2> [local count: 1073741824]:
  MEM[(struct __as_base  &)&a] ={v} {CLOBBER};
  a.m = 0;
  _5 = operator new [] (0);
  a.p = _5;
  goto <bb 4>; [100.00%]

  <bb 3> [local count: 8687547547]:
  _7 = (long unsigned int) i_6;
  _8 = _7 * 4;
  _9 = _5 + _8;
  MEM[(int *)_9] = 0;
  i_10 = i_6 + 1;

  <bb 4> [local count: 9761289362]:
  # i_6 = PHI <0(2), i_10(3)>
  _11 = a.m;
  if (i_6 < _11)
    goto <bb 3>; [89.00%]
  else
    goto <bb 5>; [11.00%]

  <bb 5> [local count: 1073741824]:
  return;

I suppose implementing the global operator new as accessing a.m would
be valid as IIRC lifetime of a starts when the CTOR is invoked, not
when it finished (otherwise the CTOR could not access the variable itself).

We somehow conclude that

_9: void * [1B, +INF]  EQUIVALENCES: { } (0 elements)

possibly because it cannot be NULL (?):

extract_range_from_stmt visiting:
_5 = operator new [] (0);
Found new range for _5: void * [1B, +INF]
marking stmt to be not simulated again

(huh?)

and then the -Warray-bounds warning concludes the access is always outside
of the allocated area.

I suspect when we'd arrive at VARYING we'd not issue the warning even
when the access would always extend beyond a zero-sized allocation.

I'm going to ignore this diagnostic issue, it concerns the 'offrange'
code I'm not familiar with at all (and maybe the value-range code for
which I now have to say sth similar).

[Bug c++/101480] [11/12 Regression] Miscompiled code involving operator new

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:09a0affdb0598a54835ac4bb0dd6b54122c12916

commit r12-4319-g09a0affdb0598a54835ac4bb0dd6b54122c12916
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Oct 11 16:06:03 2021 +0200

    middle-end/101480 - overloaded global new/delete

    The following fixes the issue of ignoring side-effects on memory
    from overloaded global new/delete operators by not marking them
    as effectively 'const' apart from other explicitely specified
    side-effects.

    This will cause

    FAIL: g++.dg/warn/Warray-bounds-16.C  -std=gnu++1? (test for excess errors)

    because we now no longer statically see the initialization loop
    never executes because the call to operator new can now clobber 'a.m'.
    This seems to be an issue with the warning code and/or ranger so
    I'm leaving this FAIL to be addressed as followup.

    2021-10-11  Richard Biener  <rguenther@suse.de>

            PR middle-end/101480
            * gimple.c (gimple_call_fnspec): Do not mark operator new/delete
            as const.

            * g++.dg/torture/pr10148.C: New testcase.

[Bug c++/101480] [11 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|12.0                        |11.2.0
      Known to work|                            |12.0
            Summary|[11/12 Regression]          |[11 Regression] Miscompiled
                   |Miscompiled code involving  |code involving operator new
                   |operator new                |

--- Comment #20 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug c++/101480] [11 Regression] Miscompiled code involving operator new

[hubicka at kam dot mff.cuni.cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #21 from hubicka at kam dot mff.cuni.cz ---
Hi,
note that also tree-ssa-structalias has:
/* 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_OPERATOR_DELETE_P (fndecl)                            
         && gimple_call_from_new_or_delete (t))                           
  ;                                                                       
I wonder if this is safe...

Honza

[Bug c++/101480] [11 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|11.1.0                      |

--- Comment #22 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to hubicka from comment #21)
> Hi,
> note that also tree-ssa-structalias has:
> /* 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_OPERATOR_DELETE_P (fndecl)                            
>          && gimple_call_from_new_or_delete (t))                           
>   ;                                                                       
> I wonder if this is safe...

I think the reasoning is that for a delete expression the storage passed
is released afterwards and thus the delete call cannot be an escape point
(as opposed to the 'new' call) for this very object.

[Bug c++/101480] [11 Regression] Miscompiled code involving operator new

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #23 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The first thing a delete expression does is invoke the destructor, after that
there is no object anyway.

[Bug c++/101480] [11 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #24 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #23)
> The first thing a delete expression does is invoke the destructor, after
> that there is no object anyway.

The situation where we avoid the escape only is also hard to exploit
since the caller would have to use the object after deletion.

But one can do sth like

static int *storage;

void operator delete (void *p)
{
  storage = (int *)p;
}

int main()
{
  int *p = new int;
  *p = 1;
  int q;
  storage = &q;
  q = 3;
  delete p;
  *storage = 2;
  if (q != 3)
    __builtin_abort ();
}

where we while we still cannot CSE q across the delete call because q
escapes (to 'storage'), we do elide the new/delete pair which then
causes a runfail.  The critical point here is (as in the original example
in this PR) that the user of new/delete expects a specific implementation
detail of new/delete (here setting of 'storage' by delete, upthread
the requirement to setting 'flag').

The whole point of making sure the argument of the delete does not make
the storage pointed to it escape (and not used) is to be able to DSE
stores to it (see the affected g++.dg/cpp1y/new1.C testcase) and thus
to elide more new/delete pairs.

[Bug c++/101480] [11 Regression] Miscompiled code involving operator new

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

--- Comment #25 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:19dcea67ac40cfdeb396fa264ebbe04fbe61fdc0

commit r11-9222-g19dcea67ac40cfdeb396fa264ebbe04fbe61fdc0
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Oct 11 16:06:03 2021 +0200

    middle-end/101480 - overloaded global new/delete

    The following fixes the issue of ignoring side-effects on memory
    from overloaded global new/delete operators by not marking them
    as effectively 'const' apart from other explicitely specified
    side-effects.

    This will cause

    FAIL: g++.dg/warn/Warray-bounds-16.C  -std=gnu++1? (test for excess errors)

    because we now no longer statically see the initialization loop
    never executes because the call to operator new can now clobber 'a.m'.
    This seems to be an issue with the warning code and/or ranger so
    I'm leaving this FAIL to be addressed as followup.

    2021-10-11  Richard Biener  <rguenther@suse.de>

            PR middle-end/101480
            * gimple.c (gimple_call_fnspec): Do not mark operator new/delete
            as const.

            * g++.dg/torture/pr10148.C: New testcase.

    (cherry picked from commit 09a0affdb0598a54835ac4bb0dd6b54122c12916)

[Bug c++/101480] [11 Regression] Miscompiled code involving operator new

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101480

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |11.2.1
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #26 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.