From 7591dcd14aa9c358fab71e40de3e70a414d7ebb3 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 27 Apr 2020 11:43:26 -0400 Subject: [PATCH 078/179] FIXME: handle escaped regions --- gcc/analyzer/region-model2.cc | 64 ++++++------------- gcc/analyzer/store2.cc | 60 ++++++++++++++--- gcc/analyzer/store2.h | 25 ++++++-- gcc/testsuite/gcc.dg/analyzer/data-model-1.c | 3 - gcc/testsuite/gcc.dg/analyzer/unknown-fns-2.c | 36 +++++++++++ 5 files changed, 128 insertions(+), 60 deletions(-) diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc index f5c37d2991b..f3d406d0788 100644 --- a/gcc/analyzer/region-model2.cc +++ b/gcc/analyzer/region-model2.cc @@ -4379,18 +4379,26 @@ public: : m_store (store), m_reachable_regs (), m_mutable_regs () {} - static void add_any_globals_cb (const region2 *base_reg, - reachable_region2s *this_ptr) + /* Callback called for each cluster when initializing this object. */ + static void init_cluster_cb (const region2 *base_reg, + reachable_region2s *this_ptr) { - this_ptr->add_any_globals (base_reg); + this_ptr->init_cluster (base_reg); } - void add_any_globals (const region2 *base_reg) + /* Called for each cluster when initializing this object. */ + void init_cluster (const region2 *base_reg) { + /* Mark any globals as mutable (and traverse what they point to). */ const region2 *parent = base_reg->get_parent_region (); gcc_assert (parent); if (parent->get_kind () == region2::RK_GLOBALS) add (base_reg, true); + + /* Mark any clusters that already escaped in previous unknown calls + as mutable (and traverse what they currently point to). */ + if (m_store->escaped_p (base_reg)) + add (base_reg, true); } /* Lazily mark the cluster containing REG as being reachable, recursively @@ -4417,8 +4425,8 @@ public: } /* If any values within the cluster are pointers, add the pointee. */ - binding_cluster2 *bind_cluster = m_store->get_cluster (base_reg); - bind_cluster->for_each_value (handle_sval_cb, this); + if (binding_cluster2 *bind_cluster = m_store->get_cluster (base_reg)) + bind_cluster->for_each_value (handle_sval_cb, this); } static void handle_sval_cb (svalue2 *sval, reachable_region2s *this_ptr) @@ -4452,7 +4460,7 @@ public: iter != m_mutable_regs.end (); ++iter) { const region2 *base_reg = *iter; - m_store->set_cluster_to_unknown (mgr, base_reg); + m_store->on_escape_to_unknown_fncall (mgr, base_reg); } } @@ -4486,14 +4494,6 @@ public: pp_flush (&pp); } -#if 0 - bool mutable_p (region2 *reg) - { - gcc_assert (reg); - return bitmap_bit_p (m_mutable_regs, rid.as_int ()); - } -#endif - private: store2 *m_store; @@ -4523,14 +4523,11 @@ region_model2::handle_unrecognized_call (const gcall *call, /* Determine the reachable region2s and their mutability. */ { - /* Globals. */ - m_store.for_each_cluster (reachable_region2s::add_any_globals_cb, + /* Add globals and regions that already escaped in previous + unknown calls. */ + m_store.for_each_cluster (reachable_region2s::init_cluster_cb, &reachable_regions); -#if 0 - region2 *globals_reg = get_globals_region2 *(); - if (!globals_reg.null_p ()) - reachable_region2s.add (globals_reg, true); -#endif + /* Params that are pointers. */ tree iter_param_types = NULL_TREE; if (fndecl) @@ -4562,29 +4559,8 @@ region_model2::handle_unrecognized_call (const gcall *call, } } -#if 1 reachable_regions.update_bindings (m_mgr->get_store2_manager ()); -#else - /* OK: we now have all reachable region2s. - Set them all to new unknown values. */ - for (unsigned i = 0; i < get_num_region2s (); i++) - { - region2 *iter_reg = region2_id::from_int (i); - if (reachable_region2s.mutable_p (iter_reg)) - { - region2 *reg = get_region2 (iter_reg); - - /* Purge any sm-state for any underlying svalue2. */ - svalue2 *curr_sval = reg->get_value_direct (); - if (!curr_sval.null_p ()) - ctxt->on_unknown_change (curr_sval); - - set_to_new_unknown_value (iter_reg, - reg->get_type (), - ctxt); - } - } -#endif + /* TODO: we also need to purge sm-state for any underlying svalue2. */ #if 0 /* Purge sm-state for any remaining svalue2s that point to region2s that diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc index 650133e9836..b6b51fe8898 100644 --- a/gcc/analyzer/store2.cc +++ b/gcc/analyzer/store2.cc @@ -175,6 +175,12 @@ binding_cluster2::dump_to_pp (pretty_printer *pp, bool simple, binding_key2s.qsort (binding_key2::cmp_ptrs); + if (m_escaped) + { + pp_string (pp, " ESCAPED"); + pp_newline (pp); + } + const binding_key2 *key; unsigned i; FOR_EACH_VEC_ELT (binding_key2s, i, key) @@ -216,14 +222,12 @@ binding_cluster2::bind (store2_manager *mgr, m_map.put (binding, val); } -/* FIXME. - Return true if the cluster is now empty of bindings. */ +/* FIXME. */ -bool +void binding_cluster2::clobber_region (store2_manager *mgr, region2 *reg) { remove_overlapping_bindings (mgr, reg); - return m_map.elements () == 0; } svalue2 * @@ -295,10 +299,29 @@ binding_cluster2::remove_overlapping_bindings (store2_manager *mgr, /* FIXME. */ void -binding_cluster2::set_cluster_to_unknown (store2_manager *mgr) +binding_cluster2::on_escape_to_unknown_fncall (store2_manager *mgr) { m_map.empty (); // TODO: need to add an "unknown" default value + + /* Mark this cluster as having escaped. */ + /* TODO: but maybe not if it's a global? + they have automatically escaped already, and we want to be able + to remove the cluster from the store if possible. + ...but what about file-local statics? + ...but do we want to be able to still mark globals with an + unknown value, to distinguish them from the "initial" svalue? */ + m_escaped = true; +} + +/* Return true if this binding_cluster2 has no information + i.e. if there are no bindings, and it hasn't been marked as having + escaped. */ + +bool +binding_cluster2::redundant_p () const +{ + return m_map.elements () == 0 && !m_escaped; } /* class store2_manager. */ @@ -509,7 +532,8 @@ store2::clobber_region (store2_manager *mgr, region2 *reg) if (!slot) return; binding_cluster2 *cluster = *slot; - if (cluster->clobber_region (mgr, reg)) + cluster->clobber_region (mgr, reg); + if (cluster->redundant_p ()) { delete cluster; m_cluster_map.remove (base_reg); @@ -536,13 +560,31 @@ store2::get_cluster (const region2 *base_reg) isn't reachable from args of those calls. */ void -store2::set_cluster_to_unknown (store2_manager *mgr, const region2 *base_reg) +store2::on_escape_to_unknown_fncall (store2_manager *mgr, + const region2 *base_reg) { gcc_assert (base_reg); gcc_assert (base_reg->get_base_region () == base_reg); if (binding_cluster2 **cluster_slot = m_cluster_map.get (base_reg)) - (*cluster_slot)->set_cluster_to_unknown (mgr); + (*cluster_slot)->on_escape_to_unknown_fncall (mgr); + // TODO + /* FIXME: but don't bother marking every global as escaped; instead, + clear these. */ +} + +/* FIXME. */ + +bool +store2::escaped_p (const region2 *base_reg) const +{ + gcc_assert (base_reg); + gcc_assert (base_reg->get_base_region () == base_reg); + + if (binding_cluster2 **cluster_slot + = const_cast (m_cluster_map).get (base_reg)) + return (*cluster_slot)->escaped_p (); + return false; } /* FIXME. */ @@ -551,7 +593,7 @@ void store2::remove_overlapping_bindings (store2_manager *mgr, region2 *reg) { const region2 *base_reg = reg->get_base_region (); - if (reg == base_reg) + if (reg == base_reg && !escaped_p (base_reg)) { /* Remove whole cluster (if any). */ m_cluster_map.remove (base_reg); diff --git a/gcc/analyzer/store2.h b/gcc/analyzer/store2.h index cde6c263171..2fe0c830f02 100644 --- a/gcc/analyzer/store2.h +++ b/gcc/analyzer/store2.h @@ -155,6 +155,8 @@ private: class binding_cluster2 { public: + binding_cluster2 () : m_map (), m_escaped (false) {} + bool operator== (const binding_cluster2 &other) const; bool operator!= (const binding_cluster2 &other) const { @@ -170,7 +172,7 @@ public: const region2 *, const svalue2 *) const; #endif - bool clobber_region (store2_manager *mgr, region2 *reg); + void clobber_region (store2_manager *mgr, region2 *reg); svalue2 *get_binding (store2_manager *mgr, region2 *reg, binding_key2::kind kind) const; @@ -184,15 +186,27 @@ public: cb ((*iter).second, user_data); } - void set_cluster_to_unknown (store2_manager *mgr); + void on_escape_to_unknown_fncall (store2_manager *mgr); + + bool escaped_p () const { return m_escaped; } + + bool redundant_p () const; private: void get_overlapping_bindings (store2_manager *mgr, region2 *reg, auto_vec *out); typedef hash_map map_t; - //typedef typename map_t::iterator iterator_t; map_t m_map; + + /* Has a pointer to this cluster "escaped" into a part of the program + we don't know about (via a call to a function with an unknown body). + Such regions could be overwritten when other such functions are called, + even if the region is no longer reachable by pointers that we are + tracking. */ + /* FIXME: Don't track this for globals, as they have automatically escaped + already. TODO: but what about file-local statics? */ + bool m_escaped; }; /* The mapping from regions to values. @@ -232,7 +246,10 @@ public: cb ((*iter).first, user_data); } - void set_cluster_to_unknown (store2_manager *mgr, const region2 *base_reg); + void on_escape_to_unknown_fncall (store2_manager *mgr, + const region2 *base_reg); + + bool escaped_p (const region2 *reg) const; private: void remove_overlapping_bindings (store2_manager *mgr, region2 *reg); diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c index e3c0339d0df..729f8b12577 100644 --- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c @@ -326,9 +326,6 @@ void test_16e (int i) __analyzer_eval (j == i); /* { dg-warning "UNKNOWN" } */ } -/* TODO: and more complicated graph-like examples, where anything that's - reachable from the pointer might be modified. */ - void test_17 (int i) { int j = 42; diff --git a/gcc/testsuite/gcc.dg/analyzer/unknown-fns-2.c b/gcc/testsuite/gcc.dg/analyzer/unknown-fns-2.c index 61194ab0b33..42d6009d4db 100644 --- a/gcc/testsuite/gcc.dg/analyzer/unknown-fns-2.c +++ b/gcc/testsuite/gcc.dg/analyzer/unknown-fns-2.c @@ -53,3 +53,39 @@ void test_2 (void) unknown_fn (NULL); __analyzer_eval (i == 17); /* { dg-warning "UNKNOWN" } */ } + +struct used_by_test_3 +{ + int *int_ptr; +}; + +void test_3 (void) +{ + int i; + + struct used_by_test_3 s; + s.int_ptr = &i; + + i = 42; + __analyzer_eval (i == 42); /* { dg-warning "TRUE" } */ + + unknown_fn (NULL); + __analyzer_eval (i == 42); /* { dg-warning "TRUE" } */ + __analyzer_eval (s.int_ptr == &i); /* { dg-warning "TRUE" } */ + + /* i should escape here. */ + unknown_fn (&s); + __analyzer_eval (i == 42); /* { dg-warning "UNKNOWN" } */ + __analyzer_eval (s.int_ptr == &i); /* { dg-warning "UNKNOWN" } */ + + s.int_ptr = NULL; + __analyzer_eval (s.int_ptr == NULL); /* { dg-warning "TRUE" } */ + + i = 17; + __analyzer_eval (i == 17); /* { dg-warning "TRUE" } */ + + /* Even though nothing we know about points to i, it escaped + above, so unknown_fn could write to it. */ + unknown_fn (NULL); + __analyzer_eval (i == 17); /* { dg-warning "UNKNOWN" } */ +} -- 2.21.0