[PING][PATCH] PR middle-end/29683 detect stack collision of split args

[Josh Conner <jconner at apple dot com>]

Ping...

Josh Conner wrote:
> In PR29683, arguments that are split between the stack and registers can
> corrupt other arguments.  In the example from the PR, we have a tree
> stmt like:
> 
>   VerifyValues (filler, 0, a$mbr1, GetConst (filler, a));
> 
> Where the third argument to VerifyValues (a) is on the stack, and the
> second argument to GetConst (a$mbr1) is split between the stack and
> registers.  expand_call processes a$mbr1 before the call to GetConst,
> and saves it onto the stack.  Then, when evaluating the call to
> GetConst, there is logic in store_one_arg that should detect that we
> need to overwrite the same stack location and save it aside:
> 
>   if (ACCUMULATE_OUTGOING_ARGS && !(flags & ECF_SIBCALL))
>     {
>       /* If this is being stored into a pre-allocated, fixed-size, \
> 	 stack area,
>          save any previous data at that location.  */
>       if (argblock && ! variable_size && arg->stack)
> 
>       ...
> 
> Unfortunately, this logic doesn't trigger for split arguments, since
> arg->stack is NULL.
> 
> The attached patch addresses this by calculating arg->stack and
> arg->stack_slot for split (partial) arguments, as well.  By doing this,
> we enable store_one_arg to detect overlaps, and to save off the
> conflicting locations as needed.  One slight modification is needed to
> store_one_arg, so that it only saves off the size of the portion on the
> stack, not the size of the whole argument.  Interestingly enough, the
> other places where the save_area is manipulated were already using the
> correct value (locate_size.constant, not expr_size (tree_value)).
> 
> I bootstrapped and verified no regressions on powerpc-apple-darwin8.8.0
> (--enable-languages=c,c++,objc,obj-c++ --disable-multilib).  I also
> verified on i686-pc-linux-gnu, although this platform is not susceptible
> to this failure.  Finally, I also verified on sh-none-elf, hoping I
> could reproduce this on a second platform, only to realize it didn't
> accumulate outgoing args.  None the less, it passed all regression tests
> as well.
> 
> Note that we should probably consider disabling TER-ing a function call
> into a function call, as setting up a stack argument just to save it
> aside is pretty inefficient.  I have a second patch for that, but I
> thought it was important to fix the expand_call failure first.
> 
> OK for mainline?  Should this be considered for 4.2/4.1, as well?
> 
> - Josh
> 
> 2006-11-01  Josh Conner  <jconner@apple.com>
> 
> 	PR middle-end/29683
> 	* calls.c (compute_argument_addresses): Set stack and stack_slot
> 	for partial args, too.
> 	(store_one_arg): Use locate.size.constant for the size when
> 	generating a save_area.
> 
> 2006-11-01  Josh Conner  <jconner@apple.com>
> 
> 	PR middle-end/29683
> 	* gcc.dg/pr29683.c: New.

Index: gcc/calls.c
===================================================================
--- gcc/calls.c	(revision 118072)
+++ gcc/calls.c	(working copy)
@@ -1357,7 +1357,8 @@ compute_argument_addresses (struct arg_d
 	  unsigned int align, boundary;
 
 	  /* Skip this parm if it will not be passed on the stack.  */
-	  if (! args[i].pass_on_stack && args[i].reg != 0)
+	  if (! args[i].pass_on_stack && args[i].reg != 0
+	      && args[i].partial == 0)
 	    continue;
 
 	  if (GET_CODE (offset) == CONST_INT)
@@ -1366,9 +1367,27 @@ compute_argument_addresses (struct arg_d
 	    addr = gen_rtx_PLUS (Pmode, arg_reg, offset);
 
 	  addr = plus_constant (addr, arg_offset);
-	  args[i].stack = gen_rtx_MEM (args[i].mode, addr);
-	  set_mem_attributes (args[i].stack,
-			      TREE_TYPE (args[i].tree_value), 1);
+
+	  if (args[i].partial != 0)
+	    {
+	      /* Only part of the parameter is being passed on the stack.
+		 Generate a simple memory reference of the correct size.
+	       */
+	      unsigned int units_on_stack;
+	      enum machine_mode mode;
+
+	      units_on_stack = args[i].locate.size.constant;
+	      mode = mode_for_size (units_on_stack * BITS_PER_UNIT,
+				    MODE_INT, 0);
+	      args[i].stack = gen_rtx_MEM (mode, addr);
+	      set_mem_size (args[i].stack, GEN_INT (units_on_stack));
+	    }
+	  else
+	    {
+	      args[i].stack = gen_rtx_MEM (args[i].mode, addr);
+	      set_mem_attributes (args[i].stack,
+				  TREE_TYPE (args[i].tree_value), 1);
+	    }
 	  align = BITS_PER_UNIT;
 	  boundary = args[i].locate.boundary;
 	  if (args[i].locate.where_pad != downward)
@@ -1386,9 +1405,27 @@ compute_argument_addresses (struct arg_d
 	    addr = gen_rtx_PLUS (Pmode, arg_reg, slot_offset);
 
 	  addr = plus_constant (addr, arg_offset);
-	  args[i].stack_slot = gen_rtx_MEM (args[i].mode, addr);
-	  set_mem_attributes (args[i].stack_slot,
-			      TREE_TYPE (args[i].tree_value), 1);
+
+	  if (args[i].partial != 0)
+	    {
+	      /* Only part of the parameter is being passed on the stack.
+		 Generate a simple memory reference of the correct size.
+	       */
+	      unsigned int units_on_stack;
+	      enum machine_mode mode;
+
+	      units_on_stack = args[i].locate.size.constant;
+	      mode = mode_for_size (units_on_stack * BITS_PER_UNIT,
+				    MODE_INT, 0);
+	      args[i].stack_slot = gen_rtx_MEM (mode, addr);
+	      set_mem_size (args[i].stack, GEN_INT (units_on_stack));
+	    }
+	  else
+	    {
+	      args[i].stack_slot = gen_rtx_MEM (args[i].mode, addr);
+	      set_mem_attributes (args[i].stack_slot,
+				  TREE_TYPE (args[i].tree_value), 1);
+	    }
 	  set_mem_align (args[i].stack_slot, args[i].locate.boundary);
 
 	  /* Function incoming arguments may overlap with sibling call
@@ -4057,7 +4094,7 @@ store_one_arg (struct arg_data *arg, rtx
 		  arg->save_area = assign_temp (nt, 0, 1, 1);
 		  preserve_temp_slots (arg->save_area);
 		  emit_block_move (validize_mem (arg->save_area), stack_area,
-				   expr_size (arg->tree_value),
+				   GEN_INT (arg->locate.size.constant),
 				   BLOCK_OP_CALL_PARM);
 		}
 	      else

/* { dg-do run } */
/* { dg-options "-Os -fno-inline-functions" } */

void abort (void);

typedef struct {
  int x[7];
} agg7;

typedef struct {
  int mbr1;
  int mbr2;
} agg2;

int expected = 31415;
agg7 filler;

int GetConst (agg7 filler, agg2 split)
{
  return expected;
}

void VerifyValues (agg7 filler, int last_reg, int first_stack, int second_stack)
{
  if (first_stack != 123 || second_stack != expected)
    abort ();
}

void RunTest (agg2 a)
{
  int result;
	
  result = GetConst (filler, a);
  VerifyValues (filler, 0, a.mbr1, result);
}

int main(void)
{
  agg2 result = {123, 456};
  RunTest (result);
  return 0;
}