Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Skip the "red zone" on AMD64
@ 2003-08-07 11:05 Michal Ludvig
  2003-08-07 11:18 ` Andreas Schwab
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Ludvig @ 2003-08-07 11:05 UTC (permalink / raw)
  To: GDB Patches

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

Hi all,
this simple patch skips the "red zone" before pushing anything to the 
stack when calling functions from the GDB prompt. If the red zone isn't 
skipped than local variables, etc. could be overwritten by called 
function parameters or return address.

This is pretty obvious patch that doesn't harm anything, but anyway - OK 
to apply?

2003-08-07  Michal Ludvig  <mludvig@suse.cz>
	* x86-64-tdep.c (x86_64_push_arguments): Skip the red zone.

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz

[-- Attachment #2: redzone-1.diff --]
[-- Type: text/plain, Size: 839 bytes --]

Index: x86-64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/x86-64-tdep.c,v
retrieving revision 1.79.2.5
diff -u -p -r1.79.2.5 x86-64-tdep.c
--- x86-64-tdep.c	7 Aug 2003 08:09:22 -0000	1.79.2.5
+++ x86-64-tdep.c	7 Aug 2003 10:56:29 -0000
@@ -624,6 +624,14 @@ x86_64_push_arguments (struct regcache *
   int stack_values_count = 0;
   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];

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Skip the "red zone" on AMD64
  2003-08-07 11:05 [RFA] Skip the "red zone" on AMD64 Michal Ludvig
@ 2003-08-07 11:18 ` Andreas Schwab
  2003-08-07 11:27   ` Michal Ludvig
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2003-08-07 11:18 UTC (permalink / raw)
  To: Michal Ludvig; +Cc: GDB Patches

Michal Ludvig <mludvig@suse.cz> writes:

|> 2003-08-07  Michal Ludvig  <mludvig@suse.cz>
|> 	* x86-64-tdep.c (x86_64_push_arguments): Skip the red zone.

Please add an empty line after the heading.

Otherwise OK.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Skip the "red zone" on AMD64
  2003-08-07 11:18 ` Andreas Schwab
@ 2003-08-07 11:27   ` Michal Ludvig
  2003-08-07 14:10     ` Andrew Cagney
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Ludvig @ 2003-08-07 11:27 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: GDB Patches

Andreas Schwab told me that:
> Michal Ludvig <mludvig@suse.cz> writes:
> 
> |> 2003-08-07  Michal Ludvig  <mludvig@suse.cz>
> |> 	* x86-64-tdep.c (x86_64_push_arguments): Skip the red zone.
> 
> Please add an empty line after the heading.

Done.

> Otherwise OK.

Thanks, committed to 6.0 and head.

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Skip the "red zone" on AMD64
  2003-08-07 11:27   ` Michal Ludvig
@ 2003-08-07 14:10     ` Andrew Cagney
  2003-08-07 14:40       ` Michal Ludvig
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cagney @ 2003-08-07 14:10 UTC (permalink / raw)
  To: Michal Ludvig; +Cc: Andreas Schwab, GDB Patches

 > This is pretty obvious patch that doesn't harm anything, but anyway - 
OK to apply?

Hmm,

> Andreas Schwab told me that:
> Michal Ludvig <mludvig@suse.cz> writes:
> 
> |> 2003-08-07  Michal Ludvig  <mludvig@suse.cz>
> |>     * x86-64-tdep.c (x86_64_push_arguments): Skip the red zone.
> 
> Please add an empty line after the heading.
> 
> Done.
> 
> Otherwise OK.
> 
> Thanks, committed to 6.0 and head.

The PowerOpen (a.k.a. AIX) and (as I only just discovered) PPC64 ABIs 
make use of the stack beyond the top-of-stack (SP) address.  A very long 
standing bug is that GDB needs to skip that area before allocating stack 
space.

This architecture's `red zone' sounds like the same [mis-]feature.  If 
it is then the posted change won't fix the problem :-(  The return 
structure, already allocated on the stack, will smash that stack area.

I think this code:

   /* Ensure that the initial SP is correctly aligned.  */
   {
     CORE_ADDR old_sp = read_sp ();
     if (gdbarch_frame_align_p (current_gdbarch))
       {
         /* NOTE: cagney/2002-09-18:

            On a RISC architecture, a void parameterless generic dummy
            frame (i.e., no parameters, no result) typically does not
            need to push anything the stack and hence can leave SP and
            FP.  Similarly, a frameless (possibly leaf) function does
            not push anything on the stack and, hence, that too can
            leave FP and SP unchanged.  As a consequence, a sequence of
            void parameterless generic dummy frame calls to frameless
            functions will create a sequence of effectively identical
            frames (SP, FP and TOS and PC the same).  This, not
            suprisingly, results in what appears to be a stack in an
            infinite loop --- when GDB tries to find a generic dummy
            frame on the internal dummy frame stack, it will always
            find the first one.

            To avoid this problem, the code below always grows the
            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))
               /* Stack grows down.  */
               sp = gdbarch_frame_align (current_gdbarch, old_sp - 1);
             else
               /* Stack grows up.  */
               sp = gdbarch_frame_align (current_gdbarch, old_sp + 1);
           }
         gdb_assert ((INNER_THAN (1, 2) && sp <= old_sp)
                     || (INNER_THAN (2, 1) && sp >= old_sp));
       }

so that it uses a new architecture method (size_of_frame_red_zone? 
default 1, assert > 0, doco) to pad out the stack before using it.

If this is the case, can you please revert the patch and post something 
more along the above lines.  Is there code in the testsuite that 
fails/passes with the fix?

Andrew

PS: In case you're wondering.  The above code was only added a few 
months ago.  Prior to that there wasn't anywhere to put the fix.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Skip the "red zone" on AMD64
  2003-08-07 14:10     ` Andrew Cagney
@ 2003-08-07 14:40       ` Michal Ludvig
  2003-08-07 15:04         ` Andrew Cagney
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Ludvig @ 2003-08-07 14:40 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Andreas Schwab, GDB Patches

Andrew Cagney told me that:

> The PowerOpen (a.k.a. AIX) and (as I only just discovered) PPC64 ABIs 
> make use of the stack beyond the top-of-stack (SP) address.  A very long 
> standing bug is that GDB needs to skip that area before allocating stack 
> space.
> 
> This architecture's `red zone' sounds like the same [mis-]feature.  If 
> it is then the posted change won't fix the problem :-(  The return 
> structure, already allocated on the stack, will smash that stack area.

I think it's something different - while on AIX or PPC64 the called 
function may clobber the stack above it's frame's top-of-stack address, 
on AMD64 it's the calling function that could have used the space below 
bottom-of-stack.

Functions on AMD64 sometimes don't allocate space for their local 
variables (i.e. don't decrement %rsp before pushing something below it), 
which is often the case for leaf functions that don't call anything else.

Imagine a simple leaf function:

int add (int a, int b)
{
	return a + b;
}

If depending on the red zone, it gets compiled to:

add:
         movl    %edi, -4(%rsp)
         movl    %esi, -8(%rsp)
         movl    -8(%rsp), %eax
         addl    -4(%rsp), %eax
         ret

But if the use of the red zone is prohibited (gcc's flag -mno-red-zone), 
it must allocate the space for storing temporary local variables:

add:
         subq    $8, %rsp	# Allocate
         movl    %edi, 4(%rsp)
         movl    %esi, (%rsp)
         movl    (%rsp), %eax
         addl    4(%rsp), %eax
         addq    $8, %rsp	# Deallocate
         ret

The problem is that in this case GDB couldn't know how much space below 
%rsp was used by the function. ABI says it can be up to 128 Bytes of 
"unregistered" space, that is not delimited by %rsp.

So this red zone is not used by the called function but could have been 
used by the calling (or "interrupted" ?) function.

Or is it the same case that AIX does?

> If this is the case, can you please revert the patch and post something 
> more along the above lines.  Is there code in the testsuite that 
> fails/passes with the fix?

It fixed about 10 failures and didn't break anything.

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Skip the "red zone" on AMD64
  2003-08-07 14:40       ` Michal Ludvig
@ 2003-08-07 15:04         ` Andrew Cagney
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Cagney @ 2003-08-07 15:04 UTC (permalink / raw)
  To: Michal Ludvig; +Cc: Andreas Schwab, GDB Patches

> I think it's something different - while on AIX or PPC64 the called function may clobber the stack above it's frame's top-of-stack address, on AMD64 it's the calling function that could have used the space below bottom-of-stack.

The SVR4 PPC32 ABI never goes beyond its end of stack, the PowerOpen 
(definitly) and PPC64 ABI (I'm told) do.  Hence, they have exactly the 
same problem as ...

> Functions on AMD64 sometimes don't allocate space for their local variables (i.e. don't decrement %rsp before pushing something below it), which is often the case for leaf functions that don't call anything else.

and the patch won't fix it :-(

Andrew



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2003-08-07 15:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-08-07 11:05 [RFA] Skip the "red zone" on AMD64 Michal Ludvig
2003-08-07 11:18 ` Andreas Schwab
2003-08-07 11:27   ` Michal Ludvig
2003-08-07 14:10     ` Andrew Cagney
2003-08-07 14:40       ` Michal Ludvig
2003-08-07 15:04         ` Andrew Cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox