Re: [PATCH] Partial fix for PR backtrace/1718

["Eli Zaretskii" <eliz at gnu dot org>]

> Date: Fri, 30 Jul 2004 22:07:56 +0200 (CEST)
> From: Mark Kettenis <kettenis@chello.nl>
> 
> Oops sorry, no that slipped through.  AFAICT the patch should work
> with the GDB 6.1 release.  So there must be something else wrong.
> Unforunately I can't really help you here.  Can you try to debug this
> yourself.  The question here is why i386_analyze_prologue() which is
> called from i386_frame_cache() isn't properly detecting the frame.

I debugged this.  The reason it doesn't work (and AFAICT shouldn't
work for anyone else on any i386 platform) is that, at least with GCC,
the opcode produced by "mov 0x375aa0,%eax" and its ilk is _not_ 0xb8
to 0xba, but rather 0xa1 for a mov to EAX and 0x8b for other
registers.  Obviously, GAS somehow produces a different opcode when
invoked for asm blocks in C code, as you did in i386-prologue.c in the
test suite, and for code produced from plain C.  Just try to
disassemble any C program and watch the opcodes produced for these
instructions; I believe you will see what I saw.

I was able to fix the problem with the following patch to i386-tdep.c.
Note that the list of possible ModR/M values for the opcodes 0x8b and
0x89 probably needs to be expanded, but for the moment I put there
only those which are needed to cover the same ground as your original
partial fix.

If you agree with the patch, I will commit it.

2004-07-31  Eli Zaretskii  <eliz@gnu.org>

	* i386-tdep.c (i386_analyze_frame_setup): Handle more opcodes that
	GCC puts into function prolugues for MOV instructions.

--- gdb/i386-tdep.c~0	2004-07-24 20:46:26.000000000 +0300
+++ gdb/i386-tdep.c	2004-07-31 16:05:46.000000000 +0300
@@ -473,8 +473,9 @@ static CORE_ADDR
 i386_analyze_frame_setup (CORE_ADDR pc, CORE_ADDR current_pc,
 			  struct i386_frame_cache *cache)
 {
-  unsigned char op;
+  unsigned char op, modrm;
   int skip = 0;
+  int break_loop = 0;
 
   if (current_pc <= pc)
     return current_pc;
@@ -502,8 +503,14 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
 	    movl $XXX, %eax
 	    movl $XXX, %ecx
 	    movl $XXX, %edx
+	    movl %eax, $XXX
+	    movl %ecx, $XXX
+	    movl %edx, $XXX
+	    movl %eax, %ebx
+	    ....
 
-	 These instructions have opcodes 0xb8, 0xb9 and 0xba.
+	 These instructions may have many different opcodes:
+	 0xb8-0xba, 0xa1, 0x89,0x8b(+modr/m), and 0xc7.
 
 	 We also check for
 
@@ -524,14 +531,64 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
 	 Make sure we only skip these instructions if we later see the
 	 `movl %esp, %ebp' that actually sets up the frame.  */
       while ((op >= 0xb8 && op <= 0xba)
+	     || op == 0xa1 || op == 0xc7
+	     || op == 0x89 || op == 0x8b
 	     || op == 0x29 || op == 0x2b
 	     || op == 0x31 || op == 0x33)
 	{
-	  if (op >= 0xb8 && op <= 0xba)
+	  if ((op >= 0xb8 && op <= 0xba) || op == 0xa1 || op ==0xc7)
 	    {
-	      /* Skip the `movl' instructions cited above.  */
+	      /* Skip the `movl' instructions cited above and their
+		 4-byte operand.  */
 	      skip += 5;
 	    }
+	  else if (op == 0x89)	/* movl r32, r32 */
+	    {
+	      /* Skip the instruction and read the ModR/M byte.  */
+	      modrm = read_memory_unsigned_integer (pc + skip + 2, 1);
+	      switch (modrm)
+		{
+		case 0xc1:	/* %ecx,%eax */
+		case 0xc2:	/* %edx,%eax */
+		case 0xc3:	/* %ebx,%eax */
+		case 0xc8:	/* %eax,%ecx */
+		case 0xca:	/* %edx,%ecx */
+		case 0xcb:	/* %ebx,%ecx */
+		case 0xd0:	/* %eax,%edx */
+		case 0xd1:	/* %ecx,%edx */
+		case 0xd3:	/* %ebx,%edx */
+		case 0xd8:	/* %eax,%ebx */
+		case 0xd9:	/* %ecx,%ebx */
+		case 0xda:	/* %edx,%ebx */
+		  skip += 2;
+		  break;
+		case 0xe5:	/* movl %esp,%ebp--found frame setup end */
+		  break_loop = 1;
+		  break;
+		default:
+		  return pc + 1;
+		}
+	    }
+	  else if (op == 0x8b)
+	    {
+	      /* Skip the instruction and read the ModR/M byte.  */
+	      modrm = read_memory_unsigned_integer (pc + skip + 2, 1);
+	      switch (modrm)
+		{
+		case 0x05:	/* %eax,... */
+		case 0x0d:	/* %ecx,... */
+		case 0x15:	/* %edx,... */
+		case 0x1d:	/* %ebx,... */
+		  /* Skip the instruction, ModR/M, and the operand.  */
+		  skip += 6;
+		  break;
+		case 0xec:	/* movl %esp,%ebp--found frame setup end */
+		  break_loop = 1;
+		  break;
+		default:
+		  return pc + 1;
+		}
+	    }
 	  else
 	    {
 	      /* Skip the `subl' and `xorl' instructions cited above.  */
@@ -547,6 +604,8 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
 		  return pc + 1;
 		}
 	    }
+	  if (break_loop)
+	    break;
 
 	  /* If that's all, return now.  */
 	  if (current_pc <= pc + skip + 1)
@@ -555,21 +614,6 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
 	  op = read_memory_unsigned_integer (pc + skip + 1, 1);
 	}
 
-      /* Check for `movl %esp, %ebp' -- can be written in two ways.  */
-      switch (op)
-	{
-	case 0x8b:
-	  if (read_memory_unsigned_integer (pc + skip + 2, 1) != 0xec)
-	    return pc + 1;
-	  break;
-	case 0x89:
-	  if (read_memory_unsigned_integer (pc + skip + 2, 1) != 0xe5)
-	    return pc + 1;
-	  break;
-	default:
-	  return pc + 1;
-	}
-
       /* OK, we actually have a frame.  We just don't know how large
 	 it is yet.  Set its size to zero.  We'll adjust it if
 	 necessary.  We also now commit to skipping the special
@@ -581,7 +625,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
       if (current_pc <= pc + 3)
 	return current_pc;
 
-      /* Check for stack adjustment 
+      /* Check for stack adjustment
 
 	    subl $XXX, %esp