[committed] analyzer: fix issues with phi handling
David Malcolm
dmalcolm@redhat.com
Wed Jul 21 21:29:59 GMT 2021
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.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as e0a7a6752dad7848eb4b29b826a551c0992256ec.
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>
---
gcc/analyzer/region-model.cc | 18 +++++++---
gcc/analyzer/region-model.h | 1 +
gcc/analyzer/state-purge.cc | 42 ++++++++++++----------
gcc/analyzer/supergraph.cc | 11 +++++-
gcc/analyzer/supergraph.h | 1 +
gcc/testsuite/gcc.dg/analyzer/explode-2.c | 2 +-
gcc/testsuite/gcc.dg/analyzer/explode-2a.c | 2 +-
gcc/testsuite/gcc.dg/analyzer/phi-2.c | 27 ++++++++++++++
8 files changed, 78 insertions(+), 26 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/phi-2.c
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 6d02c60449c..c029759cb9b 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 734ec601237..cc39929db26 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 3c3b77500a6..bfa48a9ef3f 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 8611d0f8689..1eb25436f94 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 f4090fd5e0e..877958f75fa 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 3b987e10a4a..c16982f3bc4 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 f60354cae1b..32c71ca44aa 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 00000000000..2ab8344cfe2
--- /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...] */
+}
--
2.26.3
More information about the Gcc-patches
mailing list