Bug 71157

Summary: -Wnull-dereference false alarm in wrong function
Product: gcc Reporter: Paul Eggert <eggert>
Component: cAssignee: Not yet assigned to anyone <unassigned>
Status: NEW ---    
Severity: normal CC: dimhen, egallager, manu, msebor
Priority: P3 Keywords: diagnostic
Version: 6.1.0   
Target Milestone: ---   
See Also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16351
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84203
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2018-02-20 00:00:00
Bug Depends on:    
Bug Blocks: 86172    
Attachments: preprocessed source code for emacs/lib-src/etags.c
GNU Emacs lib-src/etags.c patch to work around GCC bug
patched version of e.i that avoids GCC bug

Description Paul Eggert 2016-05-17 05:16:44 UTC
Created attachment 38504 [details]
preprocessed source code for emacs/lib-src/etags.c

I discovered this when compiling bleeding-edge GNU Emacs with GCC 6.1.0 on x86-64. For the attached file e.i, the shell command:

gcc -O2 -S -Wnull-dereference e.i

issues the diagnostics:

e.i: In function 'Forth_words':
e.i:9690:10: warning: potential null pointer dereference [-Wnull-dereference]
   while (*cp != '\0' && !c_isspace (*cp))
          ^~~
e.i:9690:10: warning: potential null pointer dereference [-Wnull-dereference]
e.i:9690:10: warning: potential null pointer dereference [-Wnull-dereference]

I see three things wrong with these diagnostics. First, line 9690 is nowhere near the function Forth_words. Second, I see no path to line 9690 in which its pointer CP can possibly be null. Furthermore, CP is dereferenced only twice in line 9690, so why are there three warnings?
Comment 1 Marek Polacek 2016-05-19 13:27:25 UTC
Interesting that with -fno-inline I see

q.i: In function ‘Ruby_functions’:
q.i:7840:12: warning: potential null pointer dereference [-Wnull-dereference]
        if (*cp != '#')
            ^~~
Comment 2 Paul Eggert 2016-05-19 22:36:30 UTC
Created attachment 38531 [details]
GNU Emacs lib-src/etags.c patch to work around GCC bug
Comment 3 Paul Eggert 2016-05-19 22:37:36 UTC
Created attachment 38532 [details]
patched version of e.i that avoids GCC bug
Comment 4 Paul Eggert 2016-05-19 22:40:00 UTC
I worked around the GCC bug by applying the attached file etags.c.patch to GNU Emacs. etags.c.patch replaces some weird but valid C code (add a small constant to a pointer and test whether the resulting pointer is non-null, a test that always yields 1) with more-normal code (add a small constant to a pointer, then yield 1). After preprocessing, this yields the attached file e-patched.i, and the command 'gcc -O2 -S -Wnull-dereference e-patched.i' does not issue the bogus warnings.
Comment 5 Manuel López-Ibáñez 2016-05-20 00:16:21 UTC
GCC does think that some path may dereference a null pointer since this block survives until the end:

;;   basic block 109, loop depth 0, count 0, freq 743, maybe hot
;;   Invalid sum of incoming frequencies 5, should be 743
;;    prev block 108, next block 1, flags: (NEW, REACHABLE)
;;    pred:       59 [15.0%]  (FALSE_VALUE,EXECUTABLE)
;;   starting at line 7841
  [pr71157.c:7841:12] # VUSE <.MEM_63>
  _402 ={v} [pr71157.c:7841:12] MEM[(charD.7 *)0B];
  # .MEM_388 = VDEF <.MEM_63>
  # USE = nonlocal null { D.5991 } (nonlocal, escaped)
  # CLB = nonlocal null { D.5991 } (nonlocal, escaped)
  __builtin_trapD.1024 ();
;;    succ:

this is because it thinks skip_space() may return NULL:

;;   basic block 59, loop depth 2, count 0, freq 36, maybe hot
;;    prev block 58, next block 60, flags: (NEW, REACHABLE)
;;    pred:       57 [29.0%]  (TRUE_VALUE,EXECUTABLE)
;;   starting at line 7764, discriminator 2
  [pr71157.c:7764:133] # PT = nonlocal escaped null
  _200 = cp_11 + 12;
  [pr71157.c:7764:122] # VUSE <.MEM_63>
  # PT = nonlocal escaped null
  # USE = nonlocal escaped null
  cp_201 = skip_spacesD.5937 (_200);
  [pr71157.c:7764:122] if (cp_201 != 0B)
    goto <bb 61>;
  else
    goto <bb 109>;
;;    succ:       61 [85.0%]  (TRUE_VALUE,EXECUTABLE)
;;                109 [15.0%]  (FALSE_VALUE,EXECUTABLE)


The location shown is just an artifact of merging expressions and not preserving the right locations. The middle-end is not very smart at that, this is why middle-end warnings are often confusing.

The testcase is too large for me to analyze further.
Comment 6 Paul Eggert 2016-07-04 07:43:16 UTC
> this is because it thinks skip_space() may return NULL:

That sounds like a bug, then, as skip_spaces immediately dereferences its argument, so it cannot possibly return NULL.

Also, skip_spaces is never passed an argument that could possibly be NULL, so even if the function simply returns its argument the result cannot be NULL.
Comment 7 Eric Gallager 2018-02-20 11:59:58 UTC
Confirmed that I get the same warnings reported in the OP.
Comment 8 Eric Gallager 2018-09-16 05:41:33 UTC
(In reply to Paul Eggert from comment #6)
> > this is because it thinks skip_space() may return NULL:
> 
> That sounds like a bug, then, as skip_spaces immediately dereferences its
> argument, so it cannot possibly return NULL.
> 
> Also, skip_spaces is never passed an argument that could possibly be NULL,
> so even if the function simply returns its argument the result cannot be
> NULL.

Try marking it up with __attribute__((returns_nonnull)) and/or __attribute__((nonnull)). If the first one works, then it's a case where GCC should suggest it, in which case it's bug 84203.
Comment 9 Paul Eggert 2018-09-16 14:55:18 UTC
(In reply to Eric Gallager from comment #8)

> Try marking it up with __attribute__((returns_nonnull)) and/or
> __attribute__((nonnull)). If the first one works, then it's a case where GCC
> should suggest it, in which case it's bug 84203.

Neither suggestion works, unfortunately. That is, I tried undoing the abovementioned patch to etags.c that works around the problem, and then modifying the declaration of skip_spaces this way:

static char *skip_spaces (char *) __attribute__ ((nonnull (1)));

or this way:

static char *skip_spaces (char *) __attribute__ ((returns_nonnull));

Neither modification worked: in both cases I still got the bogus warnings. This was with gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0.
Comment 10 Manuel López-Ibáñez 2018-09-16 16:13:14 UTC
Martin, this is one of those warnings that would benefit from printing the inline stack (and also from better location info in the middle).
Comment 11 Marc Glisse 2018-09-16 16:56:39 UTC
Trunk doesn't show the warning (gcc-8 does).
Comment 12 Martin Sebor 2018-09-17 14:36:06 UTC
(In reply to Manuel López-Ibáñez from comment #10)

Yes, the warning should use the %G directive.  It might be worth reviewing all the middle-end warnings for this improvement.  Unless we get rid of %G/%K first and make GCC print the inlining stack unconditionally.
Comment 13 Martin Sebor 2018-10-20 22:59:24 UTC
In GCC 9 the warning has disappeared with r263981.

At -O1, GCC 8
Comment 14 Martin Sebor 2018-10-20 23:01:38 UTC
At -O1, GCC 8 as well as trunk print:

pr71157.i: In function ‘TeX_commands’:
pr71157.i:8469:44: warning: ‘TEX_clgrp’ may be used uninitialized in this function [-Wmaybe-uninitialized]
8469 |       while (*p != '\0' && *p != TEX_opgrp && *p != TEX_clgrp)
     |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
pr71157.i:8469:25: warning: ‘TEX_opgrp’ may be used uninitialized in this function [-Wmaybe-uninitialized]
8469 |       while (*p != '\0' && *p != TEX_opgrp && *p != TEX_clgrp)
     |              ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~


At -O3, they issue:

In function ‘xrealloc’,
    inlined from ‘linebuffer_setlen’ at pr71157.i:10000:33,
    inlined from ‘find_entries’ at pr71157.i:4822:3:
pr71157.i:10017:18: warning: argument 2 range [9223372036854775808, 18446744073709551615] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
10017 |   void *result = realloc (ptr, size);
      |                  ^~~~~~~~~~~~~~~~~~~
pr71157.i: In function ‘find_entries’:
pr71157.i:853:14: note: in a call to allocation function ‘realloc’ declared here
853 | extern void *realloc (void *__ptr, size_t __size)
    |              ^~~~~~~
In function ‘xrealloc’,
    inlined from ‘linebuffer_setlen’ at pr71157.i:10000:33,
    inlined from ‘HTML_labels’ at pr71157.i:8593:3:
pr71157.i:10017:18: warning: argument 2 range [9223372036854775808, 18446744073709551615] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
10017 |   void *result = realloc (ptr, size);
      |                  ^~~~~~~~~~~~~~~~~~~
pr71157.i: In function ‘HTML_labels’:
pr71157.i:853:14: note: in a call to allocation function ‘realloc’ declared here
853 | extern void *realloc (void *__ptr, size_t __size)
    |              ^~~~~~~
In function ‘xrealloc’,
    inlined from ‘linebuffer_setlen’ at pr71157.i:10000:33,
    inlined from ‘HTML_labels’ at pr71157.i:8679:6:
pr71157.i:10017:18: warning: argument 2 range [9223372036854775808, 18446744073709551615] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
10017 |   void *result = realloc (ptr, size);
      |                  ^~~~~~~~~~~~~~~~~~~
pr71157.i: In function ‘HTML_labels’:
pr71157.i:853:14: note: in a call to allocation function ‘realloc’ declared here
853 | extern void *realloc (void *__ptr, size_t __size)
    |              ^~~~~~~