[Bug middle-end/98989] New: missing -Wfree-nonheap-object freeing std::strings over 15 bytes long

[Bug middle-end/98989] New: missing -Wfree-nonheap-object freeing std::strings over 15 bytes long

[msebor at gcc dot gnu.org]

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

            Bug ID: 98989
           Summary: missing -Wfree-nonheap-object freeing std::strings
                    over 15 bytes long
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor at gcc dot gnu.org
  Target Milestone: ---

Given the test case below, -Wfree-nonheap-object detects the invalid attempt to
free the std::string buffer provided the string constant it's initialized with
is no more than 15 characters long (plus the terminating nul).  But the option
fails to diagnose the invalid call once the string length is 16 or more.

#include <string>

void f ()
{
  std::string str (STR);
  char *p = &str[0];
  free (p);
}

$ g++ -DSTR='"abcdefghijklmno"' -O2 -S -Wall t.C
t.C: In function ‘void f()’:
t.C:7:8: warning: ‘void free(void*)’ called on unallocated object ‘str’
[-Wfree-nonheap-object]
    7 |   free (p);
      |   ~~~~~^~~
t.C:5:15: note: declared here
    5 |   std::string str (STR);
      |               ^~~

[Bug middle-end/98989] missing -Wfree-nonheap-object freeing std::strings over 15 bytes long

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
The reason the invalid call isn't diagnosed is because the allocation call is
"hidden" behind the call to std::__cxx11::basic_string<char>::_M_create(), and
_M_create isn't annotated as an allocation function (with attribute malloc).

In this case, though, the optimized IL shows that besides free() the function
also calls operator delete() on the same pointer.  That's almost certainly
wrong regardless of the control flow and so the warning could trigger simply on
that basis.

annotating _M_create() shouldn't be necessary

;; Function f (_Z1fv, funcdef_no=1194, decl_uid=32383, cgraph_uid=317,
symbol_order=347)

Removing basic block 5
void f ()
{
  size_type __dnew;
  struct string str;
  char * _7;
  char * _9;
  long unsigned int _11;
  long unsigned int _12;
  char * _19;
  long unsigned int __dnew.6_20;
  long unsigned int __dnew.7_22;
  char * _23;
  char * _24;

  <bb 2> [local count: 1073741824]:
  MEM[(struct basic_string *)&str] ={v} {CLOBBER};
  MEM[(struct _Alloc_hider *)&str] ={v} {CLOBBER};
  MEM[(struct _Alloc_hider *)&str]._M_p = &str.D.24447._M_local_buf;
  __dnew = 16;
  _19 = std::__cxx11::basic_string<char>::_M_create (&str, &__dnew, 0);
  str._M_dataplus._M_p = _19;
  __dnew.6_20 = __dnew;
  str.D.24447._M_allocated_capacity = __dnew.6_20;
  __builtin_memcpy (_19, "abcdefghijklmnop", 16);
  __dnew.7_22 = __dnew;
  str._M_string_length = __dnew.7_22;
  _23 = str._M_dataplus._M_p;
  _24 = _23 + __dnew.7_22;
  MEM[(char_type &)_24] = 0;
  __dnew ={v} {CLOBBER};
  _7 = str._M_dataplus._M_p;
  free (_7);
  _9 = str._M_dataplus._M_p;
  if (&str.D.24447._M_local_buf != _9)
    goto <bb 3>; [53.47%]
  else
    goto <bb 4>; [46.53%]

  <bb 3> [local count: 574129753]:
  _11 = str.D.24447._M_allocated_capacity;
  _12 = _11 + 1;
  operator delete (_9, _12);

  <bb 4> [local count: 1073741824]:
  str ={v} {CLOBBER};
  str ={v} {CLOBBER};
  return;

}

[Bug middle-end/98989] missing -Wfree-nonheap-object freeing std::strings over 15 bytes long

[msebor at gcc dot gnu.org]

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

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
Not having to annotate _M_create is also helpful when the call isn't inlined. 
For example, neither of the following functions is diagnosed because in both
cases GCC  emits a call equivalent to:

  std::__cxx11::basic_string<char>::_M_construct.isra (&str, "abc", &MEM <const
char[4]> [(void *)"abc" + 3B]);

and from that there's no way for the warning to determine how the string buffer
was allocated.

#include <string>

void f ()
{
    std::string str ("abc");
    char *p = &str[0];
    __builtin_free (p);
}

void g ()
{
    std::string str ("def");
    char *p = &str[0];
    __builtin_free (p);
}

The IL for both f() and g() ends up looking alike (below) but in both cases the
calls to free() and operator delete() are evidently made with the same pointer.
 (Technically, the use of str._M_dataplus._M_p in the assignment right after
the call to free() will be grounds for issuing -Wuse-after-free once the
warning is implemented.)

void f ()
{
  struct string str;
  char * _7;
  char * _9;
  long unsigned int _11;
  long unsigned int _12;

  <bb 2> [local count: 1073741824]:
  MEM[(struct basic_string *)&str] ={v} {CLOBBER};
  MEM[(struct _Alloc_hider *)&str] ={v} {CLOBBER};
  MEM[(struct _Alloc_hider *)&str]._M_p = &str.D.24447._M_local_buf;
  std::__cxx11::basic_string<char>::_M_construct.isra (&str, "abc", &MEM <const
char[4]> [(void *)"abc" + 3B]);
  _7 = str._M_dataplus._M_p;
  __builtin_free (_7);
  _9 = str._M_dataplus._M_p;
  if (&str.D.24447._M_local_buf != _9)
    goto <bb 3>; [53.47%]
  else
    goto <bb 4>; [46.53%]

  <bb 3> [local count: 574129753]:
  _11 = str.D.24447._M_allocated_capacity;
  _12 = _11 + 1;
  operator delete (_9, _12);

  <bb 4> [local count: 1073741824]:
  str ={v} {CLOBBER};
  str ={v} {CLOBBER};
  return;

}

[Bug middle-end/98989] missing -Wfree-nonheap-object freeing std::strings over 15 bytes long

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=98992

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
Bug 98992 shows that associating a member deallocator function with an
allocator doesn't work in GCC 11.