[Bug c/109123] New: Bogus warning: pointer used after 'realloc' -Wuse-after-free

[Bug c/109123] New: Bogus warning: pointer used after 'realloc' -Wuse-after-free

[manu at gcc dot gnu.org]

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

            Bug ID: 109123
           Summary: Bogus warning: pointer used after 'realloc'
                    -Wuse-after-free
           Product: gcc
           Version: 12.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: manu at gcc dot gnu.org
  Target Milestone: ---

```c
typedef long unsigned int size_t;
extern void *realloc (void *__ptr, size_t __size)
     __attribute__ ((__nothrow__ , __leaf__)) __attribute__
((__warn_unused_result__)) __attribute__ ((__alloc_size__ (2)));
struct vector_objective; 
typedef struct vector_objective vector_objective;
struct vector_objective { double *_begin; double *_end; double *_capacity; };
static inline size_t vector_objective_size(const vector_objective * v) { 
    return v->_end - v->_begin; 
}
static inline size_t vector_objective_capacity(const vector_objective * v) {
    return v->_capacity - v->_begin;
}
static inline void vector_objective_reserve(vector_objective * v, size_t n) {
    size_t old_capacity = vector_objective_capacity(v);
    size_t old_size = vector_objective_size(v);
    if (n > old_capacity) {
        v->_begin = realloc(v->_begin, sizeof(double) * n);
        v->_end = v->_begin + old_size;
        v->_capacity = v->_begin + n;
    }
}
static inline void vector_objective_push_back(vector_objective * v, double x) {
    if (v->_end == v->_capacity)
        vector_objective_reserve (v, (vector_objective_capacity (v) == 0) ? 8 :
2 * vector_objective_capacity (v));
    *(v->_end) = x;
    v->_end++;
}

typedef struct {
    vector_objective xy;
} eaf_polygon_t;


int
rectangle_add(eaf_polygon_t * regions, double lx)
{

    vector_objective_push_back(&regions->xy, lx);
    return 0;
}
```

With -Wall -c -O2 produces:

In function 'vector_objective_size',
    inlined from 'vector_objective_reserve' at <source>:15:23,
    inlined from 'vector_objective_push_back' at <source>:24:9,
    inlined from 'rectangle_add' at <source>:38:5:
<source>:8:20: warning: pointer used after 'realloc' [-Wuse-after-free]
    8 |     return v->_end - v->_begin;
      |                    ^
In function 'vector_objective_reserve',
    inlined from 'vector_objective_push_back' at <source>:24:9,
    inlined from 'rectangle_add' at <source>:38:5:
<source>:17:21: note: call to 'realloc' here
   17 |         v->_begin = realloc(v->_begin, sizeof(double) * n);
      | 

But the use occurs before not after the realloc.

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Bogus warning: pointer used |Bogus warning: pointer used
                   |after 'realloc'             |after 'realloc'
                   |-Wuse-after-free            |-Wuse-after-free with -O2
           Keywords|                            |diagnostic
             Blocks|                            |104075

--- Comment #1 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
Also fails in trunk (gcc 13).


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104075
[Bug 104075] bogus/missing -Wuse-after-free

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[manu at gcc dot gnu.org]

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

--- Comment #2 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
This is probably the same underlying issue as PR 106119 and PR 104215, but
unlike those, this testcase is heavily reduced without any header files (and it
could be reduced further I believe).

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
    size_t old_size = vector_objective_size(v);
    if (n > old_capacity) {
        v->_begin = realloc(v->_begin, sizeof(double) * n);
        v->_end = v->_begin + old_size;

GCC has sunk part of the old_size computation (not the loads but the
subtraction) to after the realloc but before the use of old_size.

The diagnostic code doesn't deal with this code motion very well (read:
it diagnoses it).  We do have plenty of dups for this but the author of
the diagnostic says diagnosing is done on purpose here.

There's no way to fix this.  It doesn't make sense to limit optimization
for this academic "use-after-free", nor would it be an easy task because
of the lack of explicit dependences in the IL.

Confirmed (the diagnostic is happening).

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[manu at gcc dot gnu.org]

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

--- Comment #4 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
This case is a bit different from other Wuse-after-free false positives:

* there is no path through which the warning is true
* the warning says "is used" not "may be used"
* there is no easy workaround that I can see. Wuninitialized warnings are easy
to silence but what should one do here?

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[manu at gcc dot gnu.org]

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

--- Comment #5 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
> GCC has sunk part of the old_size computation (not the loads but the
> subtraction) to after the realloc but before the use of old_size.

Is this code motion valid? Is there any point in the middle-end that checks the
validity of the pointer beyond a free/realloc?

If there is a point where such check happens, perhaps it would be a good place
to apply no_warning attribute to the pointer.

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[manu at gcc dot gnu.org]

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

--- Comment #6 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
(In reply to Manuel López-Ibáñez from comment #5)
> Is this code motion valid? Is there any point in the middle-end that checks
> the validity of the pointer beyond a free/realloc?
> 
> If there is a point where such check happens, perhaps it would be a good
> place to apply no_warning attribute to the pointer.

Answering to myself: It seems the dispute is over the meaning of "use".
According to the middle-end and for the purposes of realloc/free, "use" means
dereference, while for the warning "use" is any read of the value.

By only warning for dereferences, the warning may miss some obvious cases like:

        tmp = realloc(v->_begin, sizeof(double) * n);
        v->_end = v->_begin + old_size;
        v->_begin = tmp;

However, given that the assumption of the middle-end has worked for decades,
and it will be not possible to fix it, warning only for dereferences (or moving
warning for value-uses to a level not enabled by -Wall) would seem more
user-friendly.

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Manuel López-Ibáñez from comment #6)
> (In reply to Manuel López-Ibáñez from comment #5)
> > Is this code motion valid? Is there any point in the middle-end that checks
> > the validity of the pointer beyond a free/realloc?
> > 
> > If there is a point where such check happens, perhaps it would be a good
> > place to apply no_warning attribute to the pointer.
> 
> Answering to myself: It seems the dispute is over the meaning of "use".
> According to the middle-end and for the purposes of realloc/free, "use"
> means dereference, while for the warning "use" is any read of the value.

Yes - that's for practicality since for memory operations we do have
dependences on the realloc.

> By only warning for dereferences, the warning may miss some obvious cases
> like:
> 
>         tmp = realloc(v->_begin, sizeof(double) * n);
>         v->_end = v->_begin + old_size;
>         v->_begin = tmp;
> 
> However, given that the assumption of the middle-end has worked for decades,
> and it will be not possible to fix it, warning only for dereferences (or
> moving warning for value-uses to a level not enabled by -Wall) would seem
> more user-friendly.

Warning for "escapes" (the store is an escape point) is also sensible I think.

Warning for other uses is really only sensible before any code motion pass
took place.

Unfortunately the testsuite is full of cases expected to be diagnosed but
are no longer with any change to the operation.

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[manu at gcc dot gnu.org]

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

--- Comment #8 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> Warning for "escapes" (the store is an escape point) is also sensible I
> think.

Would it be possible to make this distinction at the point of warning? 

> Warning for other uses is really only sensible before any code motion pass
> took place.

Can you move the warning pass before the first code motion pass? 

Or split it in two warning passes: warning for any uses (before code motion)
and warn for escapes and deref (after code motion)?

> Unfortunately the testsuite is full of cases expected to be diagnosed but
> are no longer with any change to the operation.

It is better to miss some warnings rather than warn too much because otherwise
users will use any means available to silence them (often -Wno-use-after-free)
and the useful warnings will be lost.

For comprehensive code analysis, users should look at the static analyzer.

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 54668
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54668&action=edit
patch

The diagnostic pass (pass_warn_access) is already split into two modes,
unfortunately the -Wuse-after-free part only runs on the late optimized IL
(for whatever reasons ...).

Enabling it only for the early passes regresses a few cases but also resolves
some XFAILs (and your testcase), so it might be the best thing to do.

I'm testing the attached.

[Bug c/109123] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 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:0a07bfad12530bca5dc5188057ad910198780dbc

commit r13-6707-g0a07bfad12530bca5dc5188057ad910198780dbc
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Mar 15 09:12:33 2023 +0100

    tree-optimization/109123 - run -Wuse-afer-free only early

    The following switches the -Wuse-after-free diagnostics from emitted
    during the late access warning passes to the early access warning
    passes to make sure we run before passes performing code motion run
    which are the source of a lot of false positives on use-after-free
    not involving memory operations.

    The patch also fixes an issue in c-c++-common/Wuse-after-free-6.c
    and causes the name of the unused pointer to appear in the diagnostic
    for extra cases in gcc.dg/Wuse-after-free-2.c

            PR tree-optimization/109123
            * gimple-ssa-warn-access.cc (pass_waccess::warn_invalid_pointer):
            Do not emit -Wuse-after-free late.
            (pass_waccess::check_call): Always check call pointer uses.

            * gcc.dg/Wuse-after-free-pr109123.c: New testcase.
            * gcc.dg/Wuse-after-free-2.c: Amend expected diagnostic with
            the name of the pointer.
            * c-c++-common/Wuse-after-free-6.c: Un-XFAIL case.

[Bug c/109123] [12 Regression] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |13.0
           Priority|P3                          |P2
            Summary|Bogus warning: pointer used |[12 Regression] Bogus
                   |after 'realloc'             |warning: pointer used after
                   |-Wuse-after-free with -O2   |'realloc' -Wuse-after-free
                   |                            |with -O2
   Target Milestone|---                         |12.3

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk.  It's a regression in GCC 12 with -Wall

[Bug tree-optimization/109123] [12 Regression] Bogus warning: pointer used after 'realloc' -Wuse-after-free with -O2

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.