[Bug analyzer/108806] New: -Wanalyzer-null-dereference false positives due to not handling bitmasks

[Bug analyzer/108806] New: -Wanalyzer-null-dereference false positives due to not handling bitmasks

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 108806
           Summary: -Wanalyzer-null-dereference false positives due to not
                    handling bitmasks
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
            Blocks: 108562
  Target Milestone: ---

Created attachment 54470
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54470&action=edit
Reproducer

Trunk:    https://godbolt.org/z/77EbbPEW5
GCC 12.2: https://godbolt.org/z/o8bbMhPxW
GCC 11.3: https://godbolt.org/z/o4dK3zn3b
GCC 10.4: https://godbolt.org/z/GzEqzMMjW

Lots of false positives of the form:

<source>: In function 'omap2_inth_read':
<source>:75:18: warning: dereference of NULL 'bank' [CWE-476]
[-Wanalyzer-null-dereference]
   75 |       return bank->inputs;
      |              ~~~~^~~~~~~~
  'omap2_inth_read': events 1-6
    |
    |   40 |   struct omap_intr_handler_bank_s* bank = NULL;
    |      |                                    ^~~~
    |      |                                    |
    |      |                                    (1) 'bank' is NULL
    |   41 | 
    |   42 |   if ((offset & 0xf80) == 0x80) {
    |      |      ~                              
    |      |      |
    |      |      (2) following 'false' branch...
    |......
    |   52 |   switch (offset) {
    |      |   ~~~~~~                            
    |      |   |
    |      |   (3) ...to here
    |      |   (4) following 'case 128:' branch...
    |......
    |   74 |     case 0x80:
    |      |     ~~~~                            
    |      |     |
    |      |     (5) ...to here
    |   75 |       return bank->inputs;
    |      |              ~~~~~~~~~~~~           
    |      |                  |
    |      |                  (6) dereference of NULL 'bank'
    |

where if offset == 128, then the:

    |   42 |   if ((offset & 0xf80) == 0x80) {
    |      |      ~                              
    |      |      |
    |      |      (2) following 'false' branch...
    |......
    |   52 |   switch (offset) {
    |      |   ~~~~~~                            
    |      |   |
    |      |   (3) ...to here

edge is impossible, and "bank" will have been properly initialized.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108562
[Bug 108562] [meta-bug] tracker bug for issues with -Wanalyzer-null-dereference

[Bug analyzer/108806] -Wanalyzer-null-dereference false positives due to not handling bitmasks

[cvs-commit at gcc dot gnu.org]

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

--- Comment #1 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:4d3b7be281e73ecdaa233598db1a8390422b7770

commit r13-6101-g4d3b7be281e73ecdaa233598db1a8390422b7770
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Feb 16 18:12:55 2023 -0500

    analyzer: respect some conditions from bit masks [PR108806]

    PR analyzer/108806 reports false +ves seen from -fanalyzer on code like
this
    in qemu-7.2.0's hw/intc/omap_intc.c:

      [...snip...]
      struct omap_intr_handler_bank_s* bank = NULL;
      if ((offset & 0xf80) == 0x80) {
        [...set "bank" to non-NULL...]
      }
      switch (offset) {
        [...snip various cases that don't deref "bank"...]
        case 0x80:
          return bank->inputs;
        case 0x84:
          return bank->mask;
        [...etc...]
       }

    where the analyzer falsely complains about execution paths in which
    "(offset & 0xf80) == 0x80" was false (leaving "bank" as NULL), but then
    in which "switch (offset)" goes to a case for which
    "(offset & 0xf80) == 0x80" is true and dereferences NULL "bank", i.e.
    paths in which "(offset & 0xf80) == 0x80" is both true *and* false.

    This patch adds enough logic to constraint_manager for -fanalyzer to
    reject such execution paths as impossible, fixing the false +ves.

    Integration testing shows this eliminates 20 probable false positives:

    Comparison: 9.08% -> 9.34% GOOD: 66 BAD: 661 -> 641 (-20)

    where the affected warnings/projects are:

      -Wanalyzer-null-dereference: 0.00% GOOD: 0 BAD: 279 -> 269 (-10)
            qemu-7.2.0: 175 -> 165 (-10)

      -Wanalyzer-use-of-uninitialized-value: 0.00% GOOD: 0 BAD: 153 -> 143
(-10)
         coreutils-9.1:  18 ->  14 (-4)
            qemu-7.2.0:  54 ->  48 (-6)

    gcc/analyzer/ChangeLog:
            PR analyzer/108806
            * constraint-manager.cc (bounded_range::dump_to_pp): Use
            bounded_range::singleton_p.
            (constraint_manager::add_bounded_ranges): Handle singleton ranges
            by adding an EQ_EXPR constraint.
            (constraint_manager::impossible_derived_conditions_p): New.
            (constraint_manager::eval_condition): Reject EQ_EXPR when it would
            imply impossible derived conditions.
            (selftest::test_bits): New.
            (selftest::run_constraint_manager_tests): Run it.
            * constraint-manager.h (bounded_range::singleton_p): New.
            (constraint_manager::impossible_derived_conditions_p): New decl.
            * region-model.cc (region_model::get_rvalue_1): Handle
            BIT_AND_EXPR, BIT_IOR_EXPR, and BIT_XOR_EXPR.

    gcc/testsuite/ChangeLog:
            PR analyzer/108806
            * gcc.dg/analyzer/null-deref-pr108806-qemu.c: New test.
            * gcc.dg/analyzer/pr103217.c: Add -Wno-analyzer-too-complex.
            * gcc.dg/analyzer/switch.c (test_bitmask_1): New.
            (test_bitmask_2): New.
            * gcc.dg/analyzer/uninit-pr108806-qemu.c: New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug analyzer/108806] -Wanalyzer-null-dereference false positives due to not handling bitmasks

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-02-16
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed on trunk for GCC 13 by the above commit.

Keeping this open to track backporting the fix to GCC 12.
I rewrote switch-handling in GCC 12, so I don't think this is going to be
backportable to 11 or 10.

[Bug analyzer/108806] [12 Regression] -Wanalyzer-null-dereference false positives due to not handling bitmasks

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.4