[Bug tree-optimization/109238] New: [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[Bug tree-optimization/109238] New: [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[marxin at gcc dot gnu.org]

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

            Bug ID: 109238
           Summary: [13 Regression] tst-realloc.i:42:19: error: pointer
                    ‘p’ may be used after ‘realloc’
                    [-Werror=use-after-free] in glibc tests
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: rguenth at gcc dot gnu.org
  Target Milestone: ---

Created attachment 54723
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54723&action=edit
Unreduced test-case

It's the same issue as PR109170 this time in glibc's test-suite:

[ 1569s] gcc tst-realloc.c -c -std=gnu11 -fgnu89-inline  -O2 -Wall
-funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection
-Werror=return-type -g -Wall -Wwrite-strings -Wundef -Werror
-fmerge-all-constants -frounding-math -fstack-protector-strong -fno-common
-Wstrict-prototypes -Wold-style-definition -fmath-errno    -fPIE
-fcf-protection         -I../include
-I/home/abuild/rpmbuild/BUILD/glibc-2.37.9000.180.g9e2ff880f3/cc-base/malloc 
-I/home/abuild/rpmbuild/BUILD/glibc-2.37.9000.180.g9e2ff880f3/cc-base 
-I../sysdeps/unix/sysv/linux/x86_64/64  -I../sysdeps/unix/sysv/linux/x86_64 
-I../sysdeps/unix/sysv/linux/x86/include -I../sysdeps/unix/sysv/linux/x86 
-I../sysdeps/x86/nptl  -I../sysdeps/unix/sysv/linux/wordsize-64 
-I../sysdeps/x86_64/nptl  -I../sysdeps/unix/sysv/linux/include
-I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread 
-I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv 
-I../sysdeps/unix/x86_64  -I../sysdeps/unix  -I../sysdeps/posix 
-I../sysdeps/x86_64/64  -I../sysdeps/x86_64/fpu/multiarch 
-I../sysdeps/x86_64/fpu  -I../sysdeps/x86/fpu  -I../sysdeps/x86_64/multiarch 
-I../sysdeps/x86_64  -I../sysdeps/x86/include -I../sysdeps/x86 
-I../sysdeps/ieee754/float128  -I../sysdeps/ieee754/ldbl-96/include
-I../sysdeps/ieee754/ldbl-96  -I../sysdeps/ieee754/dbl-64 
-I../sysdeps/ieee754/flt-32  -I../sysdeps/wordsize-64  -I../sysdeps/ieee754 
-I../sysdeps/generic  -I.. -I../libio -I.  -D_LIBC_REENTRANT -include
/home/abuild/rpmbuild/BUILD/glibc-2.37.9000.180.g9e2ff880f3/cc-base/libc-modules.h
-DMODULE_NAME=testsuite -include ../include/libc-symbols.h  -DPIC    
-DTOP_NAMESPACE=glibc -DTEST_NO_MALLOPT -o
/home/abuild/rpmbuild/BUILD/glibc-2.37.9000.180.g9e2ff880f3/cc-base/malloc/tst-realloc.o
-MD -MP -MF
/home/abuild/rpmbuild/BUILD/glibc-2.37.9000.180.g9e2ff880f3/cc-base/malloc/tst-realloc.o.dt
-MT
/home/abuild/rpmbuild/BUILD/glibc-2.37.9000.180.g9e2ff880f3/cc-base/malloc/tst-realloc.o
[ 1569s] In function 'do_test',
[ 1569s]     inlined from 'legacy_test_function' at ../test-skeleton.c:55:10:
[ 1569s] tst-realloc.c:135:7: error: pointer 'p' may be used after 'realloc'
[-Werror=use-after-free]
[ 1569s]   135 |   p = realloc (p, 0);
[ 1569s]       |       ^~~~~~~~~~~~~~
[ 1569s] tst-realloc.c:117:7: note: call to 'realloc' here
[ 1569s]   117 |   c = realloc (p, -1);
[ 1569s]       |       ^~~~~~~~~~~~~~~
[ 1569s] tst-realloc.c:127:12: error: pointer 'p' may be used after 'realloc'
[-Werror=use-after-free]
[ 1569s]   127 |       if (c[i] != 0xff)
[ 1569s]       |            ^
[ 1569s] tst-realloc.c:117:7: note: call to 'realloc' here
[ 1569s]   117 |   c = realloc (p, -1);

$ gcc tst-realloc.i -c -O2 -Wall -Werror
In function ‘do_test’,
    inlined from ‘legacy_test_function’ at ../test-skeleton.c:55:10:
tst-realloc.c:135:7: error: pointer ‘p’ may be used after ‘realloc’
[-Werror=use-after-free]
tst-realloc.c:117:7: note: call to ‘realloc’ here
tst-realloc.c:127:12: error: pointer ‘p’ may be used after ‘realloc’
[-Werror=use-after-free]
tst-realloc.c:117:7: note: call to ‘realloc’ here
cc1: all warnings being treated as errors

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-03-21
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=109170
   Target Milestone|---                         |13.0
     Ever confirmed|0                           |1

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
  c = realloc (p, -1);

...
                        ;
  if (c != 
          ((void *)0)
              )
    support_exit_failure_impl (1, "tst-realloc.c", 120, "realloc (p, -1)
succeeded.");

  c = p;
...

  p = realloc (p, 0);

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[rguenth at gcc dot gnu.org]

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

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 #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I will have a look.

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
For the given CFG where

  dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), gimple_bb (stmt));

and we have

<bb 28> [local count: 428124]:
c_24 = realloc (p_17, 18446744073709551615);
if (c_24 != 0B)
  goto <bb 29>; [99.96%]
else
  goto <bb 33>; [0.04%]

.. on the 28->33 edge intermediate blocks, including a CFG cycle ...

<bb 36> [local count: 171]:
p_31 = realloc (p_17, 0);

but range_of_expr (vr, c_24, use_stmt) doesn't give us [0,0], instead we
get VARYING.

It looks like we go range_of_stmt of the c_24 def and then m_cache.block_range
has a VARYING entry range as well and then we're done.

176      range_of_expr(c_24) at stmt p_31 = realloc (p_17, 0);
177        range_on_entry (c_24) to BB 36
178          range_of_stmt (c_24) at stmt c_24 = realloc (p_17,
18446744073709551615);
   GLOBAL : UPDATE cache for c_24 in BB 28 : successors :   : No updates!
             TRUE : (178) range_of_stmt (c_24) [irange] unsigned char * VARYING

c_24 : CACHE: BB 36 DOM query for c_24, found [irange] unsigned char * VARYING
at BB28
179     GORI  outgoing_edge for c_24 on edge 28->33
180     GORI    compute op 1 (c_24) at if (c_24 != 0B)
        GORI      LHS =[irange] _Bool [0, 0] NONZERO 0x0
        GORI      Computes c_24 = [irange] unsigned char * [0, 0] NONZERO 0x0
intersect Known range : [irange] unsigned char * VARYING
        GORI    TRUE : (180) produces  (c_24) [irange] unsigned char * [0, 0]
NONZERO 0x0
        GORI  TRUE : (179) outgoing_edge (c_24) [irange] unsigned char * [0, 0]
NONZERO 0x0
CACHE: BB 32 DOM query for c_24, found [irange] unsigned char * VARYING at BB28
CACHE: Range for DOM returns : [irange] unsigned char * VARYING
CACHE: Range for DOM returns : [irange] unsigned char * VARYING
Filled from dominator! :  [irange] unsigned char * VARYING
           TRUE : (177) range_on_entry (c_24) [irange] unsigned char * VARYING
         TRUE : (176) range_of_expr (c_24) [irange] unsigned char * VARYING
181      range_of_expr(c_24) at stmt _27 = p_17 + _26;
182        range_on_entry (c_24) to BB 30
183          range_of_stmt (c_24) at stmt c_24 = realloc (p_17,
18446744073709551615);
             TRUE : (183)  cached (c_24) [irange] unsigned char * VARYING

c_24 : CACHE: BB 30 DOM query for c_24, found [irange] unsigned char * VARYING
at BB33
CACHE: Range for DOM returns : [irange] unsigned char * VARYING
Filled from dominator! :  [irange] unsigned char * VARYING
           TRUE : (182) range_on_entry (c_24) [irange] unsigned char * VARYING
...

(not sure where relevant dumps stop).

A reduced testcase like the following doesn't exhibit this issue - the
range is successfully computed as [0,0] there.  So to me this looks like
a ranger issue?  Andrew?

#include <stdlib.h>

int do_test(void *p)
{
  unsigned char *c = realloc (p, -1);
  if (c != 0)
    abort ();

  c = p;
  int ok = 1;

  for (int i = 0; i < 16; i++)
    {
      if (c[i] != 0xff)
        ok = 0;
    }

  if (!ok)
    abort ();

  p = realloc (p, 0);
  if (p != NULL)
    abort ();

  return ok;
}

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[amacleod at redhat dot com]

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

--- Comment #4 from Andrew Macleod <amacleod at redhat dot com> ---
I think varying is the correct result tho?   the branch is in BB28, but bb 33
is a merge poi9nt again before bb 36, so we can't tell that it is anything but
varying?

I see:

<bb 28> [local count: 428124]:
  c_24 = realloc (p_17, 18446744073709551615);
  if (c_24 != 0B)
    goto <bb 29>; [99.96%]
  else
    goto <bb 33>; [0.04%]

  <bb 29> [local count: 427952]:
  support_exit_failure_impl (1, "tst-realloc.c", 120, "realloc (p, -1)
succeeded.");

  <bb 30> [local count: 2741]:
  _26 = (sizetype) i_25;
  _27 = p_17 + _26;
  _28 = *_27;
  if (_28 != 255)
    goto <bb 31>; [66.00%]
  else
    goto <bb 32>; [34.00%]

  <bb 31> [local count: 1809]:

  <bb 32> [local count: 2741]:
  # ok_48 = PHI <ok_49(30), 0(31)>
  i_29 = i_25 + 1;

  <bb 33> [local count: 2912]:         <<--  both sides of BB 28 branch merge
here, so c_24 is varying again 
  # i_25 = PHI <0(28), i_29(32)>
  # ok_49 = PHI <1(28), ok_48(32)>
  if (i_25 != 16)
    goto <bb 30>; [94.12%]
  else
    goto <bb 34>; [5.88%]

  <bb 34> [local count: 171]:
  # ok_30 = PHI <ok_49(33)>
  if (ok_30 == 0)
    goto <bb 35>; [0.04%]
  else
    goto <bb 36>; [99.96%]

  <bb 35> [local count: 0]:
  support_exit_failure_impl (1, "tst-realloc.c", 132, "first 16 bytes were not
correct after failed realloc");

  <bb 36> [local count: 171]:
  p_31 = realloc (p_17, 0);       <<-- This is dominated by bb 33 which merged
the bb28 branch

During the cache dump the bit about
CACHE: BB 32 DOM query for c_24, found [irange] unsigned char * VARYING at BB28
CACHE: Range for DOM returns : [irange] unsigned char * VARYING
CACHE: Range for DOM returns : [irange] unsigned char * VARYING

Is a bit of a misnomer I think, its dumping in the middle of a query and didnt
show the fully updated value..  But I will look a bit closer at it.. in theory
that should have been non-zero, even tho I dont think it affects the result.

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Macleod from comment #4)
> I think varying is the correct result tho?   the branch is in BB28, but bb
> 33 is a merge poi9nt again before bb 36, so we can't tell that it is
> anything but varying?
> 
> I see:
> 
> <bb 28> [local count: 428124]:
>   c_24 = realloc (p_17, 18446744073709551615);
>   if (c_24 != 0B)
>     goto <bb 29>; [99.96%]
>   else
>     goto <bb 33>; [0.04%]
>   
>   <bb 29> [local count: 427952]:
>   support_exit_failure_impl (1, "tst-realloc.c", 120, "realloc (p, -1)
> succeeded.");

^^^ this is noreturn

> 
>   <bb 30> [local count: 2741]:
>   _26 = (sizetype) i_25;
>   _27 = p_17 + _26;
>   _28 = *_27;
>   if (_28 != 255)
>     goto <bb 31>; [66.00%]
>   else
>     goto <bb 32>; [34.00%]
>   
>   <bb 31> [local count: 1809]:
>   
>   <bb 32> [local count: 2741]:
>   # ok_48 = PHI <ok_49(30), 0(31)>
>   i_29 = i_25 + 1;
>   
>   <bb 33> [local count: 2912]:         <<--  both sides of BB 28 branch
> merge here, so c_24 is varying again 
>   # i_25 = PHI <0(28), i_29(32)>
>   # ok_49 = PHI <1(28), ok_48(32)>
>   if (i_25 != 16)
>     goto <bb 30>; [94.12%]
>   else
>     goto <bb 34>; [5.88%]
>   
>   <bb 34> [local count: 171]:
>   # ok_30 = PHI <ok_49(33)>
>   if (ok_30 == 0)
>     goto <bb 35>; [0.04%]
>   else
>     goto <bb 36>; [99.96%]
>   
>   <bb 35> [local count: 0]:
>   support_exit_failure_impl (1, "tst-realloc.c", 132, "first 16 bytes were
> not correct after failed realloc");

likewise

>   <bb 36> [local count: 171]:
>   p_31 = realloc (p_17, 0);       <<-- This is dominated by bb 33 which
> merged the bb28 branch
> 
> During the cache dump the bit about
> CACHE: BB 32 DOM query for c_24, found [irange] unsigned char * VARYING at
> BB28
> CACHE: Range for DOM returns : [irange] unsigned char * VARYING
> CACHE: Range for DOM returns : [irange] unsigned char * VARYING
> 
> Is a bit of a misnomer I think, its dumping in the middle of a query and
> didnt show the fully updated value..  But I will look a bit closer at it..
> in theory that should have been non-zero, even tho I dont think it affects
> the result.

As said, the "error" calls are noreturn so the range should be [0,0] in BB36.

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[amacleod at redhat dot com]

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

--- Comment #6 from Andrew Macleod <amacleod at redhat dot com> ---
Created attachment 54738
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54738&action=edit
Patch to fix the issue

Ah, sorry I missed that.

OK, I traced it through. The problem turns out to be in the dom_resolve code.
when we are trying to pick up a value via dominators, we walk the dominator
chain looking for outgoing edges which change the value and/or existing values.
 As we find these values, will fill the on-entry cache so that future queries
will be faster.

When we encounter a dominator node that has multiple incoming edges, as BB 33
does, we separately ask for a "quick" read-only fill and accumulate each
incoming edges values.  this allows us to pick up things where ranges are
adjusted on edges from non-dominator block ie

bb2:
   if (foo)
     if (a < 10) goto A: else goto B;
   else
     if (a >= 10) goto C:  else goto A:
   <more code>
   goto D:
A:

Block A's dominator is bb2.  It has 2 predecessors however, and on each of
those incoming edges, a has a range of [0, 10].

So by querying the outgoing range of a on each predecessor we come up with
[0,10] for a range of A, which would not be possible simply by examining the
dominator itself.

This query is done in a read-only mode so we dont go polluting the cache with a
bunch of things we may not need.  Anyway, it all works swimmingly.  usually.

What happened in this case is BB 33 has 2 predecessors. BB 28 and BB 32.
The edge from BB28 correctly picked up the range of ~[0,0], but the query for
BB32 went wrong.  BB32 is a back edge, and the query leads back to BB 33, and
in read only mode, we do not deal with these multiple incoming edges.. (and it
avoids an infinite loop).. so that query bails, and we end up with VARYING. 
that is what was generating the confusing output:

CACHE: BB 32 DOM query for c_24, found [irange] unsigned char * VARYING at BB28
CACHE: Range for DOM returns : [irange] unsigned char * VARYING
CACHE: Range for DOM returns : [irange] unsigned char * VARYING

When we are doing this inferior DOM query in read-only like this like this, we
do not need to incorporate anything from a back edge. Its intended to be
pulling a value from dominators, and there is no additional information on that
edge.  Any values from that edge can only be subsets of what the other incoming
edges have, and with the results being unioned... its pointless.

I have not yet managed to produce a reduced testcase.

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Reduced testcase:
/* PR tree-optimization/10923 */
/* { dg-do compile } */
/* { dg-options "-O2 -Wall" } */

void foo (void) __attribute__((noreturn));
void bar (void *);

void
baz (void *p)
{
  void *c = __builtin_realloc (p, 16);
  if (c)
    foo ();
  for (;;)
    bar (__builtin_realloc (p, 8));     /* { dg-bogus "pointer 'p' may be used
after '__builtin_realloc'" } */
}

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Macleod <amacleod@gcc.gnu.org>:

https://gcc.gnu.org/g:0409aa5a2de9ce3164933814a4a7adc91f6acb96

commit r13-6850-g0409aa5a2de9ce3164933814a4a7adc91f6acb96
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Thu Mar 23 10:28:34 2023 -0400

    Ranger cache dominator queries should ignore backedges.

    When querying dominators for cache values, ignore back edges in
    read-only mode.

            PR tree-optimization/109238
            gcc/
            * gimple-range-cache.cc (ranger_cache::resolve_dom): Ignore
            predecessors which this block dominates.

            gcc/testsuite/
            * gcc.dg/pr109238.c: New.

[Bug tree-optimization/109238] [13 Regression] tst-realloc.i:42:19: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] in glibc tests

[amacleod at redhat dot com]

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

Andrew Macleod <amacleod at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Andrew Macleod <amacleod at redhat dot com> ---
fixed.