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

[Luis Machado <lgustavo@codesourcery.com>]

On 12/02/2016 02:40 AM, Bernhard Heckel wrote:
> 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.
>

To add it to these sub_[esp|rsp|_imm* bits, if meaningful. I don't know 
if these are documented/used somewhere else in gdb. Just a suggestion 
that could improve visual identification of such instructions when going 
through the prologue in disassembly view.

>>
>> 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.
>

I guess you'd want "some compilers" then.