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

[Yao Qi <qiyaoltc@gmail.com>]

On 16-12-01 15:16:44, Bernhard Heckel wrote:
> Some compiler decrement stack pointer within the prologue

As Luis reviewed, it is clear to mention the name of the compilers
here.

> sequence in order to reserve memory for local variables.
> Recognize this subtraction to stop at the very end of the
> prologue.
> 
> 2016-10-20  Bernhard Heckel  <bernhard.heckel@intel.com>
> 
> gdb/Changelog:
> 	amd64-tdep.c (amd64_analyze_prologue): Recognize stack decrementation

File name should be started with "*".

> 	as prologue operation.
> 
> ---
>  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 };
>  
>    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

Please mention the name of the compilers here.

> +	     to reserve memory for local variables.
> +	     Two common variants exist to do so.  */
> +	  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

Likewise.

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

Could you add a unit test for this new prologue sequences?  Like what we
did in this patch
https://sourceware.org/ml/gdb-patches/2016-12/msg00071.html  Both x86_64
and x32 prologue should be tested, because your patch is for both of
them.

-- 
Yao