From 8e71efd87951f0ba72d81252bd7e93980c4c8652 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 21 Jul 2020 09:30:46 -0400
Subject: [PATCH 313/315] FIXME: implement store canonicalization and visitor
 class

---
 gcc/analyzer/analyzer.h                       |   4 +-
 gcc/analyzer/engine.cc                        |   8 +
 gcc/analyzer/region-model2.cc                 | 167 ++++++++++++++++++
 gcc/analyzer/region-model2.h                  |  53 +++++-
 gcc/analyzer/store2.cc                        |  58 ++++++
 gcc/analyzer/store2.h                         |   5 +
 .../gcc.dg/analyzer/malloc-in-loop.c          |  19 ++
 7 files changed, 310 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/malloc-in-loop.c

diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
index 183a11cf0fe..b82241e1b0f 100644
--- a/gcc/analyzer/analyzer.h
+++ b/gcc/analyzer/analyzer.h
@@ -59,13 +59,15 @@ struct canonicalization;
 class svalue2;
   class region_svalue2;
   class constant_svalue2;
-  class poisoned_svalue2;
   class unknown_svalue2;
+  class poisoned_svalue2;
   class setjmp_svalue2;
   class initial_svalue2;
   class unaryop_svalue2;
   class binop_svalue2;
+  class sub_svalue2;
   class unmergeable_svalue2;
+  class placeholder_svalue2;
   class widening_svalue2;
   class compound_svalue2;
 class region2;
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index bc7821bd161..6725f6e323d 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -266,6 +266,14 @@ setjmp_svalue::print_details (const region_model &model ATTRIBUTE_UNUSED,
 
 /* FIXME.  */
 
+void
+setjmp_svalue2::accept (visitor *v) const
+{
+  v->visit_setjmp_svalue2 (this);
+}
+
+/* FIXME.  */
+
 void
 setjmp_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc
index 5e6badd9fad..0d0a46ae93a 100644
--- a/gcc/analyzer/region-model2.cc
+++ b/gcc/analyzer/region-model2.cc
@@ -377,6 +377,15 @@ region_svalue2::compare_fields (const region_svalue2 &other) const
 }
 #endif
 
+/* FIXME.  */
+
+void
+region_svalue2::accept (visitor *v) const
+{
+  v->visit_region_svalue2 (this);
+  m_reg->accept (v);
+}
+
 /* Implementation of svalue2::dump_dot_to_pp for region_svalue2.  */
 
 #if 0
@@ -570,6 +579,14 @@ constant_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
     }
 }
 
+/* FIXME.  */
+
+void
+constant_svalue2::accept (visitor *v) const
+{
+  v->visit_constant_svalue2 (this);
+}
+
 /* Compare the fields of this constant_svalue2 with OTHER, returning true
    if they are equal.
    For use by svalue2::operator==.  */
@@ -681,6 +698,14 @@ unknown_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
     }
 }
 
+/* FIXME.  */
+
+void
+unknown_svalue2::accept (visitor *v) const
+{
+  v->visit_unknown_svalue2 (this);
+}
+
 /* class poisoned_svalue2 : public svalue2.  */
 
 /* Compare the fields of this poisoned_svalue2 with OTHER, returning true
@@ -696,6 +721,14 @@ poisoned_svalue2::compare_fields (const poisoned_svalue2 &other) const
 
 /* FIXME.  */
 
+void
+poisoned_svalue2::accept (visitor *v) const
+{
+  v->visit_poisoned_svalue2 (this);
+}
+
+/* FIXME.  */
+
 void
 poisoned_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -731,6 +764,15 @@ initial_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
     }
 }
 
+/* FIXME.  */
+
+void
+initial_svalue2::accept (visitor *v) const
+{
+  v->visit_initial_svalue2 (this);
+  m_reg->accept (v);
+}
+
 /* Compare the fields of this initial_svalue2 with OTHER, returning true
    if they are equal.
    For use by svalue2::operator==.  */
@@ -777,6 +819,15 @@ unaryop_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
     }
 }
 
+/* FIXME.  */
+
+void
+unaryop_svalue2::accept (visitor *v) const
+{
+  v->visit_unaryop_svalue2 (this);
+  m_arg->accept (v);
+}
+
 /* class binop_svalue2 : public svalue2.  */
 
 void
@@ -803,6 +854,16 @@ binop_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
     }
 }
 
+/* FIXME.  */
+
+void
+binop_svalue2::accept (visitor *v) const
+{
+  v->visit_binop_svalue2 (this);
+  m_arg0->accept (v);
+  m_arg1->accept (v);
+}
+
 /* class sub_svalue2 : public svalue2.  */
 
 sub_svalue2::sub_svalue2 (tree type, const svalue2 *parent_svalue,
@@ -814,6 +875,16 @@ sub_svalue2::sub_svalue2 (tree type, const svalue2 *parent_svalue,
 {
 }
 
+/* FIXME.  */
+
+void
+sub_svalue2::accept (visitor *v) const
+{
+  v->visit_sub_svalue2 (this);
+  m_parent_svalue->accept (v);
+  m_subregion->accept (v);
+}
+
 void
 sub_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -839,6 +910,16 @@ sub_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 
 /* class widening_svalue2 : public svalue2.  */
 
+/* FIXME.  */
+
+void
+widening_svalue2::accept (visitor *v) const
+{
+  v->visit_widening_svalue2 (this);
+  m_base_sval->accept (v);
+  m_iter_sval->accept (v);
+}
+
 void
 widening_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -985,6 +1066,14 @@ widening_svalue2::eval_condition_without_cm (enum tree_code op,
 
 /* class placeholder_svalue2 : public svalue2.  */
 
+/* FIXME.  */
+
+void
+placeholder_svalue2::accept (visitor *v) const
+{
+  v->visit_placeholder_svalue2 (this);
+}
+
 void
 placeholder_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -1001,6 +1090,15 @@ placeholder_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 
 /* class unmergeable_svalue2 : public svalue2.  */
 
+/* FIXME.  */
+
+void
+unmergeable_svalue2::accept (visitor *v) const
+{
+  v->visit_unmergeable_svalue2 (this);
+  m_arg->accept (v);
+}
+
 void
 unmergeable_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -1020,6 +1118,20 @@ unmergeable_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 
 /* class compound_svalue2 : public svalue2.  */
 
+/* FIXME.  */
+
+void
+compound_svalue2::accept (visitor *v) const
+{
+  v->visit_compound_svalue2 (this);
+  for (binding_map::iterator_t iter = m_map.begin ();
+       iter != m_map.end (); ++iter)
+    {
+      //(*iter).first.accept (v);
+      (*iter).second->accept (v);
+    }
+}
+
 void
 compound_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -2082,6 +2194,16 @@ region2::get_desc (bool simple) const
   return label_text::take (xstrdup (pp_formatted_text (&pp)));
 }
 
+/* FIXME.  */
+
+void
+region2::accept (visitor *v) const
+{
+  v->visit_region (this);
+  if (m_parent)
+    m_parent->accept (v);
+}
+
 /* Base implementation of region2::validate vfunc.
    Assert that the fields of "region2" are valid; subclasses should
    chain up their implementation to this one.  */
@@ -2334,6 +2456,14 @@ frame_region2::~frame_region2 ()
     delete (*iter).second;
 }
 
+void
+frame_region2::accept (visitor *v) const
+{
+  region2::accept (v);
+  if (m_calling_frame)
+    m_calling_frame->accept (v);
+}
+
 void
 frame_region2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -2944,6 +3074,15 @@ symbolic_region2::compare_fields (const symbolic_region2 &other) const
 
 /* FIXME.  */
 
+void
+symbolic_region2::accept (visitor *v) const
+{
+  region2::accept (v);
+  m_sval_ptr->accept (v);
+}
+
+/* FIXME.  */
+
 void
 symbolic_region2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -3050,6 +3189,15 @@ element_region2::compare_fields (const element_region2 &other) const
   return m_index == other.m_index;
 }
 
+/* FIXME.  */
+
+void
+element_region2::accept (visitor *v) const
+{
+  region2::accept (v);
+  m_index->accept (v);
+}
+
 void
 element_region2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -3089,6 +3237,15 @@ offset_region2::compare_fields (const offset_region2 &other) const
   return m_byte_offset == other.m_byte_offset;
 }
 
+/* FIXME.  */
+
+void
+offset_region2::accept (visitor *v) const
+{
+  region2::accept (v);
+  m_byte_offset->accept (v);
+}
+
 void
 offset_region2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -3127,6 +3284,15 @@ cast_region2::compare_fields (const cast_region2 &other) const
   return m_original_region == other.m_original_region;
 }
 
+/* FIXME.  */
+
+void
+cast_region2::accept (visitor *v) const
+{
+  region2::accept (v);
+  m_original_region->accept (v);
+}
+
 void
 cast_region2::dump_to_pp (pretty_printer *pp, bool simple) const
 {
@@ -4634,6 +4800,7 @@ region_model2::validate () const
 void
 region_model2::canonicalize ()
 {
+  m_store.canonicalize ();
   m_constraints2->canonicalize ();
 }
 
diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h
index 45986012ed0..a1ecf9c472e 100644
--- a/gcc/analyzer/region-model2.h
+++ b/gcc/analyzer/region-model2.h
@@ -145,6 +145,26 @@ struct complexity
   unsigned m_max_depth;
 };
 
+class visitor
+{
+public:
+  virtual void visit_region_svalue2 (const region_svalue2 *) {}
+  virtual void visit_constant_svalue2 (const constant_svalue2 *) {}
+  virtual void visit_unknown_svalue2 (const unknown_svalue2 *) {}
+  virtual void visit_poisoned_svalue2 (const poisoned_svalue2 *) {}
+  virtual void visit_setjmp_svalue2 (const setjmp_svalue2 *) {}
+  virtual void visit_initial_svalue2 (const initial_svalue2 *) {}
+  virtual void visit_unaryop_svalue2 (const unaryop_svalue2 *) {}
+  virtual void visit_binop_svalue2 (const binop_svalue2 *) {}
+  virtual void visit_sub_svalue2 (const sub_svalue2 *) {}
+  virtual void visit_unmergeable_svalue2 (const unmergeable_svalue2 *) {}
+  virtual void visit_placeholder_svalue2 (const placeholder_svalue2 *) {}
+  virtual void visit_widening_svalue2 (const widening_svalue2 *) {}
+  virtual void visit_compound_svalue2 (const compound_svalue2 *) {}
+
+  virtual void visit_region (const region2 *) {}
+};
+
 /* svalue and its subclasses.
 
    The class hierarchy looks like this (using indentation to show
@@ -245,6 +265,8 @@ public:
 
   const complexity &get_complexity () const { return m_complexity; }
 
+  virtual void accept (visitor *v) const  = 0;
+
  protected:
   svalue2 (complexity c, tree type)
   : m_complexity (c), m_type (type)
@@ -281,6 +303,8 @@ public:
   const region_svalue2 *
   dyn_cast_region_svalue2 () const FINAL OVERRIDE { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
+
   const region2 * get_pointee () const { return m_reg; }
 
 #if 0
@@ -329,6 +353,8 @@ public:
   const constant_svalue2 *
   dyn_cast_constant_svalue2 () const FINAL OVERRIDE { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
+
   tree get_constant () const { return m_cst_expr; }
 
 #if 0
@@ -372,6 +398,7 @@ public:
   {}
 
   enum kind get_kind () const FINAL OVERRIDE { return SK_UNKNOWN; }
+  void accept (visitor *v) const FINAL OVERRIDE;
   void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
 };
 
@@ -437,6 +464,8 @@ public:
   const poisoned_svalue2 *
   dyn_cast_poisoned_svalue2 () const FINAL OVERRIDE { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
+
   enum poison_kind get_poison_kind () const { return m_kind; }
 
  private:
@@ -508,6 +537,8 @@ public:
   const setjmp_svalue2 *
   dyn_cast_setjmp_svalue2 () const FINAL OVERRIDE { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
+
   int get_enode_index () const;
 
   const setjmp_record &get_setjmp_record () const { return m_setjmp_record; }
@@ -565,6 +596,8 @@ public:
   const initial_svalue2 *
   dyn_cast_initial_svalue2 () const FINAL OVERRIDE { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
+
   const region2 * get_region () const { return m_reg; }
 
 #if 0
@@ -639,6 +672,7 @@ public:
   const unaryop_svalue2 *
   dyn_cast_unaryop_svalue2 () const FINAL OVERRIDE { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
 
 #if 0
   void dump_dot_to_pp (const region_model2 &model,
@@ -730,6 +764,8 @@ public:
   enum kind get_kind () const FINAL OVERRIDE { return SK_BINOP; }
   virtual const binop_svalue2 *dyn_cast_binop_svalue2 () const { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
+
   enum tree_code get_op () const { return m_op; }
   const svalue2 *get_arg0 () const { return m_arg0; }
   const svalue2 *get_arg1 () const { return m_arg1; }
@@ -815,6 +851,7 @@ public:
 	       const region2 *subregion);
 
   enum kind get_kind () const FINAL OVERRIDE { return SK_SUB; }
+  void accept (visitor *v) const FINAL OVERRIDE;
 
 #if 0
   void dump_dot_to_pp (const region_model2 &model,
@@ -877,7 +914,7 @@ public:
   enum kind get_kind () const FINAL OVERRIDE { return SK_UNMERGEABLE; }
   const unmergeable_svalue2 *
   dyn_cast_unmergeable_svalue2 () const FINAL OVERRIDE { return this; }
-
+  void accept (visitor *v) const FINAL OVERRIDE;
 
 #if 0
   void dump_dot_to_pp (const region_model2 &model,
@@ -919,6 +956,7 @@ public:
   }
 
   enum kind get_kind () const FINAL OVERRIDE { return SK_PLACEHOLDER; }
+  void accept (visitor *v) const FINAL OVERRIDE;
 
 #if 0
   void dump_dot_to_pp (const region_model2 &model,
@@ -1074,6 +1112,7 @@ public:
 
   enum kind get_kind () const FINAL OVERRIDE { return SK_WIDENING; }
   const widening_svalue2 *dyn_cast_widening_svalue2 () const { return this; }
+  void accept (visitor *v) const FINAL OVERRIDE;
 
 #if 0
   void dump_dot_to_pp (const region_model2 &model,
@@ -1177,6 +1216,7 @@ public:
 
   enum kind get_kind () const FINAL OVERRIDE { return SK_COMPOUND; }
   const compound_svalue2 *dyn_cast_compound_svalue2 () const { return this; }
+  void accept (visitor *v) const FINAL OVERRIDE;
 
   iterator_t begin () const { return m_map.begin (); }
   iterator_t end () const { return m_map.end (); }
@@ -1316,6 +1356,8 @@ public:
   virtual const string_region2 *
   dyn_cast_string_region2 () const { return NULL; }
 
+  virtual void accept (visitor *v) const;
+
   const region2 *get_parent_region () const { return m_parent; }
   const region2 *get_base_region () const;
   bool base_region_p () const;
@@ -1458,6 +1500,7 @@ public:
   {
     return this;
   }
+  void accept (visitor *v) const FINAL OVERRIDE;
   void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
 #if 0
   void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE;
@@ -1784,7 +1827,7 @@ public:
   bool compare_fields (const symbolic_region2 &other) const;
 
   enum kind get_kind () const FINAL OVERRIDE { return region2::RK_SYMBOLIC; }
-
+  void accept (visitor *v) const FINAL OVERRIDE;
   void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
 
   const svalue2 *get_pointer () const { return m_sval_ptr; }
@@ -1900,6 +1943,8 @@ public:
   const element_region2 *
   dyn_cast_element_region2 () const FINAL OVERRIDE { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
+
   void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
 
   const svalue2 *get_index () const { return m_index; }
@@ -1941,6 +1986,8 @@ public:
   const offset_region2 *
   dyn_cast_offset_region2 () const FINAL OVERRIDE { return this; }
 
+  void accept (visitor *v) const FINAL OVERRIDE;
+
   void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
 
   const svalue2 *get_byte_offset () const { return m_byte_offset; }
@@ -1980,7 +2027,7 @@ public:
   enum region2::kind get_kind () const FINAL OVERRIDE { return RK_CAST; }
   const cast_region2 *
   dyn_cast_cast_region2 () const FINAL OVERRIDE { return this; }
-
+  void accept (visitor *v) const FINAL OVERRIDE;
   void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
 
   /* FIXME: should this just be the parent region? */
diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc
index 7772b477d0c..591aae47c40 100644
--- a/gcc/analyzer/store2.cc
+++ b/gcc/analyzer/store2.cc
@@ -1735,6 +1735,64 @@ store2::remove_overlapping_bindings (store2_manager *mgr, const region2 *reg)
     (*cluster_slot)->remove_overlapping_bindings (mgr, reg);
 }
 
+/* FIXME.  */
+
+struct region2_finder : public visitor
+{
+  void visit_region (const region2 *reg) FINAL OVERRIDE
+  {
+    m_regs.add (reg);
+  }
+
+  hash_set<const region2 *> m_regs;
+};
+
+/* FIXME.  */
+
+void
+store2::canonicalize ()
+{
+  /* If we have e.g.:
+         cluster for: HEAP_ALLOCATED_REGION(543)
+           ESCAPED
+           TOUCHED
+     where the heap region is empty and unreferenced, then purge that
+     cluster, to avoid unbounded state chains involving these.  */
+
+  /* Find regions that are referenced by bound values in the store.  */
+  region2_finder s;
+  for (cluster_map_t::iterator iter = m_cluster_map.begin ();
+       iter != m_cluster_map.end (); ++iter)
+    {
+      binding_cluster2 *cluster = (*iter).second;
+      for (binding_cluster2::iterator_t bind_iter = cluster->m_map.begin ();
+	   bind_iter != cluster->m_map.end (); ++bind_iter)
+	(*bind_iter).second->accept (&s);
+    }
+
+  /* Locate heap-allocated regions that have empty bindings that weren't
+     found above.  */
+  hash_set<const region2 *> purgeable_regions;
+  for (cluster_map_t::iterator iter = m_cluster_map.begin ();
+       iter != m_cluster_map.end (); ++iter)
+    {
+      const region2 *base_reg = (*iter).first;
+      binding_cluster2 *cluster = (*iter).second;
+      if (base_reg->get_kind () == region2::RK_HEAP_ALLOCATED)
+	if (cluster->empty_p ())
+	  if (!s.m_regs.contains (base_reg))
+	    purgeable_regions.add (base_reg);
+    }
+
+  /* Purge them.  */
+  for (hash_set<const region2 *>::iterator iter = purgeable_regions.begin ();
+       iter != purgeable_regions.end (); ++iter)
+    {
+      const region2 *base_reg = *iter;
+      purge_cluster (base_reg);
+    }
+}
+
 #if CHECKING_P
 
 namespace selftest {
diff --git a/gcc/analyzer/store2.h b/gcc/analyzer/store2.h
index 4a1b29df870..7981dd665b2 100644
--- a/gcc/analyzer/store2.h
+++ b/gcc/analyzer/store2.h
@@ -327,6 +327,8 @@ private:
 class binding_cluster2
 {
 public:
+  friend class store2; // FIXME
+
   typedef hash_map <const binding_key2 *, const svalue2 *> map_t;
   typedef map_t::iterator iterator_t;
 
@@ -392,6 +394,7 @@ public:
   bool touched_p () const { return m_touched; }
 
   bool redundant_p () const;
+  bool empty_p () const { return m_map.elements () == 0; }
 
   void get_representative_path_vars (const region_model2 *model,
 				     hash_set <const svalue2 *> *visited,
@@ -527,6 +530,8 @@ public:
       (*iter).second->for_each_binding (v);
   }
 
+  void canonicalize ();
+
 private:
   void remove_overlapping_bindings (store2_manager *mgr, const region2 *reg);
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-in-loop.c b/gcc/testsuite/gcc.dg/analyzer/malloc-in-loop.c
new file mode 100644
index 00000000000..a8c85a9c618
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-in-loop.c
@@ -0,0 +1,19 @@
+#include <stdlib.h>
+#include "analyzer-decls.h"
+
+extern void foo (int *);
+
+void test (int n)
+{
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */
+
+  for (int i = 0; i < n; i++)
+    {
+      int *ptr = (int *)malloc (sizeof (int) * i);
+      foo (ptr);
+      free (ptr);
+      __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 processed enodes" } */
+    }
+
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */
+}
-- 
2.26.2

