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

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

[H.J. Lu]

Hi,

X32 may use `movl %esp, %ebp' in prologue.  This patch checks it for
x32.  Tested on Linux/x86-64.  OK for trunk?

Thanks.


H.J.
--
	* amd64-tdep.c (amd64_analyze_prologue): Also check for
	`movl %esp, %ebp' if it is an x32 target.

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index d15acea..e64de21 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1900,10 +1927,14 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
       if (current_pc <= pc + 1)
         return current_pc;
 
-      /* Check for `movq %rsp, %rbp'.  */
+      /* Check for `movq %rsp, %rbp'.  Also check for `movl %esp, %ebp'
+	 if it is an x32 target.  */
       read_memory (pc + 1, buf, 3);
       if (memcmp (buf, mov_rsp_rbp_1, 3) != 0
-	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0)
+	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0
+	  && (gdbarch_ptr_bit (gdbarch) == 64
+	      || (memcmp (buf, mov_rsp_rbp_1 + 1, 2) != 0
+		  && memcmp (buf, mov_rsp_rbp_2 + 1, 2) != 0)))
 	return pc + 1;
 
       /* OK, we actually have a frame.  */

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

[H.J. Lu]

On Tue, Apr 10, 2012 at 01:29:53PM -0700, H.J. Lu wrote:
> Hi,
> 
> X32 may use `movl %esp, %ebp' in prologue.  This patch checks it for
> x32.  Tested on Linux/x86-64.  OK for trunk?
> 
> Thanks.
> 
> 
> H.J.
> --
> 	* amd64-tdep.c (amd64_analyze_prologue): Also check for
> 	`movl %esp, %ebp' if it is an x32 target.
> 
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index d15acea..e64de21 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -1900,10 +1927,14 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
>        if (current_pc <= pc + 1)
>          return current_pc;
>  
> -      /* Check for `movq %rsp, %rbp'.  */
> +      /* Check for `movq %rsp, %rbp'.  Also check for `movl %esp, %ebp'
> +	 if it is an x32 target.  */
>        read_memory (pc + 1, buf, 3);
>        if (memcmp (buf, mov_rsp_rbp_1, 3) != 0
> -	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0)
> +	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0
> +	  && (gdbarch_ptr_bit (gdbarch) == 64
> +	      || (memcmp (buf, mov_rsp_rbp_1 + 1, 2) != 0
> +		  && memcmp (buf, mov_rsp_rbp_2 + 1, 2) != 0)))
>  	return pc + 1;
>  
>        /* OK, we actually have a frame.  */

PING.

H.J.

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

[Yao Qi]

On 04/11/2012 04:29 AM, H.J. Lu wrote:
> -	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0)
> +	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0
> +	  && (gdbarch_ptr_bit (gdbarch) == 64
> +	      || (memcmp (buf, mov_rsp_rbp_1 + 1, 2) != 0
                                                  ^^
> +		  && memcmp (buf, mov_rsp_rbp_2 + 1, 2) != 0)))
                                                     ^^

I don't understand these two constants "2" here.  Does this mean the
encoding of `movl %esp, %ebp' is { 0x48, 0x89 } and { 0x48, 0x8b }?  If
my understand is correct, why don't we define two new array
"movl_esp_ebp_1" and "movl_esp_ebp_2"? which is easier to read/understand.

-- 
Yao (齐尧)

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

[H.J. Lu]

On Tue, Apr 17, 2012 at 3:49 AM, Yao Qi <yao@codesourcery.com> wrote:
> On 04/11/2012 04:29 AM, H.J. Lu wrote:
>> -       && memcmp (buf, mov_rsp_rbp_2, 3) != 0)
>> +       && memcmp (buf, mov_rsp_rbp_2, 3) != 0
>> +       && (gdbarch_ptr_bit (gdbarch) == 64
>> +           || (memcmp (buf, mov_rsp_rbp_1 + 1, 2) != 0
>                                                  ^^
>> +               && memcmp (buf, mov_rsp_rbp_2 + 1, 2) != 0)))
>                                                     ^^
>
> I don't understand these two constants "2" here.  Does this mean the
> encoding of `movl %esp, %ebp' is { 0x48, 0x89 } and { 0x48, 0x8b }?  If

movl %esp, %ebp is 0x89, 0xe5 or 0x8b, 0xec.

> my understand is correct, why don't we define two new array
> "movl_esp_ebp_1" and "movl_esp_ebp_2"? which is easier to read/understand.
>

I don't see it is necessary since the difference of movl and movq is
the 0x48 REX prefix.

-- 
H.J.

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

[Mark Kettenis]

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

> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index d15acea..e64de21 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -1900,10 +1927,14 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
>        if (current_pc <= pc + 1)
>          return current_pc;
>  
> -      /* Check for `movq %rsp, %rbp'.  */
> +      /* Check for `movq %rsp, %rbp'.  Also check for `movl %esp, %ebp'
> +	 if it is an x32 target.  */
>        read_memory (pc + 1, buf, 3);
>        if (memcmp (buf, mov_rsp_rbp_1, 3) != 0
> -	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0)
> +	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0
> +	  && (gdbarch_ptr_bit (gdbarch) == 64
> +	      || (memcmp (buf, mov_rsp_rbp_1 + 1, 2) != 0
> +		  && memcmp (buf, mov_rsp_rbp_2 + 1, 2) != 0)))
>  	return pc + 1;
>  
>        /* OK, we actually have a frame.  */
> 

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

[H.J. Lu]

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

Thanks.

-- 
H.J.

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

[H.J. Lu]

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.

-- 
H.J.

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

[H.J. Lu]

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.

-- 
H.J.

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

[Mark Kettenis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1901 bytes --]

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

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?

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

[H.J. Lu]

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.

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

Thanks.

-- 
H.J.

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

[H.J. Lu]

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.

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

[Mark Kettenis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

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

Sorry, no.  I'm really unhappy with that multi-line if clause.  It
really is hard to parse.  I'm trying to come up with a suggestion to
make this better, but so far haven't succeeded.

Does GCC ever generate the `mov %rsp,%rsp' instruction for x32?

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

[H.J. Lu]

On Thu, May 3, 2012 at 2:50 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> 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?
>
> Sorry, no.  I'm really unhappy with that multi-line if clause.  It
> really is hard to parse.  I'm trying to come up with a suggestion to
> make this better, but so far haven't succeeded.
>
> Does GCC ever generate the `mov %rsp,%rsp' instruction for x32?

Yes.  With -maddress-mode=long, gcc generates "mov %rsp,%rsp".
With -maddress-mode=short, gcc generates "mov %esp,%esp".


-- 
H.J.

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

[Mark Kettenis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3810 bytes --]

> Date: Thu, 3 May 2012 23:50:03 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> 
> > >> 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?
> 
> Sorry, no.  I'm really unhappy with that multi-line if clause.  It
> really is hard to parse.  I'm trying to come up with a suggestion to
> make this better, but so far haven't succeeded.

OK, below is what I'd prefer to check in.  No regressions on
OpenBSD/amd64 (which will only ever support the "real" LP64 ABI).
H.J. can you check that this indeed does the right thing for X32?


2012-05-06  Mark Kettenis  <kettenis@gnu.org>
	    H.J. Lu  <hongjiu.lu@intel.com>

	* amd64-tdep.c (amd64_analyze_prologue): Additionally check for
	`movl %esp, %ebp' for the X32 ABI.

Index: amd64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64-tdep.c,v
retrieving revision 1.102
diff -u -p -r1.102 amd64-tdep.c
--- amd64-tdep.c	27 Apr 2012 20:47:51 -0000	1.102
+++ amd64-tdep.c	6 May 2012 20:28:00 -0000
@@ -1867,8 +1867,14 @@ amd64_analyze_stack_align (CORE_ADDR pc,
       pushq %rbp        0x55
       movq %rsp, %rbp   0x48 0x89 0xe5 (or 0x48 0x8b 0xec)
 
-   Any function that doesn't start with this sequence will be assumed
-   to have no prologue and thus no valid frame pointer in %rbp.  */
+   or (for the X32 ABI):
+
+      pushq %rbp        0x55
+      movl %esp, %ebp   0x89 0xe5 (or 0x8b 0xec)
+
+   Any function that doesn't start with one of these sequences will be
+   assumed to have no prologue and thus no valid frame pointer in
+   %rbp.  */
 
 static CORE_ADDR
 amd64_analyze_prologue (struct gdbarch *gdbarch,
@@ -1879,6 +1885,10 @@ amd64_analyze_prologue (struct gdbarch *
   /* There are two variations of movq %rsp, %rbp.  */
   static const gdb_byte mov_rsp_rbp_1[3] = { 0x48, 0x89, 0xe5 };
   static const gdb_byte mov_rsp_rbp_2[3] = { 0x48, 0x8b, 0xec };
+  /* 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 };
+
   gdb_byte buf[3];
   gdb_byte op;
 
@@ -1900,15 +1910,30 @@ amd64_analyze_prologue (struct gdbarch *
       if (current_pc <= pc + 1)
         return current_pc;
 
-      /* Check for `movq %rsp, %rbp'.  */
       read_memory (pc + 1, buf, 3);
-      if (memcmp (buf, mov_rsp_rbp_1, 3) != 0
-	  && memcmp (buf, mov_rsp_rbp_2, 3) != 0)
-	return pc + 1;
-
-      /* OK, we actually have a frame.  */
-      cache->frameless_p = 0;
-      return pc + 4;
+
+      /* Check for `movq %rsp, %rbp'.  */
+      if (memcmp (buf, mov_rsp_rbp_1, 3) == 0
+	  || memcmp (buf, mov_rsp_rbp_2, 3) == 0)
+	{
+	  /* OK, we actually have a frame.  */
+	  cache->frameless_p = 0;
+	  return pc + 4;
+	}
+
+      /* For X32, also check for `movq %esp, %ebp'.  */
+      if (gdbarch_ptr_bit (gdbarch) == 32)
+	{
+	  if (memcmp (buf, mov_esp_ebp_1, 2) == 0
+	      || memcmp (buf, mov_esp_ebp_2, 2) == 0)
+	    {
+	      /* OK, we actually have a frame.  */
+	      cache->frameless_p = 0;
+	      return pc + 3;
+	    }
+	}
+
+      return pc + 1;
     }
 
   return pc;

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

[H.J. Lu]

On Sun, May 6, 2012 at 1:31 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Thu, 3 May 2012 23:50:03 +0200 (CEST)
>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>
>> > >> 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?
>>
>> Sorry, no.  I'm really unhappy with that multi-line if clause.  It
>> really is hard to parse.  I'm trying to come up with a suggestion to
>> make this better, but so far haven't succeeded.
>
> OK, below is what I'd prefer to check in.  No regressions on
> OpenBSD/amd64 (which will only ever support the "real" LP64 ABI).
> H.J. can you check that this indeed does the right thing for X32?
>
>
> 2012-05-06  Mark Kettenis  <kettenis@gnu.org>
>            H.J. Lu  <hongjiu.lu@intel.com>
>
>        * amd64-tdep.c (amd64_analyze_prologue): Additionally check for
>        `movl %esp, %ebp' for the X32 ABI.
>

It works fine for x32.

Thanks.

-- 
H.J.