From 74fc8aec75e52630edcb8aecc504cb91ab49afed Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 15 Mar 2019 16:10:51 -0400
Subject: [PATCH 121/169] FIXME: WIP on better expansions of call summaries

---
 gcc/analyzer/engine.cc                        | 272 +++++++++++++++++---------
 gcc/diagnostic-show-locus.c                   |   4 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-ipa-11.c |  16 +-
 3 files changed, 189 insertions(+), 103 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 520f9ce..3b06020 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -137,6 +137,11 @@ struct summary_edge
     return m_start.hash () ^ m_end.hash ();
   }
 
+  bool operator== (const summary_edge &other) const
+  {
+    return m_start == other.m_start && m_end == other.m_end;
+  }
+
   summary_point m_start;
   summary_point m_end;
 };
@@ -355,6 +360,90 @@ struct per_stmt_t
   set_of_summary_edges m_transitions;
 };
 
+class set_of_vars
+{
+public:
+  typedef hash_set<var_state_t>::iterator iterator;
+
+  set_of_vars () : m_set () {}
+  set_of_vars (const set_of_vars &other) : m_set ()
+  {
+    for (iterator i = other.begin (); i != other.end (); ++i)
+      {
+	m_set.add (*i);
+      }
+  }
+
+  set_of_vars& operator= (const set_of_vars &other)
+  {
+    m_set.empty ();
+    for (iterator i = other.begin (); i != other.end (); ++i)
+      {
+	m_set.add (*i);
+      }
+    return *this;
+  }
+
+  var_state_t *has_expr (tree expr) const
+  {
+    for (hash_set<var_state_t>::iterator iter = m_set.begin ();
+	 iter != m_set.end ();
+	 ++iter)
+      {
+	if ((*iter).m_var == expr)
+	  {
+	    // FIXME: get rid of this dynamic allocation!
+	    // FIXME: ideally we'd just return a pointer to the slot
+	    // but I'm not sure there's a way to do that.
+	    return new var_state_t (*iter);
+	    // FIXME: also: what if there's more than one?
+	  }
+      }
+    return NULL;
+  }
+
+  bool empty_p () const
+  {
+    return m_set.elements () == 0;
+  }
+
+  std::string to_str (const state_machine &sm) const
+  {
+    return ::to_str (m_set, sm);
+  }
+
+  iterator begin () const { return m_set.begin (); }
+  iterator end () const { return m_set.end (); }
+
+  hash_set<var_state_t> m_set;
+};
+
+/* FIXME: This changes as we do the traversal: */
+
+struct sm_instance
+{
+  sm_instance () : m_gstate (0), m_active_vars () {}
+
+  std::string to_str (const state_machine &sm) const
+  {
+    // FIXME: obstack?
+    std::string result = "<";
+    result += sm.get_state_name (m_gstate);
+    result += ", ";
+    result += m_active_vars.to_str (sm);
+    result += ">";
+    return result;
+  }
+
+  void add_placeholder ()
+  {
+    m_active_vars.m_set.add (var_state_t (var_state_t::get_placeholder (), 0));
+  }
+
+  state_machine::state_t m_gstate;
+  set_of_vars m_active_vars;
+};
+
 ////////////////////////////////////////////////////////////////////////////
 
 /* FIXME.  */
@@ -464,7 +553,10 @@ public:
     result += "\" (";
     result += event_kind_to_string (m_kind);
     result += ", depth ";
-    result += std::to_string (get_stack_depth ()) + ")";
+    result += std::to_string (get_stack_depth ());
+    result += ", m_loc=";
+    result += std::to_string (m_loc);
+    result += ")";
     return result;
   }
 
@@ -632,6 +724,7 @@ public:
 	    }
 	  else
 	    {
+	      free (edge_desc);
 	      return label_text::take (print ("taking edge SN:%i -> SN:%i",
 					      m_edge->m_src->m_index,
 					      m_edge->m_dest->m_index));
@@ -671,13 +764,15 @@ class call_summary : public checker_event
 {
 public:
   call_summary (const supernode *caller_node,
-		cgraph_edge *edge, int depth)
+		cgraph_edge *edge, int depth,
+		sm_instance callee_sm_inst)
   : checker_event (EK_CALL_SUMMARY,
 		   edge->call_stmt->location,
 		   caller_node->m_fun->decl,
 		   depth),
     m_caller_node (caller_node),
-    m_edge (edge)
+    m_edge (edge),
+    m_callee_sm_inst (callee_sm_inst)
   {
   }
 
@@ -691,7 +786,7 @@ public:
 
   checker_event *clone () const FINAL OVERRIDE
   {
-    return new call_summary (m_caller_node, m_edge, m_depth);
+    return new call_summary (m_caller_node, m_edge, m_depth, m_callee_sm_inst);
   }
 
   // FIXME
@@ -707,6 +802,7 @@ public:
 
   const supernode *m_caller_node;
   cgraph_edge *m_edge;
+  sm_instance m_callee_sm_inst;
 };
 
 /* FIXME: a repeat of the warning message at the end of the path.  */
@@ -1017,6 +1113,8 @@ public:
 
   checker_path *expand_call_summary (const call_summary *summary_event,
 				     tree var, state_machine::state_t state);
+  const superedge *get_next_expansion_edge (const supernode *node,
+					    summary_edge desired_suffix);
   checker_path *expand_path (const checker_path &path,
 			     tree var, state_machine::state_t state);
   void prune_path (checker_path *path, tree var, state_machine::state_t state);
@@ -1046,89 +1144,9 @@ public:
   int m_on_stmt_calls;
 };
 
-class set_of_vars
-{
-public:
-  typedef hash_set<var_state_t>::iterator iterator;
-
-  set_of_vars () : m_set () {}
-  set_of_vars (const set_of_vars &other) : m_set ()
-  {
-    for (iterator i = other.begin (); i != other.end (); ++i)
-      {
-	m_set.add (*i);
-      }
-  }
-
-  set_of_vars& operator= (const set_of_vars &other)
-  {
-    m_set.empty ();
-    for (iterator i = other.begin (); i != other.end (); ++i)
-      {
-	m_set.add (*i);
-      }
-    return *this;
-  }
-
-  var_state_t *has_expr (tree expr) const
-  {
-    for (hash_set<var_state_t>::iterator iter = m_set.begin ();
-	 iter != m_set.end ();
-	 ++iter)
-      {
-	if ((*iter).m_var == expr)
-	  {
-	    // FIXME: get rid of this dynamic allocation!
-	    // FIXME: ideally we'd just return a pointer to the slot
-	    // but I'm not sure there's a way to do that.
-	    return new var_state_t (*iter);
-	    // FIXME: also: what if there's more than one?
-	  }
-      }
-    return NULL;
-  }
-
-  bool empty_p () const
-  {
-    return m_set.elements () == 0;
-  }
-
-  std::string to_str (const state_machine &sm) const
-  {
-    return ::to_str (m_set, sm);
-  }
-
-  iterator begin () const { return m_set.begin (); }
-  iterator end () const { return m_set.end (); }
-
-  hash_set<var_state_t> m_set;
-};
-
-/* FIXME: This changes as we do the traversal: */
-
-struct sm_instance
-{
-  sm_instance () : m_gstate (0), m_active_vars () {}
-
-  std::string to_str (const state_machine &sm) const
-  {
-    // FIXME: obstack?
-    std::string result = "<";
-    result += sm.get_state_name (m_gstate);
-    result += ", ";
-    result += m_active_vars.to_str (sm);
-    result += ">";
-    return result;
-  }
-
-  void add_placeholder ()
-  {
-    m_active_vars.m_set.add (var_state_t (var_state_t::get_placeholder (), 0));
-  }
+////////////////////////////////////////////////////////////////////////////
 
-  state_machine::state_t m_gstate;
-  set_of_vars m_active_vars;
-};
+////////////////////////////////////////////////////////////////////////////
 
 void
 engine::add_events_for_node (checker_path *path, const supernode *node,
@@ -2221,7 +2239,8 @@ engine::follow_call (sm_instance sm_inst, backtrace b, path_state ps,
 
     backtrace new_b (b);
     checker_event *event
-      = new call_summary (caller_node, edge, 0/*int depth,*/);
+      = new call_summary (caller_node, edge, 0/*int depth,*/,
+			  callee_sm_inst);
     new_b.m_path.add_event (event);
 
     if (get_logger ())
@@ -2467,9 +2486,25 @@ engine::expand_call_summary (const call_summary *summary_event,
   JIT_LOG_FUNC_4 (get_logger (), "%qD called from %qD: var: %qE state: %qs",
 		  callee_fun->decl, caller_node->m_fun->decl,
 		  var, m_sm.get_state_name (state));
+  if (get_logger ())
+    log ("  m_callee_sm_inst: %s",
+	 summary_event->m_callee_sm_inst.to_str (m_sm).c_str ());
 
   const int caller_stack_depth = summary_event->m_depth;
 
+  if (TREE_CODE (var) == PARM_DECL)
+    var = ssa_default_def (callee_node->m_fun, var);
+
+  /* FIXME: potentially use any state for VAR within
+     summary_event->m_callee_sm_inst.  */
+  summary_point start
+    = summary_point (0 /*FIXME */,
+		     var_state_t (var, 0)); /* FIXME.  */
+  summary_edge state_edge (start,
+			   summary_point (0 /* FIXME */,
+					  var_state_t (var, state)));
+  if (get_logger ())
+    log (" state_edge: %s", state_edge.to_str (m_sm).c_str ());
 
   checker_path *result = new checker_path ();
 
@@ -2538,6 +2573,8 @@ engine::expand_call_summary (const call_summary *summary_event,
 		    result->add_event (event);
 		  }
 
+		// FIXME: update state_edge as needed
+
 #if 0
 		expand_call_sm_context ctxt (this, b, sm_inst,
 					     old_sm_inst, path, ps);
@@ -2576,9 +2613,9 @@ engine::expand_call_summary (const call_summary *summary_event,
 
 	if (node->return_p ())
 	  break;
-	if (node->m_succs.length () > 0)
+	const superedge *succ = get_next_expansion_edge (node, state_edge);
+	if (succ)
 	  {
-	    const superedge *succ = node->m_succs[0];
 	    // FIXME: must be in same function, etc etc
 	    if (succ->m_kind == SUPEREDGE_CFG_EDGE)
 	      {
@@ -2607,6 +2644,63 @@ engine::expand_call_summary (const call_summary *summary_event,
   return result;
 }
 
+/* FIXME.  Subroutine of expand_call_summary.  */
+
+const superedge *
+engine::get_next_expansion_edge (const supernode *node,
+				 summary_edge desired_suffix)
+{
+  JIT_LOG_FUNC_1 (get_logger (), "SN: %i", node->m_index);
+  if (get_logger ())
+    log ("desired_suffix: %s", desired_suffix.to_str (m_sm).c_str ());
+
+  /* Try to find a successor edge where the destination node contains
+     the DESIRED_SUFFIX.  */
+
+  int i;
+  superedge *succ;
+  FOR_EACH_VEC_ELT (node->m_succs, i, succ)
+    {
+      // FIXME:
+      return succ;
+#if 0
+     if (get_logger ())
+	{
+	  log ("considering edge: SN:%i -> SN:%i",
+	       succ->m_src->m_index, succ->m_dest->m_index);
+	}
+
+      per_node_t &data = const_cast <engine &> (*this).get_data_for_node (succ->m_dest);
+      const set_of_summary_edges &sfx_add = data.m_sfx_add;
+      const set_of_summary_edges &sfx_transition = data.m_sfx_transition;
+      for (set_of_summary_edges::iterator iter = sfx_add.begin ();
+	   iter != sfx_add.end ();
+	   ++iter)
+	{
+	  if ((*iter) == desired_suffix)
+	    {
+	      log ("found desired_suffix in next's sfx_add");
+	      return succ;
+	    }
+	}
+      for (set_of_summary_edges::iterator iter = sfx_transition.begin ();
+	   iter != sfx_transition.end ();
+	   ++iter)
+	{
+	  if ((*iter) == desired_suffix)
+	    {
+	      log ("found desired_suffix in next's sfx_transition");
+	      return succ;
+	    }
+	}
+#endif
+    }
+
+  log ("did not find a edge with desired_suffix");
+  return NULL;
+}
+
+
 /* Attempt to generate a path, based on IN_PATH, which should end with
    VAR in STATE.
 
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 9690536..5902c62 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -44,7 +44,7 @@ along with GCC; see the file COPYING3.  If not see
    The work is done by "class layout", which embeds and uses
    "class colorizer" and "class layout_range" to get things done.  */
 
-namespace {
+//namespace {
 
 /* The state at a given point of the source code, assuming that we're
    in a range: which range are we in, and whether we should draw a caret at
@@ -2230,7 +2230,7 @@ layout::print_line (linenum_type row)
   print_trailing_fixits (row);
 }
 
-} /* End of anonymous namespace.  */
+//} /* End of anonymous namespace.  */
 
 /* If LOC is within the spans of lines that will already be printed for
    this gcc_rich_location, then add it as a secondary location and return true.
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-11.c b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-11.c
index 0de42a5..f3cebf8 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-11.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-11.c
@@ -9,18 +9,10 @@ may_call_free (void *victim)
   if (some_condition ())
     return;
 
-  free (victim); /* { dg-warning "double-free of 'victim'" "" { xfail *-*-* } } */ 
-  /* TODO(xfail): when expanding the call summary for the first call to
-     may_call_free it's picking the early-return path, and not purging
-     locals from the path state.  Hence it gets a contradiction with
-       "_1 is true"
-     and
-       "_1 is false"
-     and decides the path is impossible.
-
-     TODO: purge the locals when returning from a function.
-     TODO: better expansion of call summary, to reflect the target state we're
-     interested in.  */
+  free (victim); /* { dg-warning "double-free of 'victim'" } */ 
+  /* { dg-message "state of 'victim': 'start' -> 'free'" "" { xfail *-*-* } .-1 } */
+  /* TODO(xfail): better expansion of call summary, to reflect the target
+     state we're interested in.  */
 }
 
 void test (void *ptr)
-- 
1.8.5.3

