[3.4 only] PR c/22061: Problems with variable-sized parameters

[Richard Sandiford <richard at codesourcery dot com>]

This patch fixes PR 22061, a 3.4-only regression from 3.3.  The original
failure was related to unit-at-a-time mode and showed up as an ICE on:

    int N = 1;
    void foo() {} /* Necessary to trigger the original ICE.  */
    void bar (char a[2][N]) { a[1][0] = N; }

expand_function_start claims the SAVE_EXPRs for a function's parameters:

    /* Evaluate now the sizes of any types declared among the arguments.  */
    expand_pending_sizes (nreverse (get_pending_sizes ()));

but expand_function_start is only called when compiling a function to
rtl.  Thus in unit-at-a-time mode, all SAVE_EXPRs for all top-level
function parameters will be accumulated in pending_sizes, and all
those SAVE_EXPRs will be claimed by the first function that gets
compiled to rtl.

This is obviously wrong, but for the record, the specific reason for the
ICE is that pending_stmts still contains the accumulated list when deciding
whether to inline functions.  c_cannot_inline_tree_fn() says:

    /* If a function has pending sizes, we must not defer its
       compilation, and we can't inline it as a tree.  */
    if (fn == current_function_decl)
      {
        t = get_pending_sizes ();
        put_pending_sizes (t);

        if (t)
          {
            ...
            goto cannot_inline;
          }
      }

and get_pending_sizes() will adjust the SAVE_EXPR_CONTEXT of each entry:

    /* Put each SAVE_EXPR into the current function.  */
    for (t = chain; t; t = TREE_CHAIN (t))
      SAVE_EXPR_CONTEXT (TREE_VALUE (t)) = current_function_decl;

Thus the SAVE_EXPR_CONTEXTs will slosh between one function and another
for each call to c_cannot_inline_tree_fn.  In the testcase above, we end
up expanding the SAVE_EXPR in bar() with its SAVE_EXPR_CONTEXT set to foo().
find_function_data() then aborts.

Another example of why this causes problems is:

    int *x;
    static void bar (char a[2][(*x)++]) {}
    int main (void) { exit (0); }

We decide not to compile bar(), so its pending_sizes end up in main(),
and main() will therefore segfault.

Also, if we have:

    int foo (char a[2][++N]) { N += 4; return sizeof (a[0]); }

we won't necessarily compute the size of "a" on entry to foo(): it might
be evaluated on entry to some other function instead.  foo() might therefore
compute the array size after the "N += 4".

At the moment, the handling of top-level and nested functions is
different in three respects:

  - Nested functions save their pending sizes in cfun.  Top-level
    functions don't.

  - Nested functions set the SAVE_EXPR_CONTEXTs of the parameter sizes
    to the parent function.  Top-level functions set it to themselves.

  - Nested functions evaluate the parameter sizes in the parent
    function's context.  Top-level functions evaluate them in
    their own context

I don't see any reason for these differences.  As far as the semantics
are concerned, parameter sizes are calculated in the same way for both
nested and top-level functions.

The patch removes the differences, changing things so that:

  - All functions save their pending sizes in cfun.

  - All functions set the SAVE_EXPR_CONTEXTs of the paraemeter sizes
    to themselves.

  - All functions evaluate the parameter sizes in their own contexts.

Specifically, the patch saves pending_sizes in the function field
set aside for that purpose and restores it in c_expand_body_1.

There was a further problem.  C functions are compiled with
cfun->x_dont_save_pending_sizes_p set to 1, and when parsing
a nested function's parameter, the "current function" is the
containing function.  We therefore don't record any SAVE_EXPRs
for nested function parameters.

For example, if we have a nested function like:

    void bar (int N)
    {
      ...
      void foo (char a[2][++N]) {}
      ...
      foo ();
      ...
    }

there will be no pending_sizes for "++N".  The body of foo() doesn't
care what size "a" has, so it never evaluates the SAVE_EXPR itself,
and the "++N" is therefore never executed.

The patch fixes the second problem by calling grokdeclarator with
cfun->x_dont_save_pending_sizes_p temporarily set to 0.

Patch boostrapped & regression tested on i686-pc-linux-gnu.  OK for 3.4?
The testcases all pass on 4.0 and mainline, so I'll apply them there too
if the patch is OK for 3.4.

Richard


	PR c/22061
	* c-decl.c (push_parm_decl): Push and pop x_dont_save_pending_sizes_p
	around the call to grokdeclarator.  Call grokdeclarator with the
	field set to 0.
	(store_parm_decls): Always store the pending_sizes in cfun.
	(c_expand_body_1): Call put_pending_sizes.
	* c-objc-common.c (c_cannot_inline_tree_fn): Always check
	pending_sizes.

testsuite/
	PR c/22061
	* gcc.c-torture/execute/pr22061-[1-4].c: New tests.

Index: c-decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.470.4.21
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.470.4.21 c-decl.c
*** c-decl.c 28 Jul 2005 23:01:12 -0000 1.470.4.21
--- c-decl.c 1 Aug 2005 17:06:05 -0000
*************** void
*** 2960,2971 ****
--- 2960,2980 ----
  push_parm_decl (tree parm)
  {
    tree decl;
+   int old_dont_save_pending_sizes_p = 0;
  
    /* Don't attempt to expand sizes while parsing this decl.
       (We can get here with i_s_e 1 somehow from Objective-C.)  */
    int save_immediate_size_expand = immediate_size_expand;
    immediate_size_expand = 0;
  
+   /* If this is a nested function, we do want to keep SAVE_EXPRs for
+      the argument sizes, regardless of the parent's setting.  */
+   if (cfun)
+     {
+       old_dont_save_pending_sizes_p = cfun->x_dont_save_pending_sizes_p;
+       cfun->x_dont_save_pending_sizes_p = 0;
+     }
+ 
    decl = grokdeclarator (TREE_VALUE (TREE_PURPOSE (parm)),
  			 TREE_PURPOSE (TREE_PURPOSE (parm)),
  			 PARM, 0, NULL);
*************** push_parm_decl (tree parm)
*** 2975,2980 ****
--- 2984,2991 ----
  
    finish_decl (decl, NULL_TREE, NULL_TREE);
  
+   if (cfun)
+     cfun->x_dont_save_pending_sizes_p = old_dont_save_pending_sizes_p;
    immediate_size_expand = save_immediate_size_expand;
  }
  
*************** store_parm_decls (void)
*** 5990,5998 ****
  {
    tree fndecl = current_function_decl;
  
-   /* The function containing FNDECL, if any.  */
-   tree context = decl_function_context (fndecl);
- 
    /* True if this definition is written with a prototype.  */
    bool prototype = (current_function_parms
  		    && TREE_CODE (current_function_parms) != TREE_LIST);
--- 6001,6006 ----
*************** store_parm_decls (void)
*** 6017,6036 ****
    /* Begin the statement tree for this function.  */
    begin_stmt_tree (&DECL_SAVED_TREE (fndecl));
  
!   /* If this is a nested function, save away the sizes of any
!      variable-size types so that we can expand them when generating
!      RTL.  */
!   if (context)
!     {
!       tree t;
! 
!       DECL_LANG_SPECIFIC (fndecl)->pending_sizes
! 	= nreverse (get_pending_sizes ());
!       for (t = DECL_LANG_SPECIFIC (fndecl)->pending_sizes;
! 	   t;
! 	   t = TREE_CHAIN (t))
! 	SAVE_EXPR_CONTEXT (TREE_VALUE (t)) = context;
!     }
  
    /* This function is being processed in whole-function mode.  */
    cfun->x_whole_function_mode_p = 1;
--- 6025,6033 ----
    /* Begin the statement tree for this function.  */
    begin_stmt_tree (&DECL_SAVED_TREE (fndecl));
  
!   /* Save away the sizes of any variable-size types so that we can
!      expand them when generating RTL.  */
!   DECL_LANG_SPECIFIC (fndecl)->pending_sizes = get_pending_sizes ();
  
    /* This function is being processed in whole-function mode.  */
    cfun->x_whole_function_mode_p = 1;
*************** static void
*** 6181,6195 ****
  c_expand_body_1 (tree fndecl, int nested_p)
  {
    if (nested_p)
!     {
!       /* Make sure that we will evaluate variable-sized types involved
! 	 in our function's type.  */
!       expand_pending_sizes (DECL_LANG_SPECIFIC (fndecl)->pending_sizes);
! 
!       /* Squirrel away our current state.  */
!       push_function_context ();
!     }
      
    tree_rest_of_compilation (fndecl, nested_p);
  
    if (nested_p)
--- 6178,6189 ----
  c_expand_body_1 (tree fndecl, int nested_p)
  {
    if (nested_p)
!     /* Squirrel away our current state.  */
!     push_function_context ();
      
+   /* Make sure that we will evaluate variable-sized types involved
+      in our function's type.  */
+   put_pending_sizes (DECL_LANG_SPECIFIC (fndecl)->pending_sizes);
    tree_rest_of_compilation (fndecl, nested_p);
  
    if (nested_p)
Index: c-objc-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-objc-common.c,v
retrieving revision 1.37.4.1
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.37.4.1 c-objc-common.c
*** c-objc-common.c 8 Feb 2004 23:12:17 -0000 1.37.4.1
--- c-objc-common.c 1 Aug 2005 17:06:05 -0000
*************** c_cannot_inline_tree_fn (tree *fnp)
*** 118,134 ****
  	}
      }
  
!   if (! DECL_FILE_SCOPE_P (fn))
      {
!       /* If a nested function has pending sizes, we may have already
!          saved them.  */
!       if (DECL_LANG_SPECIFIC (fn)->pending_sizes)
! 	{
! 	  if (do_warning)
! 	    warning ("%Jnested function '%F' can never be inlined because it "
! 		     "has possibly saved pending sizes", fn, fn);
! 	  goto cannot_inline;
! 	}
      }
  
    return 0;
--- 118,129 ----
  	}
      }
  
!   if (DECL_LANG_SPECIFIC (fn)->pending_sizes)
      {
!       if (do_warning)
! 	warning ("%Jfunction '%F' can never be inlined because it has "
! 		 "pending sizes", fn, fn);
!       goto cannot_inline;
      }
  
    return 0;
diff -c /dev/null testsuite/gcc.c-torture/execute/pr22061-1.c
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- testsuite/gcc.c-torture/execute/pr22061-1.c	2005-08-01 15:23:36.000000000 +0100
***************
*** 0 ****
--- 1,16 ----
+ int N = 1;
+ void foo() {} /* Necessary to trigger the original ICE.  */
+ void bar (char a[2][N]) { a[1][0] = N; }
+ int
+ main (void)
+ {
+   void *x;
+ 
+   N = 4;
+   x = alloca (2 * N);
+   memset (x, 0, 2 * N);
+   bar (x);
+   if (N[(char *) x] != N)
+     abort ();
+   exit (0);
+ }
diff -c /dev/null testsuite/gcc.c-torture/execute/pr22061-2.c
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- testsuite/gcc.c-torture/execute/pr22061-2.c	2005-08-01 15:31:10.000000000 +0100
***************
*** 0 ****
--- 1,7 ----
+ int *x;
+ static void bar (char a[2][(*x)++]) {}
+ int
+ main (void)
+ {
+   exit (0);
+ }
diff -c /dev/null testsuite/gcc.c-torture/execute/pr22061-3.c
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- testsuite/gcc.c-torture/execute/pr22061-3.c	2005-08-01 15:35:08.000000000 +0100
***************
*** 0 ****
--- 1,18 ----
+ void
+ bar (int N)
+ {
+   int foo (char a[2][++N]) { N += 4; return sizeof (a[0]); }
+   if (foo (0) != 2)
+     abort ();
+   if (foo (0) != 7)
+     abort ();
+   if (N != 11)
+     abort ();
+ }
+ 
+ int
+ main()
+ {
+   bar (1);
+   exit (0);
+ }
diff -c /dev/null testsuite/gcc.c-torture/execute/pr22061-4.c
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- testsuite/gcc.c-torture/execute/pr22061-4.c	2005-08-01 16:10:58.000000000 +0100
***************
*** 0 ****
--- 1,22 ----
+ void
+ bar (int N)
+ {
+   void foo (int a[2][N++]) {}
+   int a[2][N];
+   foo (a);
+   int b[2][N];
+   foo (b);
+   if (sizeof (a) != sizeof (int) * 2 * 1)
+     abort ();
+   if (sizeof (b) != sizeof (int) * 2 * 2)
+     abort ();
+   if (N != 3)
+     abort ();
+ }
+ 
+ int
+ main (void)
+ {
+   bar (1);
+   exit (0);
+ }