[Bug analyzer/106473] [12/13 Regression] -Wanalyzer-malloc-leak false positive regression when returning heap-allocation through nested pointers

["cvs-commit at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

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

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

commit r13-4276-gce917b0422c145779b83e005afd8433c0c86fb06
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Nov 23 20:43:33 2022 -0500

    analyzer: revamp of heap-allocated regions [PR106473]

    PR analyzer/106473 reports a false positive from -Wanalyzer-malloc-leak
    on:

      void foo(char **args[], int *argc) {
          *argc = 1;
          (*args)[0] = __builtin_malloc(42);
      }

    The issue is that at the write to *argc we don't know if argc could
    point within *args, and so we conservatiely set *args to be unknown.
    At the write "(*args)[0] = __builtin_malloc(42)" we have the result of
    the allocation written through an unknown pointer, so we mark the
    heap_allocated_region as having escaped.
    Unfortunately, within store::canonicalize we overzealously purge the
    heap allocated region, losing the information that it has escaped, and
    thus errnoeously report a leak.

    The first part of the fix is to update store::canonicalize so that it
    doesn't purge heap_allocated_regions that are marked as escaping.

    Doing so fixes the leak false positive, but leads to various state
    explosions relating to anywhere we have a malloc/free pair in a loop,
    where the analysis of the iteration appears to only have been reaching
    a fixed state due to a bug in the state merger code that was erroneously
    merging state about the region allocated in one iteration with that
    of another.  On touching that, the analyzer fails to reach a fixed state
    on any loops containing a malloc/free pair, since each analysis of a
    malloc was creating a new heap_allocated_region instance.

    Hence the second part of the fix is to revamp how heap_allocated_regions
    are managed within the analyzer.  Rather than create a new one at each
    analysis of a malloc call, instead we reuse them across the analysis,
    only creating a new one if the current path's state is referencing all
    of the existing ones.  Hence the heap_allocated_region instances get
    used in a fixed order along every analysis path, so e.g. at:

      if (flag)
        p = malloc (4096);
      else
        p = malloc (1024);

    both paths now use the same heap_allocated_region for their malloc
    calls - but we still end up with two enodes after the CFG merger, by
    rejecting merger of states with non-equal dynamic extents.

    gcc/analyzer/ChangeLog:
            PR analyzer/106473
            * call-summary.cc
            (call_summary_replay::convert_region_from_summary_1): Update for
            change to creation of heap-allocated regions.
            * program-state.cc (test_program_state_1): Likewise.
            (test_program_state_merging): Likewise.
            * region-model-impl-calls.cc (kf_calloc::impl_call_pre): Likewise.
            (kf_malloc::impl_call_pre): Likewise.
            (kf_operator_new::impl_call_pre): Likewise.
            (kf_realloc::impl_call_postsuccess_with_move::update_model):
Likewise.
            * region-model-manager.cc
            (region_model_manager::create_region_for_heap_alloc): Convert
            to...
            (region_model_manager::get_or_create_region_for_heap_alloc):
            ...this, reusing an existing region if it's unreferenced in the
            client state.
            * region-model-manager.h (region_model_manager::get_num_regions):
New.
             (region_model_manager::create_region_for_heap_alloc): Convert
to...
             (region_model_manager::get_or_create_region_for_heap_alloc):
...this.
            * region-model.cc (region_to_value_map::can_merge_with_p): Reject
            merger when the values are different.
            (region_model::create_region_for_heap_alloc): Convert to...
            (region_model::get_or_create_region_for_heap_alloc): ...this.
            (region_model::get_referenced_base_regions): New.
            (selftest::test_state_merging):  Update for change to creation of
            heap-allocated regions.
            (selftest::test_malloc_constraints): Likewise.
            (selftest::test_malloc): Likewise.
            * region-model.h: Include "sbitmap.h".
            (region_model::create_region_for_heap_alloc): Convert to...
            (region_model::get_or_create_region_for_heap_alloc): ...this.
            (region_model::get_referenced_base_regions): New decl.
            * store.cc (store::canonicalize): Don't purge a heap-allocated
region
            that's been marked as escaping.

    gcc/testsuite/ChangeLog:
            PR analyzer/106473
            * gcc.dg/analyzer/aliasing-pr106473.c: New test.
            * gcc.dg/analyzer/allocation-size-2.c: Add
            -fanalyzer-fine-grained".
            * gcc.dg/analyzer/allocation-size-3.c: Likewise.
            * gcc.dg/analyzer/explode-1.c: Mark leak with XFAIL.
            * gcc.dg/analyzer/explode-3.c: New test.
            * gcc.dg/analyzer/malloc-reuse.c: New test.

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