analyzer: fix issues with phi handling

The analyzer's state purging code was overzealously purging state
for ssa names that might be used within phi nodes, leading to
false positives from -Wanalyzer-use-of-uninitialized-value.

This patch updates phi handling in the analyzer to fix these issues.

gcc/analyzer/ChangeLog:
	* region-model.cc (region_model::handle_phi): Add "old_state"
	param and use it.
	(region_model::update_for_phis): Update so that all of the phi
	stmts are effectively handled simultaneously, rather than in
	order.
	* region-model.h (region_model::handle_phi): Add "old_state"
	param.
	* state-purge.cc (self_referential_phi_p): Replace with...
	(name_used_by_phis_p): ...this new function.
	(state_purge_per_ssa_name::process_point): Update to use the
	above, so that all phi stmts at a basic block are effectively
	considered simultaneously, and only consider the phi arguments for
	the pertinent in-edge.
	* supergraph.cc (cfg_superedge::get_phi_arg_idx): New.
	(cfg_superedge::get_phi_arg): Use the above.
	* supergraph.h (cfg_superedge::get_phi_arg_idx): New decl.

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/explode-2.c: Remove xfail.
	* gcc.dg/analyzer/explode-2a.c: Remove expected leak warning on
	while stmt.
	* gcc.dg/analyzer/phi-2.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

8 files changed, 78 insertions, 26 deletions

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 6d02c60..c029759 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1553,11 +1553,14 @@ region_model::on_longjmp (const gcall *longjmp_call, const gcall *setjmp_call,
 
 /* Update this region_model for a phi stmt of the form
      LHS = PHI <...RHS...>.
-   where RHS is for the appropriate edge.  */
+   where RHS is for the appropriate edge.
+   Get state from OLD_STATE so that all of the phi stmts for a basic block
+   are effectively handled simultaneously.  */
 
 void
 region_model::handle_phi (const gphi *phi,
 			  tree lhs, tree rhs,
+			  const region_model &old_state,
 			  region_model_context *ctxt)
 {
   /* For now, don't bother tracking the .MEM SSA names.  */
@@ -1566,9 +1569,10 @@ region_model::handle_phi (const gphi *phi,
       if (VAR_DECL_IS_VIRTUAL_OPERAND (var))
 	return;
 
-  const svalue *rhs_sval = get_rvalue (rhs, ctxt);
+  const svalue *src_sval = old_state.get_rvalue (rhs, ctxt);
+  const region *dst_reg = old_state.get_lvalue (lhs, ctxt);
 
-  set_value (get_lvalue (lhs, ctxt), rhs_sval, ctxt);
+  set_value (dst_reg, src_sval, ctxt);
 
   if (ctxt)
     ctxt->on_phi (phi, rhs);
@@ -3036,6 +3040,10 @@ region_model::update_for_phis (const supernode *snode,
 {
   gcc_assert (last_cfg_superedge);
 
+  /* Copy this state and pass it to handle_phi so that all of the phi stmts
+     are effectively handled simultaneously.  */
+  const region_model old_state (*this);
+
   for (gphi_iterator gpi = const_cast<supernode *>(snode)->start_phis ();
        !gsi_end_p (gpi); gsi_next (&gpi))
     {
@@ -3044,8 +3052,8 @@ region_model::update_for_phis (const supernode *snode,
       tree src = last_cfg_superedge->get_phi_arg (phi);
       tree lhs = gimple_phi_result (phi);
 
-      /* Update next_state based on phi.  */
-      handle_phi (phi, lhs, src, ctxt);
+      /* Update next_state based on phi and old_state.  */
+      handle_phi (phi, lhs, src, old_state, ctxt);
     }
 }
 
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 734ec60..cc39929 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -582,6 +582,7 @@ class region_model
 			region_model_context *ctxt);
 
   void handle_phi (const gphi *phi, tree lhs, tree rhs,
+		   const region_model &old_state,
 		   region_model_context *ctxt);
 
   bool maybe_update_for_edge (const superedge &edge,
diff --git a/gcc/analyzer/state-purge.cc b/gcc/analyzer/state-purge.cc
index 3c3b775..bfa48a9 100644
--- a/gcc/analyzer/state-purge.cc
+++ b/gcc/analyzer/state-purge.cc
@@ -288,17 +288,23 @@ state_purge_per_ssa_name::add_to_worklist (const function_point &point,
     }
 }
 
-/* Does this phi depend on itself?
-   e.g. in:
-     added_2 = PHI <added_6(2), added_2(3), added_11(4)>
-   the middle defn (from edge 3) requires added_2 itself.  */
+/* Return true iff NAME is used by any of the phi nodes in SNODE
+   when processing the in-edge with PHI_ARG_IDX.  */
 
 static bool
-self_referential_phi_p (const gphi *phi)
+name_used_by_phis_p (tree name, const supernode *snode,
+		     size_t phi_arg_idx)
 {
-  for (unsigned i = 0; i < gimple_phi_num_args (phi); i++)
-    if (gimple_phi_arg_def (phi, i) == gimple_phi_result (phi))
-      return true;
+  gcc_assert (TREE_CODE (name) == SSA_NAME);
+
+  for (gphi_iterator gpi
+	 = const_cast<supernode *> (snode)->start_phis ();
+       !gsi_end_p (gpi); gsi_next (&gpi))
+    {
+      gphi *phi = gpi.phi ();
+      if (gimple_phi_arg_def (phi, phi_arg_idx) == name)
+	return true;
+    }
   return false;
 }
 
@@ -339,27 +345,27 @@ state_purge_per_ssa_name::process_point (const function_point &point,
 	       = const_cast<supernode *> (snode)->start_phis ();
 	     !gsi_end_p (gpi); gsi_next (&gpi))
 	  {
+	    gcc_assert (point.get_from_edge ());
+	    const cfg_superedge *cfg_sedge
+	      = point.get_from_edge ()->dyn_cast_cfg_superedge ();
+	    gcc_assert (cfg_sedge);
+
 	    gphi *phi = gpi.phi ();
 	    /* Are we at the def-stmt for m_name?  */
 	    if (phi == def_stmt)
 	      {
-		/* Does this phi depend on itself?
-		   e.g. in:
-		     added_2 = PHI <added_6(2), added_2(3), added_11(4)>
-		   the middle defn (from edge 3) requires added_2 itself
-		   so we can't purge it here.  */
-		if (self_referential_phi_p (phi))
+		if (name_used_by_phis_p (m_name, snode,
+					 cfg_sedge->get_phi_arg_idx ()))
 		  {
 		    if (logger)
-		      logger->log ("self-referential def stmt within phis;"
+		      logger->log ("name in def stmt used within phis;"
 				   " continuing");
 		  }
 		else
 		  {
-		    /* Otherwise, we can stop here, so that m_name
-		       can be purged.  */
 		    if (logger)
-		      logger->log ("def stmt within phis; terminating");
+		      logger->log ("name in def stmt not used within phis;"
+				   " terminating");
 		    return;
 		  }
 	      }
diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc
index 8611d0f..1eb2543 100644
--- a/gcc/analyzer/supergraph.cc
+++ b/gcc/analyzer/supergraph.cc
@@ -1032,12 +1032,21 @@ cfg_superedge::dump_label_to_pp (pretty_printer *pp,
   /* Otherwise, no label.  */
 }
 
+/* Get the index number for this edge for use in phi stmts
+   in its destination.  */
+
+size_t
+cfg_superedge::get_phi_arg_idx () const
+{
+  return m_cfg_edge->dest_idx;
+}
+
 /* Get the phi argument for PHI for this CFG edge.  */
 
 tree
 cfg_superedge::get_phi_arg (const gphi *phi) const
 {
-  size_t index = m_cfg_edge->dest_idx;
+  size_t index = get_phi_arg_idx ();
   return gimple_phi_arg_def (phi, index);
 }
 
diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h
index f4090fd..877958f 100644
--- a/gcc/analyzer/supergraph.h
+++ b/gcc/analyzer/supergraph.h
@@ -514,6 +514,7 @@ class cfg_superedge : public superedge
   int false_value_p () const { return get_flags () & EDGE_FALSE_VALUE; }
   int back_edge_p () const { return get_flags () & EDGE_DFS_BACK; }
 
+  size_t get_phi_arg_idx () const;
   tree get_phi_arg (const gphi *phi) const;
 
  private:
diff --git a/gcc/testsuite/gcc.dg/analyzer/explode-2.c b/gcc/testsuite/gcc.dg/analyzer/explode-2.c
index 3b987e1..c16982f 100644
--- a/gcc/testsuite/gcc.dg/analyzer/explode-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/explode-2.c
@@ -24,7 +24,7 @@ void test (void)
 	  p0 = malloc (16); /* { dg-warning "leak" "" { xfail *-*-* } } */
 	  break;
 	case 1:
-	  free (p0); /* { dg-warning "double-'free' of 'p0'" "" { xfail *-*-* } } */
+	  free (p0); /* { dg-warning "double-'free' of 'p0'" } */
 	  break;
 
 	case 2:
diff --git a/gcc/testsuite/gcc.dg/analyzer/explode-2a.c b/gcc/testsuite/gcc.dg/analyzer/explode-2a.c
index f60354c..32c71ca 100644
--- a/gcc/testsuite/gcc.dg/analyzer/explode-2a.c
+++ b/gcc/testsuite/gcc.dg/analyzer/explode-2a.c
@@ -14,7 +14,7 @@ void test (void)
      explode-2.c as this code.  */
   int a = get ();
   int b = get ();
-  while (a) /* { dg-warning "leak" } */
+  while (a)
     {
       switch (b)
 	{
diff --git a/gcc/testsuite/gcc.dg/analyzer/phi-2.c b/gcc/testsuite/gcc.dg/analyzer/phi-2.c
new file mode 100644
index 0000000..2ab8344
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/phi-2.c
@@ -0,0 +1,27 @@
+/* { dg-additional-options "-O1" } */
+
+struct list_head {
+  struct list_head *next, *prev;
+};
+
+struct mbochs_dmabuf {
+  /* [...snip...] */
+  struct dma_buf *buf;
+  /* [...snip...] */
+  struct list_head next;
+  /* [...snip...] */
+};
+
+void mbochs_close(struct list_head *dmabufs,
+		  struct mbochs_dmabuf *dmabuf,
+		  struct mbochs_dmabuf *tmp)
+{
+  /* [...snip...] */
+  while (&dmabuf->next != dmabufs)
+    {
+      dmabuf = tmp;
+      tmp = ((struct mbochs_dmabuf *)((void *)(tmp->next.next) - __builtin_offsetof(struct mbochs_dmabuf, next)));
+    }
+
+  /* [...snip...] */
+}