This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Enable -fstack-protector on MIPS
- From: Adam Nemet <anemet at caviumnetworks dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: 30 Apr 2009 11:16:59 -0700
- Subject: Re: [PATCH] Enable -fstack-protector on MIPS
- References: <18934.1072.54934.5684@ropi.home> <87ocuf47az.fsf@firetop.home>
Richard Sandiford writes:
> Looks good, but...
>
> Adam Nemet <anemet@caviumnetworks.com> writes:
> > @@ -8741,10 +8742,10 @@ mips_compute_frame_info (void)
> >
> > cfun->machine->global_pointer = mips_global_pointer ();
> >
> > - /* The first STARTING_FRAME_OFFSET bytes contain the outgoing argument
> > - area and the $gp save slot. This area isn't needed in leaf functions,
> > - but if the target-independent frame size is nonzero, we're committed
> > - to allocating it anyway. */
> > + /* The first two blocks contain the outgoing argument area and the $gp save
> > + slot. This area isn't needed in leaf functions, but if the
> > + target-independent frame size is nonzero, we're committed to allocating
> > + it anyway. */
> > if (size == 0 && current_function_is_leaf)
> > {
> > /* The MIPS 3.0 linker does not like functions that dynamically
>
> ...are we really committed to allocating this area for FRAME_GROWS_DOWNWARD,
> where STARTING_FRAME_OFFSET is zero? The point is that if the target-
> independent frame size is nonzero, the target-independent code will accessed
> the first byte of the frame as frame_pointer_rtx + STARTING_FRAME_OFFSET.
Good point, thanks. I didn't make the connection that we were committed
because of how STARTING_FRAME_OFFSET was formulated. I updated the comment to
make this more explicit.
Here is the new patch. I tested as before. I also tried o32 and o64 with
-mabicalls and looked at the assembly generated. For this:
int j;
f ()
{
long long i;
asm volatile ("# %0 %1" :: "m"(i), "r"(j));
}
we no longer allocate the cprestore area with FRAME_GROWS_DOWNWARD.
OK?
Adam
* config/mips/mips.h (FRAME_GROWS_DOWNWARD,
MIPS_GP_SAVE_AREA_SIZE): Define new macros.
(STARTING_FRAME_OFFSET): Return 0 if FRAME_GROWS_DOWNWARD. Use
MIPS_GP_SAVE_AREA_SIZE.
* config/mips/mips.c (struct mips_frame_info): Update comment
before arg_pointer_offset and hard_frame_pointer_offset.
(mips_compute_frame_info): Update diagram before function: to
correctly use stack_pointer_rtx for fp_sp_offset and gp_sp_offset, to
indicate the position of frame_pointer_rtx with -fstack-protector and
to show args_size. Don't allocate cprestore area for leaf functions
if FRAME_GROWS_DOWNWARD. Use MIPS_GP_SAVE_AREA_SIZE to set
cprestore_size.
(mips_initial_elimination_offset): Update for FRAME_GROWS_DOWNWARD.
Index: gcc/config/mips/mips.h
===================================================================
--- gcc.orig/config/mips/mips.h 2009-04-26 20:38:19.000000000 -0700
+++ gcc/config/mips/mips.h 2009-04-30 10:58:21.000000000 -0700
@@ -2081,12 +2081,20 @@ enum reg_class
#define STACK_GROWS_DOWNWARD
-/* The offset of the first local variable from the beginning of the frame.
- See mips_compute_frame_info for details about the frame layout. */
+#define FRAME_GROWS_DOWNWARD flag_stack_protect
-#define STARTING_FRAME_OFFSET \
- (crtl->outgoing_args_size \
- + (TARGET_CALL_CLOBBERED_GP ? MIPS_STACK_ALIGN (UNITS_PER_WORD) : 0))
+/* Size of the area allocated in the frame to save the GP. */
+
+#define MIPS_GP_SAVE_AREA_SIZE \
+ (TARGET_CALL_CLOBBERED_GP ? MIPS_STACK_ALIGN (UNITS_PER_WORD) : 0)
+
+/* The offset of the first local variable from the frame pointer. See
+ mips_compute_frame_info for details about the frame layout. */
+
+#define STARTING_FRAME_OFFSET \
+ (FRAME_GROWS_DOWNWARD \
+ ? 0 \
+ : crtl->outgoing_args_size + MIPS_GP_SAVE_AREA_SIZE)
#define RETURN_ADDR_RTX mips_return_addr
Index: gcc/config/mips/mips.c
===================================================================
--- gcc.orig/config/mips/mips.c 2009-04-26 20:40:50.000000000 -0700
+++ gcc/config/mips/mips.c 2009-04-30 11:04:26.000000000 -0700
@@ -285,10 +285,10 @@ struct mips_frame_info GTY(()) {
HOST_WIDE_INT acc_sp_offset;
HOST_WIDE_INT cop0_sp_offset;
- /* The offset of arg_pointer_rtx from frame_pointer_rtx. */
+ /* The offset of arg_pointer_rtx from the bottom of the frame. */
HOST_WIDE_INT arg_pointer_offset;
- /* The offset of hard_frame_pointer_rtx from frame_pointer_rtx. */
+ /* The offset of hard_frame_pointer_rtx from the bottom of the frame. */
HOST_WIDE_INT hard_frame_pointer_offset;
};
@@ -8670,16 +8670,16 @@ mips_save_reg_p (unsigned int regno)
| | + UNITS_PER_WORD
| accumulator save area |
| |
- +-------------------------------+ <-- frame_pointer_rtx + fp_sp_offset
+ +-------------------------------+ <-- stack_pointer_rtx + fp_sp_offset
| | + UNITS_PER_HWFPVALUE
| FPR save area |
| |
- +-------------------------------+ <-- frame_pointer_rtx + gp_sp_offset
+ +-------------------------------+ <-- stack_pointer_rtx + gp_sp_offset
| | + UNITS_PER_WORD
| GPR save area |
| |
- +-------------------------------+
- | | \
+ +-------------------------------+ <-- frame_pointer_rtx with
+ | | \ -fstack-protector
| local variables | | var_size
| | /
+-------------------------------+
@@ -8687,16 +8687,17 @@ mips_save_reg_p (unsigned int regno)
| $gp save area | | cprestore_size
| | /
P +-------------------------------+ <-- hard_frame_pointer_rtx for
- | | MIPS16 code
- | outgoing stack arguments |
- | |
- +-------------------------------+
- | |
- | caller-allocated save area |
- | for register arguments |
- | |
+ | | \ MIPS16 code
+ | outgoing stack arguments | |
+ | | |
+ +-------------------------------+ | args_size
+ | | |
+ | caller-allocated save area | |
+ | for register arguments | |
+ | | /
+-------------------------------+ <-- stack_pointer_rtx
- frame_pointer_rtx
+ frame_pointer_rtx without
+ -fstack-protector
hard_frame_pointer_rtx for
non-MIPS16 code.
@@ -8741,11 +8742,11 @@ mips_compute_frame_info (void)
cfun->machine->global_pointer = mips_global_pointer ();
- /* The first STARTING_FRAME_OFFSET bytes contain the outgoing argument
- area and the $gp save slot. This area isn't needed in leaf functions,
- but if the target-independent frame size is nonzero, we're committed
- to allocating it anyway. */
- if (size == 0 && current_function_is_leaf)
+ /* The first two blocks contain the outgoing argument area and the $gp save
+ slot. This area isn't needed in leaf functions, but if the
+ target-independent frame size is nonzero, we have already committed to
+ allocating these in STARTING_FRAME_OFFSET for !FRAME_GROWS_DOWNWARD. */
+ if ((size == 0 || FRAME_GROWS_DOWNWARD) && current_function_is_leaf)
{
/* The MIPS 3.0 linker does not like functions that dynamically
allocate the stack and have 0 for STACK_DYNAMIC_OFFSET, since it
@@ -8760,7 +8761,7 @@ mips_compute_frame_info (void)
else
{
frame->args_size = crtl->outgoing_args_size;
- frame->cprestore_size = STARTING_FRAME_OFFSET - frame->args_size;
+ frame->cprestore_size = MIPS_GP_SAVE_AREA_SIZE;
}
offset = frame->args_size + frame->cprestore_size;
@@ -8939,12 +8940,16 @@ mips_initial_elimination_offset (int fro
mips_compute_frame_info ();
- /* Set OFFSET to the offset from the soft frame pointer, which is also
- the offset from the end-of-prologue stack pointer. */
+ /* Set OFFSET to the offset from the end-of-prologue stack pointer. */
switch (from)
{
case FRAME_POINTER_REGNUM:
- offset = 0;
+ if (FRAME_GROWS_DOWNWARD)
+ offset = (cfun->machine->frame.args_size
+ + cfun->machine->frame.cprestore_size
+ + cfun->machine->frame.var_size);
+ else
+ offset = 0;
break;
case ARG_POINTER_REGNUM: