From f7015288933adf154d6c1d5e293e32652150bb02 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 27 May 2020 16:55:25 -0400
Subject: [PATCH 186/377] 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

