Re: PATCH: Also check for `movl %esp, %ebp' for x32

["H.J. Lu" <hjl.tools@gmail.com>]

On Wed, May 2, 2012 at 3:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, May 2, 2012 at 2:43 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> Date: Sun, 29 Apr 2012 11:37:49 -0700
>>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>>
>>> On Tue, Apr 24, 2012 at 9:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> > On Tue, Apr 17, 2012 at 7:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> >> On Tue, Apr 17, 2012 at 4:43 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> >>>> Date: Tue, 10 Apr 2012 13:29:53 -0700
>>> >>>> From: "H.J. Lu" <hongjiu.lu@intel.com>
>>> >>>>
>>> >>>> Hi,
>>> >>>>
>>> >>>> X32 may use `movl %esp, %ebp' in prologue.  This patch checks it for
>>> >>>> x32.  Tested on Linux/x86-64.  OK for trunk?
>>> >>>
>>> >>> Sorry, but I'm not sure it is a good idea to mix ABIs in the code like
>>> >>> that.  Up until now, I've made a conscious attempt to keep the i386
>>> >>> and amd64 ABIs seperated out as much as possible.  Can you post a
>>> >>> complete diff of the -tdep.c related changes to support x32 in GDB,
>>> >>> such that I can judge where this is heading?
>>> >>
>>> >> Here is the complete x32 GDB patch:
>>> >>
>>> >> http://sourceware.org/ml/gdb-patches/2012-04/msg00476.html
>>> >>
>>> >
>>> > Hi Mark,
>>> >
>>> > Have you looked at my change?
>>> >
>>> > Thanks.
>>>
>>> Ping.
>>
>> Sorry; been travelling too much lately...
>>
>> I did have a look at it, but still have some questions.
>>
>>> Hi,
>>>
>>> X32 may use `movl %esp, %ebp' in prologue.  This patch checks it for
>>> x32.  Tested on Linux/x86-64.  OK for trunk?
>>
>> But the prologues generated by various compilers are expected to be
>> otherwise the same for both the x32 ABI and the normal 64-bit ABI?  I
>> guess x32 has to use "pushq %rbp" as "pushl %ebp" isn't available.
>> And I guess you want to keep the stack 16-byte aligned anyway.  I
>> suppose that "movq %rsp, %rbp" is still ok for x32, but "movl %esp,
>> %ebp" can be encoded in less bytes, so it might be a bit more
>> efficient for x32.
>
> That is correct.

Is my patch OK to install?

>> But what about the stack align code that we check for in
>> amd64_analyze_stack_align()?  Wouldn't that be different for x32 as
>> well?
>
> That is true.  I will submit a separate patch for it.
>

The patch to add x32 support to amd64_analyze_stack_align:

http://sourceware.org/ml/gdb-patches/2012-05/msg00097.html

Thanks.

-- 
H.J.