* amd64-tdep.c (amd64_skip_prefixes): Renamed from skip_prefixes.

	All callers updated.
	(amd64_get_insn_details): Handle more 3-byte opcode insns.
	(amd64_breakpoint_p): Delete.
	(amd64_displaced_step_fixup): When fixing up after stepping an int3,
	don't back up pc to the start of the int3.
	* i386-tdep.c: #include opcode/i386.h.
	(i386_skip_prefixes): New function.
	(i386_absolute_jmp_p): Constify argument.
	(i386_absolute_call_p,i386_ret_p,i386_call_p,i386_syscall_p): Ditto.
	(i386_breakpoint_p): Delete.
	(i386_displaced_step_fixup): Handle unnecessary or redundant prefixes.
	When fixing up after stepping an int3, don't back up pc to the start
	of the int3.

	* gdb.arch/amd64-disp-step.S (test_int3): New test.
	* gdb.arch/amd64-disp-step.exp (test_int3): New test.
	* gdb.arch/i386-disp-step.S (test_prefixed_abs_jump): New test.
	(test_prefixed_syscall,test_int3): New tests.
	* gdb.arch/i386-disp-step.exp (test_prefixed_abs_jump): New test.
	(test_prefixed_syscall,test_int3): New tests.

8 files changed, 271 insertions, 58 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 71c1f6a..424ae54 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2009-02-16  Doug Evans  <dje@google.com>
+
+	* amd64-tdep.c (amd64_skip_prefixes): Renamed from skip_prefixes.
+	All callers updated.
+	(amd64_get_insn_details): Handle more 3-byte opcode insns.
+	(amd64_breakpoint_p): Delete.
+	(amd64_displaced_step_fixup): When fixing up after stepping an int3,
+	don't back up pc to the start of the int3.
+	* i386-tdep.c: #include opcode/i386.h.
+	(i386_skip_prefixes): New function.
+	(i386_absolute_jmp_p): Constify argument.
+	(i386_absolute_call_p,i386_ret_p,i386_call_p,i386_syscall_p): Ditto.
+	(i386_breakpoint_p): Delete.
+	(i386_displaced_step_fixup): Handle unnecessary or redundant prefixes.
+	When fixing up after stepping an int3, don't back up pc to the start
+	of the int3.
+
 2009-02-16  Pedro Alves  <pedro@codesourcery.com>
 
 	* corelow.c (core_close): Don't hardcode the core's pid.
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index ad26493..4eef55d 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -805,7 +805,7 @@ rex_prefix_p (gdb_byte pfx)
    about falling off the end of the buffer.  */
 
 static gdb_byte *
-skip_prefixes (gdb_byte *insn)
+amd64_skip_prefixes (gdb_byte *insn)
 {
   while (1)
     {
@@ -974,7 +974,7 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details)
   details->modrm_offset = -1;
 
   /* Skip legacy instruction prefixes.  */
-  insn = skip_prefixes (insn);
+  insn = amd64_skip_prefixes (insn);
 
   /* Skip REX instruction prefix.  */
   if (rex_prefix_p (*insn))
@@ -992,13 +992,21 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details)
       need_modrm = twobyte_has_modrm[*insn];
 
       /* Check for three-byte opcode.  */
-      if (*insn == 0x38 || *insn == 0x3a)
+      switch (*insn)
 	{
+	case 0x24:
+	case 0x25:
+	case 0x38:
+	case 0x3a:
+	case 0x7a:
+	case 0x7b:
 	  ++insn;
 	  details->opcode_len = 3;
+	  break;
+	default:
+	  details->opcode_len = 2;
+	  break;
 	}
-      else
-	details->opcode_len = 2;
     }
   else
     {
@@ -1217,14 +1225,6 @@ amd64_call_p (const struct amd64_insn *details)
   return 0;
 }
 
-static int
-amd64_breakpoint_p (const struct amd64_insn *details)
-{
-  const gdb_byte *insn = &details->raw_insn[details->opcode_offset];
-
-  return insn[0] == 0xcc; /* int 3 */
-}
-
 /* Return non-zero if INSN is a system call, and set *LENGTHP to its
    length in bytes.  Otherwise, return zero.  */
 
@@ -1323,20 +1323,9 @@ amd64_displaced_step_fixup (struct gdbarch *gdbarch,
 	{
 	  ULONGEST rip = orig_rip - insn_offset;
 
-	  /* If we have stepped over a breakpoint, set %rip to
-	     point at the breakpoint instruction itself.
-
-	     (gdbarch_decr_pc_after_break was never something the core
-	     of GDB should have been concerned with; arch-specific
-	     code should be making PC values consistent before
-	     presenting them to GDB.)  */
-	  if (amd64_breakpoint_p (insn_details))
-	    {
-	      if (debug_displaced)
-		fprintf_unfiltered (gdb_stdlog,
-				    "displaced: stepped breakpoint\n");
-	      rip--;
-	    }
+	  /* If we just stepped over a breakpoint insn, we don't backup
+	     the pc on purpose; this is to match behaviour without
+	     stepping.  */
 
 	  regcache_cooked_write_unsigned (regs, AMD64_RIP_REGNUM, rip);
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 496c4ff..b74270a 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -20,6 +20,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "opcode/i386.h"
 #include "arch-utils.h"
 #include "command.h"
 #include "dummy-frame.h"
@@ -278,9 +279,44 @@ i386_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pc, int *len)
 
 /* Displaced instruction handling.  */
 
+/* Skip the legacy instruction prefixes in INSN.
+   Not all prefixes are valid for any particular insn
+   but we needn't care, the insn will fault if it's invalid.
+   The result is a pointer to the first opcode byte,
+   or NULL if we run off the end of the buffer.  */
+
+static gdb_byte *
+i386_skip_prefixes (gdb_byte *insn, size_t max_len)
+{
+  gdb_byte *end = insn + max_len;
+
+  while (insn < end)
+    {
+      switch (*insn)
+	{
+	case DATA_PREFIX_OPCODE:
+	case ADDR_PREFIX_OPCODE:
+	case CS_PREFIX_OPCODE:
+	case DS_PREFIX_OPCODE:
+	case ES_PREFIX_OPCODE:
+	case FS_PREFIX_OPCODE:
+	case GS_PREFIX_OPCODE:
+	case SS_PREFIX_OPCODE:
+	case LOCK_PREFIX_OPCODE:
+	case REPE_PREFIX_OPCODE:
+	case REPNE_PREFIX_OPCODE:
+	  ++insn;
+	  continue;
+	default:
+	  return insn;
+	}
+    }
+
+  return NULL;
+}
 
 static int
-i386_absolute_jmp_p (gdb_byte *insn)
+i386_absolute_jmp_p (const gdb_byte *insn)
 {
   /* jmp far (absolute address in operand) */
   if (insn[0] == 0xea)
@@ -301,7 +337,7 @@ i386_absolute_jmp_p (gdb_byte *insn)
 }
 
 static int
-i386_absolute_call_p (gdb_byte *insn)
+i386_absolute_call_p (const gdb_byte *insn)
 {
   /* call far, absolute */
   if (insn[0] == 0x9a)
@@ -322,7 +358,7 @@ i386_absolute_call_p (gdb_byte *insn)
 }
 
 static int
-i386_ret_p (gdb_byte *insn)
+i386_ret_p (const gdb_byte *insn)
 {
   switch (insn[0])
     {
@@ -339,7 +375,7 @@ i386_ret_p (gdb_byte *insn)
 }
 
 static int
-i386_call_p (gdb_byte *insn)
+i386_call_p (const gdb_byte *insn)
 {
   if (i386_absolute_call_p (insn))
     return 1;
@@ -351,16 +387,11 @@ i386_call_p (gdb_byte *insn)
   return 0;
 }
 
-static int
-i386_breakpoint_p (gdb_byte *insn)
-{
-  return insn[0] == 0xcc;       /* int 3 */
-}
-
 /* Return non-zero if INSN is a system call, and set *LENGTHP to its
    length in bytes.  Otherwise, return zero.  */
+
 static int
-i386_syscall_p (gdb_byte *insn, ULONGEST *lengthp)
+i386_syscall_p (const gdb_byte *insn, ULONGEST *lengthp)
 {
   if (insn[0] == 0xcd)
     {
@@ -373,6 +404,7 @@ i386_syscall_p (gdb_byte *insn, ULONGEST *lengthp)
 
 /* Fix up the state of registers and memory after having single-stepped
    a displaced instruction.  */
+
 void
 i386_displaced_step_fixup (struct gdbarch *gdbarch,
                            struct displaced_step_closure *closure,
@@ -388,6 +420,8 @@ i386_displaced_step_fixup (struct gdbarch *gdbarch,
   /* Since we use simple_displaced_step_copy_insn, our closure is a
      copy of the instruction.  */
   gdb_byte *insn = (gdb_byte *) closure;
+  /* The start of the insn, needed in case we see some prefixes.  */
+  gdb_byte *insn_start = insn;
 
   if (debug_displaced)
     fprintf_unfiltered (gdb_stdlog,
@@ -401,6 +435,18 @@ i386_displaced_step_fixup (struct gdbarch *gdbarch,
 
   /* Relocate the %eip, if necessary.  */
 
+  /* The instruction recognizers we use assume any leading prefixes
+     have been skipped.  */
+  {
+    /* This is the size of the buffer in closure.  */
+    size_t max_insn_len = gdbarch_max_insn_length (gdbarch);
+    gdb_byte *opcode = i386_skip_prefixes (insn, max_insn_len);
+    /* If there are too many prefixes, just ignore the insn.
+       It will fault when run.  */
+    if (opcode != NULL)
+      insn = opcode;
+  }
+
   /* Except in the case of absolute or indirect jump or call
      instructions, or a return instruction, the new eip is relative to
      the displaced instruction; make it relative.  Well, signal
@@ -430,7 +476,7 @@ i386_displaced_step_fixup (struct gdbarch *gdbarch,
          it unrelocated.  Goodness help us if there are PC-relative
          system calls.  */
       if (i386_syscall_p (insn, &insn_len)
-          && orig_eip != to + insn_len)
+          && orig_eip != to + (insn - insn_start) + insn_len)
         {
           if (debug_displaced)
             fprintf_unfiltered (gdb_stdlog,
@@ -441,20 +487,9 @@ i386_displaced_step_fixup (struct gdbarch *gdbarch,
         {
           ULONGEST eip = (orig_eip - insn_offset) & 0xffffffffUL;
 
-          /* If we have stepped over a breakpoint, set the %eip to
-             point at the breakpoint instruction itself.
-
-             (gdbarch_decr_pc_after_break was never something the core
-             of GDB should have been concerned with; arch-specific
-             code should be making PC values consistent before
-             presenting them to GDB.)  */
-          if (i386_breakpoint_p (insn))
-            {
-              if (debug_displaced)
-                fprintf_unfiltered (gdb_stdlog,
-                                    "displaced: stepped breakpoint\n");
-              eip--;
-            }
+	  /* If we just stepped over a breakpoint insn, we don't backup
+	     the pc on purpose; this is to match behaviour without
+	     stepping.  */
 
           regcache_cooked_write_unsigned (regs, I386_EIP_REGNUM, eip);
 
@@ -493,8 +528,6 @@ i386_displaced_step_fixup (struct gdbarch *gdbarch,
                             paddr_nz (retaddr));
     }
 }
-
-
 
 #ifdef I386_REGNO_TO_SYMMETRY
 #error "The Sequent Symmetry is no longer supported."
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e760e8a..24dc96d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2009-02-16  Doug Evans  <dje@google.com>
+
+	* gdb.arch/amd64-disp-step.S (test_int3): New test.
+	* gdb.arch/amd64-disp-step.exp (test_int3): New test.
+	* gdb.arch/i386-disp-step.S (test_prefixed_abs_jump): New test.
+	(test_prefixed_syscall,test_int3): New tests.
+	* gdb.arch/i386-disp-step.exp (test_prefixed_abs_jump): New test.
+	(test_prefixed_syscall,test_int3): New tests.
+
 2009-02-14  Vladimir Prus  <vladimir@codesourcery.com>
 
         * lib/mi-support.exp (mi_expect_stop): Adjust the order of fields.
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.S b/gdb/testsuite/gdb.arch/amd64-disp-step.S
index 45eeb9b..1ce0184 100644
--- a/gdb/testsuite/gdb.arch/amd64-disp-step.S
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step.S
@@ -23,6 +23,8 @@
 main:
 	nop
 
+/***********************************************/
+
 /* test call/ret */
 
 	.global test_call
@@ -33,6 +35,8 @@ test_call:
 test_ret_end:
 	nop
 
+/***********************************************/
+
 /* test abs-jmp/rep-ret */
 
 test_abs_jmp_setup:
@@ -48,6 +52,8 @@ test_abs_jmp_return:
 test_rep_ret_end:
 	nop
 
+/***********************************************/
+
 /* test syscall */
 
 	.global test_syscall
@@ -58,6 +64,24 @@ test_syscall:
 test_syscall_end:
 	nop
 
+/***********************************************/
+
+/* Test stepping over int3.
+   The prefixes are pointless, but it's possible, so we exercise it.  */
+
+	nop
+	.global test_int3
+test_int3:
+	repz
+	repz
+	int3
+	nop
+	.global test_int3_end
+test_int3_end:
+	nop
+
+/***********************************************/
+
 /* test rip-relative
    GDB picks a spare register to hold the rip-relative address.
    Exercise all the possibilities (rax-rdi, sans rsp).  */
@@ -118,6 +142,8 @@ test_rip_rdi_end:
 
 answer:	.8byte 42
 
+/***********************************************/
+
 /* all done */
 
 done:
@@ -139,6 +165,8 @@ test_call_end:
 test_ret:
 	ret
 
+/***********************************************/
+
 /* subroutine to help test abs-jmp/rep-ret */
 
 test_abs_jmp_subr:
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
index 26ebe59..3b0f83b 100644
--- a/gdb/testsuite/gdb.arch/amd64-disp-step.exp
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
@@ -141,6 +141,26 @@ gdb_test "continue" \
 
 ##########################################
 
+# int3 (with prefixes)
+# These don't occur in normal code, but gdb should still DTRT.
+
+gdb_test "break test_int3" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "break test_int3"
+gdb_test "break test_int3_end" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "break test_int3_end"
+
+gdb_test "continue" \
+    "Continuing.*Breakpoint.*, test_int3 ().*" \
+    "continue to test_int3"
+
+gdb_test "continue" \
+    "Continuing.*Breakpoint.*, test_int3_end ().*" \
+    "continue to test_int3_end"
+
+##########################################
+
 # Test rip-relative.
 # GDB picks a spare register to hold the rip-relative address.
 # Exercise all the possibilities (rax-rdi, sans rsp).
diff --git a/gdb/testsuite/gdb.arch/i386-disp-step.S b/gdb/testsuite/gdb.arch/i386-disp-step.S
index a56ff1c..2d5c372 100644
--- a/gdb/testsuite/gdb.arch/i386-disp-step.S
+++ b/gdb/testsuite/gdb.arch/i386-disp-step.S
@@ -23,8 +23,11 @@
 main:
 	nop
 
-/* test call/ret */
+/***********************************************/
+
+/* Test call/ret.  */
 
+	nop
 	.global test_call
 test_call:
 	call test_call_subr
@@ -33,16 +36,72 @@ test_call:
 test_ret_end:
 	nop
 
-/* test syscall */
+/***********************************************/
+
+/* Absolute jump with leading prefixes.
+   These don't occur in normal code, but gdb should still DTRT.  */
+
+	nop
+	.global test_prefixed_abs_jump
+test_prefixed_abs_jump:
+	ds
+	jmp *test_prefixed_abs_jump_addr
+	.data
+test_prefixed_abs_jump_addr:
+	.4byte test_prefixed_abs_jump_target
+	.text
+test_prefixed_abs_jump_target:
+	nop
+	.global test_prefixed_abs_jump_end
+test_prefixed_abs_jump_end:
+	nop
+
+/***********************************************/
+
+/* Test syscall.  */
 
-	.global test_syscall
 	mov $0x14,%eax /* getpid */
+	.global test_syscall
 test_syscall:
 	int $0x80
 	nop
+	.global test_syscall_end
 test_syscall_end:
 	nop
 
+/***********************************************/
+
+/* Test syscall again, this time with a prefix.
+   These don't occur in normal code, but gdb should still DTRT.  */
+
+	mov $0x14,%eax /* getpid */
+	.global test_prefixed_syscall
+test_prefixed_syscall:
+	repnz
+	int $0x80
+	nop
+	.global test_prefixed_syscall_end
+test_prefixed_syscall_end:
+	nop
+
+/***********************************************/
+
+/* Test stepping over int3.
+   The prefixes are pointless, but it's possible, so we exercise it.  */
+
+	nop
+	.global test_int3
+test_int3:
+	repz
+	repz
+	int3
+	nop
+	.global test_int3_end
+test_int3_end:
+	nop
+
+/***********************************************/
+
 /* all done */
 
 	pushl $0
diff --git a/gdb/testsuite/gdb.arch/i386-disp-step.exp b/gdb/testsuite/gdb.arch/i386-disp-step.exp
index 06c5fb2..5fc2af8 100644
--- a/gdb/testsuite/gdb.arch/i386-disp-step.exp
+++ b/gdb/testsuite/gdb.arch/i386-disp-step.exp
@@ -89,6 +89,25 @@ gdb_test "continue" \
 
 ##########################################
 
+# Absolute jump with leading prefixes.
+# These don't occur in normal code, but gdb should still DTRT.
+
+gdb_test "break test_prefixed_abs_jump" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "break test_prefixed_abs_jump"
+gdb_test "break test_prefixed_abs_jump_end" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "break test_prefixed_abs_jump_end"
+
+gdb_test "continue" \
+    "Continuing.*Breakpoint.*, test_prefixed_abs_jump ().*" \
+    "continue to test_prefixed_abs_jump"
+gdb_test "continue" \
+    "Continuing.*Breakpoint.*, test_prefixed_abs_jump_end ().*" \
+    "continue to test_prefixed_abs_jump_end"
+
+##########################################
+
 # Test syscall.
 
 gdb_test "break test_syscall" \
@@ -107,6 +126,45 @@ gdb_test "continue" \
 
 ##########################################
 
+# Test prefixed syscall.
+# These don't occur in normal code, but gdb should still DTRT.
+
+gdb_test "break test_prefixed_syscall" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "break test_prefixed_syscall"
+gdb_test "break test_prefixed_syscall_end" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "break test_prefixed_syscall_end"
+
+gdb_test "continue" \
+    "Continuing.*Breakpoint.*, test_prefixed_syscall ().*" \
+    "continue to test_prefixed_syscall"
+gdb_test "continue" \
+    "Continuing.*Breakpoint.*, test_prefixed_syscall_end ().*" \
+    "continue to test_prefixed_syscall_end"
+
+##########################################
+
+# int3 (with prefixes)
+# These don't occur in normal code, but gdb should still DTRT.
+
+gdb_test "break test_int3" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "break test_int3"
+gdb_test "break test_int3_end" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "break test_int3_end"
+
+gdb_test "continue" \
+    "Continuing.*Breakpoint.*, test_int3 ().*" \
+    "continue to test_int3"
+
+gdb_test "continue" \
+    "Continuing.*Breakpoint.*, test_int3_end ().*" \
+    "continue to test_int3_end"
+
+##########################################
+
 # Done, run program to exit.
 
 gdb_continue_to_end "i386-disp-step"