From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id B3BA4385EC56 for ; Thu, 7 Jan 2021 02:48:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B3BA4385EC56 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-172-EX0dfSOnNsG410fPAKVPeA-1; Wed, 06 Jan 2021 21:48:53 -0500 X-MC-Unique: EX0dfSOnNsG410fPAKVPeA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 17796E744 for ; Thu, 7 Jan 2021 02:48:52 +0000 (UTC) Received: from t470.redhat.com (ovpn-112-159.phx2.redhat.com [10.3.112.159]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9EC575B6A2; Thu, 7 Jan 2021 02:48:51 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Subject: [committed] analyzer: fix false leak reports when merging states [PR97074] Date: Wed, 6 Jan 2021 21:48:49 -0500 Message-Id: <20210107024849.3181454-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-13.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2021 02:48:58 -0000 Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to master as r11-6513-gbe6c485b24f2b47ac85380dd2bea025d353f1f91. gcc/analyzer/ChangeLog: PR analyzer/97074 * store.cc (binding_cluster::can_merge_p): Add "out_store" param and pass to calls to binding_cluster::make_unknown_relative_to. (binding_cluster::make_unknown_relative_to): Add "out_store" param. Use it to mark base regions that are pointed to by pointers that become unknown as having escaped. (store::can_merge_p): Pass out_store to binding_cluster::can_merge_p. * store.h (binding_cluster::can_merge_p): Add "out_store" param. (binding_cluster::make_unknown_relative_to): Likewise. * svalue.cc (region_svalue::implicitly_live_p): New vfunc. * svalue.h (region_svalue::implicitly_live_p): New vfunc decl. gcc/testsuite/ChangeLog: PR analyzer/97074 * gcc.dg/analyzer/pr97074.c: New test. --- gcc/analyzer/store.cc | 23 +++++++++++++++--- gcc/analyzer/store.h | 2 ++ gcc/analyzer/svalue.cc | 16 +++++++++++++ gcc/analyzer/svalue.h | 2 ++ gcc/testsuite/gcc.dg/analyzer/pr97074.c | 32 +++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr97074.c diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc index b4a4d5f3bb2..23118d05685 100644 --- a/gcc/analyzer/store.cc +++ b/gcc/analyzer/store.cc @@ -1177,6 +1177,7 @@ bool binding_cluster::can_merge_p (const binding_cluster *cluster_a, const binding_cluster *cluster_b, binding_cluster *out_cluster, + store *out_store, store_manager *mgr, model_merger *merger) { @@ -1197,14 +1198,14 @@ binding_cluster::can_merge_p (const binding_cluster *cluster_a, { gcc_assert (cluster_b != NULL); gcc_assert (cluster_b->m_base_region == out_cluster->m_base_region); - out_cluster->make_unknown_relative_to (cluster_b, mgr); + out_cluster->make_unknown_relative_to (cluster_b, out_store, mgr); return true; } if (cluster_b == NULL) { gcc_assert (cluster_a != NULL); gcc_assert (cluster_a->m_base_region == out_cluster->m_base_region); - out_cluster->make_unknown_relative_to (cluster_a, mgr); + out_cluster->make_unknown_relative_to (cluster_a, out_store, mgr); return true; } @@ -1298,6 +1299,7 @@ binding_cluster::can_merge_p (const binding_cluster *cluster_a, void binding_cluster::make_unknown_relative_to (const binding_cluster *other, + store *out_store, store_manager *mgr) { for (map_t::iterator iter = other->m_map.begin (); @@ -1309,6 +1311,21 @@ binding_cluster::make_unknown_relative_to (const binding_cluster *other, = mgr->get_svalue_manager ()->get_or_create_unknown_svalue (iter_sval->get_type ()); m_map.put (iter_key, unknown_sval); + + /* For any pointers in OTHER, the merger means that the + concrete pointer becomes an unknown value, which could + show up as a false report of a leak when considering what + pointers are live before vs after. + Avoid this by marking the base regions they point to as having + escaped. */ + if (const region_svalue *region_sval + = iter_sval->dyn_cast_region_svalue ()) + { + const region *base_reg + = region_sval->get_pointee ()->get_base_region (); + binding_cluster *c = out_store->get_or_create_cluster (base_reg); + c->mark_as_escaped (); + } } } @@ -2092,7 +2109,7 @@ store::can_merge_p (const store *store_a, const store *store_b, binding_cluster *out_cluster = out_store->get_or_create_cluster (base_reg); if (!binding_cluster::can_merge_p (cluster_a, cluster_b, - out_cluster, mgr, merger)) + out_cluster, out_store, mgr, merger)) return false; } return true; diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h index 2f97f4b68a5..366439ce2dd 100644 --- a/gcc/analyzer/store.h +++ b/gcc/analyzer/store.h @@ -434,9 +434,11 @@ public: static bool can_merge_p (const binding_cluster *cluster_a, const binding_cluster *cluster_b, binding_cluster *out_cluster, + store *out_store, store_manager *mgr, model_merger *merger); void make_unknown_relative_to (const binding_cluster *other_cluster, + store *out_store, store_manager *mgr); void mark_as_escaped (); diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc index b2d98cfcac6..5bbd05e8327 100644 --- a/gcc/analyzer/svalue.cc +++ b/gcc/analyzer/svalue.cc @@ -509,6 +509,22 @@ region_svalue::accept (visitor *v) const m_reg->accept (v); } +/* Implementation of svalue::implicitly_live_p vfunc for region_svalue. */ + +bool +region_svalue::implicitly_live_p (const svalue_set &, + const region_model *model) const +{ + /* Pointers into clusters that have escaped should be treated as live. */ + const region *base_reg = get_pointee ()->get_base_region (); + const store *store = model->get_store (); + if (const binding_cluster *c = store->get_cluster (base_reg)) + if (c->escaped_p ()) + return true; + + return false; +} + /* Evaluate the condition LHS OP RHS. Subroutine of region_model::eval_condition for when we have a pair of pointers. */ diff --git a/gcc/analyzer/svalue.h b/gcc/analyzer/svalue.h index 336c0b601c4..0703cac8bb3 100644 --- a/gcc/analyzer/svalue.h +++ b/gcc/analyzer/svalue.h @@ -194,6 +194,8 @@ public: void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; void accept (visitor *v) const FINAL OVERRIDE; + bool implicitly_live_p (const svalue_set &, + const region_model *) const FINAL OVERRIDE; const region * get_pointee () const { return m_reg; } diff --git a/gcc/testsuite/gcc.dg/analyzer/pr97074.c b/gcc/testsuite/gcc.dg/analyzer/pr97074.c new file mode 100644 index 00000000000..ccb3b61bd41 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/pr97074.c @@ -0,0 +1,32 @@ +#include "analyzer-decls.h" +#define NULL ((void *)0) + +void *x, *y; + +void test_1 (int flag) +{ + void *p = __builtin_malloc (1024); + if (flag) + x = p; + else + y = p; +} /* { dg-bogus "leak" } */ + +struct s2 +{ + void *f1; + void *f2; +}; + +struct s2 test_2 (int flag) +{ + struct s2 r; + r.f1 = NULL; + r.f2 = NULL; + void *p = __builtin_malloc (1024); + if (flag) + r.f1 = p; + else + r.f2 = p; + return r; +} -- 2.26.2