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>

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 6d02c60449ca7dcb6696a96679b10a223757bada..c029759cb9b94438459bb805f7ebe5b86fb132d9 100644 (file)
--- 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 734ec6012375c1281f80da7ad49f9b4ea30fef7d..cc39929db2605aadb5fd76a642e5afe875185303 100644 (file)
--- 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 3c3b77500a68d96dbe05508aff5496b65971ada4..bfa48a9ef3fa32b1bfcdcc8930eb510851d47a15 100644 (file)
--- 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 8611d0f8689868847d9d56ec9cae55b82084c307..1eb25436f94e85da50fbee2dc22d6628ad1a7937 100644 (file)
--- 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 f4090fd5e0eddfd38ac43e33d39afcff2f50105e..877958f75fab45bc2f416ca14e445023556ed46d 100644 (file)
--- 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 3b987e10a4afb728d48af5e09cd7cc8bd9f70832..c16982f3bc40fcfbaf230365b2e6ec5cb9be0858 100644 (file)
--- 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 f60354cae1bb008d7aa20d9378329fe852c0f6e9..32c71ca44aa578f4df75f4b7e380b370638e7db4 100644 (file)
--- 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 (file)
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...] */
+}