From f7015288933adf154d6c1d5e293e32652150bb02 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 27 May 2020 16:55:25 -0400 Subject: [PATCH 186/315] FIXME: store2::set_value: check both ways for clusters invalidating other clusters, and enable the check --- gcc/analyzer/region-model2.cc | 10 +++ gcc/analyzer/region-model2.h | 2 + gcc/analyzer/store2.cc | 153 ++++++++++++++++++++-------------- gcc/analyzer/store2.h | 6 +- 4 files changed, 106 insertions(+), 65 deletions(-) diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc index 323b040b125..f69b1bdbf72 100644 --- a/gcc/analyzer/region-model2.cc +++ b/gcc/analyzer/region-model2.cc @@ -927,6 +927,16 @@ region2::get_base_region () const /* FIXME. */ +tree +region2::maybe_get_decl () const +{ + if (const decl_region2 *decl_reg = dyn_cast_decl_region2 ()) + return decl_reg->get_decl (); + return NULL_TREE; +} + +/* FIXME. */ + region_offset region2::get_offset () const { diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h index 2b927ea836d..fbc4578161c 100644 --- a/gcc/analyzer/region-model2.h +++ b/gcc/analyzer/region-model2.h @@ -870,6 +870,8 @@ public: const region2 *get_parent_region () const { return m_parent; } const region2 *get_base_region () const; + tree maybe_get_decl () const; + #if 0 void set_value (region_model2 &model, region_id this_rid, svalue2_id rhs_sid, region_model2_context *ctxt); diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc index 20d014ff916..9c551b45b75 100644 --- a/gcc/analyzer/store2.cc +++ b/gcc/analyzer/store2.cc @@ -296,6 +296,14 @@ binding_cluster2::operator== (const binding_cluster2 &other) const /* FIXME. */ +bool +binding_cluster2::symbolic_p () const +{ + return m_base_region->get_kind () == region2::RK_SYMBOLIC; +} + +/* FIXME. */ + void binding_cluster2::dump_to_pp (pretty_printer *pp, bool simple, bool multiline) const @@ -1066,86 +1074,105 @@ store2::set_value (store2_manager *mgr, const region2 *lhs_reg, rhs_sval = simplify_for_binding (rhs_sval); - const region2 *base_reg = lhs_reg->get_base_region (); - binding_cluster2 *cluster = get_or_create_cluster (base_reg); - cluster->bind (mgr, lhs_reg, rhs_sval, kind); + const region2 *lhs_base_reg = lhs_reg->get_base_region (); + binding_cluster2 *lhs_cluster = get_or_create_cluster (lhs_base_reg); + lhs_cluster->bind (mgr, lhs_reg, rhs_sval, kind); -#if 0 - if (const symbolic_region2 *sym_base_reg - = base_reg->dyn_cast_symbolic_region2 ()) +#if 1 + /* Bindings to a cluster can affect other clusters if a symbolic + base region is involved. + Writes to concrete clusters can't affect other concrete clusters, + but can affect symbolic clusters. + Writes to symbolic clusters can affect both concrete and symbolic + clusters. + Invalidate our knowledge of other clusters that might have been + affected by the write. */ + /* FIXME: but what about things that don't even have a + binding cluster yet? + Does initial_value mean + - "the state of the region at the first time that we look at it" or + - "the state of the region at the start of any paths"? */ + for (cluster_map_t::iterator iter = m_cluster_map.begin (); + iter != m_cluster_map.end (); ++iter) { - /* If we set the value to a symbolic region, the code might have - written to other regions. Invalidate our knowledge of regions - that might have been affected by the write. */ - /* FIXME: but what about things that don't even have a - binding cluster yet? - Does initial_value mean - - "the state of the region at the first time that we look at it" or - - "the state of the region at the start of any paths"? */ - for (cluster_map_t::iterator iter = m_cluster_map.begin (); - iter != m_cluster_map.end (); ++iter) + const region2 *iter_base_reg = (*iter).first; + binding_cluster2 *iter_cluster = (*iter).second; + if (iter_base_reg != lhs_base_reg + && (lhs_cluster->symbolic_p () || iter_cluster->symbolic_p ())) { - const region2 *iter_base_reg = (*iter).first; - binding_cluster2 *iter_cluster = (*iter).second; - if (iter_base_reg != base_reg) + tristate t_alias = eval_alias (lhs_base_reg, iter_base_reg); + switch (t_alias.get_value ()) { - tristate t_alias = eval_alias (sym_base_reg, iter_base_reg); - switch (t_alias.get_value ()) - { - default: - gcc_unreachable (); - - case tristate::TS_UNKNOWN: - { - iter_cluster->mark_region_as_unknown (mgr, iter_base_reg); - //iter_cluster->m_touched = true; - } - break; - - case tristate::TS_TRUE: - { - gcc_unreachable (); - // TODO: apply the same change to iter_cluster - /* TODO: guard against infinite recursion - maybe with a hash_set of processed regions? */ - } - break; - - case tristate::TS_FALSE: - /* If they can't be aliases, then don't invalidate this - cluster. */ - break; - } + default: + gcc_unreachable (); + + case tristate::TS_UNKNOWN: + { + iter_cluster->mark_region_as_unknown (mgr, iter_base_reg); + // TODO: something finer-grained + //iter_cluster->m_touched = true; + } + break; + + case tristate::TS_TRUE: + { + gcc_unreachable (); + // TODO: apply the same change to iter_cluster + /* TODO: guard against infinite recursion + maybe with a hash_set of processed regions? */ + } + break; + + case tristate::TS_FALSE: + /* If they can't be aliases, then don't invalidate this + cluster. */ + break; } } } #endif } -/* Determine if SYM_BASE_REG could be an alias of OTHER_BASE_REG. */ +/* Determine if BASE_REG_A could be an alias of BASE_REG_B. */ tristate -store2::eval_alias (const symbolic_region2 *sym_base_reg, - const region2 *other_base_reg) +store2::eval_alias (const region2 *base_reg_a, + const region2 *base_reg_b) { - const svalue2 *sval = sym_base_reg->get_pointer (); - - if (const decl_region2 *decl_reg = other_base_reg->dyn_cast_decl_region2 ()) + /* SSA names can't alias. */ + tree decl_a = base_reg_a->maybe_get_decl (); + if (decl_a && TREE_CODE (decl_a) == SSA_NAME) + return tristate::TS_FALSE; + tree decl_b = base_reg_b->maybe_get_decl (); + if (decl_b && TREE_CODE (decl_b) == SSA_NAME) + return tristate::TS_FALSE; + + if (const symbolic_region2 *sym_reg_a + = base_reg_a->dyn_cast_symbolic_region2 ()) { - tree decl = decl_reg->get_decl (); - if (TREE_CODE (decl) == SSA_NAME) + const svalue2 *sval_a = sym_reg_a->get_pointer (); + if (sval_a->get_kind () == svalue2::SK_INITIAL + && decl_b + && !is_global_var (decl_b) + && !escaped_p (base_reg_b)) { - /* Can't have pointers to SSA names. */ + /* The initial value of a pointer can't point to an + unescaped local. */ + return tristate::TS_FALSE; + } + } + if (const symbolic_region2 *sym_reg_b + = base_reg_b->dyn_cast_symbolic_region2 ()) + { + const svalue2 *sval_b = sym_reg_b->get_pointer (); + if (sval_b->get_kind () == svalue2::SK_INITIAL + && decl_a + && !is_global_var (decl_a) && !escaped_p (base_reg_a)) + { + /* The initial value of a pointer can't point to an + unescaped local. */ return tristate::TS_FALSE; } - - if (sval->get_kind () == svalue2::SK_INITIAL) - if (!is_global_var (decl) && !escaped_p (decl_reg)) - { - /* The initial value of a pointer can't point to an - unescaped local. */ - return tristate::TS_FALSE; - } } // TODO: any other logic? diff --git a/gcc/analyzer/store2.h b/gcc/analyzer/store2.h index e568db81573..e75b10325fd 100644 --- a/gcc/analyzer/store2.h +++ b/gcc/analyzer/store2.h @@ -283,6 +283,8 @@ public: return !(*this == other); } + bool symbolic_p () const; + void dump_to_pp (pretty_printer *pp, bool simple, bool multiline) const; void bind (store2_manager *mgr, const region2 *, const svalue2 *, @@ -422,8 +424,8 @@ public: cluster_map_t::iterator begin () const { return m_cluster_map.begin (); } cluster_map_t::iterator end () const { return m_cluster_map.end (); } - tristate eval_alias (const symbolic_region2 *sym_base_reg, - const region2 *other_base_reg); + tristate eval_alias (const region2 *base_reg_a, + const region2 *base_reg_b); private: void remove_overlapping_bindings (store2_manager *mgr, const region2 *reg); -- 2.26.2