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

--- Comment #2 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:f573d35147ca8433c102e1721d8c99fc432cb44b

commit r12-5424-gf573d35147ca8433c102e1721d8c99fc432cb44b
Author: David Malcolm <dmalc...@redhat.com>
Date:   Thu Nov 18 15:23:30 2021 -0500

    analyzer: fix false leak due to overeager state merging [PR103217]

    PR analyzer/103217 reports a false positive from -Wanalyzer-malloc-leak.

    The root cause is due to overzealous state merger, where the
    state-merging code decided to merge these two states by merging
    the stores:

    state A:
      clusters within frame: âmainâ@1
        cluster for: one_3: CONJURED(val_4 = strdup (src_2(D));, val_4)
        cluster for: two_4: UNKNOWN(char *)
        cluster for: one_21: CONJURED(val_4 = strdup (src_2(D));, val_4)

    state B:
      clusters within frame: âmainâ@1
        cluster for: one_3: UNKNOWN(char *)
        cluster for: two_4: CONJURED(val_4 = strdup (src_2(D));, val_4)
        cluster for: two_18: CONJURED(val_4 = strdup (src_2(D));, val_4)

    into:
      clusters within frame: âmainâ@1
        cluster for: one_3: UNKNOWN(char *)
        cluster for: two_4: UNKNOWN(char *)
        cluster for: one_21: UNKNOWN(char *)
        cluster for: two_18: UNKNOWN(char *)

    despite "CONJURED(val_4 = strdup (src_2(D));, val_4)" having sm-state,
    in this case malloc:nonnull ({free}), thus leading to both references
    to the conjured svalue being lost at merger.

    This patch tweaks the state merger code so that it will not consider
    merging two different svalues for the value of a region if either svalue
    has non-purgable sm-state (in the above example, malloc:nonnull).  This
    fixes the false leak report above.

    Doing so uncovered an issue with explode-2a.c in which the warnings
    moved from the correct location to the "while" stmt.  This turned out
    to be a missing call to detect_leaks in phi-handling, which the patch
    also fixes (in the PK_BEFORE_SUPERNODE case in
    exploded_graph::process_node).  Doing this fixed the regression in
    explode-2a.c and also fixed the location of the leak warning in
    explode-1.c.

    The other side effect of the change is that pr94858-1.c now emits
    a -Wanalyzer-too-complex warning, since pertinent state is no longer
    being thrown away.  There doesn't seem to be a good way of avoiding
    this, so the patch also adds -Wno-analyzer-too-complex to that test
    case (restoring the default).

    gcc/analyzer/ChangeLog:
            PR analyzer/103217
            * engine.cc (exploded_graph::get_or_create_node): Pass in
            m_ext_state to program_state::can_merge_with_p.
            (exploded_graph::process_worklist): Likewise.
            (exploded_graph::maybe_process_run_of_before_supernode_enodes):
            Likewise.
            (exploded_graph::process_node): Add missing call to detect_leaks
            when handling phi nodes.
            * program-state.cc (program_state::can_merge_with_p): Add
            "ext_state" param.  Pass it and state ptrs to
            region_model::can_merge_with_p.
            (selftest::test_program_state_merging): Update for new ext_state
            param of program_state::can_merge_with_p.
            (selftest::test_program_state_merging_2): Likewise.
            * program-state.h (program_state::can_purge_p): Make const.
            (program_state::can_merge_with_p): Add "ext_state" param.
            * region-model.cc: Include "analyzer/program-state.h".
            (region_model::can_merge_with_p): Add params "ext_state",
            "state_a", and "state_b", use them when creating model_merger
            object.
            (model_merger::mergeable_svalue_p): New.
            * region-model.h (region_model::can_merge_with_p): Add params
            "ext_state", "state_a", and "state_b".
            (model_merger::model_merger) Likewise, initializing new fields.
            (model_merger::mergeable_svalue_p): New decl.
            (model_merger::m_ext_state): New field.
            (model_merger::m_state_a): New field.
            (model_merger::m_state_b): New field.
            * svalue.cc (svalue::can_merge_p): Call
            model_merger::mergeable_svalue_p on both states and reject the
            merger accordingly.

    gcc/testsuite/ChangeLog:
            PR analyzer/103217
            * gcc.dg/analyzer/explode-1.c: Update for improvement to location
            of leak warning.
            * gcc.dg/analyzer/pr103217.c: New test.
            * gcc.dg/analyzer/pr94858-1.c: Add -Wno-analyzer-too-complex.

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

Reply via email to