[Bug analyzer/94851] New: -fanalyzer erroniously reporting NULL dereference - simple test case attached

[Bug analyzer/94851] New: -fanalyzer erroniously reporting NULL dereference - simple test case attached

[addw at phcomp dot co.uk]

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

            Bug ID: 94851
           Summary: -fanalyzer erroniously reporting NULL dereference -
                    simple test case attached
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: addw at phcomp dot co.uk
  Target Milestone: ---

Created attachment 48409
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48409&action=edit
C source showing error

Attached is a simplified part of a bigger program.
Compiled on Fedora 32:

cc -O2 -Wall -Wno-pointer-sign -Wconversion -fanalyzer -c -o pmark.o pmark.c

Output also attached.

[Bug analyzer/94851] -fanalyzer erroniously reporting NULL dereference - simple test case attached

[addw at phcomp dot co.uk]

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

--- Comment #1 from Alain D D Williams <addw at phcomp dot co.uk> ---
Created attachment 48410
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48410&action=edit
Compiler output

[Bug analyzer/94851] -fanalyzer erroneously reporting NULL dereference - simple test case attached

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|-fanalyzer erroniously      |-fanalyzer erroneously
                   |reporting NULL dereference  |reporting NULL dereference
                   |- simple test case attached |- simple test case attached
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2020-05-08
     Ever confirmed|0                           |1

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this bug.

It does indeed look like a false positive.

With -fanalyzer-verbosity=3 I get this output:

----------------------------------------------------------------------
$ ./xgcc -B. -c -fanalyzer t.c -O2 -fanalyzer-verbosity=3
t.c: In function ‘pamark’:
t.c:48:19: warning: dereference of NULL ‘last’ [CWE-690]
[-Wanalyzer-null-dereference]
   48 |    last->m_next   = p;
      |    ~~~~~~~~~~~~~~~^~~
  ‘pamark’: events 1-5
    |
    |   28 |  while(p != (AMARK*)NULL && p->m_name != (char)c) {
    |      |       ^
    |      |       |
    |      |       (1) following ‘false’ branch (when ‘p’ is NULL)...
    |......
    |   33 |  if(p != (AMARK*) NULL) {
    |      |    ~   
    |      |    |
    |      |    (2) ...to here
    |      |    (3) following ‘false’ branch (when ‘p’ is NULL)...
    |......
    |   40 |   if((p = (AMARK*) malloc(sizeof(AMARK))) == (AMARK*) NULL)
    |      |     ~              ~~~~~~~~~~~~~~~~~~~~~
    |      |     |              |
    |      |     |              (4) ...to here
    |      |     (5) following ‘false’ branch (when ‘p’ is non-NULL)...
    |
  ‘pamark’: event 6
    |
    |   43 |   p->m_next = (AMARK*) NULL;
    |      |             ^
    |      |             |
    |      |             (6) ...to here
    |
  ‘pamark’: events 7-10
    |
    |
----------------------------------------------------------------------

It's considering the case where at:

        AMARK*  p    = curbp->b_amark;
        AMARK*  last = curbp->b_amark;

both pointers are NULL.

I'm not sure why it's filtered those events at the default verbosity level.

It's also not printing events 7-10 for some reason, but with
-fdiagnostics-path-format=separate-events they show up as:

t.c:43:13: note: (7) ‘last’ is NULL
t.c:45:5: note: (8) following ‘false’ branch...
   45 |   if(curbp->b_amark == (AMARK*) NULL)
      |     ^
t.c:48:19: note: (9) ...to here
   48 |    last->m_next   = p;
      |    ~~~~~~~~~~~~~~~^~~
t.c:48:19: note: (10) dereference of NULL ‘last’

However, it seems to have lost the fact that "curbp->b_amark == NULL" at event
(8); hence a false positive.

I've tried to simplify the reproducer but have had no success so far.

[Bug analyzer/94851] -fanalyzer erroneously reporting NULL dereference - simple test case attached

[hugo_musso_gualandi at hotmail dot com]

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

Hugo Gualandi <hugo_musso_gualandi at hotmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hugo_musso_gualandi@hotmail
                   |                            |.com

--- Comment #3 from Hugo Gualandi <hugo_musso_gualandi at hotmail dot com> ---
Created attachment 48492
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48492&action=edit
Another test case (-O1)

[Bug analyzer/94851] -fanalyzer erroneously reporting NULL dereference - simple test case attached

[hugo_musso_gualandi at hotmail dot com]

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

--- Comment #4 from Hugo Gualandi <hugo_musso_gualandi at hotmail dot com> ---
Created attachment 48493
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48493&action=edit
Another test case (-O2)

[Bug analyzer/94851] -fanalyzer erroneously reporting NULL dereference - simple test case attached

[hugo_musso_gualandi at hotmail dot com]

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

--- Comment #5 from Hugo Gualandi <hugo_musso_gualandi at hotmail dot com> ---
Hi, I came across a similar problem and I think I might have found a smaller
test case. gcc complains about a NULL pointer dereference in the p->next
despite the loop condition testing that p is not NULL.

The first test case I attached only hits the bug if we compile with -O1. The
second test case hits the bug both in -O1 and -O2.

    gcc -O1 -fanalyzer bug-O1 -o bug-O1
    gcc -O1 -fanalyzer bug-O2 -o bug-O2

The problem goes away if I refactor the code to not use the && operator

    while (p) {
        if (p == q) break;
        p = p->next;
    }

I tested with the version of GCC 10.0.1 present in Fedora 32 and with GCC
10.1.0, which I built from source.

[Bug analyzer/94851] -fanalyzer erroneously reporting NULL dereference - simple test case attached

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:df2b78d407a3fe8685343f7249b9c31c7e3af44d

commit r11-2807-gdf2b78d407a3fe8685343f7249b9c31c7e3af44d
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Sat Aug 22 06:30:17 2020 -0400

    analyzer: fix NULL deref false positives [PR94851]

    PR analyzer/94851 reports various false "NULL dereference" diagnostics.
    The first case (comment #1) affects GCC 10.2 but no longer affects
    trunk; I believe it was fixed by the state rewrite of
    r11-2694-g808f4dfeb3a95f50f15e71148e5c1067f90a126d.

    The patch adds a regression test for this case.

    The other cases (comment #3 and comment #4) still affect trunk.
    In both cases, the && in a conditional is optimized to bitwise &
      _1 = p_4 != 0B;
      _2 = p_4 != q_6(D);
      _3 = _1 & _2;
    and the analyzer fails to fold this for the case where one (or both) of
    the conditionals is false, and thus erroneously considers the path where
    "p" is non-NULL despite being passed a NULL value.

    Fix this by implementing folding for this case.

    gcc/analyzer/ChangeLog:
            PR analyzer/94851
            * region-model-manager.cc
            (region_model_manager::maybe_fold_binop): Fold bitwise "& 0" to 0.

    gcc/testsuite/ChangeLog:
            PR analyzer/94851
            * gcc.dg/analyzer/pr94851-1.c: New test.
            * gcc.dg/analyzer/pr94851-3.c: New test.
            * gcc.dg/analyzer/pr94851-4.c: New test.

[Bug analyzer/94851] -fanalyzer erroneously reporting NULL dereference - simple test case attached

[dmalcolm at gcc dot gnu.org]

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

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

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

--- Comment #7 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for reporting these.  These false positives should now be fixed in git
(for GCC 11); see the above commit.