[Bug middle-end/99714] New: warn about alloca/dealloc mismatches based on calls with same object in different functions

[Bug middle-end/99714] New: warn about alloca/dealloc mismatches based on calls with same object in different functions

[msebor at gcc dot gnu.org]

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

            Bug ID: 99714
           Summary: warn about alloca/dealloc mismatches based on calls
                    with same object in different functions
           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: ---

Deallocating an object in one function through a pointer that's assigned the
result of a mismatched allocation in another function is almost certainly a
bug.  Diagnosing such mismatched calls would help detect at compile time
problems like pr99687 that are currently only detected by dynamic analysis
tools.

#if __cplusplus

struct A {
  int *p;
  A (int);
  ~A ();
};

A::A (int n): p (new int[n]{ }) { }

A::~A ()
{
  delete p;            // missing -Wmismatched-new-delete
}
#else

struct A { int *p; };

void dealloc (void*);
__attribute__ ((malloc (dealloc))) void* alloc (int);

void init (struct A *p, int n) { p = alloc (n * sizeof *p); }
void fini (struct A *p)
{
  __builtin_free (p);  // missing -Wmismatched-dealloc
}

#endif

[Bug middle-end/99714] warn about alloca/dealloc mismatches based on calls with same object in different functions

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |documentation
             Blocks|                            |99715
           Severity|normal                      |enhancement


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99715
[Bug 99715] [meta-bug] bogus/missing Wmismatched-dealloc warnings

[Bug analyzer/99714] warn about alloca/dealloc mismatches based on calls with same object in different functions

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
The C case in comment #0 doesn't look like what I think you meant to write, and
the analyzer (correctly IMHO) complains about a leak:

<source>: In function 'init':
<source>:22:61: warning: leak of 'p' [CWE-401] [-Wanalyzer-malloc-leak]
   22 | void init (struct A *p, int n) { p = alloc (n * sizeof *p); }
      |                                                             ^
  'init': events 1-2
    |
    |   22 | void init (struct A *p, int n) { p = alloc (n * sizeof *p); }
    |      |                                      ^~~~~~~~~~~~~~~~~~~~~  ~
    |      |                                      |                      |
    |      |                                      |                      (2)
'p' leaks here; was allocated at (1)
    |      |                                      (1) allocated here
    |
Compiler returned: 0


If I fix it up thus:

struct A { int *p; };

void dealloc (void*);
__attribute__ ((malloc (dealloc))) void* alloc (int);

void init (struct A *a, int n) { a->p = alloc (n * sizeof *a->p); }
void fini (struct A *a)
{
  __builtin_free (a->p);  // missing -Wmismatched-dealloc
}

void test (void)
{
    struct A a;
    init (&a, 5);
    fini (&a);
}
/* to provide an example of usage, otherwise the analyzer doesn't "know" about
the connection between init and fini */

then -fanalyzer warns about it:

<source>: In function 'fini':
<source>:27:3: warning: 'a.p' should have been deallocated with 'dealloc' but
was deallocated with 'free' [CWE-762] [-Wanalyzer-mismatching-deallocation]
   27 |   __builtin_free (a->p);  // missing -Wmismatched-dealloc
      |   ^~~~~~~~~~~~~~~~~~~~~
  'test': events 1-2
    |
    |   30 | void test (void)
    |      |      ^~~~
    |      |      |
    |      |      (1) entry to 'test'
    |......
    |   33 |     init (&a, 5);
    |      |     ~~~~~~~~~~~~
    |      |     |
    |      |     (2) calling 'init' from 'test'
    |
    +--> 'init': events 3-4
           |
           |   24 | void init (struct A *a, int n) { a->p = alloc (n * sizeof
*a->p); }
           |      |      ^~~~                              
~~~~~~~~~~~~~~~~~~~~~~~~
           |      |      |                                  |
           |      |      |                                  (4) allocated here
(expects deallocation with 'dealloc')
           |      |      (3) entry to 'init'
           |
    <------+
    |
  'test': events 5-6
    |
    |   33 |     init (&a, 5);
    |      |     ^~~~~~~~~~~~
    |      |     |
    |      |     (5) returning to 'test' from 'init'
    |   34 |     fini (&a);
    |      |     ~~~~~~~~~
    |      |     |
    |      |     (6) calling 'fini' from 'test'
    |
    +--> 'fini': events 7-8
           |
           |   25 | void fini (struct A *a)
           |      |      ^~~~
           |      |      |
           |      |      (7) entry to 'fini'
           |   26 | {
           |   27 |   __builtin_free (a->p);  // missing -Wmismatched-dealloc
           |      |   ~~~~~~~~~~~~~~~~~~~~~
           |      |   |
           |      |   (8) deallocated with 'free' here; allocation at (4)
expects deallocation with 'dealloc'
           |
Compiler returned: 0

[Bug middle-end/99714] warn about alloca/dealloc mismatches based on calls with same object in different functions

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|dmalcolm at gcc dot gnu.org        |unassigned at gcc dot gnu.org
          Component|analyzer                    |middle-end
           Keywords|documentation               |diagnostic

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
The C test case (corrected below) is meant to allocate and deallocate the the
member pointer, just like the C++ test case.

I also raise this as an enhancement for the -Wmismatched-dealloc and
Wmismatched-new-delete warnings (hence Changing component back to middle-end),
although I think it would be worthwhile improvement to the analyzer as well. 
I'm not sure what the best way is to track enhancements to both kinds of
warnings.  Clone one to the other?

struct A { int *p; };

void dealloc (void*);
__attribute__ ((malloc (dealloc))) void* alloc (int);

void init (struct A *p, int n) { p->p = alloc (n * sizeof *p); }
void fini (struct A *p)
{
  __builtin_free (p->p);  // missing -Wmismatched-dealloc
}

#endif