[Bug analyzer/110501] New: Invalid use-after-free / realloc

[Bug analyzer/110501] New: Invalid use-after-free / realloc

[cheyenne.wills at gmail dot com]

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

            Bug ID: 110501
           Summary: Invalid use-after-free / realloc
           Product: gcc
           Version: 12.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: cheyenne.wills at gmail dot com
  Target Milestone: ---

Created attachment 55433
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55433&action=edit
testcase.i

I've ran into a problem where gcc-12 (and later versions) is producing a false
positive on a use-after-free following a realloc.

The attached information obtained from a current gentoo system using gcc
(Gentoo 12.3.1_p20230526 p2) 12.3.1 20230526) 

gcc -v -save-temps -Wall -c testcase.c &> testcase.log


I've been able to duplicate the problem on godbolt.org using different versions
of gcc from gcc 12.1 through gcc master (I also tried various architectures,
x86_64, arm, etc.). godbolt's gcc 11 does work as expected.

The gist of the problem is:
---
 S->sp = realloc(p, size * 2);
 if (S->sp == NULL && size != 0) {
     free(p);    /* << is flagged as a use after free */
     return 0;
 }
---
However the following works:
---
 char *t;
 t = realloc(p, size * 2);
 if (t == NULL && size != 0) {
     free(p);   /* << is not flagged as a use after free */
     return 0;
 }
 S->sp = t;
---

The provided testcase contains 3 simple functions.  The function fail1 and
fail2  has code that shows the invalid use-after-free, while the function
succeeds has code that does not produce the use-after-free message.  The only
difference between the failed functions and the success that the success
function uses a stack based temporary variable to hold the result of the
realloc.


==
$ $ gcc -Wall -Wextra  -c testcase.c 
testcase.c: In function ‘fail1’:
testcase.c:10:9: warning: pointer ‘p’ may be used after ‘realloc’
[-Wuse-after-free]
   10 |         free(p); /* Is flagged as a use after free */
      |         ^~~~~~~
testcase.c:8:13: note: call to ‘realloc’ here
    8 |     S->sp = realloc(p, size*2);
      |             ^~~~~~~~~~~~~~~~~~
testcase.c: In function ‘fail2’:
testcase.c:20:9: warning: pointer ‘p’ may be used after ‘realloc’
[-Wuse-after-free]
   20 |         free(p); /* Is flagged as a use after free */
      |         ^~~~~~~
testcase.c:18:9: note: call to ‘realloc’ here
   18 |     t = realloc(p, size*2);
      |         ^~~~~~~~~~~~~~~~~~
$

==

The problem was originally discovered while building from openafs's master
branch (www.openafs.org) with a gcc-13 compiler. 

  src/external/heimdal/krb5/crypto.c:1157:9: error: pointer ‘p’ may be
      used after ‘realloc’ [-Werror=use-after-free]
   1157 |         free(p);
        |         ^~~~~~~
  src/external/heimdal/krb5/crypto.c:1155:20: note: call to ‘realloc’
      here
   1155 |     result->data = realloc(p, sz);
        |                    ^~~~~~~~~~~~~~

The failing code is part of an "external library" from the heimdal project.

[Bug analyzer/110501] Invalid use-after-free / realloc

[cheyenne.wills at gmail dot com]

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

--- Comment #1 from Cheyenne Wills <cheyenne.wills at gmail dot com> ---
Created attachment 55434
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55434&action=edit
stdout/stderr from compile

[Bug tree-optimization/110501] Invalid use-after-free / realloc at -O0

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Invalid use-after-free /    |Invalid use-after-free /
                   |realloc                     |realloc at -O0

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
At -O0, the redundant load from S->sp is not optimized out ... so the warning
does not realize that it was checking against the return value of realloc ...

[Bug tree-optimization/110501] Invalid use-after-free / realloc with a store/load happening

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Oh GCC warns even with optimizations turned on ...

[Bug tree-optimization/110501] Invalid use-after-free / realloc with a store/load happening

[cheyenne.wills at gmail dot com]

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

--- Comment #4 from Cheyenne Wills <cheyenne.wills at gmail dot com> ---
(In reply to Andrew Pinski from comment #3)
> Oh GCC warns even with optimizations turned on ...

Correct.  For gcc-12 the failure only occurs with -O0.  With gcc-13 (and
later), the problem occurs with or without optimization.

[Bug tree-optimization/110501] Invalid use-after-free / realloc with a store/load happening

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-07-03
     Ever confirmed|0                           |1

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
So confirmed.

[Bug tree-optimization/110501] Invalid use-after-free / realloc with a store/load happening

[cheyenne.wills at gmail dot com]

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

--- Comment #6 from Cheyenne Wills <cheyenne.wills at gmail dot com> ---
Just another bit of information.

Specifying just -Werror=use-after-free appears to be not not enough to trigger
the problem.  Using -Wall however does trigger the problem.

(tried on gcc-12 and gcc-13)

[Bug tree-optimization/110501] Invalid use-after-free / realloc with a store/load happening

[jhb at FreeBSD dot org]

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

John Baldwin <jhb at FreeBSD dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jhb at FreeBSD dot org

--- Comment #7 from John Baldwin <jhb at FreeBSD dot org> ---
I believe I am hitting this same issue using GCC 13 to compile FreeBSD's
userland. (I looked through several of the other bugs for the Wuse-after-free
metabug and this one seems the closest to what I'm encountering).  The code in
question does not trigger the -Wuse-after-free warning when using GCC 12, but
now triggers the warning with GCC 13.  It is just calling free on the old value
after realloc fails which should be safe.  The simplest version I've
encountered so far in FreeBSD's tree is this:

static int
group_resize(void)
{
        char *buf;

        if (gbufsize == 0)
                gbufsize = 1024;
        else
                gbufsize *= 2;

        buf = gbuffer;
        gbuffer = realloc(buf, gbufsize);
        if (gbuffer == NULL) {
                free(buf);
                gbufsize = 0;
                return (ENOMEM);
        }
        memset(gbuffer, 0, gbufsize);

        return (0);
}

The warning triggers on the call to free:

    lib/libcasper/services/cap_grp/cap_grp.c: In function 'group_resize':
    lib/libcasper/services/cap_grp/cap_grp.c:65:17: error: pointer 'buf' may be
used after 'realloc' [-Werror=use-after-free]
       65 |                 free(buf);
          |                 ^~~~~~~~~
    lib/libcasper/services/cap_grp/cap_grp.c:63:19: note: call to 'realloc'
here
       63 |         gbuffer = realloc(buf, gbufsize);
          |                   ^~~~~~~~~~~~~~~~~~~~~~

[Bug tree-optimization/110501] [13/14 regression] Invalid -Wuse-after-free / realloc with a store/load happening

[sjames at gcc dot gnu.org]

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

Sam James <sjames at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13/14 regression]       |[13/14 regression] Invalid
                   |Invalid -Wuse-after-free /  |-Wuse-after-free / realloc
                   |realloc with a store/load   |with a store/load happening
                   |happening                   |

--- Comment #8 from Sam James <sjames at gcc dot gnu.org> ---
oh, right, it was added in 12. But given 12 only warns with -O0 and 13+ warn
with optimisation, let's call it a 13/14 regression.

[Bug tree-optimization/110501] [13/14 regression] Invalid -Wuse-after-free / realloc with a store/load happening

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.3

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to John Baldwin from comment #7)
> I believe I am hitting this same issue using GCC 13 to compile FreeBSD's
> userland. (I looked through several of the other bugs for the
> Wuse-after-free metabug and this one seems the closest to what I'm
> encountering).  The code in question does not trigger the -Wuse-after-free
> warning when using GCC 12, but now triggers the warning with GCC 13.  It is
> just calling free on the old value after realloc fails which should be safe.
> The simplest version I've encountered so far in FreeBSD's tree is this:
> 
> static int
> group_resize(void)
> {
> 	char *buf;
> 
> 	if (gbufsize == 0)
> 		gbufsize = 1024;
> 	else
> 		gbufsize *= 2;
> 
> 	buf = gbuffer;
> 	gbuffer = realloc(buf, gbufsize);
> 	if (gbuffer == NULL) {
> 		free(buf);

the problem with this is that 'gbuffer' is a global and we diagnose this
early before applying memory CSE so we don't see the 'gbuffer' being
stored and 'gbuffer' being tested against NULL as equal.

IIRC with GCC 13 we moved some diagnostics early from late as late has
issues with jump threading exposing very many false positives.

Testcase (which I believe we have a duplicate for):

void *gbuffer;
unsigned long gbufsize;
void foo()
{
  char *buf = gbuffer;
  gbuffer = __builtin_realloc(buf, gbufsize);
  if (!gbuffer) {
      __builtin_free(buf);
      return;
  }
  __builtin_memset (gbuffer, 0, gbufsize);
}

I'll note that for the early -Wuninitialized I added cheap value numbering
to prune unreachable code paths.  The plan was to wrap all of the early
diagnostics with such cheap VN _analysis_ (but not do any code changes)
which would allow these cases to be identified as equal.  The diagnostic
passes would need to apply some valueization then of course.

[Bug tree-optimization/110501] [13/14 regression] Invalid -Wuse-after-free / realloc with a store/load happening

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2