From be6c485b24f2b47ac85380dd2bea025d353f1f91 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 6 Jan 2021 21:44:07 -0500 Subject: [PATCH] analyzer: fix false leak reports when merging states [PR97074] 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.30.2