From c80b89988504c12a1c8bfe4fe599b52b332f3b5e Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 13 Mar 2019 16:46:53 -0400
Subject: [PATCH 101/169] FIXME: attempt to track states when expanding call
 summaries

---
 gcc/analyzer/engine.cc                        | 220 ++++++++++++++++++++------
 gcc/analyzer/supergraph.cc                    |  85 +++++++++-
 gcc/analyzer/supergraph.h                     |  26 +--
 gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c |  35 ++++
 gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c  |  12 +-
 5 files changed, 309 insertions(+), 69 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 2c0f7e7..fda64a9 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -589,6 +589,15 @@ public:
 
   // FIXME
 
+  call_superedge *get_edge_for_call (const supergraph *sg) const
+  {
+    return sg->get_edge_for_call (m_edge);
+  }
+  call_superedge *get_edge_for_return (const supergraph *sg) const
+  {
+    return sg->get_edge_for_return (m_edge);
+  }
+
   const supernode *m_caller_node;
   cgraph_edge *m_edge;
 };
@@ -2444,51 +2453,157 @@ engine::expand_call_summary (const call_summary *summary_event,
   return result;
 }
 
-/* FIXME.  */
-// Clone IN_PATH
+/* Attempt to generate a path, based on IN_PATH, which should end with
+   VAR in STATE.
+
+   Expand relevant call summaries.  */
 
 checker_path *
 engine::expand_path (const checker_path &in_path,
 		     tree var, state_machine::state_t state)
 {
+  JIT_LOG_FUNC (get_logger ());
+  in_path.maybe_log (get_logger (), "in_path");
+
+  /* Build the result backwards.  */
   checker_path *result = new checker_path ();
 
-  // FIXME:
-  // expand call summaries (recursively?)
-  for (unsigned idx = 0; idx < in_path.m_events.length (); idx++)
+  // TODO
+  /* TODO: go backwards, skipping state change events that aren't var
+     but update the pertinent var when state-copying occurs.  */
+  for (int idx = in_path.m_events.length () - 1; idx >= 0; idx--)
     {
-      const checker_event *base_event = in_path.m_events[idx];
-      if (base_event->m_kind != EK_CALL_SUMMARY)
+      checker_event *base_event = in_path.m_events[idx];
+      switch (base_event->m_kind)
 	{
+	default:
+	  gcc_unreachable ();
+
+	case EK_DEBUG:
+	case EK_FUNCTION_ENTRY:
+	case EK_WARNING:
 	  result->add_event (base_event->clone ());
-	  continue;
-	}
+	  break;
+
+	case EK_STATE_CHANGE:
+	  {
+	    state_change_event *state_change = (state_change_event *)base_event;
+	    if (state_change->m_var == var)
+	      {
+		if (state_change->m_origin)
+		  {
+		    log ("event %i: switching var of interest from %qE to %qE",
+			 idx, var, state_change->m_origin);
+		    var = state_change->m_origin;
+		  }
+	      }
+	    result->add_event (base_event->clone ());
+	  }
+	  break;
+
+	case EK_EDGE:
+	  {
+	    edge_event *event = (edge_event *)base_event;
+	    switch (event->m_edge->m_kind)
+	      {
+	      case SUPEREDGE_CFG_EDGE:
+	      case SUPEREDGE_RETURN:
+	      case SUPEREDGE_INTRAPROCEDURAL_CALL:
+		result->add_event (base_event->clone ());
+		break;
+
+	      case SUPEREDGE_CALL:
+		{
+		  // FIXME: use as_a<> here:
+		  const call_superedge *call_edge
+		    = event->m_edge->dyn_cast_call_superedge ();
+		  log ("considering call edge %i (from %qE to %qE); var: %qE",
+		       idx, call_edge->get_caller_decl (),
+		       call_edge->get_callee_decl (), var);
+		  tree maybe_parm = var;
+		  if (TREE_CODE (maybe_parm) == SSA_NAME)
+		    maybe_parm = SSA_NAME_VAR (maybe_parm);
+		  if (TREE_CODE (maybe_parm) == PARM_DECL)
+		    {
+		      // TODO: what about ecs?
+		      tree arg = call_edge->get_arg_for_parm (maybe_parm);
+		      if (arg)
+			{
+			  log ("event %i: switching var of interest from"
+			       " parm %qE to arg %qE",
+			       idx, var, arg);
+			  var = arg;
+			}
+		    }
+		  result->add_event (base_event->clone ());
+		}
+		break;
+	      }
+	  }
+	  break;
 
-      const call_summary *summary_event = (call_summary *)base_event;
-
-      /* TODO: are we interested in a state change within this function?
-	 If not, we can simply eliminate it.
-	 If we are interested, we want to find a path (ideally feasible)
-	 within it that makes the state change happen.  We can probably
-	 figure that out from the suffix summaries.  */
-
-      /* Build path representing the inside of the function.  */
-      checker_path *child_path
-	= expand_call_summary (summary_event, var, state);
-      for (unsigned j = 0; j < child_path->m_events.length (); j++)
-	result->add_event (child_path->m_events[j]->clone ());
-      delete child_path;
-      // FIXME: lots of copying/allocation here;
-      // get it working first before optimizing
+	case EK_CALL_SUMMARY:
+	  {
+	    call_summary *summary_event = (call_summary *)base_event;
+#if 0
+	    inform (event->m_loc, "considering call_summary %i: %s",
+		    idx, base_event->m_desc);
+#endif
+
+	    /* TODO: are we interested in a state change within this function?
+	       If not, we can simply eliminate it.
+	       If we are interested, we want to find a path (ideally feasible)
+	       within it that makes the state change happen.  We can probably
+	       figure that out from the suffix summaries.  */
+
+	    /* Attempt to convert "var" to the callee scope.  */
+	    call_superedge *call_edge
+	      = summary_event->get_edge_for_call (&m_sg);
+	    tree callee_var = call_edge->refine_for_call (var);
+	    if (callee_var == NULL_TREE)
+	      {
+		log ("event: %i"
+		     ": var %qE is not affected by call from %qE to %qE",
+		     idx, var, call_edge->get_caller_decl (),
+		     call_edge->get_callee_decl ());
+		result->add_event (base_event->clone ());
+		break;
+	      }
+	    log ("event %i: var %qE in %qE refined to %qE in %qE",
+		 idx, var, call_edge->get_caller_decl (),
+		 callee_var, call_edge->get_callee_decl ());
+
+	    /* Build path representing the inside of the function.  */
+	    log ("expanding event %i (call summary)", idx);
+	    checker_path *child_path
+	      = expand_call_summary (summary_event, callee_var, state);
+	    // FIXME: but here we want the thing in the end state
+	    // FIXME: maybe we don't want to filter here???
+	    // Or maybe a 2-stage filtering process?
+
+	    /* Add backwards, since we'll be reversing the result.  */
+	    for (int j = child_path->m_events.length () - 1; j >=0 ; j--)
+	      result->add_event (child_path->m_events[j]->clone ());
+	    delete child_path;
+	    // FIXME: how to update var/state???
+	  }
+	  break;
+	}
     }
 
+  /* OK, we have a backwards result; reverse it.  */
+  result->m_events.reverse ();
+
+  result->maybe_log (get_logger (), "result");
+
   return result;
 }
 
 /* FIXME.  */
 
 void
-engine::prune_path (checker_path *path, tree var, state_machine::state_t state)
+engine::prune_path (checker_path *path,
+		    tree var, state_machine::state_t state)
 {
   JIT_LOG_FUNC (get_logger ());
   path->maybe_log (get_logger (), "path");
@@ -2497,7 +2612,7 @@ engine::prune_path (checker_path *path, tree var, state_machine::state_t state)
   /* TODO: go backwards, skipping state change events that aren't var
      but update the pertinent var when state-copying occurs.  */
   int idx = path->m_events.length () - 1;
-  while (idx >= 0 && path->m_events.length () > 0)
+  while (idx >= 0 && idx < path->m_events.length ())
     {
       checker_event *base_event = path->m_events[idx];
       switch (base_event->m_kind)
@@ -2528,13 +2643,15 @@ engine::prune_path (checker_path *path, tree var, state_machine::state_t state)
 	      {
 		if (state_change->m_origin)
 		  {
-		    log ("event %i: switching to origin", idx); // FIXME: show %qE
+		    log ("event %i: switching var of interest from %qE to %qE",
+			 idx, var, state_change->m_origin);
 		    var = state_change->m_origin;
 		  }
 	      }
 	    else if (m_verbosity < 2)
 	      {
-		log ("filtering event %i: unrelated state change", idx);
+		log ("filtering event %i: state change to %qE unrelated to %qE",
+		     idx, state_change->m_var, var);
 		path->delete_event (idx);
 	      }
 	  }
@@ -2547,45 +2664,49 @@ engine::prune_path (checker_path *path, tree var, state_machine::state_t state)
 	    inform (base_event->m_loc, "considering edge_event %i: %s",
 		    idx, base_event->m_desc);
 #endif
-	    // TODO: for now, filter all intraprodcural edges:
-	    if (m_verbosity < 2
-		&& event->m_edge->m_kind == SUPEREDGE_CFG_EDGE)
+	    switch (event->m_edge->m_kind)
 	      {
-		log ("filtering event %i: CFG edge", idx);
-		path->delete_event (idx);
-	      }
-
-	    /* TODO: is this edge significant to var?
-	       See if var can be in other states in the dest, but not in other
-	       states in the src?
-	       Must have multiple sibling edges.  */
+	      case SUPEREDGE_CFG_EDGE:
+		/* TODO: is this edge significant to var?
+		   See if var can be in other states in the dest, but not
+		   in other states in the src?
+		   Must have multiple sibling edges.  */
+		// TODO: for now, filter all intraprocedural edges:
+		if (m_verbosity < 2)
+		  {
+		    log ("filtering event %i: CFG edge", idx);
+		    path->delete_event (idx);
+		  }
+		break;
 
-	    // TODO
+	      case SUPEREDGE_CALL:
+	      case SUPEREDGE_RETURN:
+	      case SUPEREDGE_INTRAPROCEDURAL_CALL:
+		// TODO
+		break;
+	      }
 	  }
 	  break;
 
 	case EK_CALL_SUMMARY:
 	  {
-	    call_summary *event = (call_summary *)base_event;
+	    call_summary *summary_event = (call_summary *)base_event;
 #if 0
 	    inform (event->m_loc, "considering call_summary %i: %s",
 		    idx, base_event->m_desc);
 #endif
-	    // TODO: for now, filter them all:
-	    if (m_verbosity < 1)
+	    /* Filter any that weren't expanded earlier.  */
+	    if (m_verbosity < 2)
 	      {
 		log ("filtering event %i: call summary", idx);
 		path->delete_event (idx);
 	      }
-
-	    // TODO
 	  }
 	  break;
 
 	case EK_WARNING:
 	  break;
 	}
-
       idx--;
     }
 
@@ -2595,7 +2716,7 @@ engine::prune_path (checker_path *path, tree var, state_machine::state_t state)
     {
       // FIXME
       int idx = path->m_events.length () - 1;
-      while (idx >= 0 && path->m_events.length () > 0)
+      while (idx >= 0 && idx < path->m_events.length ())
 	{
 	  checker_event *base_event = path->m_events[idx];
 	  if (base_event->m_kind == EK_FUNCTION_ENTRY)
@@ -2604,8 +2725,7 @@ engine::prune_path (checker_path *path, tree var, state_machine::state_t state)
 		   " function entry for purely intraprocedural path", idx);
 	      path->delete_event (idx);
 	    }
-	  else
-	    idx--;
+	  idx--;
 	}
     }
 }
diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc
index 4dfabe6..a7f1e4e 100644
--- a/gcc/analyzer/supergraph.cc
+++ b/gcc/analyzer/supergraph.cc
@@ -152,7 +152,7 @@ supergraph::supergraph ()
 	    = ENTRY_BLOCK_PTR_FOR_FN (edge->callee->get_fun ());
 	  supernode *callee_supernode
 	    = *m_bb_to_initial_node.get (callee_cfg_block);
-	  superedge *sedge
+	  call_superedge *sedge
 	    = add_callgraph_edge (caller_prev_supernode, callee_supernode,
 				  SUPEREDGE_CALL, edge);
 	  m_cgraph_edge_to_call_superedge.put (edge, sedge);
@@ -172,7 +172,7 @@ supergraph::supergraph ()
 	    = EXIT_BLOCK_PTR_FOR_FN (edge->callee->get_fun ());
 	  supernode *callee_supernode
 	    = *m_bb_to_initial_node.get (callee_cfg_block);
-	  superedge *sedge
+	  call_superedge *sedge
 	    = add_callgraph_edge (callee_supernode, caller_next_supernode,
 				  SUPEREDGE_RETURN, edge);
 	  m_cgraph_edge_to_return_superedge.put (edge, sedge);
@@ -340,11 +340,13 @@ supergraph::add_cfg_edge (supernode *src, supernode *dest, ::edge e, int idx)
     return add_edge (new cfg_superedge (src, dest, e));
 }
 
-superedge *
+call_superedge *
 supergraph::add_callgraph_edge (supernode *src, supernode *dest,
 				enum edge_kind kind, cgraph_edge *cedge)
 {
-  return add_edge (new call_superedge (src, dest, kind, cedge));
+  call_superedge *new_edge = new call_superedge (src, dest, kind, cedge);
+  add_edge (new_edge);
+  return new_edge;
 }
 
 superedge *
@@ -735,3 +737,78 @@ call_superedge::dump_label_to_pp (pretty_printer *pp) const
       break;
     }
 }
+
+/* FIXME.  */
+
+tree
+call_superedge::get_callee_decl () const
+{
+  return m_cedge->callee->get_fun ()->decl;
+}
+
+/* FIXME.  */
+
+tree
+call_superedge::get_caller_decl () const
+{
+  return m_cedge->caller->get_fun ()->decl;
+}
+
+/* FIXME.  */
+
+tree
+call_superedge::get_arg_for_parm (tree parm_to_find) const
+{
+  gcc_assert  (TREE_CODE (parm_to_find) == PARM_DECL);
+
+  tree callee = get_callee_decl ();
+
+  int i = 0;
+  for (tree iter_parm = DECL_ARGUMENTS (callee); iter_parm;
+       iter_parm = DECL_CHAIN (iter_parm), ++i)
+    {
+      if (iter_parm == parm_to_find)
+	return gimple_call_arg (get_call_stmt (), i);
+    }
+
+  /* Not found.  */
+  return NULL_TREE;
+}
+
+/* FIXME.  */
+
+tree
+call_superedge::get_parm_for_arg (tree arg_to_find) const
+{
+  tree callee = get_callee_decl ();
+
+  int i = 0;
+  for (tree iter_parm = DECL_ARGUMENTS (callee); iter_parm;
+       iter_parm = DECL_CHAIN (iter_parm), ++i)
+    {
+      if (arg_to_find == gimple_call_arg (get_call_stmt (), i))
+	return iter_parm;
+    }
+
+  // FIXME: what if more than one?
+
+  /* Not found.  */
+  return NULL_TREE;
+}
+
+/* FIXME.  */
+
+tree
+call_superedge::refine_for_call (tree caller_expr) const
+{
+  /* Is it an argument (actual param)?  If so, convert to
+     parameter (formal param).  */
+  tree parm = get_parm_for_arg (caller_expr);
+  if (parm)
+    return parm;
+  /* Otherwise try return value.  */
+  if (caller_expr == gimple_call_lhs (get_call_stmt ()))
+    return DECL_RESULT (get_callee_decl ());
+
+  return NULL_TREE;
+}
diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h
index c2c5aca..e2e26c8 100644
--- a/gcc/analyzer/supergraph.h
+++ b/gcc/analyzer/supergraph.h
@@ -98,15 +98,15 @@ public:
 	      (m_cgraph_edge_to_caller_next_node).get (edge));
   }
 
-  superedge *get_edge_for_call (cgraph_edge *edge) const
+  call_superedge *get_edge_for_call (cgraph_edge *edge) const
   {
-    return (*const_cast <cgraph_edge_to_superedge_t &>
+    return (*const_cast <cgraph_edge_to_call_superedge_t &>
 	      (m_cgraph_edge_to_call_superedge).get (edge));
   }
 
-  superedge *get_edge_for_return (cgraph_edge *edge) const
+  call_superedge *get_edge_for_return (cgraph_edge *edge) const
   {
-    return (*const_cast <cgraph_edge_to_superedge_t &>
+    return (*const_cast <cgraph_edge_to_call_superedge_t &>
 	      (m_cgraph_edge_to_return_superedge).get (edge));
   }
 
@@ -124,8 +124,8 @@ private:
   supernode *add_node (function *fun, basic_block bb, gcall *returning_call,
 		       gimple_seq phi_nodes);
   superedge *add_cfg_edge (supernode *src, supernode *dest, ::edge e, int idx);
-  superedge *add_callgraph_edge (supernode *src, supernode *dest,
-				 enum edge_kind kind, cgraph_edge *cedge);
+  call_superedge *add_callgraph_edge (supernode *src, supernode *dest,
+				      enum edge_kind kind, cgraph_edge *cedge);
   superedge *add_edge (superedge *e);
 
   /* Data.  */
@@ -141,9 +141,9 @@ private:
   cgraph_edge_to_node_t m_cgraph_edge_to_caller_prev_node;
   cgraph_edge_to_node_t m_cgraph_edge_to_caller_next_node;
 
-  typedef hash_map<cgraph_edge *, superedge *> cgraph_edge_to_superedge_t;
-  cgraph_edge_to_superedge_t m_cgraph_edge_to_call_superedge;
-  cgraph_edge_to_superedge_t m_cgraph_edge_to_return_superedge;
+  typedef hash_map<cgraph_edge *, call_superedge *> cgraph_edge_to_call_superedge_t;
+  cgraph_edge_to_call_superedge_t m_cgraph_edge_to_call_superedge;
+  cgraph_edge_to_call_superedge_t m_cgraph_edge_to_return_superedge;
 };
 
 /* FIXME.  */
@@ -264,6 +264,14 @@ class call_superedge : public superedge
     return this;
   }
 
+  tree get_callee_decl () const;
+  tree get_caller_decl () const;
+  gcall *get_call_stmt () const { return m_cedge->call_stmt; }
+  tree get_arg_for_parm (tree parm) const;
+  tree get_parm_for_arg (tree arg) const;
+
+  tree refine_for_call (tree caller_expr) const;
+
   cgraph_edge *m_cedge;
 };
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c
new file mode 100644
index 0000000..9965629
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c
@@ -0,0 +1,35 @@
+#include <stdlib.h>
+
+void
+calls_free (void *victim) /* { dg-message "\\(3\\) entry to 'calls_free'" } */
+/* { dg-message "\\(7\\) entry to 'calls_free'" "" { target *-*-* } .-1 } */
+{
+  free (victim); /* { dg-warning "double-free of 'victim'" } */
+  /* { dg-message "\\(4\\) state of 'victim': 'start' -> 'free'" "" { target *-*-* } .-1 } */
+  /* { dg-message "\\(8\\) here \\('victim' is in state 'free'\\)" "" { target *-*-* } .-2 } */
+
+  /* TODO: would this be better emitted at the callsite,
+     for such a simple wrapper?  */
+}
+
+void do_stuff (void)
+{
+  /* Empty.  Irrelevant, and thus should not be expanded into in paths.  */
+}
+
+void test (void *ptr) /* { dg-message "\\(1\\) entry to 'test'" } */
+{
+  do_stuff ();
+
+  calls_free (ptr); /* { dg-message "\\(2\\) calling 'calls_free' from 'test'" } */
+  /* { dg-message "\\(5\\) returning to 'test' from 'calls_free'" "" { target *-*-* } .-1 } */
+
+  //do_stuff ();
+
+  calls_free (ptr); /* { dg-message "\\(6\\) calling 'calls_free' from 'test'" } */
+
+  do_stuff ();
+}
+
+/* FIXME: this doesn't warn if I re-enable the intermediate "do_stuff"
+   call; why not?  */
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
index 2c09d76..f633960 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
@@ -33,9 +33,9 @@ boxed_int *
 make_boxed_int (int i) /* { dg-warning "summary: add: '<return-value>': 'start' -> 'unchecked'" } */
 /* { dg-message "\\(3\\) entry to 'make_boxed_int'" "" { target *-*-* } .-1 } */
 {
-  boxed_int *bi = (boxed_int *)wrapped_malloc (sizeof (boxed_int));
-  bi->i = i;
-  return bi;
+  boxed_int *result = (boxed_int *)wrapped_malloc (sizeof (boxed_int));
+  result->i = i;
+  return result;
 }
 
 void
@@ -53,13 +53,13 @@ free_boxed_int (boxed_int *bi) /* { dg-warning "summary: add: 'bi': 'start' -> '
 
 void test (int i) /* { dg-message "\\(1\\) entry to 'test'" } */
 {
-  boxed_int *bi = make_boxed_int (i); /* { dg-message "\\(2\\) calling 'make_boxed_int' from 'test'" } */
+  boxed_int *obj = make_boxed_int (i); /* { dg-message "\\(2\\) calling 'make_boxed_int' from 'test'" } */
   /* { dg-message "\\(5\\) returning to 'test' from 'make_boxed_int'" "" { target *-*-* } .-1 } */
 
-  free_boxed_int (bi); /* { dg-message "\\(6\\) calling 'free_boxed_int' from 'test'" } */
+  free_boxed_int (obj); /* { dg-message "\\(6\\) calling 'free_boxed_int' from 'test'" } */
   /* { dg-message "\\(13\\) returning to 'test' from 'free_boxed_int'" "" { target *-*-* } .-1 } */
 
-  free_boxed_int (bi); /* { dg-message "\\(14\\) calling 'free_boxed_int' from 'test'" } */
+  free_boxed_int (obj); /* { dg-message "\\(14\\) calling 'free_boxed_int' from 'test'" } */
 }
 
 /* TODO: various strange entries in the path.  */
-- 
1.8.5.3

