public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug analyzer/113963] New: analyzer-null-dereference, analyzer-malloc-leak false alarms in Gnulib savedir.c
@ 2024-02-17  5:30 eggert at cs dot ucla.edu
  2024-02-17  5:31 ` [Bug analyzer/113963] " eggert at cs dot ucla.edu
  0 siblings, 1 reply; 2+ messages in thread
From: eggert at cs dot ucla.edu @ 2024-02-17  5:30 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 113963
           Summary: analyzer-null-dereference, analyzer-malloc-leak false
                    alarms in Gnulib savedir.c
           Product: gcc
           Version: 13.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: eggert at cs dot ucla.edu
  Target Milestone: ---

Created attachment 57441
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57441&action=edit
test program with line number directives

I ran into this problem when compiling programs using Gnulib's lib/savedir.c
code.

This is gcc (GCC) 13.2.1 20231205 (Red Hat 13.2.1-6) on x86-64. Compile the
attached programs with:

gcc -O2 -fanalyzer -S savedir.i
gcc -O2 -fanalyzer -S savedis.i

Two problems. First, although savedis.i is merely a copy of savedir.i with line
number directives removed, the second command outputs an extra
-Wanalyzer-null-argument warning that the first command does not. Surely the
presence of line number directives should not affect which warnings are issued.

Second and more important, all the warnings are false positives. The pointers
can't possibly be null when the sizes are nonzero, and there is no memory leak.

The source files savedir.i and savedis.i are attached, compressed.

Here are the incorrect warnings that I get:

$ gcc -O2 -fanalyzer -S savedir.i
savedir.c: In function ‘streamsavedir’:
savedir.c:135:42: warning: dereference of NULL ‘entries’ [CWE-476]
[-Wanalyzer-null-dereference]
  135 |               entries[entries_used].name = used;
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
  ‘streamsavedir’: event 1
    |
    |savedir.c:106:6:
    |  106 |   if (dirp == NULL)
    |      |      ^
    |      |      |
    |      |      (1) following ‘false’ branch (when ‘dirp’ is non-NULL)...
    |
  ‘streamsavedir’: event 2
    |
    |cc1:
    | (2): ...to here
    |
  ‘streamsavedir’: events 3-5
    |
    |savedir.c:116:10:
    |  116 |       if (! dp)
    |      |          ^
    |      |          |
    |      |          (3) following ‘false’ branch (when ‘dp’ is non-NULL)...
    |......
    |  121 |       entry = dp->d_name;
    |      |       ~~~~~~~~~~~~~~~~~~
    |      |             |
    |      |             (4) ...to here
    |  122 |       if (entry[entry[0] != '.' ? 0 : entry[1] != '.' ? 1 : 2] !=
'\0')
    |      |          ~
    |      |          |
    |      |          (5) following ‘true’ branch...
    |
  ‘streamsavedir’: event 6
    |
    |savedir.c:124:30:
    |  124 |           idx_t entry_size = _D_EXACT_NAMLEN (dp) + 1;
    |
  ‘streamsavedir’: event 7
    |
    |savedir.c:125:14:
    |  125 |           if (allocated - used <= entry_size)
    |      |              ^
    |      |              |
    |      |              (7) following ‘true’ branch...
    |
  ‘streamsavedir’: event 8
    |
    |  126 |             name_space = xpalloc (name_space, &allocated,
    |
  ‘streamsavedir’: events 9-14
    |
    |savedir.c:130:14:
    |  130 |           if (cmp)
    |      |              ^
    |      |              |
    |      |              (9) following ‘true’ branch (when ‘cmp’ is
non-NULL)...
    |  131 |             {
    |  132 |               if (entries_allocated == entries_used)
    |      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                  |                  |
    |      |                  |                  (10) ...to here
    |      |                  (11) following ‘false’ branch...
    |......
    |  135 |               entries[entries_used].name = used;
    |      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                      |                   |
    |      |                      (12) ...to here     (14) dereference of NULL
‘entries + (long unsigned int)entries_used * 16’
    |      |                      (13) ‘entries’ is NULL
    |
savedir.c:169:3: warning: leak of ‘name_space’ [CWE-401]
[-Wanalyzer-malloc-leak]
  169 |   free (entries);
      |   ^~~~~~~~~~~~~~
  ‘savedir’: events 1-2
    |
    |  179 | savedir (char const *dir, enum savedir_option option)
    |      | ^~~~~~~
    |      | |
    |      | (1) entry to ‘savedir’
    |......
    |  182 |   if (! dirp)
    |      |      ~
    |      |      |
    |      |      (2) following ‘false’ branch (when ‘dirp’ is non-NULL)...
    |
  ‘savedir’: events 3-5
    |
    |savedir.c:186:26:
    |  186 |       char *name_space = streamsavedir (dirp, option);
    |      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                          |
    |      |                          (3) ...to here
    |      |                          (4) allocated here
    |      |                          (5) calling ‘streamsavedir’ from
‘savedir’
    |
    +--> ‘streamsavedir’: event 6
           |
           |savedir.c:96:1:
           |   96 | streamsavedir (DIR *dirp, enum savedir_option option)
           |      | ^~~~~~~~~~~~~
           |      | |
           |      | (6) entry to ‘streamsavedir’
           |
         ‘streamsavedir’: event 7
           |
           |savedir.c:106:6:
           |  106 |   if (dirp == NULL)
           |      |      ^
           |      |      |
           |      |      (7) following ‘false’ branch (when ‘dirp’ is
non-NULL)...
           |
         ‘streamsavedir’: event 8
           |
           |cc1:
           | (8): ...to here
           |
         ‘streamsavedir’: event 9
           |
           |savedir.c:145:6:
           |  145 |   if (errno != 0)
           |      |      ^
           |      |      |
           |      |      (9) following ‘true’ branch...
           |
         ‘streamsavedir’: event 10
           |
           |savedir.c:147:7:
           |  147 |       free (name_space);
           |      |       ^~~~~~~~~~~~~~~~~
           |      |       |
           |      |       (10) ...to here
           |
         ‘streamsavedir’: event 11
           |
           |savedir.c:169:3:
           |  169 |   free (entries);
           |      |   ^~~~~~~~~~~~~~
           |      |   |
           |      |   (11) ‘name_space’ leaks here; was allocated at (4)
           |
$ gcc -O2 -fanalyzer -S savedis.i
In function ‘memcpy’,
    inlined from ‘streamsavedir’ at savedis.i:2993:11:
savedis.i:2502:10: warning: use of NULL ‘name_space’ where non-null expected
[CWE-476] [-Wanalyzer-null-argument]
 2502 |   return __builtin___memcpy_chk (__dest, __src, __len,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2503 |      __builtin_object_size (__dest, 0));
      |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘streamsavedir’: event 1
    |
    | 2958 |   if (dirp ==
    |      |      ^
    |      |      |
    |      |      (1) following ‘false’ branch (when ‘dirp’ is non-NULL)...
    |
  ‘streamsavedir’: event 2
    |
    |cc1:
    | (2): ...to here
    |
  ‘streamsavedir’: events 3-10
    |
    | 2974 |       if (! dp)
    |      |          ^
    |      |          |
    |      |          (3) following ‘false’ branch (when ‘dp’ is non-NULL)...
    |......
    | 2979 |       entry = dp->d_name;
    |      |       ~~~~~~~~~~~~~~~~~~
    |      |             |
    |      |             (4) ...to here
    | 2980 |       if (entry[entry[0] != '.' ? 0 : entry[1] != '.' ? 1 : 2] !=
'\0')
    |      |          ~
    |      |          |
    |      |          (5) following ‘true’ branch...
    |......
    | 2983 |                             (strlen ((
    |      |                             ~~~~~~~~~~
    |      |                              |
    |      |                              (6) ...to here
    | 2984 |                             dp
    |      |                             ~~
    | 2985 |                             )->d_name))
    |      |                             ~~~~~~~~~~~
    | 2986 |                                                  + 1;
    | 2987 |           if (allocated - used <= entry_size)
    |      |              ~
    |      |              |
    |      |              (7) following ‘false’ branch...
    |......
    | 2993 |           memcpy (name_space + used, entry, entry_size);
    |      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |           |                  |
    |      |           (8) ...to here     (9) ‘name_space’ is NULL
    |      |           (10) inlined call to ‘memcpy’ from ‘streamsavedir’
    |
    +--> ‘memcpy’: event 11
           |
           | 2502 |   return __builtin___memcpy_chk (__dest, __src, __len,
           |      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |          |
           |      |          (11) argument 1 (‘name_space + (sizetype)used’)
NULL where non-null expected
           | 2503 |      __builtin_object_size (__dest, 0));
           |      |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |
<built-in>: In function ‘streamsavedir’:
<built-in>: note: argument 1 of ‘__builtin___memcpy_chk’ must be non-null
savedis.i:2999:42: warning: dereference of NULL ‘entries’ [CWE-476]
[-Wanalyzer-null-dereference]
 2999 |               entries[entries_used].name = used;
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
  ‘streamsavedir’: event 1
    |
    | 2958 |   if (dirp ==
    |      |      ^
    |      |      |
    |      |      (1) following ‘false’ branch (when ‘dirp’ is non-NULL)...
    |
  ‘streamsavedir’: event 2
    |
    |cc1:
    | (2): ...to here
    |
  ‘streamsavedir’: events 3-14
    |
    | 2974 |       if (! dp)
    |      |          ^
    |      |          |
    |      |          (3) following ‘false’ branch (when ‘dp’ is non-NULL)...
    |......
    | 2979 |       entry = dp->d_name;
    |      |       ~~~~~~~~~~~~~~~~~~
    |      |             |
    |      |             (4) ...to here
    | 2980 |       if (entry[entry[0] != '.' ? 0 : entry[1] != '.' ? 1 : 2] !=
'\0')
    |      |          ~
    |      |          |
    |      |          (5) following ‘true’ branch...
    |......
    | 2983 |                             (strlen ((
    |      |                             ~~~~~~~~~~
    |      |                              |
    |      |                              (6) ...to here
    | 2984 |                             dp
    |      |                             ~~
    | 2985 |                             )->d_name))
    |      |                             ~~~~~~~~~~~
    | 2986 |                                                  + 1;
    | 2987 |           if (allocated - used <= entry_size)
    |      |              ~
    |      |              |
    |      |              (7) following ‘true’ branch...
    | 2988 |             name_space = xpalloc (name_space, &allocated,
    |      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                          |
    |      |                          (8) ...to here
    | 2989 |                                   entry_size - (allocated - used),
    |      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    | 2990 |
    |      |
    | 2991 |                                  (9223372036854775807L)
    |      |                                  ~~~~~~~~~~~~~~~~~~~~~~
    | 2992 |                                          - 1, sizeof *name_space);
    |      |                                          ~~~~~~~~~~~~~~~~~~~~~~~~
    | 2993 |           memcpy (name_space + used, entry, entry_size);
    | 2994 |           if (cmp)
    |      |              ~
    |      |              |
    |      |              (9) following ‘true’ branch (when ‘cmp’ is
non-NULL)...
    | 2995 |             {
    | 2996 |               if (entries_allocated == entries_used)
    |      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                  |                  |
    |      |                  |                  (10) ...to here
    |      |                  (11) following ‘false’ branch...
    |......
    | 2999 |               entries[entries_used].name = used;
    |      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                      |                   |
    |      |                      (12) ...to here     (14) dereference of NULL
‘entries + (long unsigned int)entries_used * 16’
    |      |                      (13) ‘entries’ is NULL
    |
savedis.i:3037:3: warning: leak of ‘name_space’ [CWE-401]
[-Wanalyzer-malloc-leak]
 3037 |   free (entries);
      |   ^~~~~~~~~~~~~~
  ‘savedir’: events 1-5
    |
    | 3047 | savedir (char const *dir, enum savedir_option option)
    |      | ^~~~~~~
    |      | |
    |      | (1) entry to ‘savedir’
    |......
    | 3050 |   if (! dirp)
    |      |      ~
    |      |      |
    |      |      (2) following ‘false’ branch (when ‘dirp’ is non-NULL)...
    |......
    | 3056 |       char *name_space = streamsavedir (dirp, option);
    |      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                          |
    |      |                          (3) ...to here
    |      |                          (4) allocated here
    |      |                          (5) calling ‘streamsavedir’ from
‘savedir’
    |
    +--> ‘streamsavedir’: events 6-7
           |
           | 2944 | streamsavedir (DIR *dirp, enum savedir_option option)
           |      | ^~~~~~~~~~~~~
           |      | |
           |      | (6) entry to ‘streamsavedir’
           |......
           | 2958 |   if (dirp ==
           |      |      ~
           |      |      |
           |      |      (7) following ‘false’ branch (when ‘dirp’ is
non-NULL)...
           |
         ‘streamsavedir’: event 8
           |
           |cc1:
           | (8): ...to here
           |
         ‘streamsavedir’: events 9-11
           |
           | 3009 |   if (
           |      |      ^
           |      |      |
           |      |      (9) following ‘true’ branch...
           |......
           | 3013 |       free (name_space);
           |      |       ~~~~~~~~~~~~~~~~~
           |      |       |
           |      |       (10) ...to here
           |......
           | 3037 |   free (entries);
           |      |   ~~~~~~~~~~~~~~
           |      |   |
           |      |   (11) ‘name_space’ leaks here; was allocated at (4)
           |

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [Bug analyzer/113963] analyzer-null-dereference, analyzer-malloc-leak false alarms in Gnulib savedir.c
  2024-02-17  5:30 [Bug analyzer/113963] New: analyzer-null-dereference, analyzer-malloc-leak false alarms in Gnulib savedir.c eggert at cs dot ucla.edu
@ 2024-02-17  5:31 ` eggert at cs dot ucla.edu
  0 siblings, 0 replies; 2+ messages in thread
From: eggert at cs dot ucla.edu @ 2024-02-17  5:31 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #1 from Paul Eggert <eggert at cs dot ucla.edu> ---
Created attachment 57442
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57442&action=edit
test program without line number directives (also compressed)

This is the same program as savedir.i, except without line number directives.
Like the other test program, it's compressed with gzip.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-02-17  5:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-17  5:30 [Bug analyzer/113963] New: analyzer-null-dereference, analyzer-malloc-leak false alarms in Gnulib savedir.c eggert at cs dot ucla.edu
2024-02-17  5:31 ` [Bug analyzer/113963] " eggert at cs dot ucla.edu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).