[Bug analyzer/103032] New: false positive diagnostic from -fanalyzer about double-free

[Bug analyzer/103032] New: false positive diagnostic from -fanalyzer about double-free

[rhialto at falu dot nl]

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

            Bug ID: 103032
           Summary: false positive diagnostic from -fanalyzer about
                    double-free
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: rhialto at falu dot nl
  Target Milestone: ---

Created attachment 51719
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51719&action=edit
the preprocessed source file

gcc 10.3.0's -fanalyze produces diagnostics about double-frees that look
incorrect to me. Maybe they are correct but if so, the message doesn't
convince me at all.

Instead the message makes me suspect that gcc thinks the loop variable i
doesn't change through the loop. And if it doesn't do that, this makes
me doubt if the generated code is even correct.

Expected behaviour: either no diagnostic, or one that convinces me there
is an actual problem with the program.

Command line invocation:

/usr/pkg/gcc10/bin/gcc --v std=gnu99 -Wall -Wshadow -Wextra -pedantic
-Woverflow -Wstrict-overflow -ggdb -fanalyzer -O3 -DDEFAULT_OBJECTFORMAT_RT11=0
-save-temps -c -o dumpobj.o dumpobj.c


Script started on Mon Nov  1 21:43:37 2021
$ /usr/pkg/gcc10/bin/gcc -v -std=gnu99 -Wall -Wshadow -Wextra -pedantic
-Woverflow -Wstrict-overflow -ggdb -fanalyzer -O3 -DDEFAULT_OBJECTFORMAT_RT11=0
-save-temps -c -o dumpobj.o dumpobj.c
[?2004lUsing built-in specs.
COLLECT_GCC=/usr/pkg/gcc10/bin/gcc
Target: x86_64--netbsd
Configured with: ../gcc-10.3.0/configure --disable-libstdcxx-pch --enable-nls
--with-libiconv-prefix=/usr --enable-__cxa_atexit
--with-gxx-include-dir=/usr/pkg/gcc10/include/c++/ --disable-libssp
--enable-languages='c obj-c++ objc fortran c++' --enable-shared
--enable-long-long --with-local-prefix=/usr/pkg/gcc10 --enable-threads=posix
--with-boot-ldflags='-static-libstdc++ -static-libgcc
-Wl,-R/usr/pkg/gcc8/lib/gcc/x86_64--netbsd/8.4.0 -Wl,-R/usr/pkg/lib  '
--with-system-zlib --without-zstd --with-arch=nocona --with-tune=nocona
--with-fpmath=sse --with-gnu-ld --with-ld=/usr/bin/ld --with-gnu-as
--with-as=/usr/bin/as --prefix=/usr/pkg/gcc10 --build=x86_64--netbsd
--host=x86_64--netbsd --infodir=/usr/pkg/gcc10/info --mandir=/usr/pkg/gcc10/man
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 10.3.0 (GCC) 
COLLECT_GCC_OPTIONS='-v' '-std=gnu99' '-Wall' '-Wshadow' '-Wextra' '-Wpedantic'
'-Woverflow' '-Wstrict-overflow' '-ggdb' '-fanalyzer' '-O3' '-D'
'DEFAULT_OBJECTFORMAT_RT11=0' '-save-temps' '-c' '-o' 'dumpobj.o'
'-mtune=nocona' '-march=nocona'
 /usr/pkg/gcc10/libexec/gcc/x86_64--netbsd/10.3.0/cc1 -E -quiet -v -D
DEFAULT_OBJECTFORMAT_RT11=0 dumpobj.c -mtune=nocona -march=nocona -std=gnu99
-Wall -Wshadow -Wextra -Wpedantic -Woverflow -Wstrict-overflow -fanalyzer -ggdb
-fworking-directory -O3 -fpch-preprocess -o dumpobj.i
ignoring nonexistent directory
"/usr/pkg/gcc10/lib/gcc/x86_64--netbsd/10.3.0/../../../../x86_64--netbsd/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/pkg/gcc10/lib/gcc/x86_64--netbsd/10.3.0/include
 /usr/pkg/gcc10/include
 /usr/pkg/gcc10/lib/gcc/x86_64--netbsd/10.3.0/include-fixed
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-std=gnu99' '-Wall' '-Wshadow' '-Wextra' '-Wpedantic'
'-Woverflow' '-Wstrict-overflow' '-ggdb' '-fanalyzer' '-O3' '-D'
'DEFAULT_OBJECTFORMAT_RT11=0' '-save-temps' '-c' '-o' 'dumpobj.o'
'-mtune=nocona' '-march=nocona'
 /usr/pkg/gcc10/libexec/gcc/x86_64--netbsd/10.3.0/cc1 -fpreprocessed dumpobj.i
-quiet -dumpbase dumpobj.c -mtune=nocona -march=nocona -auxbase-strip dumpobj.o
-ggdb -O3 -Wall -Wshadow -Wextra -Wpedantic -Woverflow -Wstrict-overflow
-std=gnu99 -version -fanalyzer -o dumpobj.s
GNU C99 (GCC) version 10.3.0 (x86_64--netbsd)
        compiled by GNU C version 10.3.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.1.0, isl version isl-0.16.1-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C99 (GCC) version 10.3.0 (x86_64--netbsd)
        compiled by GNU C version 10.3.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.1.0, isl version isl-0.16.1-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: edfbc059e6267e32b59f120c0ada0d16
[01m[Kdumpobj.c:[m[K In function '[01m[Kgot_endgsd.part.0[m[K':
[01m[Kdumpobj.c:428:9:[m[K [01;35m[Kwarning:
[m[Kdouble-'[01m[Kfree[m[K' of '[01m[K<unknown>[m[K'
[[01;35m[K]8;;https://cwe.mitre.org/data/definitions/415.htmlCWE-415]8;;[m[K]
[[01;35m[K]8;;https://gcc.gnu.org/onlinedocs/gcc/Static-Analyzer-Options.html#index-Wanalyzer-double-free-Wanalyzer-double-free]8;;[m[K]
  428 |         [01;35m[Kfree(all_gsds[i])[m[K;
      |         [01;35m[K^~~~~~~~~~~~~~~~~[m[K
  '[01m[Kgot_endgsd[m[K': events 1-3
    [01;36m[K|[m[K
    [01;36m[K|[m[K[01m[Kdumpobj.c:409:6:[m[K
    [01;36m[K|[m[K  409 | void [01;36m[Kgot_endgsd[m[K(
    [01;36m[K|[m[K      |      [01;36m[K^~~~~~~~~~[m[K
    [01;36m[K|[m[K      |      [01;36m[K|[m[K
    [01;36m[K|[m[K      |      [01;36m[K(1)[m[K entry to
'[01m[Kgot_endgsd[m[K'
    [01;36m[K|[m[K......
    [01;36m[K|[m[K  418 |     if [01;36m[K([m[Knr_gsds == 0) {
    [01;36m[K|[m[K      |        [01;36m[K~[m[K
    [01;36m[K|[m[K      |        [01;36m[K|[m[K
    [01;36m[K|[m[K      |        [01;36m[K(2)[m[K following
'[01m[Kfalse[m[K' branch...
    [01;36m[K|[m[K......
    [01;36m[K|[m[K  422 |     [01;36m[Kqsort[m[K(all_gsds, nr_gsds,
sizeof(char *), compare_gsdlines);
    [01;36m[K|[m[K      |     [01;36m[K~~~~~[m[K
    [01;36m[K|[m[K      |     [01;36m[K|[m[K
    [01;36m[K|[m[K      |     [01;36m[K(3)[m[K ...to here
    [01;36m[K|[m[K
  '[01m[Kgot_endgsd[m[K': event 4
    [01;36m[K|[m[K
    [01;36m[K|[m[K[01m[Kcc1:[m[K
    [01;36m[K|[m[K [01;36m[K(4)[m[K: calling
'[01m[Kgot_endgsd.part.0[m[K' from '[01m[Kgot_endgsd[m[K'
    [01;36m[K|[m[K
    [01;36m[K+--> [m[K'[01m[Kgot_endgsd.part.0[m[K': events 5-13
           [01;36m[K|[m[K
           [01;36m[K|[m[K
         '[01m[Kgot_endgsd.part.0[m[K': event 14
           [01;36m[K|[m[K
           [01;36m[K|[m[K[01m[Kdumpobj.c:428:9:[m[K
           [01;36m[K|[m[K  428 |        
[01;36m[Kfree(all_gsds[i])[m[K;
           [01;36m[K|[m[K      |         [01;36m[K^~~~~~~~~~~~~~~~~[m[K
           [01;36m[K|[m[K      |         [01;36m[K|[m[K
           [01;36m[K|[m[K      |         [01;36m[K(14)[m[K second
'[01m[Kfree[m[K' here; first '[01m[Kfree[m[K' was at
[01;36m[K(11)[m[K
           [01;36m[K|[m[K
COLLECT_GCC_OPTIONS='-v' '-std=gnu99' '-Wall' '-Wshadow' '-Wextra' '-Wpedantic'
'-Woverflow' '-Wstrict-overflow' '-ggdb' '-fanalyzer' '-O3' '-D'
'DEFAULT_OBJECTFORMAT_RT11=0' '-save-temps' '-c' '-o' 'dumpobj.o'
'-mtune=nocona' '-march=nocona'
 /usr/bin/as -v -o dumpobj.o dumpobj.s
GNU assembler version 2.31.1 (x86_64--netbsd) using BFD version (NetBSD
Binutils nb1) 2.31.1
COMPILER_PATH=/usr/pkg/gcc10/libexec/gcc/x86_64--netbsd/10.3.0/:/usr/pkg/gcc10/libexec/gcc/x86_64--netbsd/10.3.0/:/usr/pkg/gcc10/libexec/gcc/x86_64--netbsd/:/usr/pkg/gcc10/lib/gcc/x86_64--netbsd/10.3.0/:/usr/pkg/gcc10/lib/gcc/x86_64--netbsd/
LIBRARY_PATH=/usr/pkg/gcc10/lib/gcc/x86_64--netbsd/10.3.0/:/usr/pkg/gcc10/lib/gcc/x86_64--netbsd/10.3.0/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-std=gnu99' '-Wall' '-Wshadow' '-Wextra' '-Wpedantic'
'-Woverflow' '-Wstrict-overflow' '-ggdb' '-fanalyzer' '-O3' '-D'
'DEFAULT_OBJECTFORMAT_RT11=0' '-save-temps' '-c' '-o' 'dumpobj.o'
'-mtune=nocona' '-march=nocona'
[?2004h$ [?2004l
exit

Script done on Mon Nov  1 21:43:45 2021

Here is a readable version of the diagnostic:

dumpobj.c: In function 'got_endgsd.part.0':
dumpobj.c:428:9: warning: double-'free' of '<unknown>' [CWE-415]
[-Wanalyzer-double-free]
  428 |         free(all_gsds[i]);
      |         ^~~~~~~~~~~~~~~~~
  'got_endgsd': events 1-3
    |
    |dumpobj.c:409:6:
    |  409 | void got_endgsd(
    |      |      ^~~~~~~~~~
    |      |      |
    |      |      (1) entry to 'got_endgsd'
    |......
    |  418 |     if (nr_gsds == 0) {
    |      |        ~
    |      |        |
    |      |        (2) following 'false' branch...
    |......
    |  422 |     qsort(all_gsds, nr_gsds, sizeof(char *), compare_gsdlines);
    |      |     ~~~~~
    |      |     |
    |      |     (3) ...to here
    |
  'got_endgsd': event 4
    |
    |cc1:
    | (4): calling 'got_endgsd.part.0' from 'got_endgsd'
    |
    +--> 'got_endgsd.part.0': events 5-13
           |
           |
         'got_endgsd.part.0': event 14
           |
           |dumpobj.c:428:9:
           |  428 |         free(all_gsds[i]);
           |      |         ^~~~~~~~~~~~~~~~~
           |      |         |
           |      |         (14) second 'free' here; first 'free' was at (11)
           |

If I add all_gsds[i] = NULL; after line 428 with the free, then gcc
seems to think that i does not change during the loop and NULL may be
passed to fputs():

Script started on Mon Nov  1 21:48:26 2021
$ /usr/pkg/gcc10/bin/gcc -std=gnu99 -Wall -Wshadow -Wextra -pedantic -Woverflow
-Wstrict-overflow -ggdb -fanalyzer -O3 -DDEFAULT_OBJECTFORMAT_RT11=0
-save-temps -c -o dumpobj.o dumpobj.c
[?2004l[01m[Kdumpobj.c:[m[K In function '[01m[Kgot_endgsd.part.0[m[K':
[01m[Kdumpobj.c:427:9:[m[K [01;35m[Kwarning: [m[Kuse of NULL
'[01m[K<unknown>[m[K' where non-null expected
[[01;35m[K]8;;https://cwe.mitre.org/data/definitions/690.htmlCWE-690]8;;[m[K]
[[01;35m[K]8;;https://gcc.gnu.org/onlinedocs/gcc/Static-Analyzer-Options.html#index-Wanalyzer-null-argument-Wanalyzer-null-argument]8;;[m[K]
  427 |         fputs(all_gsds[i], stdout);
  '[01m[Kgot_endgsd[m[K': events 1-3
    [01;36m[K|[m[K
    [01;36m[K|[m[K  409 | void [01;36m[Kgot_endgsd[m[K(
    [01;36m[K|[m[K      |      [01;36m[K^~~~~~~~~~[m[K
    [01;36m[K|[m[K      |      [01;36m[K|[m[K
    [01;36m[K|[m[K      |      [01;36m[K(1)[m[K entry to
'[01m[Kgot_endgsd[m[K'
    [01;36m[K|[m[K......
    [01;36m[K|[m[K  418 |     if [01;36m[K([m[Knr_gsds == 0) {
    [01;36m[K|[m[K      |        [01;36m[K~[m[K
    [01;36m[K|[m[K      |        [01;36m[K|[m[K
    [01;36m[K|[m[K      |        [01;36m[K(2)[m[K following
'[01m[Kfalse[m[K' branch...
    [01;36m[K|[m[K......
    [01;36m[K|[m[K  422 |     [01;36m[Kqsort[m[K(all_gsds, nr_gsds,
sizeof(char *), compare_gsdlines);
    [01;36m[K|[m[K      |     [01;36m[K~~~~~[m[K
    [01;36m[K|[m[K      |     [01;36m[K|[m[K
    [01;36m[K|[m[K      |     [01;36m[K(3)[m[K ...to here
    [01;36m[K|[m[K
  '[01m[Kgot_endgsd[m[K': event 4
    [01;36m[K|[m[K
    [01;36m[K|[m[K[01m[Kcc1:[m[K
    [01;36m[K|[m[K [01;36m[K(4)[m[K: calling
'[01m[Kgot_endgsd.part.0[m[K' from '[01m[Kgot_endgsd[m[K'
    [01;36m[K|[m[K
    [01;36m[K+--> [m[K'[01m[Kgot_endgsd.part.0[m[K': events 5-13
           [01;36m[K|[m[K
           [01;36m[K|[m[K
         '[01m[Kgot_endgsd.part.0[m[K': event 14
           [01;36m[K|[m[K
           [01;36m[K|[m[K  427 |         fputs(all_gsds[i], stdout);
           [01;36m[K|[m[K
In file included from [01m[Kdumpobj.c:37[m[K:
[01m[K/usr/pkg/gcc10/lib/gcc/x86_64--netbsd/10.3.0/include-fixed/stdio.h:248:5:[m[K
[01;36m[Knote: [m[Kargument 1 of '[01m[Kfputs[m[K' must be non-null
  248 | int [01;36m[K fput[m[Ks(const char * __restrict, FILE *
__restrict);
      |     [01;36m[K^~~~~[m[K
$ [?2004l
exit

Script done on Mon Nov  1 21:48:31 2021

$ /usr/pkg/gcc10/bin/gcc -std=gnu99 -Wall -Wshadow -Wextra -pedantic -Woverflow
-Wstrict-overflow -ggdb -fanalyzer -O3 -DDEFAULT_OBJECTFORMAT_RT11=0
-save-temps -c -o dumpobj.o dumpobj.c
dumpobj.c: In function 'got_endgsd.part.0':
dumpobj.c:427:9: warning: use of NULL '<unknown>' where non-null expected
[CWE-690] [-Wanalyzer-null-argument]
  427 |         fputs(all_gsds[i], stdout);
  'got_endgsd': events 1-3
    |
    |  409 | void got_endgsd(
    |      |      ^~~~~~~~~~
    |      |      |
    |      |      (1) entry to 'got_endgsd'
    |......
    |  418 |     if (nr_gsds == 0) {
    |      |        ~
    |      |        |
    |      |        (2) following 'false' branch...
    |......
    |  422 |     qsort(all_gsds, nr_gsds, sizeof(char *), compare_gsdlines);
    |      |     ~~~~~
    |      |     |
    |      |     (3) ...to here
    |
  'got_endgsd': event 4
    |
    |cc1:
    | (4): calling 'got_endgsd.part.0' from 'got_endgsd'
    |
    +--> 'got_endgsd.part.0': events 5-13
           |
           |
         'got_endgsd.part.0': event 14
           |
           |  427 |         fputs(all_gsds[i], stdout);
           |
In file included from dumpobj.c:37:
/usr/pkg/gcc10/lib/gcc/x86_64--netbsd/10.3.0/include-fixed/stdio.h:248:5:
note:argument 1 of 'fputs' must be non-null
  248 | int  fputs(const char * __restrict, FILE * __restrict);
      |     ^~~~~


>From reading the description, this might be a variant of bug 99044 but I don't
have a later gcc available to check.

[Bug analyzer/103032] false positive diagnostic from -fanalyzer about double-free

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
I haven't yet reproduced the precise symptoms you reported, but FWIW I'm seeing
this in got_gsd, which looks like a true warning for this code:

           | 1903 |             psects[psectid] = strdup(name);
           |      |                               ~~~~~~~~~~~~
           |      |                               |
           |      |                               (43) this call could return
NULL
           | 1904 |             trim(psects[psectid++]);
           |      |             ~~~~~~~~~~~~~~~~~~~~~~~
           |      |             |
           |      |             (44) calling ‘trim’ from ‘got_gsd’
           |
           +--> ‘trim’: events 45-46
                  |
                  | 1800 | void trim(
                  |      |      ^~~~
                  |      |      |
                  |      |      (45) entry to ‘trim’
                  |......
                  | 1805 |     for (cp = buf + strlen(buf); cp > buf; cp--)
                  |      |                     ~~~~~~~~~~~
                  |      |                     |
                  |      |                     (46) argument 1 (‘buf’) from
(43) could be NULL where non-null expected
                  |
../../src/gcc/testsuite/gcc.dg/analyzer/pr103032.c:1173:8: note: argument 1 of
‘strlen’ must be non-null
 1173 | size_t strlen(const char *);
      |        ^~~~~~

[Bug analyzer/103032] false positive diagnostic from -fanalyzer about double-free

[rhialto at falu dot nl]

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

--- Comment #2 from Olaf 'Rhialto' Seibert <rhialto at falu dot nl> ---
I tried my case on a different system with a later gcc version, and it didn't
show the problems I reported. It did however show the diagnostics you showed,
and I think it is correct. strdup() could indeed return NULL.

I don't know how the gcc policy about backporting fixes is. If you don't do it
(and this one isn't super important, just somewhat misleading), then this
report can be closed.

If you might consider a backport, then of course not :)

[Bug analyzer/103032] false positive diagnostic from -fanalyzer about double-free

[dmalcolm at gcc dot gnu.org]

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

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for the info, and the confirmation.

The analyzer code has changed a *lot* since gcc 10, and is continuing to change
quickly, so I'm not planning to backport these changes; I'll close this report
out.  Thanks again.