From e5adf51110d8edd48bc24b7eeffe4eed2153ec69 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 28 Feb 2019 19:01:57 -0500
Subject: [PATCH 020/169] FIXME: initial implementation of leak detection

---
 gcc/analyzer/constraints.cc              |  16 ++-
 gcc/analyzer/constraints.h               |   3 +
 gcc/analyzer/engine.cc                   | 186 +++++++++++++++++++++++++++++--
 gcc/analyzer/hardcoded-state-machine.cc  |   8 +-
 gcc/testsuite/gcc.dg/analyzer/file-1.c   |   8 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-1.c |  17 ++-
 6 files changed, 217 insertions(+), 21 deletions(-)

diff --git a/gcc/analyzer/constraints.cc b/gcc/analyzer/constraints.cc
index 4cbec64..1f94eb8 100644
--- a/gcc/analyzer/constraints.cc
+++ b/gcc/analyzer/constraints.cc
@@ -58,14 +58,16 @@ path_var::to_str () const
 
 /* FIXME.  */
 
-equiv_class::equiv_class () : m_constant (NULL_TREE), m_vars ()
+equiv_class::equiv_class () : m_constant (NULL_TREE), m_vars (), m_escaped (false)
 {
 }
 
 /* FIXME.  */
 
 equiv_class::equiv_class (const equiv_class &other)
-: m_constant (other.m_constant)
+: m_constant (other.m_constant),
+  m_vars (),
+  m_escaped (other.m_escaped)
 {
   // FIXME: copy-ctor for auto_vec!
   int i;
@@ -95,6 +97,8 @@ equiv_class::to_str () const
 	result += " == ";
       result += expr_to_string (m_constant);
     }
+  if (m_escaped)
+    result += " (escaped)";
   return result + "}";
 }
 
@@ -106,7 +110,13 @@ equiv_class::add (path_var v)
   if (CONSTANT_CLASS_P (v.m_tree))
     m_constant = v.m_tree;
   else
-    m_vars.safe_push (v);
+    {
+      m_vars.safe_push (v);
+
+      if (DECL_P (v.m_tree)
+	  && is_global_var (v.m_tree))
+	m_escaped = true;
+    }
 }
 
 /* Remove V from this equivalence class.
diff --git a/gcc/analyzer/constraints.h b/gcc/analyzer/constraints.h
index b82ea88..ae8011d 100644
--- a/gcc/analyzer/constraints.h
+++ b/gcc/analyzer/constraints.h
@@ -68,6 +68,9 @@ public:
 
   // FIXME: this is a bad representation, probably should be a set:
   auto_vec<path_var> m_vars;
+
+  /* Has this value escaped from this function?  */
+  bool m_escaped;
 };
 
 enum constraint_op
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 882e305..6ec1cbe 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -61,6 +61,8 @@ struct var_state_t
   : m_var (var), m_state (state)
   {}
 
+  bool is_placeholder_p () const { return m_var == get_placeholder (); }
+
   std::string to_str (const state_machine &sm) const
   {
     if (m_var == get_placeholder ())
@@ -355,6 +357,18 @@ public:
     return result;
   }
 
+  gimple *get_last_stmt () const
+  {
+    int i;
+    const supernode *node;
+    FOR_EACH_VEC_ELT_REVERSE (m_nodes, i, node)
+      {
+	if (gimple *last_stmt = node->get_last_stmt ())
+	  return last_stmt;
+      }
+    return NULL;
+  }
+
   diagnostic_path m_path;
   auto_vec <const supernode *> m_nodes; // FIXME: copy-ctor etc
 };
@@ -377,7 +391,12 @@ public:
 					  const supernode *node);
 
   void dfs_traverse (sm_instance sm_inst, backtrace b,
-		     path_state ps, const supernode *node);
+		     path_state ps, const supernode *node,
+		     const superedge *last_edge);
+  void on_phi (const supernode *node, const gphi *phi,
+	       sm_instance *sm_inst, const sm_instance *old_sm_inst,
+	       const backtrace &b, diagnostic_path *path, path_state *ps,
+	       const superedge *last_edge);
   void on_stmt (const supernode *node, const gimple *stmt,
 		sm_instance *sm_inst, const sm_instance *old_sm_inst,
 		const backtrace &b, diagnostic_path *path, path_state *ps);
@@ -405,6 +424,9 @@ public:
 
   void log_stats () const;
 
+  void detect_leaks (const supernode *node, const sm_instance &sm_inst,
+		     const backtrace &b, const path_state &ps);
+
   const supergraph &m_sg;
 
   // FIXME: this implies one traversal per state machine; should we instead
@@ -625,7 +647,8 @@ private:
 
 void
 engine::dfs_traverse (sm_instance sm_inst, backtrace b,
-		      path_state ps, const supernode *node)
+		      path_state ps, const supernode *node,
+		      const superedge *last_edge)
 {
   JIT_LOG_FUNC_1 (get_logger (), "SN: %i", node->m_index);
   m_num_traverse_calls++;
@@ -662,6 +685,18 @@ engine::dfs_traverse (sm_instance sm_inst, backtrace b,
   // FIXME: ^^^ does copy-ctor for hash_set do a deep copy?
   // Looks like it might be a shallow copy
 
+  /* Phi nodes.  */
+  // FIXME: do these have to somehow happen simultaneously?
+  for (gphi_iterator gpi = const_cast<supernode *>(node)->start_phis ();
+       !gsi_end_p (gpi); gsi_next (&gpi))
+    {
+      sm_instance per_stmt_old_sm_inst = sm_inst;
+      gphi *phi = gpi.phi ();
+      on_phi (node, phi, &sm_inst, &per_stmt_old_sm_inst,
+	      b, &b.m_path, &ps, last_edge);
+    }
+
+  /* Stmts.  */
   int i;
   gimple *stmt;
   FOR_EACH_VEC_ELT (node->m_stmts, i, stmt)
@@ -713,7 +748,7 @@ engine::dfs_traverse (sm_instance sm_inst, backtrace b,
 
   if (node->return_p ())
     {
-      // TODO: leak detection
+      detect_leaks (node, sm_inst, b, ps);
       relax (b);
     }
   else
@@ -764,12 +799,57 @@ engine::dfs_traverse (sm_instance sm_inst, backtrace b,
 				   &new_b.m_path, &new_ps);
 	   m_sm.on_edge (&ctxt, last_stmt, succ);
 
-	   dfs_traverse (per_edge_new_sm_inst, new_b, new_ps, succ->m_dest);
+	   dfs_traverse (per_edge_new_sm_inst, new_b, new_ps, succ->m_dest,
+			 succ);
 	}
     }
 }
 
 void
+engine::on_phi (const supernode *node, const gphi *phi,
+		sm_instance *sm_inst, const sm_instance *old_sm_inst,
+		const backtrace &b, /* FIXME: <<< this is purely so that we can generate full paths for debugging */
+		diagnostic_path *path, path_state *ps,
+		const superedge *last_edge)
+{
+  JIT_LOG_FUNC (get_logger ());
+  if (get_logger ())
+    {
+      pretty_printer pp;
+      pp_gimple_stmt_1 (&pp, const_cast <gphi *> (phi) , 0, (dump_flags_t)0);
+      // FIXME: const_cast!
+      log ("phi: %s", pp_formatted_text (&pp));
+      log ("sm_inst: %s", sm_inst->to_str (m_sm).c_str ());
+    }
+  
+  gcc_assert (last_edge);
+  const cfg_superedge *last_cfg_superedge = last_edge->dyn_cast_cfg_superedge ();
+  gcc_assert (last_cfg_superedge);
+  // FIXME: this ought to be an as_a<>
+
+  /* How to get from edge to phi arg?
+     PP shows: gimple_phi_arg_edge (phi, i)->src;
+     which is EDGE_PRED (gimple_bb (phi), i);
+     i.e. a lookup by index within bb->preds based on index within 
+     Aha:
+       gimple_phi_arg_location_from_edge (gphi *phi, edge e)
+       {
+         return gimple_phi_arg (phi, e->dest_idx)->locus;
+       }
+       which implies we can use the e->dest_idx to get at the phi arg.  */
+
+  size_t index = last_cfg_superedge->m_cfg_edge->dest_idx;
+  tree src = gimple_phi_arg_def (const_cast <gphi *> (phi), index); // FIXME const_cast
+  tree lhs = gimple_phi_result (phi);
+  // FIXME: this leads to infinite traversals on loops:
+  // e.g. for malloc-1.c, where we constantly hit:
+  //  "after pruning to cache, sm_inst: <start, {‘i_1’ null}>"
+#if 0
+  on_assignment (node, phi, sm_inst, old_sm_inst, lhs, src, ps);
+#endif
+}
+
+void
 engine::on_stmt (const supernode *node, const gimple *stmt,
 		 sm_instance *sm_inst, const sm_instance *old_sm_inst,
 		 const backtrace &b, /* FIXME: <<< this is purely so that we can generate full paths for debugging */
@@ -908,6 +988,7 @@ engine::on_assignment (const supernode *node, const gimple *stmt,
   if (ps)
     ps->on_assignment (path_var (lhs, 0), path_var (rhs, 0)); // FIXME stack depths
 
+  /* Copy the state of RHS to LHS.  */
   for (set_of_vars::iterator iter = old_sm_inst->m_active_vars.begin ();
        iter != old_sm_inst->m_active_vars.end ();
        ++iter)
@@ -1113,7 +1194,8 @@ engine::follow_call (sm_instance sm_inst, backtrace b, path_state ps,
     = m_sg.get_node_for_function_entry (edge->callee->get_fun ());
 
   /* Traverse into the callee.  */
-  dfs_traverse (callee_sm_inst, backtrace (), ps, callee_node);
+  dfs_traverse (callee_sm_inst, backtrace (), ps, callee_node,
+		NULL /* FIXME */);
 
   /* Build set S of transition/add edges based on callee
      function summary.  */
@@ -1187,10 +1269,13 @@ engine::follow_call (sm_instance sm_inst, backtrace b, path_state ps,
     sm_instance restored_sm
       = restore_after_call (callee_summary_sm_inst, edge, rest_of_caller_node);
     log ("rest of call is in SN: %i", rest_of_caller_node->m_index);
-    dfs_traverse (restored_sm, b, ps, rest_of_caller_node);
+    dfs_traverse (restored_sm, b, ps, rest_of_caller_node,
+		  NULL /* FIXME */);
   }
 }
 
+/* FIXME.  */
+
 void
 engine::log_stats () const
 {
@@ -1201,6 +1286,90 @@ engine::log_stats () const
   log ("m_on_stmt_calls: %i", m_on_stmt_calls);
 }
 
+/* Attempt to get the best stmt within B for a leak of VAR detected at the
+   end of B.  */
+
+static const gimple *
+get_stmt_for_leak (tree var, const backtrace &b)
+{
+  int i;
+  const supernode *node;
+  FOR_EACH_VEC_ELT_REVERSE (b.m_nodes, i, node)
+    {
+      int j;
+      gimple *stmt;
+      FOR_EACH_VEC_ELT_REVERSE (node->m_stmts, j, stmt)
+	if (stmt->location != UNKNOWN_LOCATION)
+	  return stmt;
+    }
+  return NULL;
+}
+
+/* FIXME.  */
+
+void
+engine::detect_leaks (const supernode *node, const sm_instance &sm_inst,
+		      const backtrace &b, const path_state &old_ps)
+{
+  JIT_LOG_FUNC_2 (get_logger (), "SN: %i function: %s",
+		  node->m_index, function_name (node->m_fun));
+  if (get_logger ())
+    {
+      log ("sm_inst: %s", sm_inst.to_str (m_sm).c_str ());
+      log ("ps: %s", old_ps.to_str ().c_str ());
+    }
+
+  path_state new_ps (old_ps);
+
+  // FIXME: mark ec of return value (if any) as escaping
+  {
+    tree result = DECL_RESULT (node->m_fun->decl);
+    if (result)
+      {
+	// FIXME: stack depth:
+	equiv_class_id ec_id = new_ps.get_or_add_equiv_class (path_var (result, 0));
+	equiv_class &ec = ec_id.get_obj (new_ps);
+	ec.m_escaped = true;
+      }
+  }
+
+  for (hash_set<var_state_t>::iterator iter
+	 = sm_inst.m_active_vars.m_set.begin ();
+       iter != sm_inst.m_active_vars.m_set.end ();
+       ++iter)
+    {
+      if ((*iter).is_placeholder_p ())
+	continue;
+      if (get_logger ())
+	log ("considering %s", (*iter).to_str (m_sm).c_str ());
+
+      /* Did this var's equivalence class escape?  If so, don't complain
+	 about it leaking.  */
+      // FIXME: stack depth:
+      equiv_class_id ec_id = new_ps.get_or_add_equiv_class (path_var ((*iter).m_var, 0));
+      equiv_class &ec = ec_id.get_obj (new_ps);
+      if (get_logger ())
+	log ("within %s", ec.to_str ().c_str ());
+      if (ec.m_escaped)
+	{
+	  log ("escaped");
+	  continue;
+	}
+
+      // TODO: determine most appropriate stmt
+      const gimple *stmt = get_stmt_for_leak ((*iter).m_var, b);
+
+      // FIXME: various redundant copies here:
+      diagnostic_path p (b.m_path);
+      sm_instance new_sm_inst (sm_inst);
+      engine_sm_context ctxt (this,
+			      &new_sm_inst,
+			      &sm_inst,
+			      &p, &new_ps);
+      m_sm.on_leak (&ctxt, node, stmt, (*iter).m_var, (*iter).m_state);
+    }
+}
+
 class engine_annotator : public dot_annotator
 {
 public:
@@ -1266,6 +1435,8 @@ function_comparator (const void *pn1, const void *pn2)
 void
 run_one_checker (const supergraph &sg, const state_machine &sm, logger *logger)
 {
+  JIT_LOG_FUNC_1 (logger, "sm: %s", sm.get_name ());
+
   engine e (sg, sm, logger);
 
   cgraph_node *node;
@@ -1293,7 +1464,8 @@ run_one_checker (const supergraph &sg, const state_machine &sm, logger *logger)
 
       e.log ("function: %s", function_name (fun));
       e.dfs_traverse (sm_inst, b, ps,
-		      sg.get_node_for_function_entry (fun));
+		      sg.get_node_for_function_entry (fun),
+		      NULL);
     }
 
   engine_annotator a (e);
diff --git a/gcc/analyzer/hardcoded-state-machine.cc b/gcc/analyzer/hardcoded-state-machine.cc
index a6f4039..bfe4a88 100644
--- a/gcc/analyzer/hardcoded-state-machine.cc
+++ b/gcc/analyzer/hardcoded-state-machine.cc
@@ -235,7 +235,8 @@ malloc_state_machine::on_leak (sm_context *sm_ctxt,
 			       tree var,
 			       state_machine::state_t state) const
 {
-  // TODO
+  sm_ctxt->error (node, stmt, var, m_unchecked, "leak of %qE", var);
+  sm_ctxt->error (node, stmt, var, m_nonnull, "leak of %qE", var);
 }
 
 ////////////////////////////////////////////////////////////////////////////
@@ -262,7 +263,7 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt,
   // FIXME: do all of this from data
   if (const gcall *call = dyn_cast <const gcall *> (stmt))
     {
-      if (is_named_call_p (call, "fopen", 1))
+      if (is_named_call_p (call, "fopen", 2))
 	{
 	  if (DEBUG)
 	    inform (stmt->location, "DBG: got fopen");
@@ -344,5 +345,6 @@ fileptr_state_machine::on_leak (sm_context *sm_ctxt,
 				tree var,
 				state_machine::state_t state) const
 {
-  // TODO
+  sm_ctxt->error (node, stmt, var, m_unchecked, "leak of FILE %qE", var);
+  sm_ctxt->error (node, stmt, var, m_nonnull, "leak of FILE %qE", var);
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/file-1.c b/gcc/testsuite/gcc.dg/analyzer/file-1.c
index fadd36d..86d08e0 100644
--- a/gcc/testsuite/gcc.dg/analyzer/file-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/file-1.c
@@ -19,9 +19,8 @@ test_2 (const char *src, const char *dst)
     return;
 
   FILE *f_out = fopen (src, "w");
-  if (f_out)
-    return; /* { dg-error "leak of open FILE 'f_in'" "" { xfail *-*-* } } */ 
-  /* TODO(xfail): leak detection not yet implemented.  */
+  if (!f_out)
+    return; /* { dg-error "leak of FILE 'f_in'" } */ 
 
   fclose (f_out);
   fclose (f_in);
@@ -31,6 +30,5 @@ void
 test_3 (const char *path)
 {
   FILE *f = fopen (path, "r");
-  return; /* { dg-error "leak of open FILE 'f'" "" { xfail *-*-* } } */ 
-  /* TODO(xfail): leak detection not yet implemented.  */
+  return; /* { dg-error "leak of FILE 'f'" } */ 
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index a5ef922..497d36c 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -66,6 +66,8 @@ void *test_8 (void)
     return NULL;
   memset(ptr, 0, 4096);
   return ptr;
+  /* This needs phi nodes to affect equivalence classes, or we get a false report
+     of a leak.  */
 }
 
 void test_9 (void)
@@ -75,7 +77,10 @@ void test_9 (void)
   int i;
   for (i = 0; i < 1024; i++)
     free (ptr); /* { dg-error "double-free of 'ptr'" } */
-}
+} /* { dg-error "leak of 'ptr'" } */ 
+/* FIXME: false positive about leak here: presumably we don't yet know
+   that the loop will execute at least once (presumably as our constraints
+   aren't clever enough).  */
 
 void test_10 (void)
 {
@@ -120,8 +125,7 @@ void test_13 (void)
   if (!q)
     {
       free (q);
-      return; /* { dg-error "leak of 'p'" "" { xfail *-*-* } } */ 
-      /* TODO(xfail): implement leak detection.  */
+      return; /* { dg-error "leak of 'p'" } */ 
     }
   bar ();
   free (q);
@@ -210,3 +214,10 @@ void test_19 (void)
   ptr = NULL;
   free (ptr);
 }
+
+void *global_ptr_20;
+
+void test_20 (void)
+{
+  global_ptr_20 = malloc (1024);
+}
-- 
1.8.5.3

