[PATCH] Fix displaced stepping for remote targets

[Luis Gustavo <luis_gustavo@mentor.com>]

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

Hi,

While debugging a remote target that supports hardware single-stepping 
with a GDB that supports displaced stepping, i've ran into the following 
problem...

When reaching a breakpoint, GDB should take care of relocating the 
instruction underneath that breakpoint to the scratch space where it 
will be executed out-of-line.

If a target supports hw single-stepping for displaced stepping, GDB 
should just send a vCont;s packet to tell the target to step a single 
instruction. In my case, GDB was always sending a vCont;c instead.

I've tracked it down to infrun.c:resume, where we have this check:

       if (gdbarch_cannot_step_breakpoint (gdbarch))
         {
           /* Most targets can step a breakpoint instruction, thus
              executing it normally.  But if this one cannot, just
              continue and we will hit it anyway.  */
           if (step && breakpoint_inserted_here_p (aspace, pc))
             step = 0;
         }

My target can't step breakpoints and, if we're doing displaced stepping, 
it's because we're trying to step off a breakpoint, thus 
breakpoint_inserted_here_p returns true, and we disable single-stepping 
by setting step to 0.

It seems to me we need to update the PC prior to calling 
breakpoint_inserted_here_p since the displaced stepping machinery 
adjusted the old PC to point to the space in the scratch area. That way 
we can properly command the target to step the displaced instruction and 
we can check for breakpoints at the real execution place.

The following patch fixes this by pushing the if block further down in 
the code and taking care of updating PC if displaced stepping is being used.

I've regtested this on x86 and everything looks OK. This also makes GDB 
send vCont;s now.

Ok?

Luis

[-- Attachment #2: 0001-fix_remote_displaced_stepping.diff --]
[-- Type: text/x-patch, Size: 1885 bytes --]

2012-03-19  Luis Machado  <lgustavo@codesourcery.com>

	* infrun.c (resume): Change order of statements and
	update PC to the real PC in case of displaced stepping.

Index: HEAD-git/gdb/infrun.c
===================================================================
--- HEAD-git.orig/gdb/infrun.c	2012-03-19 20:40:57.589621232 -0300
+++ HEAD-git/gdb/infrun.c	2012-03-19 22:12:08.729898472 -0300
@@ -1884,15 +1884,6 @@ a command like `return' or `jump' to con
 	  resume_ptid = inferior_ptid;
 	}
 
-      if (gdbarch_cannot_step_breakpoint (gdbarch))
-	{
-	  /* Most targets can step a breakpoint instruction, thus
-	     executing it normally.  But if this one cannot, just
-	     continue and we will hit it anyway.  */
-	  if (step && breakpoint_inserted_here_p (aspace, pc))
-	    step = 0;
-	}
-
       if (debug_displaced
           && use_displaced_stepping (gdbarch)
           && tp->control.trap_expected)
@@ -1902,12 +1893,25 @@ a command like `return' or `jump' to con
           CORE_ADDR actual_pc = regcache_read_pc (resume_regcache);
           gdb_byte buf[4];
 
+	  /* Update pc to reflect the new address from which we will execute
+	     instructions due to displaced stepping.  */
+	  pc = actual_pc;
+
           fprintf_unfiltered (gdb_stdlog, "displaced: run %s: ",
                               paddress (resume_gdbarch, actual_pc));
           read_memory (actual_pc, buf, sizeof (buf));
           displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
         }
 
+      if (gdbarch_cannot_step_breakpoint (gdbarch))
+	{
+	  /* Most targets can step a breakpoint instruction, thus
+	     executing it normally.  But if this one cannot, just
+	     continue and we will hit it anyway.  */
+	  if (step && breakpoint_inserted_here_p (aspace, pc))
+	    step = 0;
+	}
+
       /* Install inferior's terminal modes.  */
       target_terminal_inferior ();