[RFC] Handle prologue on atom.

[RFC] Handle prologue on atom.

[Yao Qi]

Hi,
I am looking at gdb testsuite fails when test cases are compiled with
-march=atom.  They are caused by gdb is unable to recognize prologue.
Usually, the i386 prologue looks like

  push   %ebp
  mov    %esp,%ebp
  sub    $0x10,%esp

and gdb can skip it correctly.  However, in my test , gcc (based on
4.7.2, with flag -march=atom) generates prologue that gdb doesn't
understand,

$ ./install/bin/i686-pc-linux-gnu-gcc -g  -march=atom gdb/testsuite/gdb.base/cursal.c -o cursal
$ ./install/bin/i686-pc-linux-gnu-gdb ./cursal
(gdb) disassemble /r main
Dump of assembler code for function main:
   0x0804841b <+0>:	55	push   %ebp
   0x0804841c <+1>:	8d 2c 24	lea    (%esp),%ebp
   0x0804841f <+4>:	8d 64 24 f0	lea    -0x10(%esp),%esp

   0x08048423 <+8>:	c7 45 fc 00 00 00 00	movl   $0x0,-0x4(%ebp)
   0x0804842a <+15>:	e8 e2 ff ff ff	call   0x8048411 <func1>
   0x0804842f <+20>:	c9	leave
   0x08048430 <+21>:	c3	ret

Looks gcc prefers lea for add/sub operation, probably done by this gcc
patch,

  PATCH: Add Intel Atom optimization
  http://gcc.gnu.org/ml/gcc-patches/2009-03/msg01722.html

In "12.3.2.2 Address Generation" of "Intel 64 and IA-32
Architectures Optimization Reference Manual", there are some
description on using LEA.

This patch teaches gdb to understand prologue for atom, and it fixes
many testsuite fails.  Regression tested on host i686-pc-linux-gnu
with different compilation options to test cases
(default/m64/-march=atom).  OK to apply?

gdb:

2012-10-18  Yao Qi  <yao@codesourcery.com>

	* i386-tdep.c (i386_analyze_frame_setup): Handle opcode 0x8d (lea).
---
 gdb/i386-tdep.c |   38 +++++++++++++++++++++++++++++++++++---
 1 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 84e9794..27e00d9 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -1388,18 +1388,40 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
       if (target_read_memory (pc + skip, &op, 1))
 	return pc + skip;
 
-      /* Check for `movl %esp, %ebp' -- can be written in two ways.  */
+      /* The i386 prologue looks like
+
+	 push   %ebp
+	 mov    %esp,%ebp
+	 sub    $0x10,%esp
+
+	 and a different prologue can be generated for atom.
+
+	 push   %ebp
+	 lea    (%esp),%ebp
+	 lea    -0x10(%esp),%esp
+
+	 We handle both of them here.  */
+
       switch (op)
 	{
+	  /* Check for `movl %esp, %ebp' -- can be written in two ways.  */
 	case 0x8b:
 	  if (read_memory_unsigned_integer (pc + skip + 1, 1, byte_order)
 	      != 0xec)
 	    return pc;
+	  pc += (skip + 2);
 	  break;
 	case 0x89:
 	  if (read_memory_unsigned_integer (pc + skip + 1, 1, byte_order)
 	      != 0xe5)
 	    return pc;
+	  pc += (skip + 2);
+	  break;
+	case 0x8d: /* Check for 'lea (%ebp), %ebp'.  */
+	  if (read_memory_unsigned_integer (pc + skip + 1, 2, byte_order)
+	      != 0x242c)
+	    return pc;
+	  pc += (skip + 3);
 	  break;
 	default:
 	  return pc;
@@ -1410,7 +1432,6 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
 	 necessary.  We also now commit to skipping the special
 	 instructions mentioned before.  */
       cache->locals = 0;
-      pc += (skip + 2);
 
       /* If that's all, return now.  */
       if (limit <= pc)
@@ -1419,6 +1440,8 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
       /* Check for stack adjustment 
 
 	    subl $XXX, %esp
+	 or
+	    lea -XXX(%esp),%esp
 
 	 NOTE: You can't subtract a 16-bit immediate from a 32-bit
 	 reg, so we don't have to worry about a data16 prefix.  */
@@ -1447,9 +1470,18 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
 	  cache->locals = read_memory_integer (pc + 2, 4, byte_order);
 	  return pc + 6;
 	}
+      else if (op == 0x8d)
+	{
+	  /* The ModR/M byte is 0x64.  */
+	  if (read_memory_unsigned_integer (pc + 1, 1, byte_order) != 0x64)
+	    return pc;
+	  /* 'lea' with 8-bit displacement.  */
+	  cache->locals = -1 * read_memory_integer (pc + 3, 1, byte_order);
+	  return pc + 4;
+	}
       else
 	{
-	  /* Some instruction other than `subl'.  */
+	  /* Some instruction other than `subl' nor 'lea'.  */
 	  return pc;
 	}
     }
-- 
1.7.7.6

Re: [RFC] Handle prologue on atom.

[Yao Qi]

On 10/18/2012 10:52 AM, Yao Qi wrote:
> gdb:
>
> 2012-10-18  Yao Qi<yao@codesourcery.com>
>
> 	* i386-tdep.c (i386_analyze_frame_setup): Handle opcode 0x8d (lea).

Ping.  http://sourceware.org/ml/gdb-patches/2012-10/msg00311.html

-- 
Yao

Re: [RFC] Handle prologue on atom.

[Mark Kettenis]

> From: Yao Qi <yao@codesourcery.com>
> Date: Thu, 18 Oct 2012 10:52:25 +0800
> 
> Hi,
> I am looking at gdb testsuite fails when test cases are compiled with
> -march=atom.  They are caused by gdb is unable to recognize prologue.
> Usually, the i386 prologue looks like
> 
>   push   %ebp
>   mov    %esp,%ebp
>   sub    $0x10,%esp
> 
> and gdb can skip it correctly.  However, in my test , gcc (based on
> 4.7.2, with flag -march=atom) generates prologue that gdb doesn't
> understand,

Thanks for the reminder Yao.  The code looks ok, but the comments need
a bit more work.  I'll try to come up with a better wording, but
unfortunately this week is extremely busy for me, so it'll probably be
next week before I'll get back to you.

> $ ./install/bin/i686-pc-linux-gnu-gcc -g  -march=atom gdb/testsuite/gdb.base/cursal.c -o cursal
> $ ./install/bin/i686-pc-linux-gnu-gdb ./cursal
> (gdb) disassemble /r main
> Dump of assembler code for function main:
>    0x0804841b <+0>:	55	push   %ebp
>    0x0804841c <+1>:	8d 2c 24	lea    (%esp),%ebp
>    0x0804841f <+4>:	8d 64 24 f0	lea    -0x10(%esp),%esp
> 
>    0x08048423 <+8>:	c7 45 fc 00 00 00 00	movl   $0x0,-0x4(%ebp)
>    0x0804842a <+15>:	e8 e2 ff ff ff	call   0x8048411 <func1>
>    0x0804842f <+20>:	c9	leave
>    0x08048430 <+21>:	c3	ret
> 
> Looks gcc prefers lea for add/sub operation, probably done by this gcc
> patch,
> 
>   PATCH: Add Intel Atom optimization
>   http://gcc.gnu.org/ml/gcc-patches/2009-03/msg01722.html
> 
> In "12.3.2.2 Address Generation" of "Intel 64 and IA-32
> Architectures Optimization Reference Manual", there are some
> description on using LEA.
> 
> This patch teaches gdb to understand prologue for atom, and it fixes
> many testsuite fails.  Regression tested on host i686-pc-linux-gnu
> with different compilation options to test cases
> (default/m64/-march=atom).  OK to apply?
> 
> gdb:
> 
> 2012-10-18  Yao Qi  <yao@codesourcery.com>
> 
> 	* i386-tdep.c (i386_analyze_frame_setup): Handle opcode 0x8d (lea).
> ---
>  gdb/i386-tdep.c |   38 +++++++++++++++++++++++++++++++++++---
>  1 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 84e9794..27e00d9 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -1388,18 +1388,40 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
>        if (target_read_memory (pc + skip, &op, 1))
>  	return pc + skip;
>  
> -      /* Check for `movl %esp, %ebp' -- can be written in two ways.  */
> +      /* The i386 prologue looks like
> +
> +	 push   %ebp
> +	 mov    %esp,%ebp
> +	 sub    $0x10,%esp
> +
> +	 and a different prologue can be generated for atom.
> +
> +	 push   %ebp
> +	 lea    (%esp),%ebp
> +	 lea    -0x10(%esp),%esp
> +
> +	 We handle both of them here.  */
> +
>        switch (op)
>  	{
> +	  /* Check for `movl %esp, %ebp' -- can be written in two ways.  */
>  	case 0x8b:
>  	  if (read_memory_unsigned_integer (pc + skip + 1, 1, byte_order)
>  	      != 0xec)
>  	    return pc;
> +	  pc += (skip + 2);
>  	  break;
>  	case 0x89:
>  	  if (read_memory_unsigned_integer (pc + skip + 1, 1, byte_order)
>  	      != 0xe5)
>  	    return pc;
> +	  pc += (skip + 2);
> +	  break;
> +	case 0x8d: /* Check for 'lea (%ebp), %ebp'.  */
> +	  if (read_memory_unsigned_integer (pc + skip + 1, 2, byte_order)
> +	      != 0x242c)
> +	    return pc;
> +	  pc += (skip + 3);
>  	  break;
>  	default:
>  	  return pc;
> @@ -1410,7 +1432,6 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
>  	 necessary.  We also now commit to skipping the special
>  	 instructions mentioned before.  */
>        cache->locals = 0;
> -      pc += (skip + 2);
>  
>        /* If that's all, return now.  */
>        if (limit <= pc)
> @@ -1419,6 +1440,8 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
>        /* Check for stack adjustment 
>  
>  	    subl $XXX, %esp
> +	 or
> +	    lea -XXX(%esp),%esp
>  
>  	 NOTE: You can't subtract a 16-bit immediate from a 32-bit
>  	 reg, so we don't have to worry about a data16 prefix.  */
> @@ -1447,9 +1470,18 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
>  	  cache->locals = read_memory_integer (pc + 2, 4, byte_order);
>  	  return pc + 6;
>  	}
> +      else if (op == 0x8d)
> +	{
> +	  /* The ModR/M byte is 0x64.  */
> +	  if (read_memory_unsigned_integer (pc + 1, 1, byte_order) != 0x64)
> +	    return pc;
> +	  /* 'lea' with 8-bit displacement.  */
> +	  cache->locals = -1 * read_memory_integer (pc + 3, 1, byte_order);
> +	  return pc + 4;
> +	}
>        else
>  	{
> -	  /* Some instruction other than `subl'.  */
> +	  /* Some instruction other than `subl' nor 'lea'.  */
>  	  return pc;
>  	}
>      }
> -- 
> 1.7.7.6
> 
> 

Re: [RFC] Handle prologue on atom.

[Yao Qi]

On 10/30/2012 06:41 PM, Mark Kettenis wrote:
> Thanks for the reminder Yao.  The code looks ok, but the comments need
> a bit more work.  I'll try to come up with a better wording, but
> unfortunately this week is extremely busy for me, so it'll probably be
> next week before I'll get back to you.

Mark, thanks for the update.

-- 
Yao

Re: [RFC] Handle prologue on atom.

[Yao Qi]

On 10/30/2012 06:41 PM, Mark Kettenis wrote:
> Thanks for the reminder Yao.  The code looks ok, but the comments need
> a bit more work.  I'll try to come up with a better wording, but
> unfortunately this week is extremely busy for me, so it'll probably be
> next week before I'll get back to you.

Mark, do you have a chance to refine the comments?  This patch fixes a 
huge number of fails of test on atom, so I'd like to get it in as early 
as we can.

-- 
Yao

Re: [RFC] Handle prologue on atom.

[Mark Kettenis]

> Date: Fri, 9 Nov 2012 09:53:33 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> X-OriginalArrivalTime: 09 Nov 2012 01:53:47.0866 (UTC) FILETIME=[0F00EBA0:01CDBE1D]
> X-XS4ALL-DNSBL-Checked: mxdrop212.xs4all.nl checked 192.94.38.131 against DNS blacklists
> X-CNFS-Analysis: v=2.0 cv=L5WqtZv8 c=1 sm=0 a=1K+y54S8amjD3nS2OSvmyw==:17
> 	a=8umaBx2KoyQA:10 a=4S5AUSBz4KsA:10 a=zbTXIsBlecMA:10
> 	a=8nJEP1OIZ-IA:10 a=lzDhS8hmAAAA:8 a=aKXOph7d5iEA:10
> 	a=a4xV_u6PtzGf4RH-6koA:9 a=wPNLvfGTeEIA:10
> 	a=1K+y54S8amjD3nS2OSvmyw==:117
> X-Virus-Scanned: by XS4ALL Virus Scanner
> X-XS4ALL-Spam-Score: 0.0 () none
> X-XS4ALL-Spam: NO
> Envelope-To: mark.kettenis@xs4all.nl
> 
> On 10/30/2012 06:41 PM, Mark Kettenis wrote:
> > Thanks for the reminder Yao.  The code looks ok, but the comments need
> > a bit more work.  I'll try to come up with a better wording, but
> > unfortunately this week is extremely busy for me, so it'll probably be
> > next week before I'll get back to you.
> 
> Mark, do you have a chance to refine the comments?  This patch fixes a 
> huge number of fails of test on atom, so I'd like to get it in as early 
> as we can.

Bleah, no.  If you don't mind me reorganizing the comments afterward,
just go ahead and commit.  The code is all good.

Re: [RFC] Handle prologue on atom.

[Yao Qi]

On 11/09/2012 06:03 PM, Mark Kettenis wrote:
> Bleah, no.  If you don't mind me reorganizing the comments afterward,
> just go ahead and commit.  The code is all good.

I don't mind at all.  Patch is committed.  Thanks for the review, Mark.

-- 
Yao