From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id B2068384F4A8; Thu, 24 Nov 2022 01:46:52 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B2068384F4A8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669254412; bh=V4eu7XBsAiB07ouyXHJciR20+sVKjFLyRAm6oJd3kkE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=YW2DIXs4F81o4QMEuI4HxHvFa+7AdHLiimL8ZCV6tFd76aNumORD1p/+lYTGNsu3j /s7hfPUM2W9u9Ho4jYiA7WsdkbzXffJa/u0LZt855xfzGiaVCYZIcTpKcPGlQ+2Ng2 AHhPVOSJmQuWRfnFR4gsaAh6punh+WC0YlUR8Rg4= From: "cvs-commit at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug analyzer/106473] [12/13 Regression] -Wanalyzer-malloc-leak false positive regression when returning heap-allocation through nested pointers Date: Thu, 24 Nov 2022 01:46:50 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: analyzer X-Bugzilla-Version: 12.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: cvs-commit at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: dmalcolm at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D106473 --- Comment #2 from CVS Commits --- The master branch has been updated by David Malcolm : https://gcc.gnu.org/g:ce917b0422c145779b83e005afd8433c0c86fb06 commit r13-4276-gce917b0422c145779b83e005afd8433c0c86fb06 Author: David Malcolm 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 =3D 1; (*args)[0] =3D __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] =3D __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 =3D malloc (4096); else p =3D 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): Likewi= se. (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): Reje= ct 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 =