From 2194a764f13ebb8aad36695f028178e84ca49546 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 20 Feb 2020 14:14:05 -0500
Subject: [PATCH 29/31] FIXME: prevent writing inherited unknown into globals
 or uninit into frames

---
 gcc/analyzer/region-model.cc | 73 +++++++++++++++++++++++++-----------
 gcc/analyzer/region-model.h  |  3 +-
 2 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 30c016a4f95..b2c76e8731a 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1017,7 +1017,7 @@ region::set_value (region_model &model, region_id this_rid, svalue_id rhs_sid,
 	gcc_assert (m_type == sval->get_type ());
     }
 
-  m_sval_id = rhs_sid;
+  set_value_direct (rhs_sid);
 
   /* Update views.
      If this is a view, it becomes its parent's active view.
@@ -1036,6 +1036,16 @@ region::set_value (region_model &model, region_id this_rid, svalue_id rhs_sid,
   }
 }
 
+/* FIXME.  */
+
+void
+region::set_value_direct (svalue_id sid)
+{
+  gcc_assert (get_kind () != RK_GLOBALS
+	      && get_kind () != RK_FRAME);
+  m_sval_id = sid;
+}
+
 /* Make this region (with id THIS_RID) the "active" view of its parent.
    Any other active view has its value set to "unknown" and descendent values
    cleared.
@@ -1074,12 +1084,12 @@ region::become_active_view (region_model &model, region_id this_rid)
 	  if (below_region.region_p (rid))
 	    {
 	      region *other_reg = model.get_region (rid);
-	      other_reg->m_sval_id = svalue_id::null ();
+	      other_reg->set_value_direct (svalue_id::null ());
 	    }
 	}
       region *parent = model.get_region (m_parent_rid);
-      parent->m_sval_id
-	= model.add_svalue (new unknown_svalue (parent->get_type ()));
+      parent->set_value_direct
+	(model.add_svalue (new unknown_svalue (parent->get_type ())));
     }
   else
     {
@@ -1122,11 +1132,11 @@ region::deactivate_view (region_model &model, region_id this_view_rid)
       if (below_view.region_p (rid))
 	{
 	  region *other_reg = model.get_region (rid);
-	  other_reg->m_sval_id = svalue_id::null ();
+	  other_reg->set_value_direct (svalue_id::null ());
 	}
     }
 
-  m_sval_id = model.add_svalue (new unknown_svalue (get_type ()));
+  set_value_direct (model.add_svalue (new unknown_svalue (get_type ())));
 }
 
 /* Get a value for this region, either its value if it has one,
@@ -1164,7 +1174,7 @@ region::get_value (region_model &model, bool non_null,
   if (non_null)
     {
       svalue_id unknown_sid = model.add_svalue (new unknown_svalue (m_type));
-      m_sval_id = unknown_sid;
+      set_value_direct (unknown_sid);
       return unknown_sid;
     }
 
@@ -1181,27 +1191,39 @@ region::get_inherited_child_sid (region *child,
 				 region_model &model,
 				 region_model_context *ctxt)
 {
-  if (m_sval_id.null_p ())
+  svalue_id this_sid = m_sval_id;
+  if (this_sid.null_p ())
     {
       /* Recurse.  */
       if (!m_parent_rid.null_p ())
 	{
 	  region *parent = model.get_region (m_parent_rid);
-	  m_sval_id = parent->get_inherited_child_sid (this, model, ctxt);
+	  this_sid = parent->get_inherited_child_sid (this, model, ctxt);
+	  if (!this_sid.null_p ())
+	    if (model.get_svalue (this_sid)->get_type ())
+	      set_value_direct (this_sid);
 	}
     }
 
-  if (!m_sval_id.null_p ())
+  if (!this_sid.null_p ())
     {
       /* Clone the parent's value, so that attempts to update it
 	 (e.g giving a specific value to an inherited "uninitialized"
 	 value) touch the child, and not the parent.  */
-      svalue *this_value = model.get_svalue (m_sval_id);
+      svalue *this_value = model.get_svalue (this_sid);
       svalue_id new_child_sid
 	= this_value->get_child_sid (this, child, model, ctxt);
       if (ctxt)
-	ctxt->on_inherited_svalue (m_sval_id, new_child_sid);
-      child->m_sval_id = new_child_sid;
+	ctxt->on_inherited_svalue (this_sid, new_child_sid);
+
+      /* Store the inherited value, but not for scope regions
+	 ("globals"  and frame), so as to avoid unnecessary duplication
+	 of "uninit" and "unknown", which would lead to enode cache misses.  */
+      // FIXME: or maybe just don't do it for typeless svalues?
+      if (!new_child_sid.null_p ())
+	if (model.get_svalue (new_child_sid)->get_type ())
+	  child->set_value_direct (new_child_sid);
+
       return new_child_sid;
     }
 
@@ -2137,6 +2159,18 @@ union_region::compare_fields (const union_region &other) const
   return struct_or_union_region::compare_fields (other);
 }
 
+/* class scope_region : public map_region.  */
+
+/* Implementation of region::validate vfunc for scope_region.  */
+
+void
+scope_region::validate (const region_model &model) const
+{
+  map_region::validate (model);
+  // FIXME: ensure frames and the "globals" region don't have values themselves
+  gcc_assert (get_value_direct ().null_p ());
+}
+
 /* class frame_region : public map_region.  */
 
 /* Compare the fields of this frame_region with OTHER, returning true
@@ -2186,15 +2220,6 @@ frame_region::print_fields (const region_model &model,
   pp_printf (pp, ", function: %qs, depth: %i", function_name (m_fun), m_depth);
 }
 
-/* Implementation of region::validate vfunc for frame_region.  */
-
-void
-frame_region::validate (const region_model &model) const
-{
-  map_region::validate (model);
-  // TODO
-}
-
 /* Implementation of region::on_purge vfunc for frame_region.  */
 
 void
@@ -4459,6 +4484,8 @@ public:
       return;
     bitmap_set_bit (m_reachable_rids, idx);
 
+    gcc_assert (m_model->get_region (rid)->get_kind () != RK_FRAME);
+
     if (is_mutable)
       {
 	if (bitmap_bit_p (m_mutable_rids, idx))
@@ -4578,6 +4605,8 @@ region_model::handle_unrecognized_call (const gcall *call,
       if (reachable_regions.mutable_p (iter_rid))
 	{
 	  region *reg = get_region (iter_rid);
+	  if (reg->get_kind () == RK_GLOBALS)
+	    continue;
 
 	  /* Purge any sm-state for any underlying svalue.  */
 	  svalue_id curr_sid = reg->get_value_direct ();
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 65502d0d798..7ff5f5778ab 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -1009,6 +1009,7 @@ public:
 			     pretty_printer *pp) const;
 
  private:
+  void set_value_direct (svalue_id  sid);
   void become_active_view (region_model &model, region_id this_rid);
   void deactivate_any_active_view (region_model &model);
   void deactivate_view (region_model &model, region_id this_view_rid);
@@ -1246,6 +1247,7 @@ class scope_region : public map_region
   }
 
   bool compare_fields (const scope_region &other) const;
+  void validate (const region_model &model) const FINAL OVERRIDE;
 };
 
 /* Concrete region subclass, representing a function frame on the stack,
@@ -1270,7 +1272,6 @@ public:
 		     region_id this_rid,
 		     pretty_printer *pp) const
     FINAL OVERRIDE;
-  void validate (const region_model &model) const FINAL OVERRIDE;
   void on_purge (const purge &p) FINAL OVERRIDE;
   void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE;
 
-- 
2.21.0

