[RFA/i386] pb reading insns if breakpoints still inserted (take 2)

[Joel Brobecker <brobecker@adacore.com>]

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

Hello,

This is the second attempt at fixing a problem that was documented in:

        http://www.sourceware.org/ml/gdb-patches/2006-04/msg00345.html

A consensus seemed to have emerged while we discussed the patch I
submitted for it in:

        http://www.sourceware.org/ml/gdb-patches/2006-04/msg00367.html

Summary of the problem: When we do next/step operations, we end up
parsing the top frame function prologue and creating a frame_info with
it. Unfortunately, at that point, the breakpoints are still inserted and
that causes the prologue analyzer to misinterpret the function prologue
and consequently breaks unwinding a bit.

I'm just only realizing that I didn't produce a new testcase for it,
so I will do that and send it tonight. Just to refresh our memory,
here is how to reproduce:

        #include <stdio.h>
        
        void
        hello (void)
        {
          printf ("Hello world.\n");
        }
        
        int
        main (void)
        {
          hello ();
        
          return 0;
        }

Compile this code on i386 using STABS. Using dwarf won't show the issue
because GDB doesn't need to do prologue analysis when the frame info is
available.

Then follow the guide:

        (gdb) b *hello
        Breakpoint 1 at 0x401050: file foo.c, line 5.
        (gdb) run
        Starting program: /[...]/foo.exe 
        
        Breakpoint 1, hello () at foo.c:5
        5       {
        (gdb) stepi
        0x00401051      5       {
        (gdb) bt
        #0  0x00401051 in hello () at foo.c:5
        #1  0x00401093 in main () at foo.c:12
        (gdb) stepi
        0x00401053 in hello () at foo.c:5
        5       {

So we're at the third instruction of the function. Here is the backtrace
we get when I request it:

        (gdb) bt
        #0  0x00401053 in hello () at foo.c:5
        #1  0x0022ee88 in ?? ()
        #2  0x00401093 in main () at foo.c:12

We get an extra frame between hello() and main().

2006-07-26  Joel Brobecker  <brobecker@adacore.com>

        * i386-tdep.c (i386_follow_jump): Use read_memory_nobpt to read
        instructions.
        (i386_analyze_struct_return): Likewise.
        (i386_skip_probe): Likewise.
        (i386_match_insn): Likewise.
        (i386_analyze_frame_setup): Likewise.
        (i386_analyze_register_saves): Likewise.
        (i386_skip_prologue): Likewise.

This patch requires that the patch submitted at the address below
be applied first (it undeprecates read_memory_nobpt):

        http://www.sourceware.org/ml/gdb-patches/2006-07/msg00397.html

Tested on x86-linux with stabs as the debugging info format.
No regression.

OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: stepi.diff --]
[-- Type: text/plain, Size: 3603 bytes --]

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.224
diff -u -p -r1.224 i386-tdep.c
--- i386-tdep.c	17 Mar 2006 00:14:24 -0000	1.224
+++ i386-tdep.c	27 Jul 2006 00:52:50 -0000
@@ -354,7 +354,7 @@ i386_follow_jump (CORE_ADDR pc)
   long delta = 0;
   int data16 = 0;
 
-  op = read_memory_unsigned_integer (pc, 1);
+  read_memory_nobpt (pc, &op, 1);
   if (op == 0x66)
     {
       data16 = 1;
@@ -420,12 +420,12 @@ i386_analyze_struct_return (CORE_ADDR pc
   if (current_pc <= pc)
     return pc;
 
-  op = read_memory_unsigned_integer (pc, 1);
+  read_memory_nobpt (pc, &op, 1);
 
   if (op != 0x58)		/* popl %eax */
     return pc;
 
-  read_memory (pc + 1, buf, 4);
+  read_memory_nobpt (pc + 1, buf, 4);
   if (memcmp (buf, proto1, 3) != 0 && memcmp (buf, proto2, 4) != 0)
     return pc;
 
@@ -464,7 +464,7 @@ i386_skip_probe (CORE_ADDR pc)
   gdb_byte buf[8];
   gdb_byte op;
 
-  op = read_memory_unsigned_integer (pc, 1);
+  read_memory_nobpt (pc, &op, 1);
 
   if (op == 0x68 || op == 0x6a)
     {
@@ -535,7 +535,7 @@ i386_match_insn (CORE_ADDR pc, struct i3
   struct i386_insn *insn;
   gdb_byte op;
 
-  op = read_memory_unsigned_integer (pc, 1);
+  read_memory_nobpt (pc, &op, 1);
 
   for (insn = skip_insns; insn->len > 0; insn++)
     {
@@ -548,7 +548,7 @@ i386_match_insn (CORE_ADDR pc, struct i3
 	  gdb_assert (insn->len > 1);
 	  gdb_assert (insn->len <= I386_MAX_INSN_LEN);
 
-	  read_memory (pc + 1, buf, insn->len - 1);
+	  read_memory_nobpt (pc + 1, buf, insn->len - 1);
 	  for (i = 1; i < insn->len; i++)
 	    {
 	      if ((buf[i - 1] & insn->mask[i]) != insn->insn[i])
@@ -634,7 +634,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
   if (limit <= pc)
     return limit;
 
-  op = read_memory_unsigned_integer (pc, 1);
+  read_memory_nobpt (pc, &op, 1);
 
   if (op == 0x55)		/* pushl %ebp */
     {
@@ -669,7 +669,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
       if (limit <= pc + skip)
 	return limit;
 
-      op = read_memory_unsigned_integer (pc + skip, 1);
+      read_memory_nobpt (pc + skip, &op, 1);
 
       /* Check for `movl %esp, %ebp' -- can be written in two ways.  */
       switch (op)
@@ -703,7 +703,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
 
 	 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.  */
-      op = read_memory_unsigned_integer (pc, 1);
+      read_memory_nobpt (pc, &op, 1);
       if (op == 0x83)
 	{
 	  /* `subl' with 8-bit immediate.  */
@@ -759,7 +759,7 @@ i386_analyze_register_saves (CORE_ADDR p
     offset -= cache->locals;
   for (i = 0; i < 8 && pc < current_pc; i++)
     {
-      op = read_memory_unsigned_integer (pc, 1);
+      read_memory_nobpt (pc, &op, 1);
       if (op < 0x50 || op > 0x57)
 	break;
 
@@ -848,7 +848,7 @@ i386_skip_prologue (CORE_ADDR start_pc)
 
   for (i = 0; i < 6; i++)
     {
-      op = read_memory_unsigned_integer (pc + i, 1);
+      read_memory_nobpt (pc + i, &op, 1);
       if (pic_pat[i] != op)
 	break;
     }
@@ -856,7 +856,7 @@ i386_skip_prologue (CORE_ADDR start_pc)
     {
       int delta = 6;
 
-      op = read_memory_unsigned_integer (pc + delta, 1);
+      read_memory_nobpt (pc + delta, &op, 1);
 
       if (op == 0x89)		/* movl %ebx, x(%ebp) */
 	{
@@ -869,7 +869,7 @@ i386_skip_prologue (CORE_ADDR start_pc)
 	  else			/* Unexpected instruction.  */
 	    delta = 0;
 
-	  op = read_memory_unsigned_integer (pc + delta, 1);
+          read_memory_nobpt (pc + delta, &op, 1);
 	}
 
       /* addl y,%ebx */