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