Re: [PATCH] AMD64, Prologue: Recognize stack decrementation as prologue operation.

[Bernhard Heckel <bernhard dot heckel at intel dot com>]

On 01/12/2016 16:31, Luis Machado wrote:
> On 12/01/2016 08:16 AM, Bernhard Heckel wrote:
> > Some compiler decrement stack pointer within the prologue
> > sequence in order to reserve memory for local variables.
> > Recognize this subtraction to stop at the very end of the
> > prologue.
>
> I suppose this was exercised with GCC as well via the testsuite?
Yes
GCC,ICC and Clang 6.0 (llvm 3.5)

No regression with GCC nor with ICC.

But, there is a major issue when running with Clang.
Clang associate this "subtraction instruction" with the line after the 
prologue sequence.
This causes regressions on Mac.

I attached disassembly of Clang and GCC for the same program. ICC 
behaves like GCC.
I was trying to file a ticket for Clang, but I don't have access to 
bugzilla. Auto-registration
is not available and manual account registration is still ongoing.

> > 2016-10-20  Bernhard Heckel  <bernhard.heckel@intel.com>
> >
> > gdb/Changelog:
> >     amd64-tdep.c (amd64_analyze_prologue): Recognize stack 
> > decrementation
> >     as prologue operation.
>
> gdb/ChangeLog above the date line, adjust date and add "*" before the 
> filename.
>
> > ---
> >  gdb/amd64-tdep.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> > index a3a1fde..795d78e 100644
> > --- a/gdb/amd64-tdep.c
> > +++ b/gdb/amd64-tdep.c
> > @@ -2283,6 +2283,12 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
> >    /* Ditto for movl %esp, %ebp.  */
> >    static const gdb_byte mov_esp_ebp_1[2] = { 0x89, 0xe5 };
> >    static const gdb_byte mov_esp_ebp_2[2] = { 0x8b, 0xec };
> > +  /* Ditto for subtraction on the stack pointer.  */
> > +  static const gdb_byte sub_rsp_imm8[3] = { 0x48, 0x83, 0xec };
> > +  static const gdb_byte sub_rsp_imm32[3] = { 0x48, 0x81, 0xec };
> > +  /* Ditto for subtraction on the stack pointer.  */
> > +  static const gdb_byte sub_esp_imm8[2] = { 0x83, 0xec };
> > +  static const gdb_byte sub_esp_imm32[2] = { 0x81, 0xec };
>
> Should we add a comment making it explicit which instruction patterns 
> we're looking at matching here?
You mean, adding it to the function description. There we have 
description for push and mov instruction.

> I looked up sub esp imm32, for example, and i got no meaningful hits 
> other than some nasm posix entry.
>
> >    gdb_byte buf[3];
> >    gdb_byte op;
> > @@ -2316,6 +2322,18 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
> >      {
> >        /* OK, we actually have a frame.  */
> >        cache->frameless_p = 0;
> > +
> > +      /* Some compiler do subtraction on the stack pointer
> > +         to reserve memory for local variables.
> > +         Two common variants exist to do so.  */
>
> What compiler exactly? Would be nice to know, otherwise this is a bit 
> vague.
Actually, GCC, ICC and Clang are using this approach.

> The comment seems to imply a specific compiler does this, or did you 
> mean "some compilers"?
>
> > +      read_code (pc + 4, buf, 3);
> > +      if (memcmp (buf, sub_rsp_imm8, 3) == 0)
> > +        /* Operand is 1 byte.  */
> > +        return pc + 8;
> > +      else if (memcmp (buf, sub_rsp_imm32, 3) == 0)
> > +        /* Operand is 4 bytes.  */
> > +        return pc + 11;
> > +
> >        return pc + 4;
> >      }
> >
> > @@ -2327,6 +2345,18 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
> >          {
> >            /* OK, we actually have a frame.  */
> >            cache->frameless_p = 0;
> > +
> > +          /* Some compiler do subtraction on the stack pointer
> > +         to reserve memory for local variables.
> > +         Two common variants exist to do so.  */
> > +          read_code (pc + 3, buf, 2);
> > +          if (memcmp (buf, sub_esp_imm8, 2) == 0)
> > +        /* Operand is 1 byte.  */
> > +        return pc + 6;
> > +          else if (memcmp (buf, sub_esp_imm32, 2) == 0)
> > +        /* Operand is 4 bytes.  */
> > +        return pc + 9;
> > +
> >            return pc + 3;
> >          }
> >      }
>
> Otherwise LGTM.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Attachment: disassembly_clang.txt
Description: Text document

Attachment: disassembly_gcc.txt
Description: Text document

Attachment: test.c
Description: Text document