This fixes the code that verifies that none of the uninitialized paths
reaching a PHI are actually taken (uninit_uses_cannot_happen).
As discussed in the PR, this is done by fixing the predicate analysis in
tree-ssa-uninit.c so that the set of predicates reaching the use of a
PHI take into the entire path from the entry block. Previously we were
starting at the immediate dominator of the PHI (for some obscure reason,
or brain fart on my part).
Fixing the predicate analysis to go all the way back to ENTRY, uncovers
some latent limitations in compute_control_dep_chain(). I've fixed
those as well.
Note, I don't know why we were excluding basic blocks with a small (< 2)
number of successors, but the exclusion was keeping us from looking at
the ENTRY block. If this was by design, I can special case the ENTRY
block. Similarly, convert_control_dep_chain_into_preds() was ignoring
basic blocks with no instructions. This was making us skip the ENTRY
block, which is just an empty forwarder block. Again, if this was by
design, I can just special case the ENTRY block.
Finally, I've split up the dumping routine into member functions so we
can get finer grained debugging help.
Tested on x86-64 Linux.
OK for trunk?
gcc/
PR middle-end/81897
* tree-ssa-uninit.c (compute_control_dep_chain): Do not bail on
basic blocks with a small number of successors.
(convert_control_dep_chain_into_preds): Special case the entry
block.
(dump_predicates): Split apart into...
(dump_pred_chain): ...here...
(dump_pred_info): ...and here.
(can_one_predicate_be_invalidated_p): Add debugging printfs.
(can_chain_union_be_invalidated_p): Return TRUE if any predicate
was invalidated.
(uninit_uses_cannot_happen): Avoid unnecessary if
convert_control_dep_chain_into_preds yielded nothing.
diff --git a/gcc/testsuite/gcc.dg/uninit-pr81897.c b/gcc/testsuite/gcc.dg/uninit-pr81897.c
new file mode 100644
index 00000000000..0323050839d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/uninit-pr81897.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wuninitialized" } */
+
+int f(void);
+static inline void rcu_read_unlock(void)
+{
+ static _Bool __warned;
+ if (f() && !__warned && !f()) {
+ __warned = 1;
+ }
+}
+int inet6_rtm_getroute(void)
+{
+ int dst;
+ int fibmatch = f();
+
+ if (!fibmatch)
+ dst = f();
+ rcu_read_unlock();
+ if (fibmatch)
+ dst = 0;
+
+ return dst;
+}
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index 6930a243deb..58590a7763b 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -543,9 +543,6 @@ compute_control_dep_chain (basic_block bb, basic_block dep_bb,
bool found_cd_chain = false;
size_t cur_chain_len = 0;
- if (EDGE_COUNT (bb->succs) < 2)
- return false;
-
if (*num_calls > PARAM_VALUE (PARAM_UNINIT_CONTROL_DEP_ATTEMPTS))
return false;
++*num_calls;
@@ -671,11 +668,9 @@ convert_control_dep_chain_into_preds (vec<edge> *dep_chains,
e = one_cd_chain[j];
guard_bb = e->src;
gsi = gsi_last_bb (guard_bb);
+ /* Ignore empty BBs as they're basically forwarder blocks. */
if (gsi_end_p (gsi))
- {
- has_valid_pred = false;
- break;
- }
+ continue;
cond_stmt = gsi_stmt (gsi);
if (is_gimple_call (cond_stmt) && EDGE_COUNT (e->src->succs) >= 2)
/* Ignore EH edge. Can add assertion on the other edge's flag. */
@@ -916,38 +911,49 @@ find_def_preds (pred_chain_union *preds, gphi *phi)
return has_valid_pred;
}
+/* Dump a pred_info. */
+
+static void
+dump_pred_info (pred_info one_pred)
+{
+ if (one_pred.invert)
+ fprintf (dump_file, " (.NOT.) ");
+ print_generic_expr (dump_file, one_pred.pred_lhs);
+ fprintf (dump_file, " %s ", op_symbol_code (one_pred.cond_code));
+ print_generic_expr (dump_file, one_pred.pred_rhs);
+}
+
+/* Dump a pred_chain. */
+
+static void
+dump_pred_chain (pred_chain one_pred_chain)
+{
+ size_t np = one_pred_chain.length ();
+ for (size_t j = 0; j < np; j++)
+ {
+ dump_pred_info (one_pred_chain[j]);
+ if (j < np - 1)
+ fprintf (dump_file, " (.AND.) ");
+ else
+ fprintf (dump_file, "\n");
+ }
+}
+
/* Dumps the predicates (PREDS) for USESTMT. */
static void
dump_predicates (gimple *usestmt, pred_chain_union preds, const char *msg)
{
- size_t i, j;
- pred_chain one_pred_chain = vNULL;
fprintf (dump_file, "%s", msg);
- print_gimple_stmt (dump_file, usestmt, 0);
- fprintf (dump_file, "is guarded by :\n\n");
+ if (usestmt)
+ {
+ print_gimple_stmt (dump_file, usestmt, 0);
+ fprintf (dump_file, "is guarded by :\n\n");
+ }
size_t num_preds = preds.length ();
- /* Do some dumping here: */
- for (i = 0; i < num_preds; i++)
+ for (size_t i = 0; i < num_preds; i++)
{
- size_t np;
-
- one_pred_chain = preds[i];
- np = one_pred_chain.length ();
-
- for (j = 0; j < np; j++)
- {
- pred_info one_pred = one_pred_chain[j];
- if (one_pred.invert)
- fprintf (dump_file, " (.NOT.) ");
- print_generic_expr (dump_file, one_pred.pred_lhs);
- fprintf (dump_file, " %s ", op_symbol_code (one_pred.cond_code));
- print_generic_expr (dump_file, one_pred.pred_rhs);
- if (j < np - 1)
- fprintf (dump_file, " (.AND.) ");
- else
- fprintf (dump_file, "\n");
- }
+ dump_pred_chain (preds[i]);
if (i < num_preds - 1)
fprintf (dump_file, "(.OR.)\n");
else
@@ -2259,12 +2265,19 @@ normalize_preds (pred_chain_union preds, gimple *use_or_def, bool is_use)
}
/* Return TRUE if PREDICATE can be invalidated by any individual
- predicate in WORKLIST. */
+ predicate in USE_GUARD. */
static bool
can_one_predicate_be_invalidated_p (pred_info predicate,
pred_chain use_guard)
{
+ if (dump_file && dump_flags & TDF_DETAILS)
+ {
+ fprintf (dump_file, "Testing if this predicate: ");
+ dump_pred_info (predicate);
+ fprintf (dump_file, "\n...can be invalidated by a USE guard of: ");
+ dump_pred_chain (use_guard);
+ }
for (size_t i = 0; i < use_guard.length (); ++i)
{
/* NOTE: This is a very simple check, and only understands an
@@ -2273,7 +2286,15 @@ can_one_predicate_be_invalidated_p (pred_info predicate,
invalidate with say [i > 5] or [i == 8]. There is certainly
room for improvement here. */
if (pred_neg_p (predicate, use_guard[i]))
- return true;
+ {
+ if (dump_file && dump_flags & TDF_DETAILS)
+ {
+ fprintf (dump_file, " Predicate was invalidated by: ");
+ dump_pred_info (use_guard[i]);
+ fputc ('\n', dump_file);
+ }
+ return true;
+ }
}
return false;
}
@@ -2287,14 +2308,17 @@ can_chain_union_be_invalidated_p (pred_chain_union uninit_pred,
{
if (uninit_pred.is_empty ())
return false;
+ if (dump_file && dump_flags & TDF_DETAILS)
+ dump_predicates (NULL, uninit_pred,
+ "Testing if anything here can be invalidated: ");
for (size_t i = 0; i < uninit_pred.length (); ++i)
{
pred_chain c = uninit_pred[i];
for (size_t j = 0; j < c.length (); ++j)
- if (!can_one_predicate_be_invalidated_p (c[j], use_guard))
- return false;
+ if (can_one_predicate_be_invalidated_p (c[j], use_guard))
+ return true;
}
- return true;
+ return false;
}
/* Return TRUE if none of the uninitialized operands in UNINT_OPNDS
@@ -2334,7 +2358,7 @@ uninit_uses_cannot_happen (gphi *phi, unsigned uninit_opnds,
/* Build the control dependency chain for uninit operand `i'... */
uninit_preds = vNULL;
- if (!compute_control_dep_chain (find_dom (e->src),
+ if (!compute_control_dep_chain (ENTRY_BLOCK_PTR_FOR_FN (cfun),
e->src, dep_chains, &num_chains,
&cur_chain, &num_calls))
{
@@ -2342,10 +2366,16 @@ uninit_uses_cannot_happen (gphi *phi, unsigned uninit_opnds,
break;
}
/* ...and convert it into a set of predicates. */
- convert_control_dep_chain_into_preds (dep_chains, num_chains,
- &uninit_preds);
+ bool has_valid_preds
+ = convert_control_dep_chain_into_preds (dep_chains, num_chains,
+ &uninit_preds);
for (size_t j = 0; j < num_chains; ++j)
dep_chains[j].release ();
+ if (!has_valid_preds)
+ {
+ ret = false;
+ break;
+ }
simplify_preds (&uninit_preds, NULL, false);
uninit_preds = normalize_preds (uninit_preds, NULL, false);
