[Bug tree-optimization/107868] New: [10 regression] Wrong code on AArch64 at -O1 with new/delete

[Bug tree-optimization/107868] New: [10 regression] Wrong code on AArch64 at -O1 with new/delete

[victor.donascimento at arm dot com]

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

            Bug ID: 107868
           Summary: [10 regression] Wrong code on AArch64 at -O1 with
                    new/delete
           Product: gcc
           Version: 10.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: victor.donascimento at arm dot com
  Target Milestone: ---

The libstdc++ execution test 20_util/allocator/1.cc test fails on the head of
the gcc-10 release branch when compiled at at the -O1 optimization level.

The cddce1 tree dump reports the following elimination:

Eliminating unnecessary statements:                                             
Deleting : _4 = ~check_delete.1_3;                                              
Deleting : operator delete (_11, 256);
Deleting : _2 = ~check_new.0_1;                                                 
Deleting : _11 = operator new (256);

leading to the spurious loss of calls to both the new and delete operators.

bisecting between basepoints/gcc-10 and basepoints/gcc-11 identified the
following commit as introducing the error:

r10-2217-g8e8e7af514344588ff3e3da25c0cb74c12dc6a0d
Author: Martin Liska <mliska@suse.cz>
Date:   Fri Aug 2 08:07:15 2019 +0200

    Mark DECL_SET_IS_OPERATOR_DELETE for user-provided delete operators.

and the following as fixing the bug:

r11-3611-g0b945f959f03a6185a3130f30bfd524d01d4142c
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Oct 1 10:44:27 2020 +0200

    make use of CALL_FROM_NEW_OR_DELETE_P

Is the above patch an appropriate fix for the issue or does it mask any further
shortcommings in the compiler?

If we're happy with the fix, should it be backported to GCC 10?
Many thanks.

Here is a reduced testcase:

#include <memory>

struct gnu { };
bool check_new = false;
bool check_delete = false;

void*
operator new(std::size_t n) noexcept(false)
{
  check_new = true;
  return NULL;
}

void operator delete(void *v) noexcept
{
  check_delete = true;
  return;
}

void operator delete(void *v, std::size_t) noexcept
{
  ::operator delete(v);
}

void test01()
{
  std::allocator<gnu> obj;

  if (!check_new)
    __builtin_abort();

  obj.deallocate(pobj, 256);
  if (!check_delete)
    __builtin_abort();
}

int main()
{
  test01();
  return 0;
}

[Bug tree-optimization/107868] [10 regression] Wrong code on AArch64 at -O1 with new/delete

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-11-25

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
@Richi: What do you think?

[Bug tree-optimization/107868] [10 regression] Wrong code on AArch64 at -O1 with new/delete

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
   Target Milestone|---                         |10.5
   Last reconfirmed|2022-11-25 00:00:00         |
             Status|NEW                         |ASSIGNED
             Target|                            |aarch64
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Possibly yes.  Let me see if it can be backported.

[Bug tree-optimization/107868] [10 regression] Wrong code on AArch64 at -O1 with new/delete

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
a backport depends on r11-3610-gb6158faacbfb7d

Your reduced testcase doesn't compile since it references undeclared 'pobj'.

[Bug tree-optimization/107868] [10 regression] Wrong code on AArch64 at -O1 with new/delete

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
I can confirm backporting the two revs fixes the -O1 execute FAIL of
20_util/allocator/1.cc on the branch.  Fully testing that now.

[Bug tree-optimization/107868] [10 regression] Wrong code on AArch64 at -O1 with new/delete

[victor.donascimento at arm dot com]

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

--- Comment #5 from Victor Do Nascimento <victor.donascimento at arm dot com> ---
(In reply to Richard Biener from comment #3)
> a backport depends on r11-3610-gb6158faacbfb7d
> 
> Your reduced testcase doesn't compile since it references undeclared 'pobj'.

How embarassing. Accidentally deleted a line of code when clearing up
unnecessary comments.

If it is of any use, the working testcase is:

#include <memory>

struct gnu { };
bool check_new = false;
bool check_delete = false;

void*
operator new(std::size_t n) noexcept(false)
{
  check_new = true;
  return NULL;
}

void operator delete(void *v) noexcept
{
  check_delete = true;
  return;
}

void operator delete(void *v, std::size_t) noexcept
{
  ::operator delete(v);
}

void test01()
{
  std::allocator<gnu> obj;

  gnu* pobj = obj.allocate(256);

  if (!check_new)
    __builtin_abort();

  obj.deallocate(pobj, 256);
  if (!check_delete)
    __builtin_abort();
}

int main()
{
  test01();
  return 0;
}

[Bug tree-optimization/107868] [10 regression] Wrong code on AArch64 at -O1 with new/delete

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
It also depends on r11-3386-g9e64f17d044767 for a testsuite adjustment and
also r11-3612-g4f4ced28826ece to further fix the C++ frontend side and
avoid regressing g++.dg/pr94314.C but that doesn't apply cleanly.

So there's definitely too much to backport for this last GCC 10 release.  At
least I'm now going to stop chasing all related commits ...

[Bug tree-optimization/107868] [10 regression] Wrong code on AArch64 at -O1 with new/delete

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.0
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed in GCC 11.