[gcc(refs/users/marxin/heads/opt-parse-enum-sanity)] analyzer: validate region subclasses

[gcc(refs/users/marxin/heads/opt-parse-enum-sanity)] analyzer: validate region subclasses

[Martin Liska]

https://gcc.gnu.org/g:3c1645a379e405c7ce33060846fa424373b1f5f4

commit 3c1645a379e405c7ce33060846fa424373b1f5f4
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Feb 20 08:44:23 2020 -0500

    analyzer: validate region subclasses
    
    This patch converts region::validate to a vfunc, implementing
    additional checking per subclass: verifying that various
    region_id fields within map_region, array_region, stack_region and
    root_region are valid, rather than just those within the base class.
    
    Doing so caught bugs earlier in follow-up work I have on
    canonicalization and purging of region_model.
    
    gcc/analyzer/ChangeLog:
            * region-model.cc (region::validate): Convert model param from ptr
            to reference.  Update comment to reflect that it's now a vfunc.
            (map_region::validate): New vfunc implementation.
            (array_region::validate): New vfunc implementation.
            (stack_region::validate): New vfunc implementation.
            (root_region::validate): New vfunc implementation.
            (region_model::validate): Pass a reference rather than a pointer
            to the region::validate vfunc.
            * region-model.h (region::validate): Make virtual.  Convert model
            param from ptr to reference.
            (map_region::validate): New vfunc decl.
            (array_region::validate): New vfunc decl.
            (stack_region::validate): New vfunc decl.
            (root_region::validate): New vfunc decl.

Diff:
---
 gcc/analyzer/ChangeLog       | 17 +++++++++++
 gcc/analyzer/region-model.cc | 70 +++++++++++++++++++++++++++++++++++++++-----
 gcc/analyzer/region-model.h  |  8 ++++-
 3 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index c4724cb090d..4a95fa60f9a 100644
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,3 +1,20 @@
+2020-03-04  David Malcolm  <dmalcolm@redhat.com>
+
+	* region-model.cc (region::validate): Convert model param from ptr
+	to reference.  Update comment to reflect that it's now a vfunc.
+	(map_region::validate): New vfunc implementation.
+	(array_region::validate): New vfunc implementation.
+	(stack_region::validate): New vfunc implementation.
+	(root_region::validate): New vfunc implementation.
+	(region_model::validate): Pass a reference rather than a pointer
+	to the region::validate vfunc.
+	* region-model.h (region::validate): Make virtual.  Convert model
+	param from ptr to reference.
+	(map_region::validate): New vfunc decl.
+	(array_region::validate): New vfunc decl.
+	(stack_region::validate): New vfunc decl.
+	(root_region::validate): New vfunc decl.
+
 2020-03-04  David Malcolm  <dmalcolm@redhat.com>
 
 	PR analyzer/93993
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 6813117968f..0ceeab45a02 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1360,21 +1360,23 @@ region::dump_child_label (const region_model &model,
     }
 }
 
-/* Assert that this object is valid.  */
+/* Base implementation of region::validate vfunc.
+   Assert that the fields of "region" are valid; subclasses should
+   chain up their implementation to this one.  */
 
 void
-region::validate (const region_model *model) const
+region::validate (const region_model &model) const
 {
-  m_parent_rid.validate (*model);
-  m_sval_id.validate (*model);
+  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 ());
-      view_rid->validate (*model);
+      view_rid->validate (model);
     }
-  m_active_view_rid.validate (*model);
+  m_active_view_rid.validate (model);
 }
 
 /* Apply MAP to svalue_ids to this region.  This updates the value
@@ -1599,6 +1601,21 @@ map_region::print_fields (const region_model &model,
   pp_string (pp, "}");
 }
 
+/* Implementation of region::validate vfunc for map_region.  */
+
+void
+map_region::validate (const region_model &model) const
+{
+  region::validate (model);
+  for (map_t::iterator iter = m_map.begin ();
+       iter != m_map.end ();
+       ++iter)
+    {
+      region_id child_rid = (*iter).second;
+      child_rid.validate (model);
+    }
+}
+
 /* Implementation of region::dump_dot_to_pp vfunc for map_region.  */
 
 void
@@ -2268,6 +2285,21 @@ array_region::print_fields (const region_model &model,
   pp_string (pp, "}");
 }
 
+/* Implementation of region::validate vfunc for array_region.  */
+
+void
+array_region::validate (const region_model &model) const
+{
+  region::validate (model);
+  for (map_t::iterator iter = m_map.begin ();
+       iter != m_map.end ();
+       ++iter)
+    {
+      region_id child_rid = (*iter).second;
+      child_rid.validate (model);
+    }
+}
+
 /* Implementation of region::dump_dot_to_pp vfunc for array_region.  */
 
 void
@@ -2544,6 +2576,18 @@ stack_region::dump_child_label (const region_model &model,
   pp_printf (pp, "frame for %qs: ", function_name (fun));
 }
 
+/* Implementation of region::validate vfunc for stack_region.  */
+
+void
+stack_region::validate (const region_model &model) const
+{
+  region::validate (model);
+  int i;
+  region_id *frame_rid;
+  FOR_EACH_VEC_ELT (m_frame_rids, i, frame_rid)
+    m_frame_rids[i].validate (model);
+}
+
 /* Push FRAME_RID (for a frame_region) onto this stack.  */
 
 void
@@ -2834,6 +2878,18 @@ root_region::print_fields (const region_model &model,
   // TODO
 }
 
+/* Implementation of region::validate vfunc for root_region.  */
+
+void
+root_region::validate (const region_model &model) const
+{
+  region::validate (model);
+  m_stack_rid.validate (model);
+  m_globals_rid.validate (model);
+  m_code_rid.validate (model);
+  m_heap_rid.validate (model);
+}
+
 /* Implementation of region::dump_child_label vfunc for root_region.  */
 
 void
@@ -3714,7 +3770,7 @@ region_model::validate () const
   unsigned i;
   region *r;
   FOR_EACH_VEC_ELT (m_regions, i, r)
-    r->validate (this);
+    r->validate (*this);
 
   // TODO: anything else?
 
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 6d49f00cfe3..c782e93a83d 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -891,7 +891,7 @@ public:
   region_id get_view (tree type, region_model *model) const;
   bool is_view_p () const { return m_is_view; }
 
-  void validate (const region_model *model) const;
+  virtual void validate (const region_model &model) const;
 
   bool non_null_p (const region_model &model) const;
 
@@ -1014,6 +1014,7 @@ public:
 		     region_id this_rid,
 		     pretty_printer *pp) const
     OVERRIDE;
+  void validate (const region_model &model) const FINAL OVERRIDE;
 
  private:
   /* Mapping from tree to child region.  */
@@ -1396,6 +1397,7 @@ public:
 		     region_id this_rid,
 		     pretty_printer *pp) const
     OVERRIDE;
+  void validate (const region_model &model) const FINAL OVERRIDE;
 
   static key_t key_from_constant (tree cst);
 
@@ -1462,6 +1464,8 @@ public:
   svalue_id get_value_by_name (tree identifier,
 			       const region_model &model) const;
 
+  void validate (const region_model &model) const FINAL OVERRIDE;
+
  private:
   void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE;
   void print_fields (const region_model &model,
@@ -1577,6 +1581,8 @@ public:
   svalue_id get_value_by_name (tree identifier,
 			       const region_model &model) const;
 
+  void validate (const region_model &model) const FINAL OVERRIDE;
+
 private:
   void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE;
   void print_fields (const region_model &model,