diff mbox

Fix nested function ICE with VLAs (PR middle-end/59011)

Message ID 20131202230255.GU892@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Dec. 2, 2013, 11:02 p.m. UTC
Hi!

tree-nested.c uses declare_vars with last argument true, which
relies on BLOCK_VARS of gimple_bind_block being a tail
of the gimple_bind_vars chain.  But unfortunately a debug info
improvement I've added to gimplify_var_or_parm_decl 4 years ago
violates this assumption, in that it adds some VAR_DECLs
at the head of BLOCK_VARS (DECL_INITIAL (current_function_decl))
chain, but doesn't adjust gimple_bind_vars chain correspondingly.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk/4.8?

2013-12-02  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/59011
	* gimplify.c (nonlocal_vla_vars): New variable.
	(gimplify_var_or_parm_decl): Put VAR_DECLs for VLAs into
	nonlocal_vla_vars chain.
	(gimplify_body): Call declare_vars on nonlocal_vla_vars chain
	if outer_bind has DECL_INITIAL (current_function_decl) block.

	* gcc.dg/pr59011.c: New test.


	Jakub

Comments

Jeff Law Dec. 3, 2013, 5:50 a.m. UTC | #1
On 12/02/13 16:02, Jakub Jelinek wrote:
> Hi!
>
> tree-nested.c uses declare_vars with last argument true, which
> relies on BLOCK_VARS of gimple_bind_block being a tail
> of the gimple_bind_vars chain.  But unfortunately a debug info
> improvement I've added to gimplify_var_or_parm_decl 4 years ago
> violates this assumption, in that it adds some VAR_DECLs
> at the head of BLOCK_VARS (DECL_INITIAL (current_function_decl))
> chain, but doesn't adjust gimple_bind_vars chain correspondingly.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk/4.8?
>
> 2013-12-02  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR middle-end/59011
> 	* gimplify.c (nonlocal_vla_vars): New variable.
> 	(gimplify_var_or_parm_decl): Put VAR_DECLs for VLAs into
> 	nonlocal_vla_vars chain.
> 	(gimplify_body): Call declare_vars on nonlocal_vla_vars chain
> 	if outer_bind has DECL_INITIAL (current_function_decl) block.
>
> 	* gcc.dg/pr59011.c: New test.
OK for the trunk.  Branch maintainers have final call on their 
respective branches.

jeff
Richard Biener Dec. 3, 2013, 9:04 a.m. UTC | #2
On Tue, 3 Dec 2013, Jakub Jelinek wrote:

> Hi!
> 
> tree-nested.c uses declare_vars with last argument true, which
> relies on BLOCK_VARS of gimple_bind_block being a tail
> of the gimple_bind_vars chain.  But unfortunately a debug info
> improvement I've added to gimplify_var_or_parm_decl 4 years ago
> violates this assumption, in that it adds some VAR_DECLs
> at the head of BLOCK_VARS (DECL_INITIAL (current_function_decl))
> chain, but doesn't adjust gimple_bind_vars chain correspondingly.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk/4.8?

Ok.

Thanks,
Richard.

> 2013-12-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/59011
> 	* gimplify.c (nonlocal_vla_vars): New variable.
> 	(gimplify_var_or_parm_decl): Put VAR_DECLs for VLAs into
> 	nonlocal_vla_vars chain.
> 	(gimplify_body): Call declare_vars on nonlocal_vla_vars chain
> 	if outer_bind has DECL_INITIAL (current_function_decl) block.
> 
> 	* gcc.dg/pr59011.c: New test.
> 
> --- gcc/gimplify.c.jj	2013-12-02 14:33:34.000000000 +0100
> +++ gcc/gimplify.c	2013-12-02 20:32:02.883491995 +0100
> @@ -1689,6 +1689,9 @@ gimplify_conversion (tree *expr_p)
>  /* Nonlocal VLAs seen in the current function.  */
>  static struct pointer_set_t *nonlocal_vlas;
>  
> +/* The VAR_DECLs created for nonlocal VLAs for debug info purposes.  */
> +static tree nonlocal_vla_vars;
> +
>  /* Gimplify a VAR_DECL or PARM_DECL.  Return GS_OK if we expanded a
>     DECL_VALUE_EXPR, and it's worth re-examining things.  */
>  
> @@ -1737,14 +1740,13 @@ gimplify_var_or_parm_decl (tree *expr_p)
>  	    ctx = ctx->outer_context;
>  	  if (!ctx && !pointer_set_insert (nonlocal_vlas, decl))
>  	    {
> -	      tree copy = copy_node (decl), block;
> +	      tree copy = copy_node (decl);
>  
>  	      lang_hooks.dup_lang_specific_decl (copy);
>  	      SET_DECL_RTL (copy, 0);
>  	      TREE_USED (copy) = 1;
> -	      block = DECL_INITIAL (current_function_decl);
> -	      DECL_CHAIN (copy) = BLOCK_VARS (block);
> -	      BLOCK_VARS (block) = copy;
> +	      DECL_CHAIN (copy) = nonlocal_vla_vars;
> +	      nonlocal_vla_vars = copy;
>  	      SET_DECL_VALUE_EXPR (copy, unshare_expr (value_expr));
>  	      DECL_HAS_VALUE_EXPR_P (copy) = 1;
>  	    }
> @@ -8562,6 +8564,21 @@ gimplify_body (tree fndecl, bool do_parm
>  
>    if (nonlocal_vlas)
>      {
> +      if (nonlocal_vla_vars)
> +	{
> +	  /* tree-nested.c may later on call declare_vars (..., true);
> +	     which relies on BLOCK_VARS chain to be the tail of the
> +	     gimple_bind_vars chain.  Ensure we don't violate that
> +	     assumption.  */
> +	  if (gimple_bind_block (outer_bind)
> +	      == DECL_INITIAL (current_function_decl))
> +	    declare_vars (nonlocal_vla_vars, outer_bind, true);
> +	  else
> +	    BLOCK_VARS (DECL_INITIAL (current_function_decl))
> +	      = chainon (BLOCK_VARS (DECL_INITIAL (current_function_decl)),
> +			 nonlocal_vla_vars);
> +	  nonlocal_vla_vars = NULL_TREE;
> +	}
>        pointer_set_destroy (nonlocal_vlas);
>        nonlocal_vlas = NULL;
>      }
> --- gcc/testsuite/gcc.dg/pr59011.c.jj	2013-12-02 20:14:22.350702153 +0100
> +++ gcc/testsuite/gcc.dg/pr59011.c	2013-12-02 20:15:10.902455660 +0100
> @@ -0,0 +1,22 @@
> +/* PR middle-end/59011 */
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu99" } */
> +
> +void
> +foo (int m)
> +{
> +  int a[m];
> +  void
> +  bar (void)
> +  {
> +    {
> +      int
> +      baz (void)
> +      {
> +	return a[0];
> +      }
> +    }
> +    a[0] = 42;
> +  }
> +  bar ();
> +}
> 
> 	Jakub
> 
>
diff mbox

Patch

--- gcc/gimplify.c.jj	2013-12-02 14:33:34.000000000 +0100
+++ gcc/gimplify.c	2013-12-02 20:32:02.883491995 +0100
@@ -1689,6 +1689,9 @@  gimplify_conversion (tree *expr_p)
 /* Nonlocal VLAs seen in the current function.  */
 static struct pointer_set_t *nonlocal_vlas;
 
+/* The VAR_DECLs created for nonlocal VLAs for debug info purposes.  */
+static tree nonlocal_vla_vars;
+
 /* Gimplify a VAR_DECL or PARM_DECL.  Return GS_OK if we expanded a
    DECL_VALUE_EXPR, and it's worth re-examining things.  */
 
@@ -1737,14 +1740,13 @@  gimplify_var_or_parm_decl (tree *expr_p)
 	    ctx = ctx->outer_context;
 	  if (!ctx && !pointer_set_insert (nonlocal_vlas, decl))
 	    {
-	      tree copy = copy_node (decl), block;
+	      tree copy = copy_node (decl);
 
 	      lang_hooks.dup_lang_specific_decl (copy);
 	      SET_DECL_RTL (copy, 0);
 	      TREE_USED (copy) = 1;
-	      block = DECL_INITIAL (current_function_decl);
-	      DECL_CHAIN (copy) = BLOCK_VARS (block);
-	      BLOCK_VARS (block) = copy;
+	      DECL_CHAIN (copy) = nonlocal_vla_vars;
+	      nonlocal_vla_vars = copy;
 	      SET_DECL_VALUE_EXPR (copy, unshare_expr (value_expr));
 	      DECL_HAS_VALUE_EXPR_P (copy) = 1;
 	    }
@@ -8562,6 +8564,21 @@  gimplify_body (tree fndecl, bool do_parm
 
   if (nonlocal_vlas)
     {
+      if (nonlocal_vla_vars)
+	{
+	  /* tree-nested.c may later on call declare_vars (..., true);
+	     which relies on BLOCK_VARS chain to be the tail of the
+	     gimple_bind_vars chain.  Ensure we don't violate that
+	     assumption.  */
+	  if (gimple_bind_block (outer_bind)
+	      == DECL_INITIAL (current_function_decl))
+	    declare_vars (nonlocal_vla_vars, outer_bind, true);
+	  else
+	    BLOCK_VARS (DECL_INITIAL (current_function_decl))
+	      = chainon (BLOCK_VARS (DECL_INITIAL (current_function_decl)),
+			 nonlocal_vla_vars);
+	  nonlocal_vla_vars = NULL_TREE;
+	}
       pointer_set_destroy (nonlocal_vlas);
       nonlocal_vlas = NULL;
     }
--- gcc/testsuite/gcc.dg/pr59011.c.jj	2013-12-02 20:14:22.350702153 +0100
+++ gcc/testsuite/gcc.dg/pr59011.c	2013-12-02 20:15:10.902455660 +0100
@@ -0,0 +1,22 @@ 
+/* PR middle-end/59011 */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" } */
+
+void
+foo (int m)
+{
+  int a[m];
+  void
+  bar (void)
+  {
+    {
+      int
+      baz (void)
+      {
+	return a[0];
+      }
+    }
+    a[0] = 42;
+  }
+  bar ();
+}