diff mbox

Warn on undefined loop exit

Message ID 546CFB3C.5030307@codesourcery.com
State New
Headers show

Commit Message

Andrew Stubbs Nov. 19, 2014, 8:19 p.m. UTC
On 19/11/14 16:39, Marek Polacek wrote:
> On Wed, Nov 19, 2014 at 04:32:43PM +0000, Andrew Stubbs wrote:
>> +		     if (warning_at (gimple_location (elt->stmt),
>> +				     OPT_Waggressive_loop_optimizations,
>> +				     "Loop exit may only be reached after undefined behaviour."))
>
> Warnings should start with a lowercase and should be without
> a fullstop at the end.

Fixed, and I spotted a britishism too.

Andrew

Comments

Richard Biener Nov. 20, 2014, 4:27 p.m. UTC | #1
On Wed, Nov 19, 2014 at 9:19 PM, Andrew Stubbs <ams@codesourcery.com> wrote:
> On 19/11/14 16:39, Marek Polacek wrote:
>>
>> On Wed, Nov 19, 2014 at 04:32:43PM +0000, Andrew Stubbs wrote:
>>>
>>> +                    if (warning_at (gimple_location (elt->stmt),
>>> +                                    OPT_Waggressive_loop_optimizations,
>>> +                                    "Loop exit may only be reached after
>>> undefined behaviour."))
>>
>>
>> Warnings should start with a lowercase and should be without
>> a fullstop at the end.
>
>
> Fixed, and I spotted a britishism too.

If it's really duplicated code can you split it out to a function?

+      if (OPT_Waggressive_loop_optimizations)
+    {

this doesn't do what you think it does ;)  The variable to check is
warn_aggressive_loop_optimizations.

+      if (exit_warned && problem_stmts != vNULL)
+        {

!problem_stmts.empty ()

Otherwise it looks ok.

Thanks,
Richard.

> Andrew
Jakub Jelinek Feb. 18, 2015, 2:05 p.m. UTC | #2
On Thu, Nov 20, 2014 at 05:27:35PM +0100, Richard Biener wrote:
> On Wed, Nov 19, 2014 at 9:19 PM, Andrew Stubbs <ams@codesourcery.com> wrote:
> > On 19/11/14 16:39, Marek Polacek wrote:
> >>
> >> On Wed, Nov 19, 2014 at 04:32:43PM +0000, Andrew Stubbs wrote:
> >>>
> >>> +                    if (warning_at (gimple_location (elt->stmt),
> >>> +                                    OPT_Waggressive_loop_optimizations,
> >>> +                                    "Loop exit may only be reached after
> >>> undefined behaviour."))
> >>
> >>
> >> Warnings should start with a lowercase and should be without
> >> a fullstop at the end.
> >
> >
> > Fixed, and I spotted a britishism too.
> 
> If it's really duplicated code can you split it out to a function?
> 
> +      if (OPT_Waggressive_loop_optimizations)
> +    {
> 
> this doesn't do what you think it does ;)  The variable to check is
> warn_aggressive_loop_optimizations.
> 
> +      if (exit_warned && problem_stmts != vNULL)
> +        {
> 
> !problem_stmts.empty ()
> 
> Otherwise it looks ok.

This caused PR64491.  If the loop has multiple exits, the loop might be
exit earlier and so it would be just fine if the other loop exit may only be
reached after undefined behavior.

	Jakub
diff mbox

Patch

2014-11-19  Andrew Stubbs  <ams@codesourcery.com>

	gcc/
	* tree-ssa-loop-niter.c (maybe_lower_iteration_bound): Warn if a loop
	condition would be removed due to undefined behaviour.

	gcc/testsuite/
	* gcc.dg/undefined-loop-1.c: New file.
	* gcc.dg/undefined-loop-2.c: New file.
---
 gcc/testsuite/gcc.dg/undefined-loop-1.c |   18 ++++++++++++
 gcc/testsuite/gcc.dg/undefined-loop-2.c |   22 +++++++++++++++
 gcc/tree-ssa-loop-niter.c               |   46 +++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/undefined-loop-1.c
 create mode 100644 gcc/testsuite/gcc.dg/undefined-loop-2.c

diff --git a/gcc/testsuite/gcc.dg/undefined-loop-1.c b/gcc/testsuite/gcc.dg/undefined-loop-1.c
new file mode 100644
index 0000000..80260cc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/undefined-loop-1.c
@@ -0,0 +1,18 @@ 
+/* 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]  /* { dg-message "note: possible undefined statement is here" } */
+       && i < 5; /* { dg-warning "loop exit may only be reached after undefined behavior" } */
+       i++)
+    doSomething(array[i]);
+}
diff --git a/gcc/testsuite/gcc.dg/undefined-loop-2.c b/gcc/testsuite/gcc.dg/undefined-loop-2.c
new file mode 100644
index 0000000..dbea62c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/undefined-loop-2.c
@@ -0,0 +1,22 @@ 
+/* Check that loops whose final iteration is undefined are detected.  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Waggressive-loop-optimizations" } */
+
+void doSomething(char);
+
+char array1[5];
+char array2[5];
+
+void
+foo (int p)
+{
+  int i;
+  for (i=0;
+       (p
+        ? array1[i]  /* { dg-message "note: possible undefined statement is here" } */
+        : array2[i]) /* { dg-message "note: possible undefined statement is here" } */
+       && i < 5      /* { dg-warning "loop exit may only be reached after undefined behavior" } */
+       && i < 100;   /* { dg-warning "loop exit may only be reached after undefined behavior" } */
+       i++)
+    doSomething(array1[i]);
+}
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index fd32d28..0365505 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -3289,6 +3289,7 @@  maybe_lower_iteration_bound (struct loop *loop)
   struct nb_iter_bound *elt;
   bool found_exit = false;
   vec<basic_block> queue = vNULL;
+  vec<gimple> problem_stmts = vNULL;
   bitmap visited;
 
   /* Collect all statements with interesting (i.e. lower than
@@ -3334,6 +3335,7 @@  maybe_lower_iteration_bound (struct loop *loop)
 	  if (not_executed_last_iteration->contains (stmt))
 	    {
 	      stmt_found = true;
+	      problem_stmts.safe_push (stmt);
 	      break;
 	    }
 	  if (gimple_has_side_effects (stmt))
@@ -3377,9 +3379,53 @@  maybe_lower_iteration_bound (struct loop *loop)
 		 "undefined statement must be executed at the last iteration.\n");
       record_niter_bound (loop, loop->nb_iterations_upper_bound - 1,
 			  false, true);
+
+      if (OPT_Waggressive_loop_optimizations)
+	{
+	  bool exit_warned = false;
+	  for (elt = loop->bounds; elt; elt = elt->next)
+	    {
+	      if (elt->is_exit
+		  && wi::gtu_p (elt->bound, loop->nb_iterations_upper_bound))
+		{
+		  basic_block bb = gimple_bb (elt->stmt);
+		  edge exit_edge = EDGE_SUCC (bb, 0);
+		  struct tree_niter_desc niter;
+
+		  if (!loop_exit_edge_p (loop, exit_edge))
+		    exit_edge = EDGE_SUCC (bb, 1);
+
+		  if(number_of_iterations_exit (loop, exit_edge,
+						&niter, false, false)
+		     && integer_onep (niter.assumptions)
+		     && integer_zerop (niter.may_be_zero)
+		     && niter.niter
+		     && TREE_CODE (niter.niter) == INTEGER_CST
+		     && wi::ltu_p (loop->nb_iterations_upper_bound,
+				   wi::to_widest (niter.niter)))
+		   {
+		     if (warning_at (gimple_location (elt->stmt),
+				     OPT_Waggressive_loop_optimizations,
+				     "loop exit may only be reached after undefined behavior"))
+		       exit_warned = true;
+		   }
+		}
+	    }
+
+	  if (exit_warned && problem_stmts != vNULL)
+	    {
+	      gimple stmt;
+	      int index;
+	      FOR_EACH_VEC_ELT (problem_stmts, index, stmt)
+		inform (gimple_location (stmt),
+			"possible undefined statement is here");
+	    }
+      }
     }
+
   BITMAP_FREE (visited);
   queue.release ();
+  problem_stmts.release ();
   delete not_executed_last_iteration;
 }