From 56defae49481a789a4466b487f4b9b90d307e7dd Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 25 Jun 2020 13:16:19 -0400
Subject: [PATCH 253/377] FIXME: implement poisoning of regions

---
 gcc/analyzer/region-model2.cc           | 195 ++++++++++++++++++------
 gcc/analyzer/region-model2.h            |   7 +-
 gcc/analyzer/store2.cc                  |  16 ++
 gcc/analyzer/store2.h                   |  29 +++-
 gcc/testsuite/gcc.dg/analyzer/bzero-1.c |   2 +-
 5 files changed, 194 insertions(+), 55 deletions(-)

diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc
index b7043a0514a..1907fbe4c9a 100644
--- a/gcc/analyzer/region-model2.cc
+++ b/gcc/analyzer/region-model2.cc
@@ -1204,7 +1204,10 @@ region2::descendent_of_p (const region2 *elder) const
     {
       if (iter == elder)
 	return true;
-      iter = iter->get_parent_region ();
+      if (iter->get_kind () == RK_CAST)
+	iter = iter->dyn_cast_cast_region2 ()->get_original_region ();
+      else
+	iter = iter->get_parent_region ();
     }
   return false;
 }
@@ -4815,6 +4818,86 @@ call_details::dump (bool simple) const
   pp_flush (&pp);
 }
 
+/* A subclass of pending_diagnostic for complaining about uses of
+   poisoned values.  */
+
+class poisoned_value_diagnostic2
+: public pending_diagnostic_subclass<poisoned_value_diagnostic2>
+{
+public:
+  poisoned_value_diagnostic2 (tree expr, enum poison_kind pkind)
+  : m_expr (expr), m_pkind (pkind)
+  {}
+
+  const char *get_kind () const FINAL OVERRIDE { return "poisoned_value_diagnostic2"; }
+
+  bool operator== (const poisoned_value_diagnostic2 &other) const
+  {
+    return m_expr == other.m_expr;
+  }
+
+  bool emit (rich_location *rich_loc) FINAL OVERRIDE
+  {
+    switch (m_pkind)
+      {
+      default:
+	gcc_unreachable ();
+      case POISON_KIND_UNINIT:
+	{
+	  diagnostic_metadata m;
+	  m.add_cwe (457); /* "CWE-457: Use of Uninitialized Variable".  */
+	  return warning_meta (rich_loc, m,
+			       OPT_Wanalyzer_use_of_uninitialized_value,
+			       "use of uninitialized value %qE",
+			       m_expr);
+	}
+	break;
+      case POISON_KIND_FREED:
+	{
+	  diagnostic_metadata m;
+	  m.add_cwe (416); /* "CWE-416: Use After Free".  */
+	  return warning_meta (rich_loc, m,
+			       OPT_Wanalyzer_use_after_free,
+			       "use after %<free%> of %qE",
+			       m_expr);
+	}
+	break;
+      case POISON_KIND_POPPED_STACK:
+	{
+	  /* TODO: which CWE?  */
+	  return warning_at (rich_loc,
+			     OPT_Wanalyzer_use_of_pointer_in_stale_stack_frame,
+			     "use of pointer %qE within stale stack frame",
+			     m_expr);
+	}
+	break;
+      }
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+  {
+    switch (m_pkind)
+      {
+      default:
+	gcc_unreachable ();
+      case POISON_KIND_UNINIT:
+	return ev.formatted_print ("use of uninitialized value %qE here",
+				   m_expr);
+      case POISON_KIND_FREED:
+	return ev.formatted_print ("use after %<free%> of %qE here",
+				   m_expr);
+      case POISON_KIND_POPPED_STACK:
+	return ev.formatted_print
+	  ("use of pointer %qE within stale stack frame here",
+	   m_expr);
+      }
+  }
+
+private:
+  tree m_expr;
+  enum poison_kind m_pkind;
+};
+
 /* Update this model for the CALL stmt, using CTXT to report any
    diagnostics - the first half.
 
@@ -4908,13 +4991,12 @@ region_model2::on_call_pre (const gcall *call, region_model2_context *ctxt)
 
    If UNKNOWN_SIDE_EFFECTS is true, also call handle_unrecognized_call
    to purge state.  */
-#if 1
+
 void
 region_model2::on_call_post (const gcall *call,
 			     bool unknown_side_effects,
 			     region_model2_context *ctxt)
 {
-#if 0
   /* Update for "free" here, after sm-handling.
 
      If the ptr points to an underlying heap region2, delete the region2,
@@ -4934,33 +5016,24 @@ region_model2::on_call_post (const gcall *call,
       {
 	tree ptr = gimple_call_arg (call, 0);
 	const svalue2 *ptr_sval = get_rvalue (ptr, ctxt);
-	if (region_const svalue2 *ptr_to_region2_sval
-	    = ptr_sval->dyn_cast_region_svalue2 ())
+	if (const region_svalue2 *ptr_to_region2_sval
+	      = ptr_sval->dyn_cast_region_svalue2 ())
 	  {
 	    /* If the ptr points to an underlying heap region2, delete it,
 	       poisoning pointers.  */
-	    region2 *pointee_reg = ptr_to_region2_sval->get_pointee ();
-	    region2 *heap_reg = get_root_region2 ()->ensure_heap_region2 (this);
-	    if (!pointee_reg.null_p ()
-		&& get_region2 (pointee_reg)->get_parent () == heap_reg)
-	      {
-		purge_stats stats;
-		delete_region2_and_descendents (pointee_reg,
-					       POISON_KIND_FREED,
-					       &stats, ctxt->get_logger ());
-		purge_unused_svalue2s (&stats, ctxt);
-		validate ();
-		// TODO: do anything with stats?
-	      }
+	    const region2 *freed_reg = ptr_to_region2_sval->get_pointee ();
+	    purge_stats2 stats;
+	    unbind_region2_and_descendents (freed_reg,
+					    POISON_KIND_FREED,
+					    &stats, ctxt->get_logger ());
+	    // TODO: do anything with stats?
 	  }
 	return;
       }
-#endif
 
   if (unknown_side_effects)
     handle_unrecognized_call (call, ctxt);
 }
-#endif
 
 /* Handling for the known behavior of various specific functions.  */
 
@@ -6067,7 +6140,6 @@ region_model2::deref_rvalue (const svalue2 *ptr_sval,
     case svalue2::SK_UNARYOP:
       goto create_symbolic_region2;
 
-#if 0
     case svalue2::SK_POISONED:
       {
 	if (ctxt)
@@ -6076,11 +6148,10 @@ region_model2::deref_rvalue (const svalue2 *ptr_sval,
 	      const poisoned_svalue2 *poisoned_sval
 		= as_a <const poisoned_svalue2 *> (ptr_sval);
 	      enum poison_kind pkind = poisoned_sval->get_poison_kind ();
-	      ctxt->warn (new poisoned_value_diagnostic (ptr, pkind));
+	      ctxt->warn (new poisoned_value_diagnostic2 (ptr, pkind));
 	    }
 	goto create_symbolic_region2;
       }
-#endif
 
     case svalue2::SK_UNKNOWN:
       {
@@ -7585,38 +7656,60 @@ region_model2::unbind_region2_and_descendents (const region2 *reg,
     }
   for (hash_set<const region2 *>::iterator iter = base_regs.begin ();
        iter != base_regs.end (); ++iter)
-    m_store.purge_region (m_mgr->get_store2_manager (), (*iter));
+    m_store.purge_cluster (*iter);
 
-  // TODO: unit test for descendent_of_p
-  // TODO: poison pointers to regions that are descendents of REG.
-#if 0
-  /* Find any pointers to such region2s; convert to poisoned.  */
-  poison_any_pointers_to_bad_region2s (descendents, pkind);
-#endif
+  /* Find any pointers to REG or its descendents; convert to poisoned.  */
+  poison_any_pointers_to_descendents (reg, pkind);
 }
 
-/* Find any pointers to region2s within BAD_REGION2S; convert them to
-   poisoned values of kind PKIND.  */
-#if 0
-void
-region_model2::poison_any_pointers_to_bad_region2s (const region2_id_set &
-						    bad_region2s,
-						  enum poison_kind pkind)
+/* Implementation of BindingVisitor.
+   Update the bound svalues for regions below REG to use poisoned
+   values instead.  */
+
+struct bad_pointer_finder
 {
-  int i;
-  const svalue2 *sval;
-  FOR_EACH_VEC_ELT (m_svalue2s, i, sval)
-    if (region_svalue2 *ptr_sval = sval->dyn_cast_region_svalue2 ())
+  bad_pointer_finder (const region2 *reg, enum poison_kind pkind,
+		      region_model2_manager *mgr)
+  : m_reg (reg), m_pkind (pkind), m_mgr (mgr), m_count (0)
+  {}
+
+  void on_binding (const binding_key2 *key, const svalue2 *&sval)
+  {
+    if (const region_svalue2 *ptr_sval = sval->dyn_cast_region_svalue2 ())
       {
-	region2 *ptr_dst = ptr_sval->get_pointee ();
-	if (!ptr_dst.null_p ())
-	  if (bad_region2s.region2_p (ptr_dst))
-	    replace_svalue2
-	      (svalue2_id::from_int (i),
-	       new poisoned_svalue2 (pkind, sval->get_type ()));
+	const region2 *ptr_dst = ptr_sval->get_pointee ();
+	/* Poison ptrs to descendents of REG, but not to REG itself,
+	   otherwise double-free detection doesn't work (since sm-state
+	   for "free" is stored on the original ptr svalue).  */
+	if (ptr_dst->descendent_of_p (m_reg)
+	    && ptr_dst != m_reg)
+	  {
+	    sval = m_mgr->get_or_create_poisoned_svalue2 (m_pkind,
+							  sval->get_type ());
+	    ++m_count;
+	  }
       }
+  }
+
+  const region2 *m_reg;
+  enum poison_kind m_pkind;
+  region_model2_manager *const m_mgr;
+  int m_count;
+};
+
+/* Find any pointers to REG or its descendents; convert them to
+   poisoned values of kind PKIND.
+   Return the number of pointers that were poisoned.  */
+// FIXME: ^^ unit test for this?
+
+int
+region_model2::poison_any_pointers_to_descendents (const region2 *reg,
+						   enum poison_kind pkind)
+{
+  bad_pointer_finder bv (reg, pkind, m_mgr);
+  m_store.for_each_binding (bv);
+  return bv.m_count;
 }
-#endif
 
 /* Attempt to merge THIS with OTHER_MODEL, writing the result
    to OUT_MODEL.
@@ -8871,6 +8964,10 @@ test_descendent_of_p ()
   tree x = build_global_decl ("x", integer_type_node);
   const region2 *x_reg = mgr.get_region_for_global (x);
   ASSERT_TRUE (x_reg->descendent_of_p (globals));
+
+  /* A cast_region2 should be a descendent of the original region.  */
+  const region2 *cast_reg = mgr.get_cast_region (x_reg, ptr_type_node);
+  ASSERT_TRUE (cast_reg->descendent_of_p (x_reg));
 }
 
 #if 0
@@ -9243,11 +9340,9 @@ test_stack_frames ()
   /* Verify that p (which was pointing at the local "x" in the popped
      frame) has been poisoned.  */
   const svalue2 *new_p_sval = model.get_rvalue (p, &ctxt);
-#if 0
   ASSERT_EQ (new_p_sval->get_kind (), svalue2::SK_POISONED);
   ASSERT_EQ (new_p_sval->dyn_cast_poisoned_svalue2 ()->get_poison_kind (),
 	     POISON_KIND_POPPED_STACK);
-#endif
 
   /* Verify that q still points to p, in spite of the region2
      renumbering.  */
diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h
index 967b8734d94..32ba5f8df1c 100644
--- a/gcc/analyzer/region-model2.h
+++ b/gcc/analyzer/region-model2.h
@@ -2369,10 +2369,11 @@ private:
   bool apply_constraints_for_gswitch (const switch_cfg_superedge &edge,
 				      const gswitch *switch_stmt,
 				      region_model2_context *ctxt);
-#if 0
-  void poison_any_pointers_to_bad_region2s (const region2_id_set &bad_region2s,
-					    enum poison_kind pkind);
 
+  int poison_any_pointers_to_descendents (const region2 *reg,
+					  enum poison_kind pkind);
+
+#if 0
   void dump_summary_of_rep_path_vars (pretty_printer *pp,
 				      auto_vec<path_var> *rep_path_vars,
 				      bool *is_first);
diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc
index e6fcfe8d23b..aca5003f363 100644
--- a/gcc/analyzer/store2.cc
+++ b/gcc/analyzer/store2.cc
@@ -1364,6 +1364,22 @@ store2::get_or_create_cluster (const region2 *base_reg)
   return cluster;
 }
 
+/* Remove any cluster for BASE_REG, for use by
+   region_model2::unbind_region2_and_descendents
+   when popping stack frames and handling deleted heap regions.  */
+
+void
+store2::purge_cluster (const region2 *base_reg)
+{
+  gcc_assert (base_reg->get_base_region () == base_reg);
+  binding_cluster2 **slot = m_cluster_map.get (base_reg);
+  if (!slot)
+    return;
+  binding_cluster2 *cluster = *slot;
+  delete cluster;
+  m_cluster_map.remove (base_reg);
+}
+
 /* FIXME.  */
 
 bool
diff --git a/gcc/analyzer/store2.h b/gcc/analyzer/store2.h
index b80213e2268..48e71c7696e 100644
--- a/gcc/analyzer/store2.h
+++ b/gcc/analyzer/store2.h
@@ -65,7 +65,7 @@ along with GCC; see the file COPYING3.  If not see
 
      arr[3] = c;  (3)
        After (3), the concrete binding for bits 24-31 is replaced with the
-       svalue "INIT_VAL(b)":
+       svalue "INIT_VAL(c)":
        cluster: {bits 16-23: "INIT_VAL(a)",  (from before)
                  bits 24-31: "INIT_VAL(c)";  (updated)
                  flags: {}}
@@ -267,6 +267,13 @@ private:
   const region2 *m_region;
 };
 
+/* Concept: BindingVisitor, for use by binding_cluster2::for_each_binding
+   and store2::for_each_binding.
+
+   Should implement:
+     void on_binding (const binding_key2 *key, const svalue2 *&sval);
+*/
+
 /* All of the bindings within a store for regions that share the same
    base region.  */
 
@@ -341,6 +348,17 @@ public:
 
   const svalue2 *maybe_get_simple_value (store2_manager *mgr) const;
 
+  template <typename BindingVisitor>
+  void for_each_binding (BindingVisitor &v)
+  {
+    for (map_t::iterator iter = m_map.begin (); iter != m_map.end (); ++iter)
+      {
+	const binding_key2 *key = (*iter).first;
+	const svalue2 *&sval = (*iter).second;
+	v.on_binding (key, sval);
+      }
+  }
+
 private:
   const svalue2 *get_any_value (const binding_key2 *key) const;
   void get_overlapping_bindings (store2_manager *mgr, const region2 *reg,
@@ -409,6 +427,7 @@ public:
   const binding_cluster2 *get_cluster (const region2 *base_reg) const;
   binding_cluster2 *get_cluster (const region2 *base_reg);
   binding_cluster2 *get_or_create_cluster (const region2 *base_reg);
+  void purge_cluster (const region2 *base_reg);
 
   template <typename T>
   void for_each_cluster (void (*cb) (const region2 *base_reg, T user_data),
@@ -439,6 +458,14 @@ public:
   tristate eval_alias (const region2 *base_reg_a,
 		       const region2 *base_reg_b);
 
+  template <typename BindingVisitor>
+  void for_each_binding (BindingVisitor &v)
+  {
+    for (cluster_map_t::iterator iter = m_cluster_map.begin ();
+	 iter != m_cluster_map.end (); ++iter)
+      (*iter).second->for_each_binding (v);
+  }
+
 private:
   void remove_overlapping_bindings (store2_manager *mgr, const region2 *reg);
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/bzero-1.c b/gcc/testsuite/gcc.dg/analyzer/bzero-1.c
index 66bb8833468..894508611fe 100644
--- a/gcc/testsuite/gcc.dg/analyzer/bzero-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/bzero-1.c
@@ -7,5 +7,5 @@ void test_1 (void)
   char tmp[1024];
   bzero (tmp, 1024);
   __analyzer_eval (tmp[0] == 0); /* { dg-warning "TRUE" } */
-  __analyzer_eval (tmp[023] == 0); /* { dg-warning "TRUE" } */
+  __analyzer_eval (tmp[1023] == 0); /* { dg-warning "TRUE" } */
 }
-- 
2.26.2

