[Bug analyzer/108455] -Wanalyzer-deref-before-check false positive seen in git pack-revindex.c

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

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

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

https://gcc.gnu.org/g:0d6f7b1dd62e9c9dccb0b9b673f9cc3238b7ea6d

commit r13-5261-g0d6f7b1dd62e9c9dccb0b9b673f9cc3238b7ea6d
Author: David Malcolm <dmalc...@redhat.com>
Date:   Thu Jan 19 13:51:16 2023 -0500

    analyzer: use dominator info in -Wanalyzer-deref-before-check [PR108455]

    My integration testing [1] of -fanalyzer in GCC 13 is showing a lot of
    diagnostics from the new -Wanalyzer-deref-before-check warning on
    real-world C projects, and most of these seem to be false positives.

    This patch updates the warning to make it much less likely to fire:
    - only intraprocedural cases are now reported
    - reject cases in which there are control flow paths to the check
      that didn't come through the dereference, by looking at BB dominator
      information.  This fixes a false positive seen in git-2.39.0's
      pack-revindex.c: load_revindex_from_disk (PR analyzer/108455), in
      which a shared "cleanup:" section checks "data" for NULL, and
      depending on how much of the function is executed "data" might or
      might not have already been dereferenced.

    The counts of -Wanalyzer-deref-before-check diagnostics in [1]
    before/after this patch show this improvement:
      Known false positives:    6 ->  0  (-6)
      Known true positives:     1 ->  1
      Unclassified positives: 123 -> 63 (-60)

    [1] https://github.com/davidmalcolm/gcc-analyzer-integration-tests

    gcc/analyzer/ChangeLog:
            PR analyzer/108455
            * analyzer.h (class checker_event): New forward decl.
            (class state_change_event): Indent.
            (class warning_event): New forward decl.
            * checker-event.cc (state_change_event::state_change_event): Add
            "enode" param.
            (warning_event::get_desc): Update for new param of
            evdesc::final_event ctor.
            * checker-event.h (state_change_event::state_change_event): Add
            "enode" param.
            (state_change_event::get_exploded_node): New accessor.
            (state_change_event::m_enode): New field.
            (warning_event::warning_event): New "enode" param.
            (warning_event::get_exploded_node): New accessor.
            (warning_event::m_enode): New field.
            * diagnostic-manager.cc
            (state_change_event_creator::on_global_state_change): Pass
            src_node to state_change_event ctor.
            (state_change_event_creator::on_state_change): Likewise.
            (null_assignment_sm_context::set_next_state): Pass NULL for
            new param of state_change_event ctor.
            * infinite-recursion.cc
            (infinite_recursion_diagnostic::add_final_event): Update for new
            param of warning_event ctor.
            * pending-diagnostic.cc (pending_diagnostic::add_final_event):
            Pass enode to warning_event ctor.
            * pending-diagnostic.h (evdesc::final_event): Add reference to
            warning_event.
            * sm-malloc.cc: Include "analyzer/checker-event.h" and
            "analyzer/exploded-graph.h".
            (deref_before_check::deref_before_check): Initialize new fields.
            (deref_before_check::emit): Reject warnings in which we were
            unable to determine the enodes of the dereference and the check.
            Reject warnings interprocedural warnings. Reject warnings in which
            the dereference doesn't dominate the check.
            (deref_before_check::describe_state_change): Set m_deref_enode.
            (deref_before_check::describe_final_event): Set m_check_enode.
            (deref_before_check::m_deref_enode): New field.
            (deref_before_check::m_check_enode): New field.

    gcc/testsuite/ChangeLog:
            PR analyzer/108455
            * gcc.dg/analyzer/deref-before-check-1.c: Add test coverage
            involving dominance.
            * gcc.dg/analyzer/deref-before-check-pr108455-1.c: New test.
            * gcc.dg/analyzer/deref-before-check-pr108455-git-pack-revindex.c:
            New test.

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