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

[Mark Kettenis <mark.kettenis@xs4all.nl>]

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