[PATCH] Fix -fstack-protector -m32 on ppc with stdarg functions

Jakub Jelinek jakub@redhat.com
Sat Jul 2 00:15:00 GMT 2005


Hi!

I have missed the fact that on ppc ABI_V4 the varargs save area
location is hardcoded as virtual_stack_vars_rtx - RS6000_VARARGS_SIZE,
which of course does not work very well with -fstack-protector (well,
FRAME_GROWS_DOWNWARD), since virtual_stack_vars_rtx in that case is
above the vars area, while varargs save area was accounted space
below that area, which lead to corruptions.
Now, to fix this, either we can do it the easy way and for
FRAME_GROWS_DOWNWARD put varargs area above vars area, i.e.
the location of varargs save area would be virtual_stack_vars_rtx
and varargs_size would not be accounted for in elimination offsets
etc.  But I think it is preferrable to keep the area below buffers,
so that they can't be corrupted by buffer overflows.
Furthermore, especially with stdarg optimization, we often can
use much smaller save area, e.g. when we know we don't need to save
any floating point registers, there is no point in allocating an area
for them, similarly if we don't need to save any gpr registers
(whether because there are already 8 integer arguments for the routine,
or only floating types used in va_arg and va_list doesn't escape, etc.).

The following patch allocates the varargs save area using assign_stack_local
with the size that is really needed to held them (and keeping the ABI
constraint that reg_save_area must be 8 byte aligned).
On some routines it means saving up to 96 bytes on the stack.

I have moved machine_function type to rs6000.c, because a HOST_WIDE_INT
field was added to that structure and that caused problems to target files
(crtstuff, libgcc2.c, etc.) that include rs6000.h, but don't define
HOST_WIDE_INT anywhere.  The structure is not used outside of rs6000.c,
so it doesn't matter much, but if you prefer to keep it in rs6000.h,
it would need to be surrounded by #ifndef IN_LIBGCC2 or #ifdef HOST_WIDE_INT
or something like that.

Earlier version of this patch bootstrapped/regtested on ppc64-linux
and also regtested with RUNTESTFLAGS=--target_board=unix/-fstack-protector
(and on ppc32-linux, where it caused regressions).  Those should
be fixed in this patch, but bootstrap/regression testing on ppc32-linux
with this patch (as well as
RUNTESTFLAGS=--target_board=unix/-fstack-protector regtesting)
is still pending.

Ok to commit if it succeeds?

2005-07-02  Jakub Jelinek  <jakub@redhat.com>

	* config/rs6000/rs6000.h (RS6000_VARARGS_AREA, RS6000_VARARGS_SIZE):
	Remove.
	(STARTING_FRAME_OFFSET): Don't add RS6000_VARARGS_AREA.
	(machine_function): Move typedef to...
	* config/rs6000/rs6000.c (machine_function): ... here.  Add
	varargs_save_offset field.
	(rs6000_stack_t): Remove varargs_size field.
	(setup_incoming_varargs): Allocate varargs save area using
	assign_stack_local, try to make it as small as possible.
	Save offset from virtual_stack_vars_rtx to the save area
	in cfun->machine->varargs_save_offset.
	(rs6000_va_start): Use cfun->machine->varargs_save_offset
	instead of -RS6000_VARARGS_SIZE.
	(rs6000_stack_info, debug_stack_info,
	rs6000_initial_elimination_offset): Remove all traces of
	varargs_size.
	* config/rs6000/sysv4.h (RS6000_VARARGS_AREA): Remove.
	* config/rs6000/darwin.h (STARTING_FRAME_OFFSET): Don't add
	RS6000_VARARGS_AREA.

--- gcc/config/rs6000/rs6000.c.jj	2005-06-30 16:26:56.000000000 +0200
+++ gcc/config/rs6000/rs6000.c	2005-07-02 01:33:24.000000000 +0200
@@ -93,7 +93,6 @@ typedef struct rs6000_stack {
   int varargs_save_offset;	/* offset to save the varargs registers */
   int ehrd_offset;		/* offset to EH return data */
   int reg_size;			/* register size (4 or 8) */
-  int varargs_size;		/* size to hold V.4 args passed in regs */
   HOST_WIDE_INT vars_size;	/* variable save area size */
   int parm_size;		/* outgoing parameter size */
   int save_size;		/* save area size */
@@ -113,6 +112,23 @@ typedef struct rs6000_stack {
   int spe_64bit_regs_used;
 } rs6000_stack_t;
 
+/* A C structure for machine-specific, per-function data.
+   This is added to the cfun structure.  */
+typedef struct machine_function GTY(())
+{
+  /* Flags if __builtin_return_address (n) with n >= 1 was used.  */
+  int ra_needs_full_frame;
+  /* Some local-dynamic symbol.  */
+  const char *some_ld_name;
+  /* Whether the instruction chain has been scanned already.  */
+  int insn_chain_scanned_p;
+  /* Flags if __builtin_return_address (0) was used.  */
+  int ra_need_lr;
+  /* Offset from virtual_stack_vars_rtx to the start of the ABI_V4
+     varargs save area.  */
+  HOST_WIDE_INT varargs_save_offset;
+} machine_function;
+
 /* Target cpu type */
 
 enum processor_type rs6000_cpu;
@@ -5218,11 +5234,67 @@ setup_incoming_varargs (CUMULATIVE_ARGS 
 
   if (DEFAULT_ABI == ABI_V4)
     {
+      first_reg_offset = next_cum.sysv_gregno - GP_ARG_MIN_REG;
+
       if (! no_rtl)
-	save_area = plus_constant (virtual_stack_vars_rtx,
-				   - RS6000_VARARGS_SIZE);
+	{
+	  int gpr_size = 0, fpr_size = 0;
+	  HOST_WIDE_INT offset = 0;
 
-      first_reg_offset = next_cum.sysv_gregno - GP_ARG_MIN_REG;
+	  /* Try to optimize the size of the varargs save area.
+	     The ABI requires that ap.reg_save_area is doubleword
+	     aligned, but we don't need to allocate space for all
+	     the bytes, only those to which we actually will save
+	     anything.  */
+	  if (cfun->va_list_gpr_size && first_reg_offset < GP_ARG_NUM_REG)
+	    gpr_size = GP_ARG_NUM_REG - first_reg_offset;
+	  if (TARGET_HARD_FLOAT && TARGET_FPRS
+	      && next_cum.fregno <= FP_ARG_V4_MAX_REG
+	      && cfun->va_list_fpr_size)
+	    {
+	      if (gpr_size)
+		fpr_size = (next_cum.fregno - FP_ARG_MIN_REG) * 8;
+	      if (cfun->va_list_fpr_size
+		  < FP_ARG_V4_MAX_REG + 1 - next_cum.fregno)
+		fpr_size += cfun->va_list_fpr_size * 8;
+	      else
+		fpr_size += (FP_ARG_V4_MAX_REG + 1 - next_cum.fregno) * 8;
+	    }
+	  if (gpr_size)
+	    {
+	      offset = -((first_reg_offset * reg_size) & ~7);
+	      if (!fpr_size && gpr_size > cfun->va_list_gpr_size)
+		{
+		  gpr_size = cfun->va_list_gpr_size;
+		  if (reg_size == 4 && (first_reg_offset & 1))
+		    gpr_size++;
+		}
+	      gpr_size = (gpr_size * reg_size + 7) & ~7;
+	    }
+	  else if (fpr_size)
+	    offset = - (int) (next_cum.fregno - FP_ARG_MIN_REG) * 8
+		     - (int) (GP_ARG_NUM_REG * reg_size);
+
+	  if (gpr_size + fpr_size)
+	    {
+	      rtx reg_save_area
+		= assign_stack_local (BLKmode, gpr_size + fpr_size, 64);
+	      gcc_assert (GET_CODE (reg_save_area) == MEM);
+	      reg_save_area = XEXP (reg_save_area, 0);
+	      if (GET_CODE (reg_save_area) == PLUS)
+		{
+		  gcc_assert (XEXP (reg_save_area, 0)
+			      == virtual_stack_vars_rtx);
+		  gcc_assert (GET_CODE (XEXP (reg_save_area, 1)) == CONST_INT);
+		  offset += INTVAL (XEXP (reg_save_area, 1));
+		}
+	      else
+		gcc_assert (reg_save_area == virtual_stack_vars_rtx);
+	    }
+
+	  cfun->machine->varargs_save_offset = offset;
+	  save_area = plus_constant (virtual_stack_vars_rtx, offset);
+	}
     }
   else
     {
@@ -5423,8 +5495,9 @@ rs6000_va_start (tree valist, rtx nextar
 
   /* Find the register save area.  */
   t = make_tree (TREE_TYPE (sav), virtual_stack_vars_rtx);
-  t = build (PLUS_EXPR, TREE_TYPE (sav), t,
-	     build_int_cst (NULL_TREE, -RS6000_VARARGS_SIZE));
+  if (cfun->machine->varargs_save_offset)
+    t = build (PLUS_EXPR, TREE_TYPE (sav), t,
+	       build_int_cst (NULL_TREE, cfun->machine->varargs_save_offset));
   t = build (MODIFY_EXPR, TREE_TYPE (sav), sav, t);
   TREE_SIDE_EFFECTS (t) = 1;
   expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
@@ -12116,17 +12189,16 @@ rs6000_stack_info (void)
   /* Determine various sizes.  */
   info_ptr->reg_size     = reg_size;
   info_ptr->fixed_size   = RS6000_SAVE_AREA;
-  info_ptr->varargs_size = RS6000_VARARGS_AREA;
   info_ptr->vars_size    = RS6000_ALIGN (get_frame_size (), 8);
   info_ptr->parm_size    = RS6000_ALIGN (current_function_outgoing_args_size,
 					 TARGET_ALTIVEC ? 16 : 8);
   if (FRAME_GROWS_DOWNWARD)
     info_ptr->vars_size
-      += RS6000_ALIGN (info_ptr->fixed_size + info_ptr->varargs_size
-		       + info_ptr->vars_size + info_ptr->parm_size,
+      += RS6000_ALIGN (info_ptr->fixed_size + info_ptr->vars_size
+		       + info_ptr->parm_size,
 		       ABI_STACK_BOUNDARY / BITS_PER_UNIT)
-	 - (info_ptr->fixed_size + info_ptr->varargs_size
-	    + info_ptr->vars_size + info_ptr->parm_size);
+	 - (info_ptr->fixed_size + info_ptr->vars_size
+	    + info_ptr->parm_size);
 
   if (TARGET_SPE_ABI && info_ptr->spe_64bit_regs_used != 0)
     info_ptr->spe_gp_size = 8 * (32 - info_ptr->first_gp_reg_save);
@@ -12251,8 +12323,7 @@ rs6000_stack_info (void)
 
   non_fixed_size	 = (info_ptr->vars_size
 			    + info_ptr->parm_size
-			    + info_ptr->save_size
-			    + info_ptr->varargs_size);
+			    + info_ptr->save_size);
 
   info_ptr->total_size = RS6000_ALIGN (non_fixed_size + info_ptr->fixed_size,
 				       ABI_STACK_BOUNDARY / BITS_PER_UNIT);
@@ -12452,9 +12523,6 @@ debug_stack_info (rs6000_stack_t *info)
     fprintf (stderr, "\ttotal_size          = "HOST_WIDE_INT_PRINT_DEC"\n",
 	     info->total_size);
 
-  if (info->varargs_size)
-    fprintf (stderr, "\tvarargs_size        = %5d\n", info->varargs_size);
-
   if (info->vars_size)
     fprintf (stderr, "\tvars_size           = "HOST_WIDE_INT_PRINT_DEC"\n",
 	     info->vars_size);
@@ -18261,13 +18329,11 @@ rs6000_initial_elimination_offset (int f
     {
       offset = info->push_p ? 0 : -info->total_size;
       if (FRAME_GROWS_DOWNWARD)
-	offset += info->fixed_size + info->varargs_size
-		  + info->vars_size + info->parm_size;
+	offset += info->fixed_size + info->vars_size + info->parm_size;
     }
   else if (from == FRAME_POINTER_REGNUM && to == HARD_FRAME_POINTER_REGNUM)
     offset = FRAME_GROWS_DOWNWARD
-	     ? info->fixed_size + info->varargs_size
-	       + info->vars_size + info->parm_size
+	     ? info->fixed_size + info->vars_size + info->parm_size
 	     : 0;
   else if (from == ARG_POINTER_REGNUM && to == HARD_FRAME_POINTER_REGNUM)
     offset = info->total_size;
--- gcc/config/rs6000/rs6000.h.jj	2005-06-30 16:26:56.000000000 +0200
+++ gcc/config/rs6000/rs6000.h	2005-07-01 17:01:27.000000000 +0200
@@ -1251,16 +1251,9 @@ extern enum rs6000_abi rs6000_current_ab
 				     plus_constant (stack_pointer_rtx, \
 						    (TARGET_32BIT ? 20 : 40)))
 
-/* Size of the V.4 varargs area if needed */
-#define RS6000_VARARGS_AREA 0
-
 /* Align an address */
 #define RS6000_ALIGN(n,a) (((n) + (a) - 1) & ~((a) - 1))
 
-/* Size of V.4 varargs area in bytes */
-#define RS6000_VARARGS_SIZE \
-  ((GP_ARG_NUM_REG * (TARGET_32BIT ? 4 : 8)) + (FP_ARG_NUM_REG * 8) + 8)
-
 /* Offset within stack frame to start allocating local variables at.
    If FRAME_GROWS_DOWNWARD, this is the offset to the END of the
    first local allocated.  Otherwise, it is the offset to the BEGINNING
@@ -1275,7 +1268,6 @@ extern enum rs6000_abi rs6000_current_ab
    ? 0									\
    : (RS6000_ALIGN (current_function_outgoing_args_size,		\
 		    TARGET_ALTIVEC ? 16 : 8)				\
-      + RS6000_VARARGS_AREA						\
       + RS6000_SAVE_AREA))
 
 /* Offset from the stack pointer register to an item dynamically
@@ -1413,20 +1405,6 @@ extern enum rs6000_abi rs6000_current_ab
    || ((unsigned) (N) - FP_ARG_MIN_REG < FP_ARG_NUM_REG			\
        && TARGET_HARD_FLOAT && TARGET_FPRS))
 
-/* A C structure for machine-specific, per-function data.
-   This is added to the cfun structure.  */
-typedef struct machine_function GTY(())
-{
-  /* Flags if __builtin_return_address (n) with n >= 1 was used.  */
-  int ra_needs_full_frame;
-  /* Some local-dynamic symbol.  */
-  const char *some_ld_name;
-  /* Whether the instruction chain has been scanned already.  */
-  int insn_chain_scanned_p;
-  /* Flags if __builtin_return_address (0) was used.  */
-  int ra_need_lr;
-} machine_function;
-
 /* Define a data type for recording info about an argument list
    during the scan of that argument list.  This data type should
    hold all necessary information about the function itself
--- gcc/config/rs6000/darwin.h.jj	2005-07-01 10:19:30.000000000 +0200
+++ gcc/config/rs6000/darwin.h	2005-07-01 16:09:59.000000000 +0200
@@ -164,7 +164,6 @@ do {									\
   (FRAME_GROWS_DOWNWARD							\
    ? 0									\
    : (RS6000_ALIGN (current_function_outgoing_args_size, 16)		\
-      + RS6000_VARARGS_AREA						\
       + RS6000_SAVE_AREA))
 
 #undef STACK_DYNAMIC_OFFSET
--- gcc/config/rs6000/sysv4.h.jj	2005-06-30 16:21:17.000000000 +0200
+++ gcc/config/rs6000/sysv4.h	2005-07-01 16:10:25.000000000 +0200
@@ -249,13 +249,6 @@ do {									\
    so it is not available to the normal user.  */
 #define FIXED_R13 1
 
-/* Size of the V.4 varargs area if needed.  */
-/* Override rs6000.h definition.  */
-#undef	RS6000_VARARGS_AREA
-#define RS6000_VARARGS_AREA \
-  ((DEFAULT_ABI == ABI_V4 && current_function_stdarg)		\
-   ? RS6000_VARARGS_SIZE : 0)
-
 /* Override default big endianism definitions in rs6000.h.  */
 #undef	BYTES_BIG_ENDIAN
 #undef	WORDS_BIG_ENDIAN

	Jakub



More information about the Gcc-patches mailing list