[patch, rfa:doco, rfa:ppc] Generic red zone arch method

[patch, rfa:doco, rfa:ppc] Generic red zone arch method

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

Hello,

Ref: Skip the "red zone" on AMD64
http://sources.redhat.com/ml/gdb-patches/2003-08/msg00092.html

The attached implements the suggested fix in that thread.  For x86-64, 
it gives the same results as the original change (need more test cases).

Eli, doco ok?

Kevin, for PPC64 and AIX I pulled the numbers from ABI doco.  It isn't 
yet tested.  For PPC64 I've too many other failures for results to be 
meaningful, and for AIX probably won't happen :-(.

The remaining bits (framework and x86-64) I'll commit in a few days.
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 7249 bytes --]

Index: ChangeLog
2003-08-16  Andrew Cagney  <cagney@redhat.com>

	* gdbarch.sh (FRAME_RED_ZONE_SIZE): New architecture method.
	* gdbarch.h, gdbarch.c: Re-generate.
	* infcall.c (call_function_by_hand): Adjust the SP by
	frame_red_zone_size before allocating any stack space.
	* rs6000-tdep.c (rs6000_gdbarch_init): Set "frame_red_zone_size".
	* x86-64-tdep.c (x86_64_frame_align): New function.
	(x86_64_init_abi): Set "frame_red_zone_size" and "frame_align".

	* x86-64-tdep.c (x86_64_push_arguments): Revert 2003-08-07 change.
	Remove code adjusting SP so that it skips over the Red Zone.

Index: doc/ChangeLog
2003-08-13  Andrew Cagney  <cagney@redhat.com>

	* gdbint.texinfo (Target Architecture Definition): Document
	"frame_red_zone_size".

Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.261
diff -u -r1.261 gdbarch.sh
--- gdbarch.sh	4 Aug 2003 22:24:44 -0000	1.261
+++ gdbarch.sh	16 Aug 2003 18:51:07 -0000
@@ -643,6 +643,7 @@
 F:2:STACK_ALIGN:CORE_ADDR:stack_align:CORE_ADDR sp:sp
 M:::CORE_ADDR:frame_align:CORE_ADDR address:address
 F:2:REG_STRUCT_HAS_ADDR:int:reg_struct_has_addr:int gcc_p, struct type *type:gcc_p, type
+v::FRAME_RED_ZONE_SIZE:int:frame_red_zone_size
 v:2:PARM_BOUNDARY:int:parm_boundary
 #
 v:2:TARGET_FLOAT_FORMAT:const struct floatformat *:float_format::::::default_float_format (gdbarch)::%s:(TARGET_FLOAT_FORMAT)->name
Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.22
diff -u -r1.22 infcall.c
--- infcall.c	10 Aug 2003 17:19:23 -0000	1.22
+++ infcall.c	16 Aug 2003 18:51:08 -0000
@@ -440,6 +440,18 @@
     CORE_ADDR old_sp = read_sp ();
     if (gdbarch_frame_align_p (current_gdbarch))
       {
+	sp = gdbarch_frame_align (current_gdbarch, old_sp);
+	/* NOTE: cagney/2003-08-13: Skip the "red zone".  For some
+	   ABIs, a function can use memory beyond the inner most stack
+	   address.  AMD64 called that region the "red zone".  Skip at
+	   least the "red zone" size before allocating any space on
+	   the stack.  */
+	if (INNER_THAN (1, 2))
+	  sp -= gdbarch_frame_red_zone_size (current_gdbarch);
+	else
+	  sp += gdbarch_frame_red_zone_size (current_gdbarch);
+	/* Still aligned?  */
+	gdb_assert (sp == gdbarch_frame_align (current_gdbarch, sp));
 	/* NOTE: cagney/2002-09-18:
 	   
 	   On a RISC architecture, a void parameterless generic dummy
@@ -460,7 +472,6 @@
 	   stack.  That way, two dummy frames can never be identical.
 	   It does burn a few bytes of stack but that is a small price
 	   to pay :-).  */
-	sp = gdbarch_frame_align (current_gdbarch, old_sp);
 	if (sp == old_sp)
 	  {
 	    if (INNER_THAN (1, 2))
@@ -476,12 +487,16 @@
     else
       /* FIXME: cagney/2002-09-18: Hey, you loose!
 
-	 Who knows how badly aligned the SP is!  Further, per comment
-	 above, if the generic dummy frame ends up empty (because
-	 nothing is pushed) GDB won't be able to correctly perform
-	 back traces.  If a target is having trouble with backtraces,
-	 first thing to do is add FRAME_ALIGN() to the architecture
-	 vector. If that fails, try unwind_dummy_id().  */
+	 Who knows how badly aligned the SP is!
+
+	 If the generic dummy frame ends up empty (because nothing is
+	 pushed) GDB won't be able to correctly perform back traces.
+	 If a target is having trouble with backtraces, first thing to
+	 do is add FRAME_ALIGN() to the architecture vector. If that
+	 fails, try unwind_dummy_id().
+
+         If the ABI specifies a "Red Zone" (see the doco) the code
+         below will quietly trash it.  */
       sp = old_sp;
   }
 
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.147
diff -u -r1.147 rs6000-tdep.c
--- rs6000-tdep.c	26 Jun 2003 17:18:42 -0000	1.147
+++ rs6000-tdep.c	16 Aug 2003 18:51:08 -0000
@@ -2944,6 +2944,12 @@
 
   set_gdbarch_deprecated_fix_call_dummy (gdbarch, rs6000_fix_call_dummy);
   set_gdbarch_frame_align (gdbarch, rs6000_frame_align);
+  if (sysv_abi && wordsize == 8)
+    /* PPC64 SYSV.  */
+    set_gdbarch_frame_red_zone_size (gdbarch, 288);
+  else if (!sysv_abi && wordsize == 4)
+    /* PowerOpen / AIX 32 bit.  */
+    set_gdbarch_frame_red_zone_size (gdbarch, 220);
   set_gdbarch_deprecated_save_dummy_frame_tos (gdbarch, generic_save_dummy_frame_tos);
   set_gdbarch_deprecated_push_return_address (gdbarch, ppc_push_return_address);
   set_gdbarch_believe_pcc_promotion (gdbarch, 1);
Index: x86-64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/x86-64-tdep.c,v
retrieving revision 1.87
diff -u -r1.87 x86-64-tdep.c
--- x86-64-tdep.c	7 Aug 2003 11:26:42 -0000	1.87
+++ x86-64-tdep.c	16 Aug 2003 18:51:09 -0000
@@ -614,13 +614,6 @@
   int *stack_values;
   stack_values = alloca (nargs * sizeof (int));
 
-  /* Before storing anything to the stack we must skip
-     the "Red zone" (see the "Function calling sequence" section
-     of AMD64 ABI).
-     It could have already been skipped in the function's
-     prologue, but we don't care and will easily skip it once again.  */
-  sp -= 128;
-
   for (i = 0; i < nargs; i++)
     {
       enum x86_64_reg_class class[MAX_CLASSES];
@@ -1203,6 +1196,14 @@
   return frame_id_build (fp + 16, frame_pc_unwind (next_frame));
 }
 
+/* 16 byte align the SP per frame requirements.  */
+
+static CORE_ADDR
+x86_64_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
+{
+  return sp & -(CORE_ADDR)16;
+}
+
 void
 x86_64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -1246,6 +1247,8 @@
 
   /* Call dummy code.  */
   set_gdbarch_push_dummy_call (gdbarch, x86_64_push_dummy_call);
+  set_gdbarch_frame_align (gdbarch, x86_64_frame_align);
+  set_gdbarch_frame_red_zone_size (gdbarch, 128);
 
   set_gdbarch_convert_register_p (gdbarch, x86_64_convert_register_p);
   set_gdbarch_register_to_value (gdbarch, i387_register_to_value);
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.158
diff -u -r1.158 gdbint.texinfo
--- doc/gdbint.texinfo	4 Aug 2003 20:43:57 -0000	1.158
+++ doc/gdbint.texinfo	16 Aug 2003 18:51:14 -0000
@@ -3229,6 +3229,24 @@
 
 By default, no frame based stack alignment is performed.
 
+@item int frame_red_zone_size
+
+The number of bytes, beyond the innermost-stack-address, reserved by the
+@sc{abi}.  A function is permitted to use this scratch area (instead of
+allocating extra stack space).
+
+When performing an inferior function call, to ensure that it does not
+modify this area, @value{GDBN} adjusts the innermost-stack-address by
+@var{frame_red_zone_size} bytes before pushing parameters onto the
+stack.
+
+By default, zero bytes are allocated.  The value must be aligned
+(@pxref{frame_align}).
+
+The @sc{amd64} (nee x86-64) @sc{abi} documentation refers to the
+@emph{red zone} when describing this scratch area.
+@kindex red zone
+
 @item DEPRECATED_FRAME_CHAIN(@var{frame})
 @findex DEPRECATED_FRAME_CHAIN
 Given @var{frame}, return a pointer to the calling frame.

Re: [patch, rfa:doco, rfa:ppc] Generic red zone arch method

[Daniel Jacobowitz]

On Sat, Aug 16, 2003 at 03:07:28PM -0400, Andrew Cagney wrote:
> Hello,
> 
> Ref: Skip the "red zone" on AMD64
> http://sources.redhat.com/ml/gdb-patches/2003-08/msg00092.html
> 
> The attached implements the suggested fix in that thread.  For x86-64, 
> it gives the same results as the original change (need more test cases).
> 
> Eli, doco ok?
> 
> Kevin, for PPC64 and AIX I pulled the numbers from ABI doco.  It isn't 
> yet tested.  For PPC64 I've too many other failures for results to be 
> meaningful, and for AIX probably won't happen :-(.
> 
> The remaining bits (framework and x86-64) I'll commit in a few days.
> Andrew

I'm sure there's a reason this won't work but... is gdbarch_frame_align
called from anywhere else, i.e. that should not include the red zone? 
Why isn't skipping the red zone there sufficient?


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch, rfa:doco, rfa:ppc] Generic red zone arch method

[Andrew Cagney]

> On Sat, Aug 16, 2003 at 03:07:28PM -0400, Andrew Cagney wrote:
> 
>> Hello,
>> 
>> Ref: Skip the "red zone" on AMD64
>> http://sources.redhat.com/ml/gdb-patches/2003-08/msg00092.html
>> 
>> The attached implements the suggested fix in that thread.  For x86-64, 
>> it gives the same results as the original change (need more test cases).
>> 
>> Eli, doco ok?
>> 
>> Kevin, for PPC64 and AIX I pulled the numbers from ABI doco.  It isn't 
>> yet tested.  For PPC64 I've too many other failures for results to be 
>> meaningful, and for AIX probably won't happen :-(.
>> 
>> The remaining bits (framework and x86-64) I'll commit in a few days.
>> Andrew
> 
> 
> I'm sure there's a reason this won't work but... is gdbarch_frame_align
> called from anywhere else, i.e. that should not include the red zone?
> Why isn't skipping the red zone there sufficient?

Please look in infcall.c at how frame_align is used :-/

Andrew

Re: [patch, rfa:doco, rfa:ppc] Generic red zone arch method

[Daniel Jacobowitz]

On Sat, Aug 16, 2003 at 03:32:31PM -0400, Andrew Cagney wrote:
> >On Sat, Aug 16, 2003 at 03:07:28PM -0400, Andrew Cagney wrote:
> >
> >>Hello,
> >>
> >>Ref: Skip the "red zone" on AMD64
> >>http://sources.redhat.com/ml/gdb-patches/2003-08/msg00092.html
> >>
> >>The attached implements the suggested fix in that thread.  For x86-64, 
> >>it gives the same results as the original change (need more test cases).
> >>
> >>Eli, doco ok?
> >>
> >>Kevin, for PPC64 and AIX I pulled the numbers from ABI doco.  It isn't 
> >>yet tested.  For PPC64 I've too many other failures for results to be 
> >>meaningful, and for AIX probably won't happen :-(.
> >>
> >>The remaining bits (framework and x86-64) I'll commit in a few days.
> >>Andrew
> >
> >
> >I'm sure there's a reason this won't work but... is gdbarch_frame_align
> >called from anywhere else, i.e. that should not include the red zone?
> >Why isn't skipping the red zone there sufficient?
> 
> Please look in infcall.c at how frame_align is used :-/

Blah.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch, rfa:doco, rfa:ppc] Generic red zone arch method

[Eli Zaretskii]

> Date: Sat, 16 Aug 2003 15:07:28 -0400
> From: Andrew Cagney <ac131313@redhat.com>
> 
> Eli, doco ok?

Yes, except that

> +@kindex red zone

Should be a @cindex, I think ("red zone" is not a key nor a function).

Re: [patch, rfa:doco, rfa:ppc] Generic red zone arch method

[Kevin Buettner]

On Aug 16,  3:07pm, Andrew Cagney wrote:

> Kevin, for PPC64 and AIX I pulled the numbers from ABI doco.  It isn't 
> yet tested.  For PPC64 I've too many other failures for results to be 
> meaningful, and for AIX probably won't happen :-(.

The rs6000-tdep.c changes are okay with me.

Kevin

Re: [patch, rfa:doco, rfa:ppc] Generic red zone arch method

[Andrew Cagney]

> On Aug 16,  3:07pm, Andrew Cagney wrote:
> 
> 
>> Kevin, for PPC64 and AIX I pulled the numbers from ABI doco.  It isn't 
>> yet tested.  For PPC64 I've too many other failures for results to be 
>> meaningful, and for AIX probably won't happen :-(.
> 
> 
> The rs6000-tdep.c changes are okay with me.

Thanks.  I've got as far as figuring out that the PPC64 part doesn't 
make things worse :-/

Andrew

Re: [patch, rfa:doco, rfa:ppc] Generic red zone arch method

[Andrew Cagney]

> Date: Sat, 16 Aug 2003 15:07:28 -0400
>> From: Andrew Cagney <ac131313@redhat.com>
>> 
>> Eli, doco ok?
> 
> 
> Yes, except that
> 
> 
>> +@kindex red zone
> 
> 
> Should be a @cindex, I think ("red zone" is not a key nor a function).

Ok.  With that tweak, I've checked this in to trunk and branch.

> Index: ChangeLog
> 2003-08-16  Andrew Cagney  <cagney@redhat.com>
> 
> 	* gdbarch.sh (FRAME_RED_ZONE_SIZE): New architecture method.
> 	* gdbarch.h, gdbarch.c: Re-generate.
> 	* infcall.c (call_function_by_hand): Adjust the SP by
> 	frame_red_zone_size before allocating any stack space.
> 	* rs6000-tdep.c (rs6000_gdbarch_init): Set "frame_red_zone_size".
> 	* x86-64-tdep.c (x86_64_frame_align): New function.
> 	(x86_64_init_abi): Set "frame_red_zone_size" and "frame_align".
> 
> 	* x86-64-tdep.c (x86_64_push_arguments): Revert 2003-08-07 change.
> 	Remove code adjusting SP so that it skips over the Red Zone.
> 


Andrew