From 62d3a17253b4d4a26e510fe670e5226ff94737d9 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 29 Oct 2019 11:12:58 -0400
Subject: [PATCH 084/119] FIXME: more on view activation

---
 gcc/analyzer/region-model.cc                  | 180 ++++++++++++++++----------
 gcc/analyzer/region-model.h                   |  40 +++++-
 gcc/testsuite/gcc.dg/analyzer/data-model-13.c |  13 ++
 gcc/testsuite/gcc.dg/analyzer/data-model-14.c |  21 +++
 4 files changed, 186 insertions(+), 68 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/data-model-13.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/data-model-14.c

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 6381e8c..052c8de 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -161,6 +161,14 @@ svalue_id::dump_node_name_to_pp (pretty_printer *pp) const
   pp_printf (pp, "svalue_%i", m_idx);
 }
 
+/* Assert that this object is valid (w.r.t. MODEL).  */
+
+void
+svalue_id::validate (const region_model &model) const
+{
+  gcc_assert (null_p () || m_idx < (int)model.get_num_svalues ());
+}
+
 ////////////////////////////////////////////////////////////////////////////
 
 /* class region_id.  */
@@ -185,6 +193,14 @@ region_id::dump_node_name_to_pp (pretty_printer *pp) const
   pp_printf (pp, "region_%i", m_idx);
 }
 
+/* Assert that this object is valid (w.r.t. MODEL).  */
+
+void
+region_id::validate (const region_model &model) const
+{
+  gcc_assert (null_p () || m_idx < (int)model.get_num_regions ());
+}
+
 ////////////////////////////////////////////////////////////////////////////
 
 /* class id_set.  */
@@ -1079,18 +1095,15 @@ region::set_value (region_model &model, region_id this_rid, svalue_id rhs_sid)
       if (sval->get_type ())
 	rhs_sid = model.maybe_cast (m_type, rhs_sid);
 
-      /* FIXME: we can't enable this until maybe_cast works fully.  */
-#if 1
       sval = model.get_svalue (rhs_sid);
       if (sval->get_type ())
 	gcc_assert (m_type == sval->get_type ());
-#endif
     }
 
   m_sval_id = rhs_sid;
 
-  // TODO: views
-  {
+  /* Update views.  */
+   {
     /* FIXME: find other views for same region.  */
     /* FIXME: set to "unknown" all other views for the same region,
        and recursively clear their child values.
@@ -1106,13 +1119,25 @@ region::set_value (region_model &model, region_id this_rid, svalue_id rhs_sid)
        active view might imply we've already invalidated inactive views,
        perhaps? */
     /* If this is a view, it becomes its parent's active view.
-       All other views have their values invalidated.  */
+       If there was already an active views, invalidate its value; otherwise
+       if the parent itself had a value, invalidate it.
+       If it's not a view, then deactivate any view that is active on this
+       region.  */
     if (m_is_view)
       become_active_view (model, this_rid);
+    else
+      {
+	deactivate_any_active_view (model, this_rid);
+	gcc_assert (m_active_view_rid.null_p ());
+      }
   }
 }
 
-/* FIXME.  */
+/* 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.
+   If there wasn't an active view, then set the parent's value to unknown, and
+   clear its descendent values (apart from this view).  */
 
 void
 region::become_active_view (region_model &model, region_id this_rid)
@@ -1122,7 +1147,6 @@ region::become_active_view (region_model &model, region_id this_rid)
   region *parent_reg = model.get_region (m_parent_rid);
   gcc_assert (parent_reg);
 
-
   region_id old_active_view_rid = parent_reg->m_active_view_rid;
 
   if (old_active_view_rid == this_rid)
@@ -1136,62 +1160,71 @@ region::become_active_view (region_model &model, region_id this_rid)
 
   if (old_active_view_rid.null_p ())
     {
-      /* No previous active view; there ought not to be anything to
-	 invalidate.  */
-      return;
+      /* No previous active view, but the parent and its other children
+	 might have values.
+	 If so, invalidate those values - but not that of the new view.  */
+      region_id_set below_region (&model);
+      model.get_descendents (m_parent_rid, &below_region, this_rid);
+      // FIXME: use an iterator class for this (or a FOR_EACH macro)
+      for (unsigned i = 0; i < model.get_num_regions (); i++)
+	{
+	  region_id rid (region_id::from_int (i));
+	  if (below_region.region_p (rid))
+	    {
+	      region *other_reg = model.get_region (rid);
+	      other_reg->m_sval_id = svalue_id::null ();
+	    }
+	}
+      region *parent = model.get_region (m_parent_rid);
+      parent->m_sval_id
+	= model.add_svalue (new unknown_svalue (parent->get_type ()));
     }
+  else
+    {
+      /* If there was an active view, invalidate it.  */
+      region *old_active_view = model.get_region (old_active_view_rid);
+      old_active_view->deactivate_view (model, old_active_view_rid);
+    }
+}
+
+/* If this region (with id THIS_RID) has an active view, deactivate it,
+   clearing m_active_view_rid.  */
+
+void
+region::deactivate_any_active_view (region_model &model, region_id this_rid)
+{
+  if (m_active_view_rid.null_p ())
+    return;
+  region *view = model.get_region (m_active_view_rid);
+  view->deactivate_view (model, m_active_view_rid);
+  m_active_view_rid = region_id::null ();
+}
+
+/* Clear any values for regions below THIS_RID.
+   Set the view's value to unknown.  */
 
-  /* Purge values from old_active_view_rid and all its
+void
+region::deactivate_view (region_model &model, region_id this_view_rid)
+{
+  gcc_assert (is_view_p ());
+
+  /* Purge values from old_active_this_view_rid and all its
      descendents.  Potentially we could use a poison value
      for this, but let's use unknown for now.  */
-  region_id_set below_other_view (&model);
-  model.get_descendents (old_active_view_rid, &below_other_view);
+  region_id_set below_view (&model);
+  model.get_descendents (this_view_rid, &below_view, region_id::null ());
 
   for (unsigned i = 0; i < model.get_num_regions (); i++)
     {
       region_id rid (region_id::from_int (i));
-      if (below_other_view.region_p (rid))
+      if (below_view.region_p (rid))
 	{
 	  region *other_reg = model.get_region (rid);
 	  other_reg->m_sval_id = svalue_id::null ();
 	}
     }
 
-  region *old_active_view = model.get_region (old_active_view_rid);
-  old_active_view->m_sval_id
-    = model.add_svalue (new unknown_svalue (old_active_view->get_type ()));
-
-#if 0
-  /* Iterate over the other views of the parent's region.  */
-  unsigned i;
-  region_id *sibling_rid;
-  FOR_EACH_VEC_ELT (parent_reg->m_view_rids, i, sibling_rid)
-    {
-      if (*sibling_rid != this_rid)
-	{
-	  /* Purge values from sibling_rid and all its
-	     descendents.  Potentially we could use a poison value
-	     for this, but let's use unknown for now.  */
-	  region_id_set below_other_view (&model);
-	  model.get_descendents (*sibling_rid, &below_other_view);
-
-	  for (unsigned i = 0; i < model.get_num_regions (); i++)
-	    {
-	      region_id rid (region_id::from_int (i));
-	      if (below_other_view.region_p (rid))
-		{
-		  region *other_reg = model.get_region (rid);
-		  if (!other_reg->m_sval_id.null_p ())
-		    {
-		      
-		    }
-		    gcc_unreachable (); // TODO
-		}
-	    }
-	}
-    }
-  //#error
-#endif
+  m_sval_id = model.add_svalue (new unknown_svalue (get_type ()));
 }
 
 /* Get a value for this region, either its value if it has one,
@@ -1462,21 +1495,20 @@ POP_IGNORE_WFORMAT
 }
 
 /* FIXME.  */
+
 void
 region::validate (const region_model *model) const
 {
-  svalue_id sid = get_value_direct ();
-  if (!sid.null_p ())
-    gcc_assert (sid.as_int () < model->get_num_svalues ());
+  m_parent_rid.validate (*model);
+  m_sval_id.validate (*model);
   unsigned i;
   region_id *view_rid;
   FOR_EACH_VEC_ELT (m_view_rids, i, view_rid)
     {
       gcc_assert (!view_rid->null_p ());
-      gcc_assert (view_rid->as_int () < model->get_num_regions ());
+      view_rid->validate (*model);
     }
-  gcc_assert (m_active_view_rid.null_p ()
-	      || m_active_view_rid.as_int () < model->get_num_regions ());
+  m_active_view_rid.validate (*model);
 }
 
 /* Apply MAP to svalue_ids to this region.  This updates the value
@@ -3889,7 +3921,8 @@ region_model::on_assignment (const gassign *assign, region_model_context *ctxt)
 	/* FIXME: offset is in bytes; we presumably want to
 	   map back to an index.  */
 #endif
-	region_id view_rid = add_view (ptr_sid, offset_sid, ctxt);
+	region_id view_rid = add_view (TREE_TYPE (lhs),
+				       ptr_sid, offset_sid, ctxt);
 
 	svalue_id element_ptr_sid
 	  = get_or_create_ptr_svalue (TREE_TYPE (ptr), view_rid);
@@ -4366,7 +4399,7 @@ region_model::get_lvalue_1 (path_var pv, region_model_context *ctxt)
 	svalue_id offset_sid = get_rvalue (offset, ctxt);
 
 #if 1
-	return add_view (ptr_sid, offset_sid, ctxt);
+	return add_view (TREE_TYPE (expr), ptr_sid, offset_sid, ctxt);
 #else
 	return deref_rvalue (ptr, ctxt);
 #endif
@@ -4671,6 +4704,10 @@ region_model::get_field_region (region_id struct_or_union_rid, tree field)
   region_id child_rid
     = sou_reg->get_or_create (this, struct_or_union_rid, field,
 			      TREE_TYPE (field));
+#if 0
+  if (sou_reg->get_kind == RK_UNION)
+    sou_reg->add_view (child_rid);
+#endif
   return child_rid;
 }
 
@@ -4715,9 +4752,9 @@ region_model::deref_rvalue (svalue_id ptr_sid, region_model_context *ctxt)
 
     case SK_POISONED:
       {
+#if 0
 	poisoned_svalue *poisoned_sval = as_a <poisoned_svalue *> (ptr_svalue);
 	enum poison_kind pkind = poisoned_sval->get_poison_kind ();
-#if 0
 	if (ctxt)
 	  ctxt->warn (new poisoned_value_diagnostic (ptr, pkind));
 #endif
@@ -5831,10 +5868,12 @@ region_model::purge_regions (const region_id_set &set_to_purge,
   // FIXME: what about views???
 }
 
-/* Populate *OUT with RID and all of its descendents.  */
+/* Populate *OUT with RID and all of its descendents.
+   If EXCLUDE_RID is non-null, then don't add it or its descendents.  */
 
 void
-region_model::get_descendents (region_id rid, region_id_set *out) const
+region_model::get_descendents (region_id rid, region_id_set *out,
+			       region_id exclude_rid) const
 {
   out->add_region (rid);
 
@@ -5847,6 +5886,8 @@ region_model::get_descendents (region_id rid, region_id_set *out) const
       FOR_EACH_VEC_ELT (m_regions, i, r)
 	{
 	  region_id iter_rid = region_id::from_int (i);
+	  if (iter_rid == exclude_rid)
+	    continue;
 	  if (!out->region_p (iter_rid))
 	    {
 	      region_id parent_rid = r->get_parent ();
@@ -5874,7 +5915,7 @@ region_model::delete_region_and_descendents (region_id rid,
 {
   /* Find all child and descendent regions.  */
   region_id_set descendents (this);
-  get_descendents (rid, &descendents);
+  get_descendents (rid, &descendents, region_id::null ());
 
   /* Find any pointers to such regions; convert to poisoned.  */
   poison_any_pointers_to_bad_regions (descendents, pkind);
@@ -5976,7 +6017,7 @@ region_model::get_value_by_name (const char *name) const
    (might be easier if we let regions know their own IDs).  */
 
 region_id
-region_model::add_view (svalue_id ptr_sid, svalue_id offset_sid,
+region_model::add_view (tree type, svalue_id ptr_sid, svalue_id offset_sid,
 			region_model_context *ctxt)
 {
   svalue *ptr_sval = get_svalue (ptr_sid);
@@ -5986,7 +6027,7 @@ region_model::add_view (svalue_id ptr_sid, svalue_id offset_sid,
 
   // TODO: what about offset_sid
 
-  return add_view (raw_rid, TREE_TYPE (ptr_type));
+  return add_view (raw_rid, type);
 }
 
 /* FIXME.  */
@@ -6837,11 +6878,20 @@ test_stack_frames ()
 
   /* Test get_descendents.  */
   region_id_set descendents (&model);
-  model.get_descendents (child_frame_rid, &descendents);
+  model.get_descendents (child_frame_rid, &descendents, region_id::null ());
   ASSERT_TRUE (descendents.region_p (child_frame_rid));
   ASSERT_TRUE (descendents.region_p (x_in_child_rid));
   ASSERT_FALSE (descendents.region_p (a_in_parent_rid));
   ASSERT_EQ (descendents.num_regions (), 3);
+#if 0
+  auto_vec<region_id> test_vec;
+  for (region_id_set::iterator_t iter = descendents.begin ();
+       iter != descendents.end ();
+       ++iter)
+    test_vec.safe_push (*iter);
+  gcc_unreachable (); // TODO
+  //ASSERT_EQ ();
+#endif
 
   ASSERT_DUMP_EQ (model, true,
 		  "x: 0, {y}: unknown, p: &x, q: &p, b < 10, y != 5");
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index b7c04e3..67ad3ce 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -97,6 +97,8 @@ public:
   void print (pretty_printer *pp) const;
   void dump_node_name_to_pp (pretty_printer *pp) const;
 
+  void validate (const region_model &model) const;
+
 private:
   svalue_id (int idx) : m_idx (idx) {}
 
@@ -131,6 +133,8 @@ public:
   void print (pretty_printer *pp) const;
   void dump_node_name_to_pp (pretty_printer *pp) const;
 
+  void validate (const region_model &model) const;
+
 private:
   region_id (int idx) : m_idx (idx) {}
 
@@ -399,6 +403,33 @@ public:
     return bitmap_count_bits (m_bitmap);
   }
 
+#if 0
+  // FIXME: not a great impl, but hopefully good enough for now
+ class iterator_t
+  {
+  public:
+    iterator_t (auto_sbitmap *bm, unsigned bit_idx)
+    {
+      bmp_iter_set_init (&m_bi, bm, bit_idx, &m_bit_idx);
+    }
+    T operator * ()
+    {
+      return T::from_int (m_bit_idx);
+    }
+    void operator++ ()
+    {
+      bmp_iter_next (&m_bi, &m_bit_idx);
+    }
+
+  private:
+    sbitmap_iterator m_bi;
+    unsigned m_bit_idx;
+  };
+
+  iterator_t begin () { return iterator_t (this, 0); }
+  iterator_t end () { return iterator_t (this, num_regions ()); }
+#endif
+
 private:
   auto_sbitmap m_bitmap;
 };
@@ -867,7 +898,7 @@ public:
 
   void add_view (region_id view_rid, region_model *model);
   region_id get_view (tree type, region_model *model) const;
-  //bool is_view_p () const { return m_is_view; }
+  bool is_view_p () const { return m_is_view; }
 
   void validate (const region_model *model) const;
 
@@ -882,6 +913,8 @@ public:
 
  private:
   void become_active_view (region_model &model, region_id this_rid);
+  void deactivate_any_active_view (region_model &model, region_id this_rid);
+  void deactivate_view (region_model &model, region_id this_view_rid);
 
   region_id m_parent_rid;
   svalue_id m_sval_id;
@@ -1571,7 +1604,8 @@ class region_model
     return m_constraints;
   }
 
-  void get_descendents (region_id rid, region_id_set *out) const;
+  void get_descendents (region_id rid, region_id_set *out,
+			region_id exclude_rid) const;
 
   void delete_region_and_descendents (region_id rid,
 				      enum poison_kind pkind,
@@ -1586,7 +1620,7 @@ class region_model
 
   svalue_id get_value_by_name (const char *name) const;
 
-  region_id add_view (svalue_id ptr_sid, svalue_id offset_sid,
+  region_id add_view (tree type, svalue_id ptr_sid, svalue_id offset_sid,
 		      region_model_context *ctxt);
   region_id add_view (region_id raw_rid, tree type);
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-13.c b/gcc/testsuite/gcc.dg/analyzer/data-model-13.c
new file mode 100644
index 0000000..a746a89
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-13.c
@@ -0,0 +1,13 @@
+#include <stdlib.h>
+
+union
+{
+  void *ptr_val;
+  int int_val;
+} global_union;
+
+void test (void)
+{
+  global_union.ptr_val = malloc (1024);
+  global_union.int_val = 0; // FIXME: ought to complain about a leak here
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-14.c b/gcc/testsuite/gcc.dg/analyzer/data-model-14.c
new file mode 100644
index 0000000..32fed4c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-14.c
@@ -0,0 +1,21 @@
+#include <stdlib.h>
+
+void *global_ptr;
+
+void test_1 (int i)
+{
+  global_ptr = malloc (1024); /* { dg-message "allocated here" } */
+  *(int *)&global_ptr = i; /* { dg-warning "leak of '<unknown>'" } */
+  // TODO: something better than "<unknown>" here ^^^
+}
+
+void test_2 (int i)
+{
+  void *p = malloc (1024); /* { dg-message "allocated here" "" { xfail *-*-* } } */
+  // TODO(xfail)
+  global_ptr = p;
+  *(int *)&p = i;
+  p = global_ptr;
+  free (p);
+  free (global_ptr); /* { dg-warning "double-'free' of 'p'" } */
+}
-- 
1.8.5.3

