[Bug middle-end/105204] New: -Wuse-after-free=1 inconsistency with conditional free

[Bug middle-end/105204] New: -Wuse-after-free=1 inconsistency with conditional free

[piotr.grabowski at scylladb dot com]

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

            Bug ID: 105204
           Summary: -Wuse-after-free=1 inconsistency with conditional free
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: piotr.grabowski at scylladb dot com
  Target Milestone: ---

Created attachment 52772
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52772&action=edit
two examples of conditional free

Below, I added two examples of conditional free(), which cause inconsistent
behavior of -Wuse-after-free=1. 

In the first case, GCC 12 does not issue -Wuse-after-free=1 warning, but in the
second similar example, the warning is triggered.


// Compile with: g++ -Wuse-after-free=1 -O2 -c example-inconsistency.cpp
void example1(int* ptr, bool condition) {
    if (condition) {
        free(ptr);
    }
    ++*ptr; // No -Wuse-after-free=1 warning
}

void example2(int* ptr) {
    if (*ptr == 1234) {
        free(ptr);
    }
    ++*ptr; // -Wuse-after-free=1 warning issued
}


Compiled with: g++ (GCC) 12.0.1 20220404 (experimental)

We hit that second case in our production code in our implementation of shared
pointer in Seastar
(https://github.com/scylladb/seastar/blob/05cdfc2d30c553ec73b5cdbfb6c4318c232b3a6d/include/seastar/core/shared_ptr.hh#L255).
Below is a simplified version of it, which triggers -Wuse-after-free=1:


// Compile with: g++ -Wuse-after-free=1 -O2 -c example-shared-ptr.cpp
struct shared_ptr {
    size_t* ref_count;
public:
    shared_ptr(const shared_ptr& other) : ref_count(other.ref_count) {
        (*ref_count)++;
    }
    ~shared_ptr() {
        if (--(*ref_count) == 0) {
            free(ref_count);
        }
    }
};

void example3(shared_ptr& sp) {
    shared_ptr sp2(sp);
    shared_ptr sp3(sp);
    // -Wuse-after-free=1 is issued
}


Is this the expected behavior of -Wuse-after-free=1 and we should work around
it in our code?

[Bug middle-end/105204] -Wuse-after-free=1 inconsistency with conditional free

[piotr.grabowski at scylladb dot com]

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

--- Comment #1 from Piotr Grabowski <piotr.grabowski at scylladb dot com> ---
Created attachment 52773
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52773&action=edit
example shared pointer implementation

[Bug middle-end/105204] -Wuse-after-free=1 inconsistency with conditional free

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
The diagnostic is confused about a PRE where we transform

  <bb 2> [local count: 1073741824]:
  _8 = MEM[(const struct shared_ptr &)sp_2(D)].ref_count;
  _9 = *_8;
  _20 = _9 + 1;
  *_8 = _20;
  if (_20 == 0)
    goto <bb 3>; [33.00%]
  else
    goto <bb 4>; [67.00%]

  <bb 3> [local count: 354334800]:
  free (_8);

  <bb 4> [local count: 1073741824]:
  _16 = *_8;
  _17 = _16 + 18446744073709551615;
  *_8 = _17;

to

  <bb 2> [local count: 1073741824]:
  _8 = MEM[(const struct shared_ptr &)sp_2(D)].ref_count;
  _9 = *_8;
  _20 = _9 + 1;
  *_8 = _20;
  if (_20 == 0)
    goto <bb 3>; [33.00%]
  else
    goto <bb 4>; [67.00%]

  <bb 3> [local count: 354334800]:
  free (_8);
  pretmp_5 = *_8;
  _25 = pretmp_5 + 18446744073709551615;

  <bb 4> [local count: 1073741824]:
  # prephitmp_10 = PHI <_9(2), _25(3)>
  *_8 = prephitmp_10;

I think this is both a missed diagnostic without PRE (the free () falls
through to a dereference to the freed pointer) and a missed optimization
where the

  _8 = MEM[(const struct shared_ptr &)sp_2(D)].ref_count;
  _9 = *_8;
  _20 = _9 + 1;
  *_8 = _20;
  if (_20 == 0)

test is not constant folded.  Note this is because your code is not
resilent against overflow of ref_count and also not resilent against
a ref_count of zero incoming into the copy ctor.  The latter is what
provokes the diagnostic in the end and also causes the missed
optimization.

changing the CTOR to sth like

    shared_ptr(const shared_ptr& other) : ref_count(other.ref_count) {
        if (*ref_count == 0)
          __builtin_unreachable ();
        (*ref_count)++;
    }

avoids the diagnostic and optimizes the example3 to an empty function
(since we never will need to free the reference count).

So what this bug shows is a missed diagnostic without PRE, but otherwise
it works as expected.

[Bug middle-end/105204] -Wuse-after-free=1 inconsistency with conditional free

[avi at scylladb dot com]

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

--- Comment #3 from Avi Kivity <avi at scylladb dot com> ---
Does one have to declare all preconditions? I happen to know that shared_ptr
with a reference count of zero is impossible, and I can understand the compiler
not being able to infer it, but just because it's unable to infer it doesn't
mean my code is wrong.

Maybe the warning should be renamed -Wmaybe-use-after-free to indicate the
compiler just thinks there may be a bug, but also acknowledges it doesn't know
everything.