Bug 94458 - -Wanalyzer-malloc-leak false positive when returning a heap-allocated struct by value holding a heap-allocated pointer
Summary: -Wanalyzer-malloc-leak false positive when returning a heap-allocated struct ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: analyzer (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: David Malcolm
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-02 15:46 UTC by Simon Marchi
Modified: 2021-08-05 22:59 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-05-08 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Marchi 2020-04-02 15:46:06 UTC
Variation of [1], where the returned struct is heap-allocated, rather than returned by value.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94378

Source:

#include <stdlib.h>

struct ret
{
  int **array;
};

struct ret *allocate_stuff(void)
{
        struct ret *ret;

        ret = calloc(1, sizeof (struct ret));
        if (!ret) {
            abort();
        }

        ret->array = calloc (10, sizeof(int *));
        if (!ret->array) {
            abort();
        }

        return ret;
}

Analyzer report:

$ /opt/gcc/git/bin/gcc a.c -g3 -O0 -fanalyzer -c -Wall -Werror
a.c: In function ‘allocate_stuff’:
a.c:18:10: error: leak of ‘<unknown>’ [CWE-401] [-Werror=analyzer-malloc-leak]
   18 |  if (!ret->array) {
      |       ~~~^~~~~~~
  ‘allocate_stuff’: events 1-7
    |
    |   13 |  if (!ret) {
    |      |     ^
    |      |     |
    |      |     (1) following ‘false’ branch (when ‘ret’ is non-NULL)...
    |......
    |   17 |  ret->array = calloc (10, sizeof(int *));
    |      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |               |
    |      |               (2) ...to here
    |      |               (3) allocated here
    |   18 |  if (!ret->array) {
    |      |     ~ ~~~~~~~~~~
    |      |     |    |
    |      |     |    (7) ‘<unknown>’ leaks here; was allocated at (3)
    |      |     (4) assuming ‘<unknown>’ is non-NULL
    |      |     (5) following ‘false’ branch...
    |......
    |   22 |  return ret;
    |      |         ~~~
    |      |         |
    |      |         (6) ...to here
    |
cc1: all warnings being treated as errors

$ /opt/gcc/git/bin/gcc --version                              
gcc (GCC) 10.0.1 20200401 (experimental)


The analyzer says the memory allocated at (3) is leaked, but it is in fact pointed by the returned struct.
Comment 1 David Malcolm 2020-05-08 15:09:39 UTC
Thanks for filing this bug; confirmed.
Comment 2 Paul Eggert 2020-06-19 00:55:53 UTC
I ran into what appear to be several instances of this bug when compiling GNU coreutils. My instances didn't necessarily involve two allocations; one sufficed. Here is a stripped-down version of the first instance:

void *malloc (unsigned long);

struct hash_table;
void *hash_insert (struct hash_table *, const void *);

struct di_ent
{
  unsigned long dev;
  struct hash_table *ino_set;
};
struct di_set
{
  struct hash_table *dev_map;
  struct di_ent *probe;
};

void
map_device (struct di_set *dis, unsigned long dev)
{
  struct di_ent *probe = dis->probe;
  if (probe)
    {
      if (probe->dev == dev)
        return;
    }
  else
    {
      probe = malloc (sizeof *probe);
      if (!probe)
        return;
      dis->probe = probe;
    }
  probe->dev = dev;
  struct di_ent *ent = hash_insert (dis->dev_map, probe);
  if (ent == probe)
    dis->probe = 0;
}



in the file t3.i, and here is the incorrect output when I compiled with 'gcc -fanalyzer -S t3.i':

In function 'map_device':
t3.i:36:16: warning: leak of 'probe' [CWE-401] [-Wanalyzer-malloc-leak]
   36 |     dis->probe = 0;
      |     ~~~~~~~~~~~^~~
  'map_device': events 1-9
    |
    |   21 |   if (probe)
    |      |      ^
    |      |      |
    |      |      (1) following 'false' branch (when 'probe' is NULL)...
    |......
    |   28 |       probe = malloc (sizeof *probe);
    |      |               ~~~~~~~~~~~~~~~~~~~~~~
    |      |               |
    |      |               (2) ...to here
    |      |               (3) allocated here
    |   29 |       if (!probe)
    |      |          ~
    |      |          |
    |      |          (4) assuming 'probe' is non-NULL
    |      |          (5) following 'false' branch (when 'probe' is non-NULL)...
    |   30 |  return;
    |   31 |       dis->probe = probe;
    |      |       ~~~~~~~~~~~~~~~~~~
    |      |                  |
    |      |                  (6) ...to here
    |......
    |   35 |   if (ent == probe)
    |      |      ~
    |      |      |
    |      |      (7) following 'true' branch (when 'ent == probe')...
    |   36 |     dis->probe = 0;
    |      |     ~~~~~~~~~~~~~~
    |      |                |
    |      |                (8) ...to here
    |      |                (9) 'probe' leaks here; was allocated at (3)
    |
Comment 3 David Malcolm 2020-08-13 20:30:09 UTC
The leak false positive should be fixed by g:808f4dfeb3a95f50f15e71148e5c1067f90a126d (for GCC 11).  Marking this as fixed.