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

Mark Kettenis
2012-05-06T20:31:29Z

[-- 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;