Source: void foo(char **args[], int *argc) { *argc = 1; (*args)[0] = __builtin_malloc(42); } Compiler output: $ gcc-12 -Wall -fanalyzer -c -o foo.o foo.c foo.c: In function 'foo': foo.c:4:1: warning: leak of '<unknown>' [CWE-401] [-Wanalyzer-malloc-leak] 4 | } | ^ 'foo': events 1-2 | | 3 | (*args)[0] = __builtin_malloc(42); | | ^~~~~~~~~~~~~~~~~~~~ | | | | | (1) allocated here | 4 | } | | ~ | | | | | (2) '<unknown>' leaks here; was allocated at (1) | Notes: This is only reported with the write to argc happening first, which should be considered completely unrelated to args. Reordering the two statements resolves the analyzer report. Tested versions: gcc 10.3: FAIL gcc 11.2: OK gcc 12.0: FAIL I therefore consider this a regression as it was not reported by gcc 11. Compiler Explorer link: https://gcc.godbolt.org/z/zGanPa3fs
Thanks for filing this bug. Confirmed; am investigating.
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>
This should be fixed on trunk for GCC 13 by the above patch. Unfortunately the patch feels too invasive to backport to GCC 12. I'm going to mark this as resolved.