This patch adds the following warning message:
undefined.c:9:20: warning: statement may be undefined in the final loop
iteration. [-Waggressive-loop-optimizations]
for (i = 0; array[i] && i < 5; i++)
^
(Where the code ought to read "i < 5 && array[i]".)
The tree-ssa loop optimizations already eliminate useless loop-exit
conditions (i.e. conditions that will never be true). Unfortunately,
they also eliminate exit conditions that can be true, but only after
undefined behaviour has occurred. Typically, that means that the
undefined behaviour becomes an infinite loop (if it doesn't happen to
crash, of course), and that's surprising. It also looks more like a
compiler bug than a crash does.
The new warning should highlight these cases but does not actually
change anything. I've included a comment where the compiler could be
adjusted to avoid the surprising optimization. Would it be appropriate
to do so?
OK to commit?
Andrew
2014-11-05 Andrew Stubbs <a...@codesourcery.com>
gcc/
* tree-ssa-loop-niter.c (maybe_lower_iteration_bound): Set
loop->possibly_undefined_stmt appropriately.
* tree-ssa-loop-ivcanon.c (remove_redundant_iv_tests): Warn if
any tests have loop->possibly_undefined_stmt set.
* cfgloop.h (struct loop): Add field "possibly_undefined_stmt".
gcc/testsuite/
* gcc.dg/undefined-loop.c: New file.
Index: gcc/tree-ssa-loop-niter.c
===================================================================
--- gcc/tree-ssa-loop-niter.c (revision 217085)
+++ gcc/tree-ssa-loop-niter.c (working copy)
@@ -3320,6 +3320,7 @@
visited = BITMAP_ALLOC (NULL);
bitmap_set_bit (visited, loop->header->index);
found_exit = false;
+ gimple problem_stmt = NULL;
do
{
@@ -3334,6 +3335,8 @@
if (not_executed_last_iteration->contains (stmt))
{
stmt_found = true;
+ if (!problem_stmt)
+ problem_stmt = stmt;
break;
}
if (gimple_has_side_effects (stmt))
@@ -3375,6 +3378,8 @@
if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file, "Reducing loop iteration estimate by 1; "
"undefined statement must be executed at the last iteration.\n");
+ if (!loop->possibly_undefined_stmt)
+ loop->possibly_undefined_stmt = problem_stmt;
record_niter_bound (loop, loop->nb_iterations_upper_bound - 1,
false, true);
}
Index: gcc/testsuite/gcc.dg/undefined-loop.c
===================================================================
--- gcc/testsuite/gcc.dg/undefined-loop.c (revision 0)
+++ gcc/testsuite/gcc.dg/undefined-loop.c (revision 0)
@@ -0,0 +1,15 @@
+/* Check that loops whose final iteration is undefined are detected. */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Waggressive-loop-optimizations" } */
+
+void doSomething(char);
+
+char array[5];
+
+void
+foo (void)
+{
+ int i;
+ for (i = 0; array[i] && i < 5; i++) /* { dg-warning "statement may be undefined in the final loop iteration" } */
+ doSomething(array[i]);
+}
Index: gcc/tree-ssa-loop-ivcanon.c
===================================================================
--- gcc/tree-ssa-loop-ivcanon.c (revision 217085)
+++ gcc/tree-ssa-loop-ivcanon.c (working copy)
@@ -86,6 +86,7 @@
#include "target.h"
#include "tree-cfgcleanup.h"
#include "builtins.h"
+#include "diagnostic-core.h"
/* Specifies types of loops that may be unrolled. */
@@ -586,6 +587,18 @@
wi::to_widest (niter.niter)))
continue;
+ /* If another loop exit has previously been suspected of causing
+ undefined behavior then removing other exit statements may be
+ unsafe. */
+ if (loop->possibly_undefined_stmt)
+ {
+ warning_at (gimple_location (loop->possibly_undefined_stmt),
+ OPT_Waggressive_loop_optimizations,
+ "statement may be undefined in the final loop iteration.");
+ /* We could avoid the unsafe optimzation here:
+ continue; */
+ }
+
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, "Removed pointless exit: ");
Index: gcc/cfgloop.h
===================================================================
--- gcc/cfgloop.h (revision 217085)
+++ gcc/cfgloop.h (working copy)
@@ -179,6 +179,10 @@
already. */
bool warned_aggressive_loop_optimizations;
+ /* Set if the loop count was reduced do to a statement that is undefined
+ in the final iteration. */
+ gimple possibly_undefined_stmt;
+
/* An integer estimation of the number of iterations. Estimate_state
describes what is the state of the estimation. */
enum loop_estimation estimate_state;