[Bug middle-end/106776] New: Unexpected use-after-free warning

[Bug middle-end/106776] New: Unexpected use-after-free warning

[drfiemost at email dot it]

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

            Bug ID: 106776
           Summary: Unexpected use-after-free warning
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: drfiemost at email dot it
  Target Milestone: ---

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

When compiling the attached program with the -O2 and -Wuse-after-free flags I
get the following warning, which seems bogus as the pointer is not actually
dereferenced in case it is deleted:

$ g++ -O2 -Wuse-after-free test_ref.cpp
In member function 'unsigned int counter::decrease()',
    inlined from 'matrix_t::~matrix_t()' at test_ref.cpp:32:38,
    inlined from 'matrix_t* cache(int, const char*)' at test_ref.cpp:53:1:
test_ref.cpp:14:40: warning: pointer used after 'void operator delete(void*,
std::size_t)' [-Wuse-after-free]
   14 |     unsigned int decrease() { return --c; }
      |                                        ^
In destructor 'matrix_t::~matrix_t()',
    inlined from 'std::pair<const int, matrix_t>::~pair()' at
C:/msys64/mingw64/include/c++/12.2.0/bits/stl_pair.h:185:12,
    inlined from 'matrix_t* cache(int, const char*)' at test_ref.cpp:52:40:
test_ref.cpp:32:56: note: call to 'void operator delete(void*, std::size_t)'
here
   32 |     ~matrix_t() { if (count->decrease() == 0) { delete count; delete
data; } }
      |                                                        ^~~~~


The IL dump confirms that the pointer _1 is either accessed <bb 3> or deleted
<bb 5>:

void matrix<short int>::~matrix (struct matrix * const this)
{
  struct counter * _1;
  short int * _2;
  unsigned int _10;
  unsigned int _11;

  <bb 2> [local count: 1073741824]:
  _1 = this_5(D)->count;
  _10 = _1->c;
  _11 = _10 + 4294967295;
  if (_11 == 0)
    goto <bb 5>; [33.00%]
  else
    goto <bb 3>; [67.00%]

  <bb 3> [local count: 719407024]:
  _1->c = _11;

  <bb 4> [local count: 884279007]:
  goto <bb 7>; [100.00%]

  <bb 5> [local count: 354334800]:
  operator delete (_1, 4);
  _2 = this_5(D)->data;
  if (_2 != 0B)
    goto <bb 6>; [53.47%]
  else
    goto <bb 8>; [46.53%]

  <bb 8> [local count: 164871983]:
  goto <bb 4>; [100.00%]

  <bb 6> [local count: 189462817]:
  operator delete [] (_2); [tail call]

  <bb 7> [local count: 1073741824]:
  return;

}

The warning doesn't appear when using the -O1 flag or if the variable is
decalared volatile.
If I'm not missing anything obvious the warning should not be issued at all.

[Bug middle-end/106776] Unexpected use-after-free warning

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

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

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
Reproduced with the following reduced test case:

$ cat pr106776.C && g++ -O2 -S -Wall pr106776.C
#include <map>

struct matrix_t
{
  int* count;

  matrix_t() :
    count(new int(1)) {}

  matrix_t(const matrix_t& p) :
    count(p.count) { ++*count; }

  ~matrix_t() { if (--*count == 0) { delete count; } }
};

typedef std::map<int, matrix_t> cache_t;

cache_t CACHE;

matrix_t* cache(cache_t::iterator lb)
{
  matrix_t wftable;

  return &CACHE.insert(lb, cache_t::value_type(1, wftable))->second;
}
In destructor ‘matrix_t::~matrix_t()’,
    inlined from ‘matrix_t* cache(std::map<int, matrix_t>::iterator)’ at
pr106776.C:25:1:
pr106776.C:13:23: warning: pointer used after ‘void operator delete(void*,
std::size_t)’ [-Wuse-after-free]
   13 |   ~matrix_t() { if (--*count == 0) { delete count; } }
      |                       ^~~~~~
In destructor ‘matrix_t::~matrix_t()’,
    inlined from ‘std::pair<const int, matrix_t>::~pair()’ at
/build/gcc-master/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_pair.h:187:12,
    inlined from ‘matrix_t* cache(std::map<int, matrix_t>::iterator)’ at
pr106776.C:24:37:
pr106776.C:13:45: note: call to ‘void operator delete(void*, std::size_t)’ here
   13 |   ~matrix_t() { if (--*count == 0) { delete count; } }
      |                                             ^~~~~

The IL in shows the reason for the warning in bb 8 in struct matrix_t * cache
(struct iterator lb), seen in the output of -fdump-tree-waccess3:

  ...
  <bb 8> [local count: 335388518]:
  operator delete (_3, 4);      <<< _3 deleted
  pretmp_50 = MEM[(int *)_3];   <<< _3 dereferenced

[Bug middle-end/106776] Unexpected use-after-free warning

[drfiemost at email dot it]

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

--- Comment #2 from Leandro Nini <drfiemost at email dot it> ---
Oh, now I see it, it wasn't that obvious in the first test. But why is the
compiler allowed to postpone the store after deleting the pointer? Is there
some undefined behavior involved here or what?

[Bug middle-end/106776] Unexpected use-after-free warning

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Before PRE
 
[/opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/bits/stl_tree.h:1061:28]
__position ={v} {CLOBBER(eol)};
  [/app/example.cpp:25:62] _4 = [/app/example.cpp:25:10]
&[/app/example.cpp:25:62] [/app/example.cpp:25:62] MEM[(struct pair *)SR.163_98
+ 32B].second;
  [/app/example.cpp:14:23] _57 = MEM[(int *)_3];
  [/app/example.cpp:14:21] _58 = _57 + -1;
  [/app/example.cpp:14:30] MEM[(int *)_3] = _58;
  [/app/example.cpp:14:17] if (_58 == 0)
    goto <bb 9>; [33.00%]
  else
    goto <bb 10>; [67.00%]

  <bb 9> [local count: 335388518]:
  MEM[(int *)_3] ={v} {CLOBBER};
  [/app/example.cpp:14:45] operator delete (_3, 4);

  <bb 10> [local count: 1016328849]:
  [/opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/bits/stl_pair.h:185:12]
D.26938 ={v} {CLOBBER};
  [/app/example.cpp:25:37] D.26938 ={v} {CLOBBER(eol)};
  [/app/example.cpp:14:23] _54 = MEM[(int *)_3];

So it is not PRE causing the issue ... Look further back.

[Bug middle-end/106776] Unexpected use-after-free warning

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Here is a reduced testcase without any std::map or otherwise:
```
struct matrix_t
{
  int* count;

  matrix_t() :
    count(new int(1)) {}

  matrix_t(const matrix_t& p) :
    count(p.count) { ++*count; }

  ~matrix_t() { if (--*count == 0) { delete count;  } }
};

void f();
void cache1(void)
{
  matrix_t wftable;
  matrix_t wftable1(wftable);
  f();
}

```
basically what is going on is GCC cannot figure out the load from the count in
the first deconstructor will be greater than 1. GCC didn't inline the
deconstructor on the unwinding from exception which is causing GCC to think
count escapes and is read by f; the original case there is similar non-inlining
which is causing a similar issue.

The warning is a false positive but it is hard to prove unless you understand
the IR and such.

[Bug middle-end/106776] Unexpected use-after-free warning

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Leandro Nini from comment #2)
> Oh, now I see it, it wasn't that obvious in the first test. But why is the
> compiler allowed to postpone the store after deleting the pointer? Is there
> some undefined behavior involved here or what?

So the branch where the read after delete happens just happens to be dead code
but GCC cannot prove it statically due to what I mentioned in comment #4.

Oh if you compile the testcase in comment #4 with -fno-exceptions you don't get
the warning. Or marking f as noexcept. I have not looked into what could be
done for either other testcase though.

[Bug middle-end/106776] Unexpected use-after-free warning

[drfiemost at yahoo dot com]

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

--- Comment #6 from Leandro Nini <drfiemost at yahoo dot com> ---
Can't reproduce anymore with gcc 13.1.0
Still there in gcc 12.3.0