Re: [PATCH] AMD64, Prologue: Recognize stack decrementation as prologue operation.

[Luis Machado <lgustavo@codesourcery.com>]

On 12/01/2016 08:16 AM, Bernhard Heckel wrote:
> Some compiler decrement stack pointer within the prologue
> sequence in order to reserve memory for local variables.
> Recognize this subtraction to stop at the very end of the
> prologue.

I suppose this was exercised with GCC as well via the testsuite?

>
> 2016-10-20  Bernhard Heckel  <bernhard.heckel@intel.com>
>
> gdb/Changelog:
> 	amd64-tdep.c (amd64_analyze_prologue): Recognize stack decrementation
> 	as prologue operation.

gdb/ChangeLog above the date line, adjust date and add "*" before the 
filename.

>
> ---
>  gdb/amd64-tdep.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index a3a1fde..795d78e 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -2283,6 +2283,12 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
>    /* 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 };
> +  /* Ditto for subtraction on the stack pointer.  */
> +  static const gdb_byte sub_rsp_imm8[3] = { 0x48, 0x83, 0xec };
> +  static const gdb_byte sub_rsp_imm32[3] = { 0x48, 0x81, 0xec };
> +  /* Ditto for subtraction on the stack pointer.  */
> +  static const gdb_byte sub_esp_imm8[2] = { 0x83, 0xec };
> +  static const gdb_byte sub_esp_imm32[2] = { 0x81, 0xec };

Should we add a comment making it explicit which instruction patterns 
we're looking at matching here?

I looked up sub esp imm32, for example, and i got no meaningful hits 
other than some nasm posix entry.

>
>    gdb_byte buf[3];
>    gdb_byte op;
> @@ -2316,6 +2322,18 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
>  	{
>  	  /* OK, we actually have a frame.  */
>  	  cache->frameless_p = 0;
> +
> +	  /* Some compiler do subtraction on the stack pointer
> +	     to reserve memory for local variables.
> +	     Two common variants exist to do so.  */

What compiler exactly? Would be nice to know, otherwise this is a bit vague.

The comment seems to imply a specific compiler does this, or did you 
mean "some compilers"?

> +	  read_code (pc + 4, buf, 3);
> +	  if (memcmp (buf, sub_rsp_imm8, 3) == 0)
> +	    /* Operand is 1 byte.  */
> +	    return pc + 8;
> +	  else if (memcmp (buf, sub_rsp_imm32, 3) == 0)
> +	    /* Operand is 4 bytes.  */
> +	    return pc + 11;
> +
>  	  return pc + 4;
>  	}
>
> @@ -2327,6 +2345,18 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
>  	    {
>  	      /* OK, we actually have a frame.  */
>  	      cache->frameless_p = 0;
> +
> +	      /* Some compiler do subtraction on the stack pointer
> +		 to reserve memory for local variables.
> +		 Two common variants exist to do so.  */
> +	      read_code (pc + 3, buf, 2);
> +	      if (memcmp (buf, sub_esp_imm8, 2) == 0)
> +		/* Operand is 1 byte.  */
> +		return pc + 6;
> +	      else if (memcmp (buf, sub_esp_imm32, 2) == 0)
> +		/* Operand is 4 bytes.  */
> +		return pc + 9;
> +
>  	      return pc + 3;
>  	    }
>  	}
>

Otherwise LGTM.