diff mbox

fix middle-end/71476

Message ID 20160610123623.GB16494@redhat.com
State New
Headers show

Commit Message

Marek Polacek June 10, 2016, 12:36 p.m. UTC
While working on something else I discovered this ICE-on-invalid.
The problem is that when looking into the innermost lexical scope,
gimple_bind_body might be null, so check 'seq' first before checking
its gimple_code.
I factored the code to its own function so that I can simply 'return'.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2016-06-10  Marek Polacek  <polacek@redhat.com>

	PR middle-end/71476
	* gimplify.c (maybe_warn_switch_unreachable): Factored out of
	gimplify_switch_expr.

	* gcc.dg/noncompile/pr71476.c: New test.


	Marek

Comments

Jakub Jelinek June 10, 2016, 12:52 p.m. UTC | #1
On Fri, Jun 10, 2016 at 02:36:23PM +0200, Marek Polacek wrote:
> --- gcc/gimplify.c
> +++ gcc/gimplify.c
> @@ -1559,6 +1559,50 @@ gimplify_statement_list (tree *expr_p, gimple_seq *pre_p)
>    return GS_ALL_DONE;
>  }
>  
> +/* Possibly warn about unreachable statements between switch's controlling
> +   expression and the first case.  SEQ is the body of a switch expression.  */
> +
> +static void
> +maybe_warn_switch_unreachable (gimple_seq seq)
> +{
> +  if (!warn_switch_unreachable
> +      /* This warning doesn't play well with Fortran when optimizations
> +	 are on.  */
> +      || lang_GNU_Fortran ()
> +      || seq == NULL)
> +    return;
> +
> +  /* Look into the innermost lexical scope.  */
> +  while (seq && gimple_code (seq) == GIMPLE_BIND)
> +    seq = gimple_bind_body (as_a <gbind *> (seq));
> +
> +  /* If it is empty, there's nothing to do.  */
> +  if (seq == NULL)
> +    return;

Won't this just give up on say:
void
foo (int a, int b)
{
  switch (a)
    {
      { int c; }
      { int d; }
      { int e; }
      b++;
    case 1:
      break;
    }
}

?  Such blocks can also be can be arbitrarily nested.
Wouldn't it be better to just walk_gimple_seq with NULL callback_op
and non-NULL callback_stmt that would stop on the first real stmt in there?
Also, the above loop looks confusing, I'd be expecting gimple_seq_first_stmt
(seq) before testing it for GIMPLE_BIND or casting to gbind.

	Jakub
diff mbox

Patch

diff --git gcc/gimplify.c gcc/gimplify.c
index f12c6a1..542e0b2 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -1559,6 +1559,50 @@  gimplify_statement_list (tree *expr_p, gimple_seq *pre_p)
   return GS_ALL_DONE;
 }
 
+/* Possibly warn about unreachable statements between switch's controlling
+   expression and the first case.  SEQ is the body of a switch expression.  */
+
+static void
+maybe_warn_switch_unreachable (gimple_seq seq)
+{
+  if (!warn_switch_unreachable
+      /* This warning doesn't play well with Fortran when optimizations
+	 are on.  */
+      || lang_GNU_Fortran ()
+      || seq == NULL)
+    return;
+
+  /* Look into the innermost lexical scope.  */
+  while (seq && gimple_code (seq) == GIMPLE_BIND)
+    seq = gimple_bind_body (as_a <gbind *> (seq));
+
+  /* If it is empty, there's nothing to do.  */
+  if (seq == NULL)
+    return;
+
+  gimple *stmt = gimple_seq_first_stmt (seq);
+  if (gimple_code (stmt) == GIMPLE_TRY)
+    {
+      /* A compiler-generated cleanup or a user-written try block.
+	 Try to get the first statement in its try-block, for better
+	 location.  */
+      if ((seq = gimple_try_eval (stmt)))
+	stmt = gimple_seq_first_stmt (seq);
+    }
+
+  if (gimple_code (stmt) != GIMPLE_LABEL)
+    {
+      if (gimple_code (stmt) == GIMPLE_GOTO
+	  && TREE_CODE (gimple_goto_dest (stmt)) == LABEL_DECL
+	  && DECL_ARTIFICIAL (gimple_goto_dest (stmt)))
+	/* Don't warn for compiler-generated gotos.  These occur
+	   in Duff's devices, for example.  */;
+      else
+	warning_at (gimple_location (stmt), OPT_Wswitch_unreachable,
+		    "statement will never be executed");
+    }
+}
+
 
 /* Gimplify a SWITCH_EXPR, and collect the vector of labels it can
    branch to.  */
@@ -1596,39 +1640,8 @@  gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
 
       gimplify_stmt (&SWITCH_BODY (switch_expr), &switch_body_seq);
 
-      /* Possibly warn about unreachable statements between switch's
-	 controlling expression and the first case.  */
-      if (warn_switch_unreachable
-	  /* This warning doesn't play well with Fortran when optimizations
-	     are on.  */
-	  && !lang_GNU_Fortran ()
-	  && switch_body_seq != NULL)
-	{
-	  gimple_seq seq = switch_body_seq;
-	  /* Look into the innermost lexical scope.  */
-	  while (gimple_code (seq) == GIMPLE_BIND)
-	    seq = gimple_bind_body (as_a <gbind *> (seq));
-	  gimple *stmt = gimple_seq_first_stmt (seq);
-	  if (gimple_code (stmt) == GIMPLE_TRY)
-	    {
-	      /* A compiler-generated cleanup or a user-written try block.
-		 Try to get the first statement in its try-block, for better
-		 location.  */
-	      if ((seq = gimple_try_eval (stmt)))
-		stmt = gimple_seq_first_stmt (seq);
-	    }
-	  if (gimple_code (stmt) != GIMPLE_LABEL)
-	    {
-	      if (gimple_code (stmt) == GIMPLE_GOTO
-		  && TREE_CODE (gimple_goto_dest (stmt)) == LABEL_DECL
-		  && DECL_ARTIFICIAL (gimple_goto_dest (stmt)))
-		/* Don't warn for compiler-generated gotos.  These occur
-		   in Duff's devices, for example.  */;
-	      else
-		warning_at (gimple_location (stmt), OPT_Wswitch_unreachable,
-			    "statement will never be executed");
-	    }
-	}
+      maybe_warn_switch_unreachable (switch_body_seq);
+
       labels = gimplify_ctxp->case_labels;
       gimplify_ctxp->case_labels = saved_labels;
 
diff --git gcc/testsuite/gcc.dg/noncompile/pr71476.c gcc/testsuite/gcc.dg/noncompile/pr71476.c
index e69de29..138eeeb 100644
--- gcc/testsuite/gcc.dg/noncompile/pr71476.c
+++ gcc/testsuite/gcc.dg/noncompile/pr71476.c
@@ -0,0 +1,12 @@ 
+/* PR middle-end/71476 */
+/* { dg-do compile } */
+/* { dg-options "-Wswitch-unreachable" } */
+
+void
+foo (int a)
+{
+  switch (a)
+    {
+      void f (void) { break; } /* { dg-error "break statement not within loop or switch" } */
+    }
+}