PATCH: Update x86 stack align analyzer

PATCH: Update x86 stack align analyzer

[H.J. Lu]

Gcc 4.4 revision 138335:

http://gcc.gnu.org/ml/gcc-cvs/2008-07/msg01048.html

introduced new ways to align stack.  This patch teaches gdb how to
recognize the new stack prologue. OK for trunk?

Please CC me since I am not on the gdb-patches mailing list.

Thanks.


H.J.
----
2008-07-30  Xuepeng Guo  <xuepeng.guo@intel.com>
	    H.J. Lu  <hongjiu.lu@intel.com>

	* amd64-tdep.c (amd64_frame_cache): Add saved_sp_reg.
	(amd64_init_frame_cache): Initialize saved_sp_reg.
	(amd64_analyze_stack_align): New.
	(amd64_analyze_prologue): Call it.
	(amd64_frame_cache): Try to use saved_sp_reg if frame is invalid.

	* amd64-tdep.h (amd64_regnum): Add AMD64_R9_REGNUM to
	AMD64_R14_REGNUM.

	* i386-tdep.c (i386_frame_cache): Remove stack_align.  Add
	saved_sp_reg.
	(i386_alloc_frame_cache): Remove stack_align.  Initialize
	saved_sp_reg to I386_ESP_REGNUM.
	(i386_analyze_stack_align): Rewrite.
	(i386_frame_cache): Use saved_sp_reg only if frame is invalid.

--- ../gdb/src/gdb/amd64-tdep.c	2008-07-16 22:30:04.000000000 -0700
+++ gdb/gdb/amd64-tdep.c	2008-07-25 11:57:37.000000000 -0700
@@ -680,6 +680,7 @@ struct amd64_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[AMD64_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
+  enum i386_regnum saved_sp_reg;
 
   /* Do we have a frame?  */
   int frameless_p;
@@ -702,6 +703,7 @@ amd64_init_frame_cache (struct amd64_fra
   for (i = 0; i < AMD64_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
+  cache->saved_sp_reg = AMD64_RSP_REGNUM;
 
   /* Frameless until proven otherwise.  */
   cache->frameless_p = 1;
@@ -719,6 +721,204 @@ amd64_alloc_frame_cache (void)
   return cache;
 }
 
+/* GCC 4.4 and later, can put code in the prologue to realign the
+   stack pointer.  Check whether PC points to such code, and update
+   CACHE accordingly.  Return the first instruction after the code
+   sequence or CURRENT_PC, whichever is smaller.  If we don't
+   recognize the code, return PC.  */
+
+static CORE_ADDR
+amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
+			   struct amd64_frame_cache *cache)
+{
+  /* There are 3 code sequences to re-align stack:
+
+     	1. Use %ebp:
+
+		pushq %rbp
+		movq  %rsp, %rbp
+		andq  $-XXX, %rsp
+
+	2. Use a caller-saved saved register:
+
+		leaq  8(%rsp), %reg
+		andq  $-XXX, %rsp
+		pushq -8(%reg)
+
+	3. Use a callee-saved saved register:
+
+		pushq %reg
+		leaq  16(%rsp), %reg
+		andq  $-XXX, %rsp
+		pushq -8(%reg)
+
+     "andq $-XXX, %rsp" can be either 4 bytes or 7 bytes:
+     
+     	0x48 0x83 0xe4 0xf0			andq $-16, %rsp
+     	0x48 0x81 0xe4 0x00 0xff 0xff 0xff	andq $-256, %rsp
+   */
+
+  gdb_byte buf[18];
+  int reg, r;
+  int offset, offset_and;
+  static enum amd64_regnum regnums[16] = {
+    AMD64_RAX_REGNUM,		/* %rax */
+    AMD64_RCX_REGNUM,		/* %rcx */
+    AMD64_RDX_REGNUM,		/* %rdx */
+    AMD64_RBX_REGNUM,		/* %rbx */
+    AMD64_RSP_REGNUM,		/* %rsp */
+    AMD64_RBP_REGNUM,		/* %rbp */
+    AMD64_RSI_REGNUM,		/* %rsi */
+    AMD64_RDI_REGNUM,		/* %rdi */
+    AMD64_R8_REGNUM,		/* %r8 */
+    AMD64_R9_REGNUM,		/* %r9 */
+    AMD64_R10_REGNUM,		/* %r10 */
+    AMD64_R11_REGNUM,		/* %r11 */
+    AMD64_R12_REGNUM,		/* %r12 */
+    AMD64_R13_REGNUM,		/* %r13 */
+    AMD64_R14_REGNUM,		/* %r14 */
+    AMD64_R15_REGNUM,		/* %r15 */
+  };
+
+  if (target_read_memory (pc, buf, sizeof buf))
+    return pc;
+
+  /* First check "pushq %rbp".  */
+  if (buf[0] == 0x55)
+    {
+      /* The next instruction has to be "movq %rsp, %rbp".  */
+      if (buf[1] != 0x48 || buf[2] != 0x89 || buf[3] != 0xe5)
+	return pc;
+
+      /* The next instruction has to be "andq $-XXX, %rsp".  */
+      if (buf[4] != 0x48
+	  || buf[6] != 0xe4
+	  || (buf[5] != 0x81 && buf[5] != 0x83))
+	return pc;
+
+      if (current_pc > pc + 4)
+	cache->saved_sp_reg = AMD64_RBP_REGNUM;
+
+      /* Keep the prologue for amd64_analyze_prologue.  */
+      return pc;
+    }
+
+  /* Check caller-saved saved register.  The first instruction has
+     to be "leaq 8(%rsp), %reg".  */
+  if ((buf[0] & 0xfb) == 0x48
+      && buf[1] == 0x8d
+      && buf[3] == 0x24
+      && buf[4] == 0x8)
+    {
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[2] & 0xc7) != 0x44)
+	return pc;
+
+      /* REG has register number.  */
+      reg = (buf[2] >> 3) & 7;
+
+      /* Check the REX.R bit.  */
+      if (buf[0] == 0x4c)
+	reg += 8;
+
+      offset = 5;
+    }
+  else
+    {
+      /* Check callee-saved saved register.  The first instruction
+	 has to be "pushq %reg".  */
+      reg = 0;
+      if ((buf[0] & 0xf8) == 0x50)
+	offset = 0;
+      else if ((buf[0] & 0xf6) == 0x40
+	       && (buf[1] & 0xf8) == 0x50)
+	{
+	  /* Check the REX.B bit.  */
+	  if ((buf[0] & 1) != 0)
+	    reg = 8;
+
+	  offset = 1;
+	}
+      else
+	return pc;
+
+      /* Get register.  */
+      reg += buf[offset] & 0x7;
+
+      offset++;
+
+      /* The next instruction has to be "leaq 16(%rsp), %reg".  */
+      if ((buf[offset] & 0xfb) != 0x48
+	  || buf[offset + 1] != 0x8d
+	  || buf[offset + 3] != 0x24
+	  || buf[offset + 4] != 0x10)
+	return pc;
+
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[offset + 2] & 0xc7) != 0x44)
+	return pc;
+      
+      /* REG has register number.  */
+      r = (buf[offset + 2] >> 3) & 7;
+
+      /* Check the REX.R bit.  */
+      if (buf[offset] == 0x4c)
+	r += 8;
+
+      /* Registers in pushq and leaq have to be the same.  */
+      if (reg != r)
+	return pc;
+
+      offset += 5;
+    }
+
+  /* Rigister can't be %rsp nor %rbp.  */
+  if (reg == 4 || reg == 5)
+    return pc;
+
+  /* The next instruction has to be "andq $-XXX, %rsp".  */
+  if (buf[offset] != 0x48
+      || buf[offset + 2] != 0xe4
+      || (buf[offset + 1] != 0x81 && buf[offset + 1] != 0x83))
+    return pc;
+
+  offset_and = offset;
+  offset += buf[offset + 1] == 0x81 ? 7 : 4;
+
+  /* The next instruction has to be "pushq -8(%reg)".  */
+  r = 0;
+  if (buf[offset] == 0xff)
+    offset++;
+  else if ((buf[offset] & 0xf6) == 0x40
+	   && buf[offset + 1] == 0xff)
+    {
+      /* Check the REX.B bit.  */
+      if ((buf[offset] & 0x1) != 0)
+	r = 8;
+      offset += 2;
+    }
+  else
+    return pc;
+
+  /* 8bit -8 is 0xf8.  REG must be binary 110 and MOD must be binary
+     01.  */
+  if (buf[offset + 1] != 0xf8
+      || (buf[offset] & 0xf8) != 0x70)
+    return pc;
+
+  /* R/M has register.  */
+  r += buf[offset] & 7;
+
+  /* Registers in leaq and pushq have to be the same.  */
+  if (reg != r)
+    return pc;
+
+  if (current_pc > pc + offset_and)
+    cache->saved_sp_reg = regnums[reg];
+
+  return min (pc + offset + 2, current_pc);
+}
+
 /* Do a limited analysis of the prologue at PC and update CACHE
    accordingly.  Bail out early if CURRENT_PC is reached.  Return the
    address where the analysis stopped.
@@ -742,6 +942,8 @@ amd64_analyze_prologue (CORE_ADDR pc, CO
   if (current_pc <= pc)
     return current_pc;
 
+  pc = amd64_analyze_stack_align (pc, current_pc, cache);
+
   op = read_memory_unsigned_integer (pc, 1);
 
   if (op == 0x55)		/* pushq %rbp */
@@ -813,8 +1015,23 @@ amd64_frame_cache (struct frame_info *th
 	 at the stack pointer.  For truly "frameless" functions this
 	 might work too.  */
 
-      get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
-      cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
+      if (cache->saved_sp_reg != AMD64_RSP_REGNUM)
+	{
+	  /* We're halfway aligning the stack.  Saved stack pointer
+	     has been saved in saved_sp_reg.  */
+	  get_frame_register (this_frame, cache->saved_sp_reg, buf);
+	  cache->saved_sp = extract_unsigned_integer(buf, 8);
+	  cache->base = ((cache->saved_sp - 8) & 0xfffffffffffffff0LL) - 8;
+	  cache->saved_regs[AMD64_RIP_REGNUM] = cache->saved_sp - 8;
+
+	  /* This will be added back below.  */
+	  cache->saved_regs[AMD64_RIP_REGNUM] -= cache->base;
+	}
+      else
+	{
+	  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+	  cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
+	}
     }
   else
     {
--- ../gdb/src/gdb/amd64-tdep.h	2008-01-17 09:38:41.000000000 -0800
+++ gdb/gdb/amd64-tdep.h	2008-07-18 15:41:32.000000000 -0700
@@ -39,8 +39,14 @@ enum amd64_regnum
   AMD64_RDI_REGNUM,		/* %rdi */
   AMD64_RBP_REGNUM,		/* %rbp */
   AMD64_RSP_REGNUM,		/* %rsp */
-  AMD64_R8_REGNUM = 8,		/* %r8 */
-  AMD64_R15_REGNUM = 15,	/* %r15 */
+  AMD64_R8_REGNUM,		/* %r8 */
+  AMD64_R9_REGNUM,		/* %r9 */
+  AMD64_R10_REGNUM,		/* %r10 */
+  AMD64_R11_REGNUM,		/* %r11 */
+  AMD64_R12_REGNUM,		/* %r12 */
+  AMD64_R13_REGNUM,		/* %r13 */
+  AMD64_R14_REGNUM,		/* %r14 */
+  AMD64_R15_REGNUM,		/* %r15 */
   AMD64_RIP_REGNUM,		/* %rip */
   AMD64_EFLAGS_REGNUM,		/* %eflags */
   AMD64_CS_REGNUM,		/* %cs */
Only in gdb/gdb: ChangeLog.stack
diff -x .svn -upr ../gdb/src/gdb/i386-tdep.c gdb/gdb/i386-tdep.c
--- ../gdb/src/gdb/i386-tdep.c	2008-07-16 22:30:17.000000000 -0700
+++ gdb/gdb/i386-tdep.c	2008-07-18 17:42:03.000000000 -0700
@@ -518,7 +518,7 @@ struct i386_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[I386_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
-  int stack_align;
+  enum i386_regnum saved_sp_reg;
   int pc_in_eax;
 
   /* Stack space reserved for local variables.  */
@@ -545,7 +545,7 @@ i386_alloc_frame_cache (void)
   for (i = 0; i < I386_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
-  cache->stack_align = 0;
+  cache->saved_sp_reg = I386_ESP_REGNUM;
   cache->pc_in_eax = 0;
 
   /* Frameless until proven otherwise.  */
@@ -707,37 +707,134 @@ static CORE_ADDR
 i386_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
 			  struct i386_frame_cache *cache)
 {
-  /* The register used by the compiler to perform the stack re-alignment 
-     is, in order of preference, either %ecx, %edx, or %eax.  GCC should
-     never use %ebx as it always treats it as callee-saved, whereas
-     the compiler can only use caller-saved registers.  */
-  static const gdb_byte insns_ecx[10] = { 
-    0x8d, 0x4c, 0x24, 0x04,	/* leal  4(%esp), %ecx */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x71, 0xfc		/* pushl -4(%ecx) */
-  };
-  static const gdb_byte insns_edx[10] = { 
-    0x8d, 0x54, 0x24, 0x04,	/* leal  4(%esp), %edx */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x72, 0xfc		/* pushl -4(%edx) */
-  };
-  static const gdb_byte insns_eax[10] = { 
-    0x8d, 0x44, 0x24, 0x04,	/* leal  4(%esp), %eax */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x70, 0xfc		/* pushl -4(%eax) */
+  /* There are 3 code sequences to re-align stack:
+
+     	1. Use %ebp:
+
+		pushl %ebp
+		movl  %esp, %ebp
+		andl  $-XXX, %esp
+
+	2. Use a caller-saved saved register:
+
+		leal  4(%esp), %reg
+		andl  $-XXX, %esp
+		pushl -4(%reg)
+
+	3. Use a callee-saved saved register:
+
+		pushl %reg
+		leal  8(%esp), %reg
+		andl  $-XXX, %esp
+		pushl -4(%reg)
+
+     "andl $-XXX, %esp" can be either 3 bytes or 6 bytes:
+     
+     	0x83 0xe4 0xf0			andl $-16, %esp
+     	0x81 0xe4 0x00 0xff 0xff 0xff	andl $-256, %esp
+   */
+
+  gdb_byte buf[14];
+  int reg;
+  int offset, offset_and;
+  static enum i386_regnum regnums[8] = {
+    I386_EAX_REGNUM,		/* %eax */
+    I386_ECX_REGNUM,		/* %ecx */
+    I386_EDX_REGNUM,		/* %edx */
+    I386_EBX_REGNUM,		/* %ebx */
+    I386_ESP_REGNUM,		/* %esp */
+    I386_EBP_REGNUM,		/* %ebp */
+    I386_ESI_REGNUM,		/* %esi */
+    I386_EDI_REGNUM		/* %edi */
   };
-  gdb_byte buf[10];
 
-  if (target_read_memory (pc, buf, sizeof buf)
-      || (memcmp (buf, insns_ecx, sizeof buf) != 0
-          && memcmp (buf, insns_edx, sizeof buf) != 0
-          && memcmp (buf, insns_eax, sizeof buf) != 0))
+  if (target_read_memory (pc, buf, sizeof buf))
+    return pc;
+
+  /* First check "pushl %ebp".  */
+  if (buf[0] == 0x55)
+    {
+      /* The next instruction has to be "movl %esp, %ebp".  */
+      if (buf[1] != 0x89 || buf[2] != 0xe5)
+	return pc;
+
+      /* The next instruction has to be "andl $-XXX, %esp".  */
+      if (buf[4] != 0xe4 || (buf[3] != 0x81 && buf[3] != 0x83))
+	return pc;
+
+      if (current_pc > pc + 3)
+	cache->saved_sp_reg = I386_EBP_REGNUM;
+
+      /* Keep the prologue for i386_analyze_frame_setup.  */
+      return pc;
+    }
+
+  /* Check caller-saved saved register.  The first instruction has
+     to be "leal 4(%esp), %reg".  */
+  if (buf[0] == 0x8d && buf[2] == 0x24 && buf[3] == 0x4)
+    {
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[1] & 0xc7) != 0x44)
+	return pc;
+
+      /* REG has register number.  */
+      reg = (buf[1] >> 3) & 7;
+      offset = 4;
+    }
+  else
+    {
+      /* Check callee-saved saved register.  The first instruction
+	 has to be "pushl %reg".  */
+      if ((buf[0] & 0xf8) != 0x50)
+	return pc;
+
+      /* Get register.  */
+      reg = buf[0] & 0x7;
+
+      /* The next instruction has to be "leal 8(%esp), %reg".  */
+      if (buf[1] != 0x8d || buf[3] != 0x24 || buf[4] != 0x8)
+	return pc;
+
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[2] & 0xc7) != 0x44)
+	return pc;
+      
+      /* REG has register number.  Registers in pushl and leal have to
+	 be the same.  */
+      if (reg != ((buf[2] >> 3) & 7))
+	return pc;
+
+      offset = 5;
+    }
+
+  /* Rigister can't be %esp nor %ebp.  */
+  if (reg == 4 || reg == 5)
+    return pc;
+
+  /* The next instruction has to be "andl $-XXX, %esp".  */
+  if (buf[offset + 1] != 0xe4
+      || (buf[offset] != 0x81 && buf[offset] != 0x83))
     return pc;
 
-  if (current_pc > pc + 4)
-    cache->stack_align = 1;
+  offset_and = offset;
+  offset += buf[offset] == 0x81 ? 6 : 3;
 
-  return min (pc + 10, current_pc);
+  /* The next instruction has to be "pushl -4(%reg)".  8bit -4 is
+     0xfc.  REG must be binary 110 and MOD must be binary 01.  */
+  if (buf[offset] != 0xff
+      || buf[offset + 2] != 0xfc
+      || (buf[offset + 1] & 0xf8) != 0x70)
+    return pc;
+
+  /* R/M has register.  Registers in leal and pushl have to be the
+     same.  */
+  if (reg != (buf[offset + 1] & 7))
+    return pc;
+
+  if (current_pc > pc + offset_and)
+    cache->saved_sp_reg = regnums[reg];
+
+  return min (pc + offset + 3, current_pc);
 }
 
 /* Maximum instruction length we need to handle.  */
@@ -1241,13 +1338,6 @@ i386_frame_cache (struct frame_info *thi
   if (cache->pc != 0)
     i386_analyze_prologue (cache->pc, get_frame_pc (this_frame), cache);
 
-  if (cache->stack_align)
-    {
-      /* Saved stack pointer has been saved in %ecx.  */
-      get_frame_register (this_frame, I386_ECX_REGNUM, buf);
-      cache->saved_sp = extract_unsigned_integer(buf, 4);
-    }
-
   if (cache->locals < 0)
     {
       /* We didn't find a valid frame, which means that CACHE->base
@@ -1258,9 +1348,12 @@ i386_frame_cache (struct frame_info *thi
 	 frame by looking at the stack pointer.  For truly "frameless"
 	 functions this might work too.  */
 
-      if (cache->stack_align)
+      if (cache->saved_sp_reg != I386_ESP_REGNUM)
 	{
-	  /* We're halfway aligning the stack.  */
+	  /* We're halfway aligning the stack.  Saved stack pointer
+	     has been saved in saved_sp_reg.  */
+	  get_frame_register (this_frame, cache->saved_sp_reg, buf);
+	  cache->saved_sp = extract_unsigned_integer(buf, 4);
 	  cache->base = ((cache->saved_sp - 4) & 0xfffffff0) - 4;
 	  cache->saved_regs[I386_EIP_REGNUM] = cache->saved_sp - 4;
 

Re: PATCH: Update x86 stack align analyzer

[Mark Kettenis]

> Date: Wed, 30 Jul 2008 14:11:52 -0700
> From: "H.J. Lu" <hongjiu.lu@intel.com>
> 
> Gcc 4.4 revision 138335:
> 
> http://gcc.gnu.org/ml/gcc-cvs/2008-07/msg01048.html
> 
> introduced new ways to align stack.  This patch teaches gdb how to
> recognize the new stack prologue. OK for trunk?

A few comments inline below.

> --- ../gdb/src/gdb/amd64-tdep.c	2008-07-16 22:30:04.000000000 -0700
> +++ gdb/gdb/amd64-tdep.c	2008-07-25 11:57:37.000000000 -0700
> @@ -680,6 +680,7 @@ struct amd64_frame_cache
>    /* Saved registers.  */
>    CORE_ADDR saved_regs[AMD64_NUM_SAVED_REGS];
>    CORE_ADDR saved_sp;
> +  enum i386_regnum saved_sp_reg;

I suppose you meant 'enum amd64_regnum' here.  I'd prefer if you'd
simply use 'int' as the type for registers instead of the enum.
That's what all the other code in GDB does.

> +/* GCC 4.4 and later, can put code in the prologue to realign the
> +   stack pointer.  Check whether PC points to such code, and update
> +   CACHE accordingly.  Return the first instruction after the code
> +   sequence or CURRENT_PC, whichever is smaller.  If we don't
> +   recognize the code, return PC.  */

There are older versions of GCC that use at least method #2, so this
comment is a bit misleading.

> +static CORE_ADDR
> +amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
> +			   struct amd64_frame_cache *cache)
> +{
> +  /* There are 3 code sequences to re-align stack:
> +
> +     	1. Use %ebp:
> +
> +		pushq %rbp
> +		movq  %rsp, %rbp
> +		andq  $-XXX, %rsp

But this is basically the standard prologue sequence.  I don't see why
you need to handle this sequence here, since it is already handled in
amd64_analyze_prologue().

> +	2. Use a caller-saved saved register:
> +
> +		leaq  8(%rsp), %reg
> +		andq  $-XXX, %rsp
> +		pushq -8(%reg)
> +
> +	3. Use a callee-saved saved register:
> +
> +		pushq %reg
> +		leaq  16(%rsp), %reg
> +		andq  $-XXX, %rsp
> +		pushq -8(%reg)
> +
> +     "andq $-XXX, %rsp" can be either 4 bytes or 7 bytes:
> +     
> +     	0x48 0x83 0xe4 0xf0			andq $-16, %rsp
> +     	0x48 0x81 0xe4 0x00 0xff 0xff 0xff	andq $-256, %rsp
> +   */
> +
> +  gdb_byte buf[18];
> +  int reg, r;
> +  int offset, offset_and;
> +  static enum amd64_regnum regnums[16] = {
> +    AMD64_RAX_REGNUM,		/* %rax */
> +    AMD64_RCX_REGNUM,		/* %rcx */
> +    AMD64_RDX_REGNUM,		/* %rdx */
> +    AMD64_RBX_REGNUM,		/* %rbx */
> +    AMD64_RSP_REGNUM,		/* %rsp */
> +    AMD64_RBP_REGNUM,		/* %rbp */
> +    AMD64_RSI_REGNUM,		/* %rsi */
> +    AMD64_RDI_REGNUM,		/* %rdi */
> +    AMD64_R8_REGNUM,		/* %r8 */
> +    AMD64_R9_REGNUM,		/* %r9 */
> +    AMD64_R10_REGNUM,		/* %r10 */
> +    AMD64_R11_REGNUM,		/* %r11 */
> +    AMD64_R12_REGNUM,		/* %r12 */
> +    AMD64_R13_REGNUM,		/* %r13 */
> +    AMD64_R14_REGNUM,		/* %r14 */
> +    AMD64_R15_REGNUM,		/* %r15 */
> +  };
> +
> +  if (target_read_memory (pc, buf, sizeof buf))
> +    return pc;
> +
> +  /* First check "pushq %rbp".  */
> +  if (buf[0] == 0x55)
> +    {
> +      /* The next instruction has to be "movq %rsp, %rbp".  */
> +      if (buf[1] != 0x48 || buf[2] != 0x89 || buf[3] != 0xe5)
> +	return pc;
> +
> +      /* The next instruction has to be "andq $-XXX, %rsp".  */
> +      if (buf[4] != 0x48
> +	  || buf[6] != 0xe4
> +	  || (buf[5] != 0x81 && buf[5] != 0x83))
> +	return pc;
> +
> +      if (current_pc > pc + 4)
> +	cache->saved_sp_reg = AMD64_RBP_REGNUM;
> +
> +      /* Keep the prologue for amd64_analyze_prologue.  */
> +      return pc;
> +    }
> +
> +  /* Check caller-saved saved register.  The first instruction has
> +     to be "leaq 8(%rsp), %reg".  */
> +  if ((buf[0] & 0xfb) == 0x48
> +      && buf[1] == 0x8d
> +      && buf[3] == 0x24
> +      && buf[4] == 0x8)
> +    {
> +      /* MOD must be binary 10 and R/M must be binary 100.  */
> +      if ((buf[2] & 0xc7) != 0x44)
> +	return pc;
> +
> +      /* REG has register number.  */
> +      reg = (buf[2] >> 3) & 7;
> +
> +      /* Check the REX.R bit.  */
> +      if (buf[0] == 0x4c)
> +	reg += 8;
> +
> +      offset = 5;
> +    }
> +  else
> +    {
> +      /* Check callee-saved saved register.  The first instruction
> +	 has to be "pushq %reg".  */
> +      reg = 0;
> +      if ((buf[0] & 0xf8) == 0x50)
> +	offset = 0;
> +      else if ((buf[0] & 0xf6) == 0x40
> +	       && (buf[1] & 0xf8) == 0x50)
> +	{
> +	  /* Check the REX.B bit.  */
> +	  if ((buf[0] & 1) != 0)
> +	    reg = 8;
> +
> +	  offset = 1;
> +	}
> +      else
> +	return pc;
> +
> +      /* Get register.  */
> +      reg += buf[offset] & 0x7;
> +
> +      offset++;
> +
> +      /* The next instruction has to be "leaq 16(%rsp), %reg".  */
> +      if ((buf[offset] & 0xfb) != 0x48
> +	  || buf[offset + 1] != 0x8d
> +	  || buf[offset + 3] != 0x24
> +	  || buf[offset + 4] != 0x10)
> +	return pc;
> +
> +      /* MOD must be binary 10 and R/M must be binary 100.  */
> +      if ((buf[offset + 2] & 0xc7) != 0x44)
> +	return pc;
> +      
> +      /* REG has register number.  */
> +      r = (buf[offset + 2] >> 3) & 7;
> +
> +      /* Check the REX.R bit.  */
> +      if (buf[offset] == 0x4c)
> +	r += 8;
> +
> +      /* Registers in pushq and leaq have to be the same.  */
> +      if (reg != r)
> +	return pc;
> +
> +      offset += 5;
> +    }
> +
> +  /* Rigister can't be %rsp nor %rbp.  */
> +  if (reg == 4 || reg == 5)
> +    return pc;
> +
> +  /* The next instruction has to be "andq $-XXX, %rsp".  */
> +  if (buf[offset] != 0x48
> +      || buf[offset + 2] != 0xe4
> +      || (buf[offset + 1] != 0x81 && buf[offset + 1] != 0x83))
> +    return pc;
> +
> +  offset_and = offset;
> +  offset += buf[offset + 1] == 0x81 ? 7 : 4;
> +
> +  /* The next instruction has to be "pushq -8(%reg)".  */
> +  r = 0;
> +  if (buf[offset] == 0xff)
> +    offset++;
> +  else if ((buf[offset] & 0xf6) == 0x40
> +	   && buf[offset + 1] == 0xff)
> +    {
> +      /* Check the REX.B bit.  */
> +      if ((buf[offset] & 0x1) != 0)
> +	r = 8;
> +      offset += 2;
> +    }
> +  else
> +    return pc;
> +
> +  /* 8bit -8 is 0xf8.  REG must be binary 110 and MOD must be binary
> +     01.  */
> +  if (buf[offset + 1] != 0xf8
> +      || (buf[offset] & 0xf8) != 0x70)
> +    return pc;
> +
> +  /* R/M has register.  */
> +  r += buf[offset] & 7;
> +
> +  /* Registers in leaq and pushq have to be the same.  */
> +  if (reg != r)
> +    return pc;
> +
> +  if (current_pc > pc + offset_and)
> +    cache->saved_sp_reg = regnums[reg];
> +
> +  return min (pc + offset + 2, current_pc);
> +}
> +
>  /* Do a limited analysis of the prologue at PC and update CACHE
>     accordingly.  Bail out early if CURRENT_PC is reached.  Return the
>     address where the analysis stopped.
> @@ -742,6 +942,8 @@ amd64_analyze_prologue (CORE_ADDR pc, CO
>    if (current_pc <= pc)
>      return current_pc;
>  
> +  pc = amd64_analyze_stack_align (pc, current_pc, cache);
> +
>    op = read_memory_unsigned_integer (pc, 1);
>  
>    if (op == 0x55)		/* pushq %rbp */
> @@ -813,8 +1015,23 @@ amd64_frame_cache (struct frame_info *th
>  	 at the stack pointer.  For truly "frameless" functions this
>  	 might work too.  */
>  
> -      get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
> -      cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
> +      if (cache->saved_sp_reg != AMD64_RSP_REGNUM)
> +	{
> +	  /* We're halfway aligning the stack.  Saved stack pointer
> +	     has been saved in saved_sp_reg.  */
> +	  get_frame_register (this_frame, cache->saved_sp_reg, buf);
> +	  cache->saved_sp = extract_unsigned_integer(buf, 8);
> +	  cache->base = ((cache->saved_sp - 8) & 0xfffffffffffffff0LL) - 8;
> +	  cache->saved_regs[AMD64_RIP_REGNUM] = cache->saved_sp - 8;

I don't see how this could possibly work if saved_regs[AMD64_RIP_REGNUM]
is set to 8 unconditionally a few lines further on.

> +
> +	  /* This will be added back below.  */
> +	  cache->saved_regs[AMD64_RIP_REGNUM] -= cache->base;
> +	}
> +      else
> +	{
> +	  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
> +	  cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
> +	}
>      }
>    else
>      {
> diff -x .svn -upr ../gdb/src/gdb/i386-tdep.c gdb/gdb/i386-tdep.c
> --- ../gdb/src/gdb/i386-tdep.c	2008-07-16 22:30:17.000000000 -0700
> +++ gdb/gdb/i386-tdep.c	2008-07-18 17:42:03.000000000 -0700
> @@ -707,37 +707,134 @@ static CORE_ADDR
>  i386_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
>  			  struct i386_frame_cache *cache)
>  {
> -  /* The register used by the compiler to perform the stack re-alignment 
> -     is, in order of preference, either %ecx, %edx, or %eax.  GCC should
> -     never use %ebx as it always treats it as callee-saved, whereas
> -     the compiler can only use caller-saved registers.  */
> -  static const gdb_byte insns_ecx[10] = { 
> -    0x8d, 0x4c, 0x24, 0x04,	/* leal  4(%esp), %ecx */
> -    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
> -    0xff, 0x71, 0xfc		/* pushl -4(%ecx) */
> -  };
> -  static const gdb_byte insns_edx[10] = { 
> -    0x8d, 0x54, 0x24, 0x04,	/* leal  4(%esp), %edx */
> -    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
> -    0xff, 0x72, 0xfc		/* pushl -4(%edx) */
> -  };
> -  static const gdb_byte insns_eax[10] = { 
> -    0x8d, 0x44, 0x24, 0x04,	/* leal  4(%esp), %eax */
> -    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
> -    0xff, 0x70, 0xfc		/* pushl -4(%eax) */
> +  /* There are 3 code sequences to re-align stack:
> +
> +     	1. Use %ebp:
> +
> +		pushl %ebp
> +		movl  %esp, %ebp
> +		andl  $-XXX, %esp

See my comment about amd64.

Re: PATCH: Update x86 stack align analyzer

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 2832 bytes --]

On Wed, Aug 6, 2008 at 2:05 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>
>> introduced new ways to align stack.  This patch teaches gdb how to
>> recognize the new stack prologue. OK for trunk?
>
> A few comments inline below.

Thanks for you review.

>
>> --- ../gdb/src/gdb/amd64-tdep.c       2008-07-16 22:30:04.000000000 -0700
>> +++ gdb/gdb/amd64-tdep.c      2008-07-25 11:57:37.000000000 -0700
>> @@ -680,6 +680,7 @@ struct amd64_frame_cache
>>    /* Saved registers.  */
>>    CORE_ADDR saved_regs[AMD64_NUM_SAVED_REGS];
>>    CORE_ADDR saved_sp;
>> +  enum i386_regnum saved_sp_reg;
>
> I suppose you meant 'enum amd64_regnum' here.  I'd prefer if you'd
> simply use 'int' as the type for registers instead of the enum.
> That's what all the other code in GDB does.
>

Done.

>> +/* GCC 4.4 and later, can put code in the prologue to realign the
>> +   stack pointer.  Check whether PC points to such code, and update
>> +   CACHE accordingly.  Return the first instruction after the code
>> +   sequence or CURRENT_PC, whichever is smaller.  If we don't
>> +   recognize the code, return PC.  */
>
> There are older versions of GCC that use at least method #2, so this
> comment is a bit misleading.

Gcc 4.1 can align stack to 16byte, but only for ia32, not for x86-64. We
added this capability to gcc 4.4.

>
>> +static CORE_ADDR
>> +amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
>> +                        struct amd64_frame_cache *cache)
>> +{
>> +  /* There are 3 code sequences to re-align stack:
>> +
>> +             1. Use %ebp:
>> +
>> +             pushq %rbp
>> +             movq  %rsp, %rbp
>> +             andq  $-XXX, %rsp
>
> But this is basically the standard prologue sequence.  I don't see why
> you need to handle this sequence here, since it is already handled in
> amd64_analyze_prologue().

We need it to set saved_sp_reg.

>
>> -      get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
>> -      cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
>> +      if (cache->saved_sp_reg != AMD64_RSP_REGNUM)
>> +     {
>> +       /* We're halfway aligning the stack.  Saved stack pointer
>> +          has been saved in saved_sp_reg.  */
>> +       get_frame_register (this_frame, cache->saved_sp_reg, buf);
>> +       cache->saved_sp = extract_unsigned_integer(buf, 8);
>> +       cache->base = ((cache->saved_sp - 8) & 0xfffffffffffffff0LL) - 8;
>> +       cache->saved_regs[AMD64_RIP_REGNUM] = cache->saved_sp - 8;
>
> I don't see how this could possibly work if saved_regs[AMD64_RIP_REGNUM]
> is set to 8 unconditionally a few lines further on.

I changed it not to set %rip to 8 when halfway aligning the stack. That
is similar to i386 code.

>
> See my comment about amd64.
>

I changed to int.

I am enclosed my updated patch.

Thanks.


-- 
H.J.

[-- Attachment #2: gdb-stack-3.patch.txt --]
[-- Type: text/plain, Size: 15957 bytes --]

2008-08-06  Xuepeng Guo  <xuepeng.guo@intel.com>
	    H.J. Lu  <hongjiu.lu@intel.com>

	* amd64-tdep.c (amd64_frame_cache): Add saved_sp_reg.
	(amd64_init_frame_cache): Initialize saved_sp_reg.
	(amd64_analyze_stack_align): New.
	(amd64_analyze_prologue): Call it.
	(amd64_frame_cache): Try to use saved_sp_reg if frame is invalid.
	Don't set %rip to 8 when halfway aligning the stack.

	* amd64-tdep.h (amd64_regnum): Add AMD64_R9_REGNUM to
	AMD64_R14_REGNUM.

	* i386-tdep.c (i386_frame_cache): Remove stack_align.  Add
	saved_sp_reg.
	(i386_alloc_frame_cache): Remove stack_align.  Initialize
	saved_sp_reg to I386_ESP_REGNUM.
	(i386_analyze_stack_align): Rewrite.
	(i386_frame_cache): Use saved_sp_reg only if frame is invalid.

diff -x .svn -upr ../gdb/src/gdb gdb/gdb
diff -x .svn -upr ../gdb/src/gdb/amd64-tdep.c gdb/gdb/amd64-tdep.c
--- ../gdb/src/gdb/amd64-tdep.c	2008-07-16 22:30:04.000000000 -0700
+++ gdb/gdb/amd64-tdep.c	2008-08-06 14:25:49.000000000 -0700
@@ -680,6 +680,7 @@ struct amd64_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[AMD64_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
+  int saved_sp_reg;
 
   /* Do we have a frame?  */
   int frameless_p;
@@ -702,6 +703,7 @@ amd64_init_frame_cache (struct amd64_fra
   for (i = 0; i < AMD64_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
+  cache->saved_sp_reg = AMD64_RSP_REGNUM;
 
   /* Frameless until proven otherwise.  */
   cache->frameless_p = 1;
@@ -719,6 +721,204 @@ amd64_alloc_frame_cache (void)
   return cache;
 }
 
+/* GCC 4.4 and later, can put code in the prologue to realign the
+   stack pointer.  Check whether PC points to such code, and update
+   CACHE accordingly.  Return the first instruction after the code
+   sequence or CURRENT_PC, whichever is smaller.  If we don't
+   recognize the code, return PC.  */
+
+static CORE_ADDR
+amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
+			   struct amd64_frame_cache *cache)
+{
+  /* There are 3 code sequences to re-align stack:
+
+     	1. Use %rbp:
+
+		pushq %rbp
+		movq  %rsp, %rbp
+		andq  $-XXX, %rsp
+
+	2. Use a caller-saved saved register:
+
+		leaq  8(%rsp), %reg
+		andq  $-XXX, %rsp
+		pushq -8(%reg)
+
+	3. Use a callee-saved saved register:
+
+		pushq %reg
+		leaq  16(%rsp), %reg
+		andq  $-XXX, %rsp
+		pushq -8(%reg)
+
+     "andq $-XXX, %rsp" can be either 4 bytes or 7 bytes:
+     
+     	0x48 0x83 0xe4 0xf0			andq $-16, %rsp
+     	0x48 0x81 0xe4 0x00 0xff 0xff 0xff	andq $-256, %rsp
+   */
+
+  gdb_byte buf[18];
+  int reg, r;
+  int offset, offset_and;
+  static int regnums[16] = {
+    AMD64_RAX_REGNUM,		/* %rax */
+    AMD64_RCX_REGNUM,		/* %rcx */
+    AMD64_RDX_REGNUM,		/* %rdx */
+    AMD64_RBX_REGNUM,		/* %rbx */
+    AMD64_RSP_REGNUM,		/* %rsp */
+    AMD64_RBP_REGNUM,		/* %rbp */
+    AMD64_RSI_REGNUM,		/* %rsi */
+    AMD64_RDI_REGNUM,		/* %rdi */
+    AMD64_R8_REGNUM,		/* %r8 */
+    AMD64_R9_REGNUM,		/* %r9 */
+    AMD64_R10_REGNUM,		/* %r10 */
+    AMD64_R11_REGNUM,		/* %r11 */
+    AMD64_R12_REGNUM,		/* %r12 */
+    AMD64_R13_REGNUM,		/* %r13 */
+    AMD64_R14_REGNUM,		/* %r14 */
+    AMD64_R15_REGNUM,		/* %r15 */
+  };
+
+  if (target_read_memory (pc, buf, sizeof buf))
+    return pc;
+
+  /* First check "pushq %rbp".  */
+  if (buf[0] == 0x55)
+    {
+      /* The next instruction has to be "movq %rsp, %rbp".  */
+      if (buf[1] != 0x48 || buf[2] != 0x89 || buf[3] != 0xe5)
+	return pc;
+
+      /* The next instruction has to be "andq $-XXX, %rsp".  */
+      if (buf[4] != 0x48
+	  || buf[6] != 0xe4
+	  || (buf[5] != 0x81 && buf[5] != 0x83))
+	return pc;
+
+      if (current_pc > pc + 4)
+	cache->saved_sp_reg = AMD64_RBP_REGNUM;
+
+      /* Keep the prologue for amd64_analyze_prologue.  */
+      return pc;
+    }
+
+  /* Check caller-saved saved register.  The first instruction has
+     to be "leaq 8(%rsp), %reg".  */
+  if ((buf[0] & 0xfb) == 0x48
+      && buf[1] == 0x8d
+      && buf[3] == 0x24
+      && buf[4] == 0x8)
+    {
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[2] & 0xc7) != 0x44)
+	return pc;
+
+      /* REG has register number.  */
+      reg = (buf[2] >> 3) & 7;
+
+      /* Check the REX.R bit.  */
+      if (buf[0] == 0x4c)
+	reg += 8;
+
+      offset = 5;
+    }
+  else
+    {
+      /* Check callee-saved saved register.  The first instruction
+	 has to be "pushq %reg".  */
+      reg = 0;
+      if ((buf[0] & 0xf8) == 0x50)
+	offset = 0;
+      else if ((buf[0] & 0xf6) == 0x40
+	       && (buf[1] & 0xf8) == 0x50)
+	{
+	  /* Check the REX.B bit.  */
+	  if ((buf[0] & 1) != 0)
+	    reg = 8;
+
+	  offset = 1;
+	}
+      else
+	return pc;
+
+      /* Get register.  */
+      reg += buf[offset] & 0x7;
+
+      offset++;
+
+      /* The next instruction has to be "leaq 16(%rsp), %reg".  */
+      if ((buf[offset] & 0xfb) != 0x48
+	  || buf[offset + 1] != 0x8d
+	  || buf[offset + 3] != 0x24
+	  || buf[offset + 4] != 0x10)
+	return pc;
+
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[offset + 2] & 0xc7) != 0x44)
+	return pc;
+      
+      /* REG has register number.  */
+      r = (buf[offset + 2] >> 3) & 7;
+
+      /* Check the REX.R bit.  */
+      if (buf[offset] == 0x4c)
+	r += 8;
+
+      /* Registers in pushq and leaq have to be the same.  */
+      if (reg != r)
+	return pc;
+
+      offset += 5;
+    }
+
+  /* Rigister can't be %rsp nor %rbp.  */
+  if (reg == 4 || reg == 5)
+    return pc;
+
+  /* The next instruction has to be "andq $-XXX, %rsp".  */
+  if (buf[offset] != 0x48
+      || buf[offset + 2] != 0xe4
+      || (buf[offset + 1] != 0x81 && buf[offset + 1] != 0x83))
+    return pc;
+
+  offset_and = offset;
+  offset += buf[offset + 1] == 0x81 ? 7 : 4;
+
+  /* The next instruction has to be "pushq -8(%reg)".  */
+  r = 0;
+  if (buf[offset] == 0xff)
+    offset++;
+  else if ((buf[offset] & 0xf6) == 0x40
+	   && buf[offset + 1] == 0xff)
+    {
+      /* Check the REX.B bit.  */
+      if ((buf[offset] & 0x1) != 0)
+	r = 8;
+      offset += 2;
+    }
+  else
+    return pc;
+
+  /* 8bit -8 is 0xf8.  REG must be binary 110 and MOD must be binary
+     01.  */
+  if (buf[offset + 1] != 0xf8
+      || (buf[offset] & 0xf8) != 0x70)
+    return pc;
+
+  /* R/M has register.  */
+  r += buf[offset] & 7;
+
+  /* Registers in leaq and pushq have to be the same.  */
+  if (reg != r)
+    return pc;
+
+  if (current_pc > pc + offset_and)
+    cache->saved_sp_reg = regnums[reg];
+
+  return min (pc + offset + 2, current_pc);
+}
+
 /* Do a limited analysis of the prologue at PC and update CACHE
    accordingly.  Bail out early if CURRENT_PC is reached.  Return the
    address where the analysis stopped.
@@ -742,6 +942,8 @@ amd64_analyze_prologue (CORE_ADDR pc, CO
   if (current_pc <= pc)
     return current_pc;
 
+  pc = amd64_analyze_stack_align (pc, current_pc, cache);
+
   op = read_memory_unsigned_integer (pc, 1);
 
   if (op == 0x55)		/* pushq %rbp */
@@ -813,8 +1015,23 @@ amd64_frame_cache (struct frame_info *th
 	 at the stack pointer.  For truly "frameless" functions this
 	 might work too.  */
 
-      get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
-      cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
+      if (cache->saved_sp_reg != AMD64_RSP_REGNUM)
+	{
+	  /* We're halfway aligning the stack.  Saved stack pointer
+	     has been saved in saved_sp_reg.  */
+	  get_frame_register (this_frame, cache->saved_sp_reg, buf);
+	  cache->saved_sp = extract_unsigned_integer(buf, 8);
+	  cache->base = ((cache->saved_sp - 8) & 0xfffffffffffffff0LL) - 8;
+	  cache->saved_regs[AMD64_RIP_REGNUM] = cache->saved_sp - 8;
+
+	  /* This will be added back below.  */
+	  cache->saved_regs[AMD64_RIP_REGNUM] -= cache->base;
+	}
+      else
+	{
+	  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+	  cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
+	}
     }
   else
     {
@@ -828,8 +1045,11 @@ amd64_frame_cache (struct frame_info *th
 
   /* For normal frames, %rip is stored at 8(%rbp).  If we don't have a
      frame we find it at the same offset from the reconstructed base
-     address.  */
-  cache->saved_regs[AMD64_RIP_REGNUM] = 8;
+     address.  If we're halfway aligning the stack, %rip is handled
+     differently (see above).  */
+  if (!cache->frameless_p
+      || cache->saved_sp_reg == AMD64_RSP_REGNUM)
+    cache->saved_regs[AMD64_RIP_REGNUM] = 8;
 
   /* Adjust all the saved registers such that they contain addresses
      instead of offsets.  */
diff -x .svn -upr ../gdb/src/gdb/amd64-tdep.h gdb/gdb/amd64-tdep.h
--- ../gdb/src/gdb/amd64-tdep.h	2008-01-17 09:38:41.000000000 -0800
+++ gdb/gdb/amd64-tdep.h	2008-07-18 15:41:32.000000000 -0700
@@ -39,8 +39,14 @@ enum amd64_regnum
   AMD64_RDI_REGNUM,		/* %rdi */
   AMD64_RBP_REGNUM,		/* %rbp */
   AMD64_RSP_REGNUM,		/* %rsp */
-  AMD64_R8_REGNUM = 8,		/* %r8 */
-  AMD64_R15_REGNUM = 15,	/* %r15 */
+  AMD64_R8_REGNUM,		/* %r8 */
+  AMD64_R9_REGNUM,		/* %r9 */
+  AMD64_R10_REGNUM,		/* %r10 */
+  AMD64_R11_REGNUM,		/* %r11 */
+  AMD64_R12_REGNUM,		/* %r12 */
+  AMD64_R13_REGNUM,		/* %r13 */
+  AMD64_R14_REGNUM,		/* %r14 */
+  AMD64_R15_REGNUM,		/* %r15 */
   AMD64_RIP_REGNUM,		/* %rip */
   AMD64_EFLAGS_REGNUM,		/* %eflags */
   AMD64_CS_REGNUM,		/* %cs */
Only in gdb/gdb: ChangeLog.stack
diff -x .svn -upr ../gdb/src/gdb/i386-tdep.c gdb/gdb/i386-tdep.c
--- ../gdb/src/gdb/i386-tdep.c	2008-07-16 22:30:17.000000000 -0700
+++ gdb/gdb/i386-tdep.c	2008-08-06 14:24:23.000000000 -0700
@@ -518,7 +518,7 @@ struct i386_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[I386_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
-  int stack_align;
+  int saved_sp_reg;
   int pc_in_eax;
 
   /* Stack space reserved for local variables.  */
@@ -545,7 +545,7 @@ i386_alloc_frame_cache (void)
   for (i = 0; i < I386_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
-  cache->stack_align = 0;
+  cache->saved_sp_reg = I386_ESP_REGNUM;
   cache->pc_in_eax = 0;
 
   /* Frameless until proven otherwise.  */
@@ -707,37 +707,134 @@ static CORE_ADDR
 i386_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
 			  struct i386_frame_cache *cache)
 {
-  /* The register used by the compiler to perform the stack re-alignment 
-     is, in order of preference, either %ecx, %edx, or %eax.  GCC should
-     never use %ebx as it always treats it as callee-saved, whereas
-     the compiler can only use caller-saved registers.  */
-  static const gdb_byte insns_ecx[10] = { 
-    0x8d, 0x4c, 0x24, 0x04,	/* leal  4(%esp), %ecx */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x71, 0xfc		/* pushl -4(%ecx) */
-  };
-  static const gdb_byte insns_edx[10] = { 
-    0x8d, 0x54, 0x24, 0x04,	/* leal  4(%esp), %edx */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x72, 0xfc		/* pushl -4(%edx) */
-  };
-  static const gdb_byte insns_eax[10] = { 
-    0x8d, 0x44, 0x24, 0x04,	/* leal  4(%esp), %eax */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x70, 0xfc		/* pushl -4(%eax) */
+  /* There are 3 code sequences to re-align stack:
+
+     	1. Use %ebp:
+
+		pushl %ebp
+		movl  %esp, %ebp
+		andl  $-XXX, %esp
+
+	2. Use a caller-saved saved register:
+
+		leal  4(%esp), %reg
+		andl  $-XXX, %esp
+		pushl -4(%reg)
+
+	3. Use a callee-saved saved register:
+
+		pushl %reg
+		leal  8(%esp), %reg
+		andl  $-XXX, %esp
+		pushl -4(%reg)
+
+     "andl $-XXX, %esp" can be either 3 bytes or 6 bytes:
+     
+     	0x83 0xe4 0xf0			andl $-16, %esp
+     	0x81 0xe4 0x00 0xff 0xff 0xff	andl $-256, %esp
+   */
+
+  gdb_byte buf[14];
+  int reg;
+  int offset, offset_and;
+  static int regnums[8] = {
+    I386_EAX_REGNUM,		/* %eax */
+    I386_ECX_REGNUM,		/* %ecx */
+    I386_EDX_REGNUM,		/* %edx */
+    I386_EBX_REGNUM,		/* %ebx */
+    I386_ESP_REGNUM,		/* %esp */
+    I386_EBP_REGNUM,		/* %ebp */
+    I386_ESI_REGNUM,		/* %esi */
+    I386_EDI_REGNUM		/* %edi */
   };
-  gdb_byte buf[10];
 
-  if (target_read_memory (pc, buf, sizeof buf)
-      || (memcmp (buf, insns_ecx, sizeof buf) != 0
-          && memcmp (buf, insns_edx, sizeof buf) != 0
-          && memcmp (buf, insns_eax, sizeof buf) != 0))
+  if (target_read_memory (pc, buf, sizeof buf))
+    return pc;
+
+  /* First check "pushl %ebp".  */
+  if (buf[0] == 0x55)
+    {
+      /* The next instruction has to be "movl %esp, %ebp".  */
+      if (buf[1] != 0x89 || buf[2] != 0xe5)
+	return pc;
+
+      /* The next instruction has to be "andl $-XXX, %esp".  */
+      if (buf[4] != 0xe4 || (buf[3] != 0x81 && buf[3] != 0x83))
+	return pc;
+
+      if (current_pc > pc + 3)
+	cache->saved_sp_reg = I386_EBP_REGNUM;
+
+      /* Keep the prologue for i386_analyze_frame_setup.  */
+      return pc;
+    }
+
+  /* Check caller-saved saved register.  The first instruction has
+     to be "leal 4(%esp), %reg".  */
+  if (buf[0] == 0x8d && buf[2] == 0x24 && buf[3] == 0x4)
+    {
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[1] & 0xc7) != 0x44)
+	return pc;
+
+      /* REG has register number.  */
+      reg = (buf[1] >> 3) & 7;
+      offset = 4;
+    }
+  else
+    {
+      /* Check callee-saved saved register.  The first instruction
+	 has to be "pushl %reg".  */
+      if ((buf[0] & 0xf8) != 0x50)
+	return pc;
+
+      /* Get register.  */
+      reg = buf[0] & 0x7;
+
+      /* The next instruction has to be "leal 8(%esp), %reg".  */
+      if (buf[1] != 0x8d || buf[3] != 0x24 || buf[4] != 0x8)
+	return pc;
+
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[2] & 0xc7) != 0x44)
+	return pc;
+      
+      /* REG has register number.  Registers in pushl and leal have to
+	 be the same.  */
+      if (reg != ((buf[2] >> 3) & 7))
+	return pc;
+
+      offset = 5;
+    }
+
+  /* Rigister can't be %esp nor %ebp.  */
+  if (reg == 4 || reg == 5)
+    return pc;
+
+  /* The next instruction has to be "andl $-XXX, %esp".  */
+  if (buf[offset + 1] != 0xe4
+      || (buf[offset] != 0x81 && buf[offset] != 0x83))
     return pc;
 
-  if (current_pc > pc + 4)
-    cache->stack_align = 1;
+  offset_and = offset;
+  offset += buf[offset] == 0x81 ? 6 : 3;
 
-  return min (pc + 10, current_pc);
+  /* The next instruction has to be "pushl -4(%reg)".  8bit -4 is
+     0xfc.  REG must be binary 110 and MOD must be binary 01.  */
+  if (buf[offset] != 0xff
+      || buf[offset + 2] != 0xfc
+      || (buf[offset + 1] & 0xf8) != 0x70)
+    return pc;
+
+  /* R/M has register.  Registers in leal and pushl have to be the
+     same.  */
+  if (reg != (buf[offset + 1] & 7))
+    return pc;
+
+  if (current_pc > pc + offset_and)
+    cache->saved_sp_reg = regnums[reg];
+
+  return min (pc + offset + 3, current_pc);
 }
 
 /* Maximum instruction length we need to handle.  */
@@ -1241,13 +1338,6 @@ i386_frame_cache (struct frame_info *thi
   if (cache->pc != 0)
     i386_analyze_prologue (cache->pc, get_frame_pc (this_frame), cache);
 
-  if (cache->stack_align)
-    {
-      /* Saved stack pointer has been saved in %ecx.  */
-      get_frame_register (this_frame, I386_ECX_REGNUM, buf);
-      cache->saved_sp = extract_unsigned_integer(buf, 4);
-    }
-
   if (cache->locals < 0)
     {
       /* We didn't find a valid frame, which means that CACHE->base
@@ -1258,9 +1348,12 @@ i386_frame_cache (struct frame_info *thi
 	 frame by looking at the stack pointer.  For truly "frameless"
 	 functions this might work too.  */
 
-      if (cache->stack_align)
+      if (cache->saved_sp_reg != I386_ESP_REGNUM)
 	{
-	  /* We're halfway aligning the stack.  */
+	  /* We're halfway aligning the stack.  Saved stack pointer
+	     has been saved in saved_sp_reg.  */
+	  get_frame_register (this_frame, cache->saved_sp_reg, buf);
+	  cache->saved_sp = extract_unsigned_integer(buf, 4);
 	  cache->base = ((cache->saved_sp - 4) & 0xfffffff0) - 4;
 	  cache->saved_regs[I386_EIP_REGNUM] = cache->saved_sp - 4;
 

Re: PATCH: Update x86 stack align analyzer

[Mark Kettenis]

> Date: Wed, 6 Aug 2008 14:36:01 -0700
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> >> +/* GCC 4.4 and later, can put code in the prologue to realign the
> >> +   stack pointer.  Check whether PC points to such code, and update
> >> +   CACHE accordingly.  Return the first instruction after the code
> >> +   sequence or CURRENT_PC, whichever is smaller.  If we don't
> >> +   recognize the code, return PC.  */
> >
> > There are older versions of GCC that use at least method #2, so this
> > comment is a bit misleading.
> 
> Gcc 4.1 can align stack to 16byte, but only for ia32, not for x86-64. We
> added this capability to gcc 4.4.

Ah ok, guess I confused amd64 with i386 there.  Sorry 'bout that.

> >> +static CORE_ADDR
> >> +amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
> >> +                        struct amd64_frame_cache *cache)
> >> +{
> >> +  /* There are 3 code sequences to re-align stack:
> >> +
> >> +             1. Use %ebp:
> >> +
> >> +             pushq %rbp
> >> +             movq  %rsp, %rbp
> >> +             andq  $-XXX, %rsp
> >
> > But this is basically the standard prologue sequence.  I don't see why
> > you need to handle this sequence here, since it is already handled in
> > amd64_analyze_prologue().
> 
> We need it to set saved_sp_reg.

I feel, rather strongly, that the stack align stuff stuff should only
deal with stack alignment done *before* a frame gets setup.  I think
setting saved_sp_reg to -1, and proceeding as usual if it is still set
at that value after the stack alignment analysis, works just fine.  I
also noticed a problem with your diff in that it doesn't use the
proper saved sp value if you do have a frame (not sure GCC actually
generates such code).

How about the attached diff?  Does it work with GCC 4.4?


Index: amd64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64-tdep.c,v
retrieving revision 1.52
diff -u -p -r1.52 amd64-tdep.c
--- amd64-tdep.c	16 May 2008 00:27:22 -0000	1.52
+++ amd64-tdep.c	9 Aug 2008 11:56:43 -0000
@@ -680,6 +680,7 @@ struct amd64_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[AMD64_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
+  int saved_sp_reg;
 
   /* Do we have a frame?  */
   int frameless_p;
@@ -702,6 +703,7 @@ amd64_init_frame_cache (struct amd64_fra
   for (i = 0; i < AMD64_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
+  cache->saved_sp_reg = -1;
 
   /* Frameless until proven otherwise.  */
   cache->frameless_p = 1;
@@ -719,6 +721,179 @@ amd64_alloc_frame_cache (void)
   return cache;
 }
 
+/* GCC 4.4 and later, can put code in the prologue to realign the
+   stack pointer.  Check whether PC points to such code, and update
+   CACHE accordingly.  Return the first instruction after the code
+   sequence or CURRENT_PC, whichever is smaller.  If we don't
+   recognize the code, return PC.  */
+
+static CORE_ADDR
+amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
+			   struct amd64_frame_cache *cache)
+{
+  /* There are 2 code sequences to re-align stack before the frame
+     gets set up:
+
+	1. Use a caller-saved saved register:
+
+		leaq  8(%rsp), %reg
+		andq  $-XXX, %rsp
+		pushq -8(%reg)
+
+	2. Use a callee-saved saved register:
+
+		pushq %reg
+		leaq  16(%rsp), %reg
+		andq  $-XXX, %rsp
+		pushq -8(%reg)
+
+     "andq $-XXX, %rsp" can be either 4 bytes or 7 bytes:
+     
+     	0x48 0x83 0xe4 0xf0			andq $-16, %rsp
+     	0x48 0x81 0xe4 0x00 0xff 0xff 0xff	andq $-256, %rsp
+   */
+
+  gdb_byte buf[18];
+  int reg, r;
+  int offset, offset_and;
+  static int regnums[16] = {
+    AMD64_RAX_REGNUM,		/* %rax */
+    AMD64_RCX_REGNUM,		/* %rcx */
+    AMD64_RDX_REGNUM,		/* %rdx */
+    AMD64_RBX_REGNUM,		/* %rbx */
+    AMD64_RSP_REGNUM,		/* %rsp */
+    AMD64_RBP_REGNUM,		/* %rbp */
+    AMD64_RSI_REGNUM,		/* %rsi */
+    AMD64_RDI_REGNUM,		/* %rdi */
+    AMD64_R8_REGNUM,		/* %r8 */
+    AMD64_R9_REGNUM,		/* %r9 */
+    AMD64_R10_REGNUM,		/* %r10 */
+    AMD64_R11_REGNUM,		/* %r11 */
+    AMD64_R12_REGNUM,		/* %r12 */
+    AMD64_R13_REGNUM,		/* %r13 */
+    AMD64_R14_REGNUM,		/* %r14 */
+    AMD64_R15_REGNUM,		/* %r15 */
+  };
+
+  if (target_read_memory (pc, buf, sizeof buf))
+    return pc;
+
+  /* Check caller-saved saved register.  The first instruction has
+     to be "leaq 8(%rsp), %reg".  */
+  if ((buf[0] & 0xfb) == 0x48
+      && buf[1] == 0x8d
+      && buf[3] == 0x24
+      && buf[4] == 0x8)
+    {
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[2] & 0xc7) != 0x44)
+	return pc;
+
+      /* REG has register number.  */
+      reg = (buf[2] >> 3) & 7;
+
+      /* Check the REX.R bit.  */
+      if (buf[0] == 0x4c)
+	reg += 8;
+
+      offset = 5;
+    }
+  else
+    {
+      /* Check callee-saved saved register.  The first instruction
+	 has to be "pushq %reg".  */
+      reg = 0;
+      if ((buf[0] & 0xf8) == 0x50)
+	offset = 0;
+      else if ((buf[0] & 0xf6) == 0x40
+	       && (buf[1] & 0xf8) == 0x50)
+	{
+	  /* Check the REX.B bit.  */
+	  if ((buf[0] & 1) != 0)
+	    reg = 8;
+
+	  offset = 1;
+	}
+      else
+	return pc;
+
+      /* Get register.  */
+      reg += buf[offset] & 0x7;
+
+      offset++;
+
+      /* The next instruction has to be "leaq 16(%rsp), %reg".  */
+      if ((buf[offset] & 0xfb) != 0x48
+	  || buf[offset + 1] != 0x8d
+	  || buf[offset + 3] != 0x24
+	  || buf[offset + 4] != 0x10)
+	return pc;
+
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[offset + 2] & 0xc7) != 0x44)
+	return pc;
+      
+      /* REG has register number.  */
+      r = (buf[offset + 2] >> 3) & 7;
+
+      /* Check the REX.R bit.  */
+      if (buf[offset] == 0x4c)
+	r += 8;
+
+      /* Registers in pushq and leaq have to be the same.  */
+      if (reg != r)
+	return pc;
+
+      offset += 5;
+    }
+
+  /* Rigister can't be %rsp nor %rbp.  */
+  if (reg == 4 || reg == 5)
+    return pc;
+
+  /* The next instruction has to be "andq $-XXX, %rsp".  */
+  if (buf[offset] != 0x48
+      || buf[offset + 2] != 0xe4
+      || (buf[offset + 1] != 0x81 && buf[offset + 1] != 0x83))
+    return pc;
+
+  offset_and = offset;
+  offset += buf[offset + 1] == 0x81 ? 7 : 4;
+
+  /* The next instruction has to be "pushq -8(%reg)".  */
+  r = 0;
+  if (buf[offset] == 0xff)
+    offset++;
+  else if ((buf[offset] & 0xf6) == 0x40
+	   && buf[offset + 1] == 0xff)
+    {
+      /* Check the REX.B bit.  */
+      if ((buf[offset] & 0x1) != 0)
+	r = 8;
+      offset += 2;
+    }
+  else
+    return pc;
+
+  /* 8bit -8 is 0xf8.  REG must be binary 110 and MOD must be binary
+     01.  */
+  if (buf[offset + 1] != 0xf8
+      || (buf[offset] & 0xf8) != 0x70)
+    return pc;
+
+  /* R/M has register.  */
+  r += buf[offset] & 7;
+
+  /* Registers in leaq and pushq have to be the same.  */
+  if (reg != r)
+    return pc;
+
+  if (current_pc > pc + offset_and)
+    cache->saved_sp_reg = regnums[reg];
+
+  return min (pc + offset + 2, current_pc);
+}
+
 /* Do a limited analysis of the prologue at PC and update CACHE
    accordingly.  Bail out early if CURRENT_PC is reached.  Return the
    address where the analysis stopped.
@@ -742,6 +917,8 @@ amd64_analyze_prologue (CORE_ADDR pc, CO
   if (current_pc <= pc)
     return current_pc;
 
+  pc = amd64_analyze_stack_align (pc, current_pc, cache);
+
   op = read_memory_unsigned_integer (pc, 1);
 
   if (op == 0x55)		/* pushq %rbp */
@@ -804,6 +981,13 @@ amd64_frame_cache (struct frame_info *th
   if (cache->pc != 0)
     amd64_analyze_prologue (cache->pc, get_frame_pc (this_frame), cache);
 
+  if (cache->saved_sp_reg != -1)
+    {
+      /* Stack pointer has been saved.  */
+      get_frame_register (this_frame, cache->saved_sp_reg, buf);
+      cache->saved_sp = extract_unsigned_integer(buf, 8);
+    }
+
   if (cache->frameless_p)
     {
       /* We didn't find a valid frame.  If we're at the start of a
@@ -813,8 +997,20 @@ amd64_frame_cache (struct frame_info *th
 	 at the stack pointer.  For truly "frameless" functions this
 	 might work too.  */
 
-      get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
-      cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
+      if (cache->saved_sp_reg != -1)
+	{
+	  /* We're halfway aligning the stack.  */
+	  cache->base = ((cache->saved_sp - 8) & 0xfffffffffffffff0LL) - 8;
+	  cache->saved_regs[AMD64_RIP_REGNUM] = cache->saved_sp - 8;
+
+	  /* This will be added back below.  */
+	  cache->saved_regs[AMD64_RIP_REGNUM] -= cache->base;
+	}
+      else
+	{
+	  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+	  cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
+	}
     }
   else
     {
@@ -828,8 +1024,10 @@ amd64_frame_cache (struct frame_info *th
 
   /* For normal frames, %rip is stored at 8(%rbp).  If we don't have a
      frame we find it at the same offset from the reconstructed base
-     address.  */
-  cache->saved_regs[AMD64_RIP_REGNUM] = 8;
+     address.  If we're halfway aligning the stack, %rip is handled
+     differently (see above).  */
+  if (!cache->frameless_p || cache->saved_sp_reg == -1)
+    cache->saved_regs[AMD64_RIP_REGNUM] = 8;
 
   /* Adjust all the saved registers such that they contain addresses
      instead of offsets.  */
Index: amd64-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/amd64-tdep.h,v
retrieving revision 1.11
diff -u -p -r1.11 amd64-tdep.h
--- amd64-tdep.h	1 Jan 2008 22:53:09 -0000	1.11
+++ amd64-tdep.h	9 Aug 2008 11:56:43 -0000
@@ -39,8 +39,14 @@ enum amd64_regnum
   AMD64_RDI_REGNUM,		/* %rdi */
   AMD64_RBP_REGNUM,		/* %rbp */
   AMD64_RSP_REGNUM,		/* %rsp */
-  AMD64_R8_REGNUM = 8,		/* %r8 */
-  AMD64_R15_REGNUM = 15,	/* %r15 */
+  AMD64_R8_REGNUM,		/* %r8 */
+  AMD64_R9_REGNUM,		/* %r9 */
+  AMD64_R10_REGNUM,		/* %r10 */
+  AMD64_R11_REGNUM,		/* %r11 */
+  AMD64_R12_REGNUM,		/* %r12 */
+  AMD64_R13_REGNUM,		/* %r13 */
+  AMD64_R14_REGNUM,		/* %r14 */
+  AMD64_R15_REGNUM,		/* %r15 */
   AMD64_RIP_REGNUM,		/* %rip */
   AMD64_EFLAGS_REGNUM,		/* %eflags */
   AMD64_CS_REGNUM,		/* %cs */
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.261
diff -u -p -r1.261 i386-tdep.c
--- i386-tdep.c	3 Jul 2008 00:19:58 -0000	1.261
+++ i386-tdep.c	9 Aug 2008 11:56:44 -0000
@@ -518,7 +518,7 @@ struct i386_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[I386_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
-  int stack_align;
+  int saved_sp_reg;
   int pc_in_eax;
 
   /* Stack space reserved for local variables.  */
@@ -545,7 +545,7 @@ i386_alloc_frame_cache (void)
   for (i = 0; i < I386_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
-  cache->stack_align = 0;
+  cache->saved_sp_reg = -1;
   cache->pc_in_eax = 0;
 
   /* Frameless until proven otherwise.  */
@@ -707,37 +707,111 @@ static CORE_ADDR
 i386_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
 			  struct i386_frame_cache *cache)
 {
-  /* The register used by the compiler to perform the stack re-alignment 
-     is, in order of preference, either %ecx, %edx, or %eax.  GCC should
-     never use %ebx as it always treats it as callee-saved, whereas
-     the compiler can only use caller-saved registers.  */
-  static const gdb_byte insns_ecx[10] = { 
-    0x8d, 0x4c, 0x24, 0x04,	/* leal  4(%esp), %ecx */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x71, 0xfc		/* pushl -4(%ecx) */
-  };
-  static const gdb_byte insns_edx[10] = { 
-    0x8d, 0x54, 0x24, 0x04,	/* leal  4(%esp), %edx */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x72, 0xfc		/* pushl -4(%edx) */
-  };
-  static const gdb_byte insns_eax[10] = { 
-    0x8d, 0x44, 0x24, 0x04,	/* leal  4(%esp), %eax */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x70, 0xfc		/* pushl -4(%eax) */
+  /* There are 2 code sequences to re-align stack before the frame
+     gets set up:
+
+	1. Use a caller-saved saved register:
+
+		leal  4(%esp), %reg
+		andl  $-XXX, %esp
+		pushl -4(%reg)
+
+	2. Use a callee-saved saved register:
+
+		pushl %reg
+		leal  8(%esp), %reg
+		andl  $-XXX, %esp
+		pushl -4(%reg)
+
+     "andl $-XXX, %esp" can be either 3 bytes or 6 bytes:
+     
+     	0x83 0xe4 0xf0			andl $-16, %esp
+     	0x81 0xe4 0x00 0xff 0xff 0xff	andl $-256, %esp
+   */
+
+  gdb_byte buf[14];
+  int reg;
+  int offset, offset_and;
+  static int regnums[8] = {
+    I386_EAX_REGNUM,		/* %eax */
+    I386_ECX_REGNUM,		/* %ecx */
+    I386_EDX_REGNUM,		/* %edx */
+    I386_EBX_REGNUM,		/* %ebx */
+    I386_ESP_REGNUM,		/* %esp */
+    I386_EBP_REGNUM,		/* %ebp */
+    I386_ESI_REGNUM,		/* %esi */
+    I386_EDI_REGNUM		/* %edi */
   };
-  gdb_byte buf[10];
 
-  if (target_read_memory (pc, buf, sizeof buf)
-      || (memcmp (buf, insns_ecx, sizeof buf) != 0
-          && memcmp (buf, insns_edx, sizeof buf) != 0
-          && memcmp (buf, insns_eax, sizeof buf) != 0))
+  if (target_read_memory (pc, buf, sizeof buf))
+    return pc;
+
+  /* Check caller-saved saved register.  The first instruction has
+     to be "leal 4(%esp), %reg".  */
+  if (buf[0] == 0x8d && buf[2] == 0x24 && buf[3] == 0x4)
+    {
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[1] & 0xc7) != 0x44)
+	return pc;
+
+      /* REG has register number.  */
+      reg = (buf[1] >> 3) & 7;
+      offset = 4;
+    }
+  else
+    {
+      /* Check callee-saved saved register.  The first instruction
+	 has to be "pushl %reg".  */
+      if ((buf[0] & 0xf8) != 0x50)
+	return pc;
+
+      /* Get register.  */
+      reg = buf[0] & 0x7;
+
+      /* The next instruction has to be "leal 8(%esp), %reg".  */
+      if (buf[1] != 0x8d || buf[3] != 0x24 || buf[4] != 0x8)
+	return pc;
+
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[2] & 0xc7) != 0x44)
+	return pc;
+      
+      /* REG has register number.  Registers in pushl and leal have to
+	 be the same.  */
+      if (reg != ((buf[2] >> 3) & 7))
+	return pc;
+
+      offset = 5;
+    }
+
+  /* Rigister can't be %esp nor %ebp.  */
+  if (reg == 4 || reg == 5)
+    return pc;
+
+  /* The next instruction has to be "andl $-XXX, %esp".  */
+  if (buf[offset + 1] != 0xe4
+      || (buf[offset] != 0x81 && buf[offset] != 0x83))
+    return pc;
+
+  offset_and = offset;
+  offset += buf[offset] == 0x81 ? 6 : 3;
+
+  /* The next instruction has to be "pushl -4(%reg)".  8bit -4 is
+     0xfc.  REG must be binary 110 and MOD must be binary 01.  */
+  if (buf[offset] != 0xff
+      || buf[offset + 2] != 0xfc
+      || (buf[offset + 1] & 0xf8) != 0x70)
+    return pc;
+
+  /* R/M has register.  Registers in leal and pushl have to be the
+     same.  */
+  if (reg != (buf[offset + 1] & 7))
     return pc;
 
-  if (current_pc > pc + 4)
-    cache->stack_align = 1;
+  if (current_pc > pc + offset_and)
+    cache->saved_sp_reg = regnums[reg];
 
-  return min (pc + 10, current_pc);
+  return min (pc + offset + 3, current_pc);
 }
 
 /* Maximum instruction length we need to handle.  */
@@ -1241,10 +1315,10 @@ i386_frame_cache (struct frame_info *thi
   if (cache->pc != 0)
     i386_analyze_prologue (cache->pc, get_frame_pc (this_frame), cache);
 
-  if (cache->stack_align)
+  if (cache->saved_sp_reg != -1)
     {
-      /* Saved stack pointer has been saved in %ecx.  */
-      get_frame_register (this_frame, I386_ECX_REGNUM, buf);
+      /* Saved stack pointer has been saved.  */
+      get_frame_register (this_frame, cache->saved_sp_reg, buf);
       cache->saved_sp = extract_unsigned_integer(buf, 4);
     }
 
@@ -1258,7 +1332,7 @@ i386_frame_cache (struct frame_info *thi
 	 frame by looking at the stack pointer.  For truly "frameless"
 	 functions this might work too.  */
 
-      if (cache->stack_align)
+      if (cache->saved_sp_reg != -1)
 	{
 	  /* We're halfway aligning the stack.  */
 	  cache->base = ((cache->saved_sp - 4) & 0xfffffff0) - 4;

Re: PATCH: Update x86 stack align analyzer

[H.J. Lu]

On Sat, Aug 09, 2008 at 02:02:39PM +0200, Mark Kettenis wrote:
> I feel, rather strongly, that the stack align stuff stuff should only
> deal with stack alignment done *before* a frame gets setup.  I think
> setting saved_sp_reg to -1, and proceeding as usual if it is still set
> at that value after the stack alignment analysis, works just fine.  I
> also noticed a problem with your diff in that it doesn't use the
> proper saved sp value if you do have a frame (not sure GCC actually
> generates such code).
> 
> How about the attached diff?  Does it work with GCC 4.4?
> 
> 

Yes, it works.  OK to install?

Thanks.


H.J.
---
2008-08-09  Xuepeng Guo  <xuepeng.guo@intel.com>
	    H.J. Lu  <hongjiu.lu@intel.com>
	    Mark Kettenis <mark.kettenis@xs4all.nl>

	* amd64-tdep.c (amd64_frame_cache): Add saved_sp_reg.
	(amd64_init_frame_cache): Initialize saved_sp_reg.
	(amd64_analyze_stack_align): New.
	(amd64_analyze_prologue): Call it.
	(amd64_frame_cache): Use saved_sp_reg if it is invalid.  Don't set
	%rip to 8 when halfway aligning the stack.

	* amd64-tdep.h (amd64_regnum): Add AMD64_R9_REGNUM to
	AMD64_R14_REGNUM.

	* i386-tdep.c (i386_frame_cache): Remove stack_align.  Add
	saved_sp_reg.
	(i386_alloc_frame_cache): Remove stack_align.  Initialize
	saved_sp_reg to -1.
	(i386_analyze_stack_align): Rewrite.
	(i386_frame_cache): Use saved_sp_reg if it is valid.

Index: amd64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64-tdep.c,v
retrieving revision 1.52
diff -u -p -r1.52 amd64-tdep.c
--- amd64-tdep.c	16 May 2008 00:27:22 -0000	1.52
+++ amd64-tdep.c	9 Aug 2008 11:56:43 -0000
@@ -680,6 +680,7 @@ struct amd64_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[AMD64_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
+  int saved_sp_reg;
 
   /* Do we have a frame?  */
   int frameless_p;
@@ -702,6 +703,7 @@ amd64_init_frame_cache (struct amd64_fra
   for (i = 0; i < AMD64_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
+  cache->saved_sp_reg = -1;
 
   /* Frameless until proven otherwise.  */
   cache->frameless_p = 1;
@@ -719,6 +721,179 @@ amd64_alloc_frame_cache (void)
   return cache;
 }
 
+/* GCC 4.4 and later, can put code in the prologue to realign the
+   stack pointer.  Check whether PC points to such code, and update
+   CACHE accordingly.  Return the first instruction after the code
+   sequence or CURRENT_PC, whichever is smaller.  If we don't
+   recognize the code, return PC.  */
+
+static CORE_ADDR
+amd64_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
+			   struct amd64_frame_cache *cache)
+{
+  /* There are 2 code sequences to re-align stack before the frame
+     gets set up:
+
+	1. Use a caller-saved saved register:
+
+		leaq  8(%rsp), %reg
+		andq  $-XXX, %rsp
+		pushq -8(%reg)
+
+	2. Use a callee-saved saved register:
+
+		pushq %reg
+		leaq  16(%rsp), %reg
+		andq  $-XXX, %rsp
+		pushq -8(%reg)
+
+     "andq $-XXX, %rsp" can be either 4 bytes or 7 bytes:
+     
+     	0x48 0x83 0xe4 0xf0			andq $-16, %rsp
+     	0x48 0x81 0xe4 0x00 0xff 0xff 0xff	andq $-256, %rsp
+   */
+
+  gdb_byte buf[18];
+  int reg, r;
+  int offset, offset_and;
+  static int regnums[16] = {
+    AMD64_RAX_REGNUM,		/* %rax */
+    AMD64_RCX_REGNUM,		/* %rcx */
+    AMD64_RDX_REGNUM,		/* %rdx */
+    AMD64_RBX_REGNUM,		/* %rbx */
+    AMD64_RSP_REGNUM,		/* %rsp */
+    AMD64_RBP_REGNUM,		/* %rbp */
+    AMD64_RSI_REGNUM,		/* %rsi */
+    AMD64_RDI_REGNUM,		/* %rdi */
+    AMD64_R8_REGNUM,		/* %r8 */
+    AMD64_R9_REGNUM,		/* %r9 */
+    AMD64_R10_REGNUM,		/* %r10 */
+    AMD64_R11_REGNUM,		/* %r11 */
+    AMD64_R12_REGNUM,		/* %r12 */
+    AMD64_R13_REGNUM,		/* %r13 */
+    AMD64_R14_REGNUM,		/* %r14 */
+    AMD64_R15_REGNUM,		/* %r15 */
+  };
+
+  if (target_read_memory (pc, buf, sizeof buf))
+    return pc;
+
+  /* Check caller-saved saved register.  The first instruction has
+     to be "leaq 8(%rsp), %reg".  */
+  if ((buf[0] & 0xfb) == 0x48
+      && buf[1] == 0x8d
+      && buf[3] == 0x24
+      && buf[4] == 0x8)
+    {
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[2] & 0xc7) != 0x44)
+	return pc;
+
+      /* REG has register number.  */
+      reg = (buf[2] >> 3) & 7;
+
+      /* Check the REX.R bit.  */
+      if (buf[0] == 0x4c)
+	reg += 8;
+
+      offset = 5;
+    }
+  else
+    {
+      /* Check callee-saved saved register.  The first instruction
+	 has to be "pushq %reg".  */
+      reg = 0;
+      if ((buf[0] & 0xf8) == 0x50)
+	offset = 0;
+      else if ((buf[0] & 0xf6) == 0x40
+	       && (buf[1] & 0xf8) == 0x50)
+	{
+	  /* Check the REX.B bit.  */
+	  if ((buf[0] & 1) != 0)
+	    reg = 8;
+
+	  offset = 1;
+	}
+      else
+	return pc;
+
+      /* Get register.  */
+      reg += buf[offset] & 0x7;
+
+      offset++;
+
+      /* The next instruction has to be "leaq 16(%rsp), %reg".  */
+      if ((buf[offset] & 0xfb) != 0x48
+	  || buf[offset + 1] != 0x8d
+	  || buf[offset + 3] != 0x24
+	  || buf[offset + 4] != 0x10)
+	return pc;
+
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[offset + 2] & 0xc7) != 0x44)
+	return pc;
+      
+      /* REG has register number.  */
+      r = (buf[offset + 2] >> 3) & 7;
+
+      /* Check the REX.R bit.  */
+      if (buf[offset] == 0x4c)
+	r += 8;
+
+      /* Registers in pushq and leaq have to be the same.  */
+      if (reg != r)
+	return pc;
+
+      offset += 5;
+    }
+
+  /* Rigister can't be %rsp nor %rbp.  */
+  if (reg == 4 || reg == 5)
+    return pc;
+
+  /* The next instruction has to be "andq $-XXX, %rsp".  */
+  if (buf[offset] != 0x48
+      || buf[offset + 2] != 0xe4
+      || (buf[offset + 1] != 0x81 && buf[offset + 1] != 0x83))
+    return pc;
+
+  offset_and = offset;
+  offset += buf[offset + 1] == 0x81 ? 7 : 4;
+
+  /* The next instruction has to be "pushq -8(%reg)".  */
+  r = 0;
+  if (buf[offset] == 0xff)
+    offset++;
+  else if ((buf[offset] & 0xf6) == 0x40
+	   && buf[offset + 1] == 0xff)
+    {
+      /* Check the REX.B bit.  */
+      if ((buf[offset] & 0x1) != 0)
+	r = 8;
+      offset += 2;
+    }
+  else
+    return pc;
+
+  /* 8bit -8 is 0xf8.  REG must be binary 110 and MOD must be binary
+     01.  */
+  if (buf[offset + 1] != 0xf8
+      || (buf[offset] & 0xf8) != 0x70)
+    return pc;
+
+  /* R/M has register.  */
+  r += buf[offset] & 7;
+
+  /* Registers in leaq and pushq have to be the same.  */
+  if (reg != r)
+    return pc;
+
+  if (current_pc > pc + offset_and)
+    cache->saved_sp_reg = regnums[reg];
+
+  return min (pc + offset + 2, current_pc);
+}
+
 /* Do a limited analysis of the prologue at PC and update CACHE
    accordingly.  Bail out early if CURRENT_PC is reached.  Return the
    address where the analysis stopped.
@@ -742,6 +917,8 @@ amd64_analyze_prologue (CORE_ADDR pc, CO
   if (current_pc <= pc)
     return current_pc;
 
+  pc = amd64_analyze_stack_align (pc, current_pc, cache);
+
   op = read_memory_unsigned_integer (pc, 1);
 
   if (op == 0x55)		/* pushq %rbp */
@@ -804,6 +981,13 @@ amd64_frame_cache (struct frame_info *th
   if (cache->pc != 0)
     amd64_analyze_prologue (cache->pc, get_frame_pc (this_frame), cache);
 
+  if (cache->saved_sp_reg != -1)
+    {
+      /* Stack pointer has been saved.  */
+      get_frame_register (this_frame, cache->saved_sp_reg, buf);
+      cache->saved_sp = extract_unsigned_integer(buf, 8);
+    }
+
   if (cache->frameless_p)
     {
       /* We didn't find a valid frame.  If we're at the start of a
@@ -813,8 +997,20 @@ amd64_frame_cache (struct frame_info *th
 	 at the stack pointer.  For truly "frameless" functions this
 	 might work too.  */
 
-      get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
-      cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
+      if (cache->saved_sp_reg != -1)
+	{
+	  /* We're halfway aligning the stack.  */
+	  cache->base = ((cache->saved_sp - 8) & 0xfffffffffffffff0LL) - 8;
+	  cache->saved_regs[AMD64_RIP_REGNUM] = cache->saved_sp - 8;
+
+	  /* This will be added back below.  */
+	  cache->saved_regs[AMD64_RIP_REGNUM] -= cache->base;
+	}
+      else
+	{
+	  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+	  cache->base = extract_unsigned_integer (buf, 8) + cache->sp_offset;
+	}
     }
   else
     {
@@ -828,8 +1024,10 @@ amd64_frame_cache (struct frame_info *th
 
   /* For normal frames, %rip is stored at 8(%rbp).  If we don't have a
      frame we find it at the same offset from the reconstructed base
-     address.  */
-  cache->saved_regs[AMD64_RIP_REGNUM] = 8;
+     address.  If we're halfway aligning the stack, %rip is handled
+     differently (see above).  */
+  if (!cache->frameless_p || cache->saved_sp_reg == -1)
+    cache->saved_regs[AMD64_RIP_REGNUM] = 8;
 
   /* Adjust all the saved registers such that they contain addresses
      instead of offsets.  */
Index: amd64-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/amd64-tdep.h,v
retrieving revision 1.11
diff -u -p -r1.11 amd64-tdep.h
--- amd64-tdep.h	1 Jan 2008 22:53:09 -0000	1.11
+++ amd64-tdep.h	9 Aug 2008 11:56:43 -0000
@@ -39,8 +39,14 @@ enum amd64_regnum
   AMD64_RDI_REGNUM,		/* %rdi */
   AMD64_RBP_REGNUM,		/* %rbp */
   AMD64_RSP_REGNUM,		/* %rsp */
-  AMD64_R8_REGNUM = 8,		/* %r8 */
-  AMD64_R15_REGNUM = 15,	/* %r15 */
+  AMD64_R8_REGNUM,		/* %r8 */
+  AMD64_R9_REGNUM,		/* %r9 */
+  AMD64_R10_REGNUM,		/* %r10 */
+  AMD64_R11_REGNUM,		/* %r11 */
+  AMD64_R12_REGNUM,		/* %r12 */
+  AMD64_R13_REGNUM,		/* %r13 */
+  AMD64_R14_REGNUM,		/* %r14 */
+  AMD64_R15_REGNUM,		/* %r15 */
   AMD64_RIP_REGNUM,		/* %rip */
   AMD64_EFLAGS_REGNUM,		/* %eflags */
   AMD64_CS_REGNUM,		/* %cs */
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.261
diff -u -p -r1.261 i386-tdep.c
--- i386-tdep.c	3 Jul 2008 00:19:58 -0000	1.261
+++ i386-tdep.c	9 Aug 2008 11:56:44 -0000
@@ -518,7 +518,7 @@ struct i386_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[I386_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
-  int stack_align;
+  int saved_sp_reg;
   int pc_in_eax;
 
   /* Stack space reserved for local variables.  */
@@ -545,7 +545,7 @@ i386_alloc_frame_cache (void)
   for (i = 0; i < I386_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
-  cache->stack_align = 0;
+  cache->saved_sp_reg = -1;
   cache->pc_in_eax = 0;
 
   /* Frameless until proven otherwise.  */
@@ -707,37 +707,111 @@ static CORE_ADDR
 i386_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
 			  struct i386_frame_cache *cache)
 {
-  /* The register used by the compiler to perform the stack re-alignment 
-     is, in order of preference, either %ecx, %edx, or %eax.  GCC should
-     never use %ebx as it always treats it as callee-saved, whereas
-     the compiler can only use caller-saved registers.  */
-  static const gdb_byte insns_ecx[10] = { 
-    0x8d, 0x4c, 0x24, 0x04,	/* leal  4(%esp), %ecx */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x71, 0xfc		/* pushl -4(%ecx) */
-  };
-  static const gdb_byte insns_edx[10] = { 
-    0x8d, 0x54, 0x24, 0x04,	/* leal  4(%esp), %edx */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x72, 0xfc		/* pushl -4(%edx) */
-  };
-  static const gdb_byte insns_eax[10] = { 
-    0x8d, 0x44, 0x24, 0x04,	/* leal  4(%esp), %eax */
-    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
-    0xff, 0x70, 0xfc		/* pushl -4(%eax) */
+  /* There are 2 code sequences to re-align stack before the frame
+     gets set up:
+
+	1. Use a caller-saved saved register:
+
+		leal  4(%esp), %reg
+		andl  $-XXX, %esp
+		pushl -4(%reg)
+
+	2. Use a callee-saved saved register:
+
+		pushl %reg
+		leal  8(%esp), %reg
+		andl  $-XXX, %esp
+		pushl -4(%reg)
+
+     "andl $-XXX, %esp" can be either 3 bytes or 6 bytes:
+     
+     	0x83 0xe4 0xf0			andl $-16, %esp
+     	0x81 0xe4 0x00 0xff 0xff 0xff	andl $-256, %esp
+   */
+
+  gdb_byte buf[14];
+  int reg;
+  int offset, offset_and;
+  static int regnums[8] = {
+    I386_EAX_REGNUM,		/* %eax */
+    I386_ECX_REGNUM,		/* %ecx */
+    I386_EDX_REGNUM,		/* %edx */
+    I386_EBX_REGNUM,		/* %ebx */
+    I386_ESP_REGNUM,		/* %esp */
+    I386_EBP_REGNUM,		/* %ebp */
+    I386_ESI_REGNUM,		/* %esi */
+    I386_EDI_REGNUM		/* %edi */
   };
-  gdb_byte buf[10];
 
-  if (target_read_memory (pc, buf, sizeof buf)
-      || (memcmp (buf, insns_ecx, sizeof buf) != 0
-          && memcmp (buf, insns_edx, sizeof buf) != 0
-          && memcmp (buf, insns_eax, sizeof buf) != 0))
+  if (target_read_memory (pc, buf, sizeof buf))
+    return pc;
+
+  /* Check caller-saved saved register.  The first instruction has
+     to be "leal 4(%esp), %reg".  */
+  if (buf[0] == 0x8d && buf[2] == 0x24 && buf[3] == 0x4)
+    {
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[1] & 0xc7) != 0x44)
+	return pc;
+
+      /* REG has register number.  */
+      reg = (buf[1] >> 3) & 7;
+      offset = 4;
+    }
+  else
+    {
+      /* Check callee-saved saved register.  The first instruction
+	 has to be "pushl %reg".  */
+      if ((buf[0] & 0xf8) != 0x50)
+	return pc;
+
+      /* Get register.  */
+      reg = buf[0] & 0x7;
+
+      /* The next instruction has to be "leal 8(%esp), %reg".  */
+      if (buf[1] != 0x8d || buf[3] != 0x24 || buf[4] != 0x8)
+	return pc;
+
+      /* MOD must be binary 10 and R/M must be binary 100.  */
+      if ((buf[2] & 0xc7) != 0x44)
+	return pc;
+      
+      /* REG has register number.  Registers in pushl and leal have to
+	 be the same.  */
+      if (reg != ((buf[2] >> 3) & 7))
+	return pc;
+
+      offset = 5;
+    }
+
+  /* Rigister can't be %esp nor %ebp.  */
+  if (reg == 4 || reg == 5)
+    return pc;
+
+  /* The next instruction has to be "andl $-XXX, %esp".  */
+  if (buf[offset + 1] != 0xe4
+      || (buf[offset] != 0x81 && buf[offset] != 0x83))
+    return pc;
+
+  offset_and = offset;
+  offset += buf[offset] == 0x81 ? 6 : 3;
+
+  /* The next instruction has to be "pushl -4(%reg)".  8bit -4 is
+     0xfc.  REG must be binary 110 and MOD must be binary 01.  */
+  if (buf[offset] != 0xff
+      || buf[offset + 2] != 0xfc
+      || (buf[offset + 1] & 0xf8) != 0x70)
+    return pc;
+
+  /* R/M has register.  Registers in leal and pushl have to be the
+     same.  */
+  if (reg != (buf[offset + 1] & 7))
     return pc;
 
-  if (current_pc > pc + 4)
-    cache->stack_align = 1;
+  if (current_pc > pc + offset_and)
+    cache->saved_sp_reg = regnums[reg];
 
-  return min (pc + 10, current_pc);
+  return min (pc + offset + 3, current_pc);
 }
 
 /* Maximum instruction length we need to handle.  */
@@ -1241,10 +1315,10 @@ i386_frame_cache (struct frame_info *thi
   if (cache->pc != 0)
     i386_analyze_prologue (cache->pc, get_frame_pc (this_frame), cache);
 
-  if (cache->stack_align)
+  if (cache->saved_sp_reg != -1)
     {
-      /* Saved stack pointer has been saved in %ecx.  */
-      get_frame_register (this_frame, I386_ECX_REGNUM, buf);
+      /* Saved stack pointer has been saved.  */
+      get_frame_register (this_frame, cache->saved_sp_reg, buf);
       cache->saved_sp = extract_unsigned_integer(buf, 4);
     }
 
@@ -1258,7 +1332,7 @@ i386_frame_cache (struct frame_info *thi
 	 frame by looking at the stack pointer.  For truly "frameless"
 	 functions this might work too.  */
 
-      if (cache->stack_align)
+      if (cache->saved_sp_reg != -1)
 	{
 	  /* We're halfway aligning the stack.  */
 	  cache->base = ((cache->saved_sp - 4) & 0xfffffff0) - 4;

Re: PATCH: Update x86 stack align analyzer

[Mark Kettenis]

> Date: Sat, 9 Aug 2008 07:51:47 -0700
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> On Sat, Aug 09, 2008 at 02:02:39PM +0200, Mark Kettenis wrote:
> > I feel, rather strongly, that the stack align stuff stuff should only
> > deal with stack alignment done *before* a frame gets setup.  I think
> > setting saved_sp_reg to -1, and proceeding as usual if it is still set
> > at that value after the stack alignment analysis, works just fine.  I
> > also noticed a problem with your diff in that it doesn't use the
> > proper saved sp value if you do have a frame (not sure GCC actually
> > generates such code).
> > 
> > How about the attached diff?  Does it work with GCC 4.4?
> > 
> > 
> 
> Yes, it works.  OK to install?

Sure, but can you change the ChangeLog to use kettenis@gnu.org as my
mail-address?

> 2008-08-09  Xuepeng Guo  <xuepeng.guo@intel.com>
> 	    H.J. Lu  <hongjiu.lu@intel.com>
> 	    Mark Kettenis <mark.kettenis@xs4all.nl>
> 
> 	* amd64-tdep.c (amd64_frame_cache): Add saved_sp_reg.
> 	(amd64_init_frame_cache): Initialize saved_sp_reg.
> 	(amd64_analyze_stack_align): New.
> 	(amd64_analyze_prologue): Call it.
> 	(amd64_frame_cache): Use saved_sp_reg if it is invalid.  Don't set
> 	%rip to 8 when halfway aligning the stack.
> 
> 	* amd64-tdep.h (amd64_regnum): Add AMD64_R9_REGNUM to
> 	AMD64_R14_REGNUM.
> 
> 	* i386-tdep.c (i386_frame_cache): Remove stack_align.  Add
> 	saved_sp_reg.
> 	(i386_alloc_frame_cache): Remove stack_align.  Initialize
> 	saved_sp_reg to -1.
> 	(i386_analyze_stack_align): Rewrite.
> 	(i386_frame_cache): Use saved_sp_reg if it is valid.