Patchwork [v3,15/17] PPC64: Fix gdb.arch/ppc64-atomic-inst.exp with displaced stepping

login
register
mail settings
Submitter Pedro Alves
Date April 17, 2015, 10:45 a.m.
Message ID <1429267521-21047-16-git-send-email-palves@redhat.com>
Download mbox | patch
Permalink /patch/6287/
State New
Headers show

Comments

Pedro Alves - April 17, 2015, 10:45 a.m.
The ppc64 displaced step code can't handle atomic sequences.  Fallback
to stepping over the breakpoint in-line if we detect one.

gdb/ChangeLog:
2015-04-17  Pedro Alves  <palves@redhat.com>

	* infrun.c (displaced_step_prepare): Return -1 if
	gdbarch_displaced_step_copy_insn returns NULL.
	(resume): When displaced stepping doesn't work, try software
	single-stepping.
	* rs6000-tdep.c (LWARX_MASK, LWARX_INSTRUCTION, LDARX_INSTRUCTION)
	(STWCX_MASK, STWCX_INSTRUCTION, STDCX_INSTRUCTION): Move higher up
	in file.
	(ppc_displaced_step_copy_insn): New function.
	(ppc_displaced_step_fixup): Update comment.
	(rs6000_gdbarch_init): Install ppc_displaced_step_copy_insn as
	gdbarch_displaced_step_copy_insn hook.
	* gdbarch.sh (displaced_step_copy_insn): Document what happens on
	NULL return.
	* gdbarch.h: Regenerate.

gdb/testsuite/ChangeLog:
2015-04-17  Pedro Alves  <palves@redhat.com>

	* gdb.arch/ppc64-atomic-inst.exp (do_test): New procedure, move
	tests here.
	(top level): Run do_test with and without displaced stepping.

v3:

   - Document that gdbarch_displaced_step_copy_insn can return NULL.
---
 gdb/gdbarch.h                                |  6 ++-
 gdb/gdbarch.sh                               |  4 ++
 gdb/infrun.c                                 | 11 +++--
 gdb/rs6000-tdep.c                            | 68 +++++++++++++++++++++++-----
 gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp | 50 ++++++++++++--------
 5 files changed, 106 insertions(+), 33 deletions(-)
Yao Qi - April 21, 2015, 11:20 a.m.
Pedro Alves <palves@redhat.com> writes:

> The ppc64 displaced step code can't handle atomic sequences.  Fallback
> to stepping over the breakpoint in-line if we detect one.

Probably arm atom sequences instructions can't be executed out-of-line
either, I'll write a test for this.

>
> gdb/ChangeLog:
> 2015-04-17  Pedro Alves  <palves@redhat.com>
>
> 	* infrun.c (displaced_step_prepare): Return -1 if
> 	gdbarch_displaced_step_copy_insn returns NULL.
> 	(resume): When displaced stepping doesn't work, try software
> 	single-stepping.

This chunk isn't shown in the patch below, has been committed already?

> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index be186a7..a89eefc 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -1797,9 +1797,14 @@ displaced_step_prepare_throw (ptid_t ptid)
>  
>    closure = gdbarch_displaced_step_copy_insn (gdbarch,
>  					      original, copy, regcache);
> -
> -  /* We don't support the fully-simulated case at present.  */
> -  gdb_assert (closure);
> +  if (closure == NULL)
> +    {
> +      /* The architecture doesn't know how or want to displaced step
> +	 this instruction or instruction sequence.  Fallback to
> +	 stepping over the breakpoint in-line.  */
> +      do_cleanups (old_cleanups);
> +      return -1;
> +    }
>  
>    /* Save the information we need to fix things up if the step
>       succeeds.  */

We need to update the comments to displaced_step_prepare_throw about
return value -1.

Patch

diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index c94c19c..b02225a 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -923,7 +923,11 @@  extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i
   
    If your architecture doesn't need to adjust instructions before
    single-stepping them, consider using simple_displaced_step_copy_insn
-   here. */
+   here.
+  
+   If the instruction cannot execute out of line, return NULL.  The
+   core falls back to stepping past the instruction in-line instead in
+   that case. */
 
 extern int gdbarch_displaced_step_copy_insn_p (struct gdbarch *gdbarch);
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 0f303a4..749cbda 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -763,6 +763,10 @@  V:ULONGEST:max_insn_length:::0:0
 # If your architecture doesn't need to adjust instructions before
 # single-stepping them, consider using simple_displaced_step_copy_insn
 # here.
+#
+# If the instruction cannot execute out of line, return NULL.  The
+# core falls back to stepping past the instruction in-line instead in
+# that case.
 M:struct displaced_step_closure *:displaced_step_copy_insn:CORE_ADDR from, CORE_ADDR to, struct regcache *regs:from, to, regs
 
 # Return true if GDB should use hardware single-stepping to execute
diff --git a/gdb/infrun.c b/gdb/infrun.c
index be186a7..a89eefc 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1797,9 +1797,14 @@  displaced_step_prepare_throw (ptid_t ptid)
 
   closure = gdbarch_displaced_step_copy_insn (gdbarch,
 					      original, copy, regcache);
-
-  /* We don't support the fully-simulated case at present.  */
-  gdb_assert (closure);
+  if (closure == NULL)
+    {
+      /* The architecture doesn't know how or want to displaced step
+	 this instruction or instruction sequence.  Fallback to
+	 stepping over the breakpoint in-line.  */
+      do_cleanups (old_cleanups);
+      return -1;
+    }
 
   /* Save the information we need to fix things up if the step
      succeeds.  */
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index a125000..6106029 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -974,6 +974,61 @@  rs6000_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *bp_addr,
 #define BXL_INSN 0x4c000000
 #define BP_INSN 0x7C000008
 
+/* Instruction masks used during single-stepping of atomic
+   sequences.  */
+#define LWARX_MASK 0xfc0007fe
+#define LWARX_INSTRUCTION 0x7c000028
+#define LDARX_INSTRUCTION 0x7c0000A8
+#define STWCX_MASK 0xfc0007ff
+#define STWCX_INSTRUCTION 0x7c00012d
+#define STDCX_INSTRUCTION 0x7c0001ad
+
+/* We can't displaces step atomic sequenes.  Otherwise this is just
+   simple_displaced_step_copy_insn.  */
+
+static struct displaced_step_closure *
+ppc_displaced_step_copy_insn (struct gdbarch *gdbarch,
+			      CORE_ADDR from, CORE_ADDR to,
+			      struct regcache *regs)
+{
+  size_t len = gdbarch_max_insn_length (gdbarch);
+  gdb_byte *buf = xmalloc (len);
+  struct cleanup *old_chain = make_cleanup (xfree, buf);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  int insn;
+
+  read_memory (from, buf, len);
+
+  insn = extract_signed_integer (buf, PPC_INSN_SIZE, byte_order);
+
+  /* Assume all atomic sequences start with a lwarx/ldarx instruction.  */
+  if ((insn & LWARX_MASK) == LWARX_INSTRUCTION
+      || (insn & LWARX_MASK) == LDARX_INSTRUCTION)
+    {
+      if (debug_displaced)
+	{
+	  fprintf_unfiltered (gdb_stdlog,
+			      "displaced: can't displaced step "
+			      "atomic sequence at %s\n",
+			      paddress (gdbarch, from));
+	}
+      do_cleanups (old_chain);
+      return NULL;
+    }
+
+  write_memory (to, buf, len);
+
+  if (debug_displaced)
+    {
+      fprintf_unfiltered (gdb_stdlog, "displaced: copy %s->%s: ",
+                          paddress (gdbarch, from), paddress (gdbarch, to));
+      displaced_step_dump_bytes (gdb_stdlog, buf, len);
+    }
+
+  discard_cleanups (old_chain);
+  return (struct displaced_step_closure *) buf;
+}
+
 /* Fix up the state of registers and memory after having single-stepped
    a displaced instruction.  */
 static void
@@ -983,8 +1038,7 @@  ppc_displaced_step_fixup (struct gdbarch *gdbarch,
 			  struct regcache *regs)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  /* Since we use simple_displaced_step_copy_insn, our closure is a
-     copy of the instruction.  */
+  /* Our closure is a copy of the instruction.  */
   ULONGEST insn  = extract_unsigned_integer ((gdb_byte *) closure,
 					      PPC_INSN_SIZE, byte_order);
   ULONGEST opcode = 0;
@@ -1077,14 +1131,6 @@  ppc_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
   return 1;
 }
 
-/* Instruction masks used during single-stepping of atomic sequences.  */
-#define LWARX_MASK 0xfc0007fe
-#define LWARX_INSTRUCTION 0x7c000028
-#define LDARX_INSTRUCTION 0x7c0000A8
-#define STWCX_MASK 0xfc0007ff
-#define STWCX_INSTRUCTION 0x7c00012d
-#define STDCX_INSTRUCTION 0x7c0001ad
-
 /* Checks for an atomic sequence of instructions beginning with a LWARX/LDARX
    instruction and ending with a STWCX/STDCX instruction.  If such a sequence
    is found, attempt to step through it.  A breakpoint is placed at the end of 
@@ -5923,7 +5969,7 @@  rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* Setup displaced stepping.  */
   set_gdbarch_displaced_step_copy_insn (gdbarch,
-					simple_displaced_step_copy_insn);
+					ppc_displaced_step_copy_insn);
   set_gdbarch_displaced_step_hw_singlestep (gdbarch,
 					    ppc_displaced_step_hw_singlestep);
   set_gdbarch_displaced_step_fixup (gdbarch, ppc_displaced_step_fixup);
diff --git a/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
index d251425..08518fb 100644
--- a/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
+++ b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
@@ -32,27 +32,41 @@  if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug quiet}]
     return -1
 }
 
-if ![runto_main] then {
-    untested "could not run to main"
-    return -1
-}
+# The test proper.  DISPLACED is true if we should try with displaced
+# stepping.
+proc do_test { displaced } {
+    global decimal hex
+
+    if ![runto_main] then {
+	untested "could not run to main"
+	return -1
+    }
+
+    gdb_test_no_output "set displaced-stepping $displaced"
 
-set bp1 [gdb_get_line_number "lwarx"]
-gdb_breakpoint "$bp1" "Breakpoint $decimal at $hex" \
-  "Set the breakpoint at the start of the lwarx/stwcx sequence"
+    set bp1 [gdb_get_line_number "lwarx"]
+    gdb_breakpoint "$bp1" "Breakpoint $decimal at $hex" \
+	"Set the breakpoint at the start of the lwarx/stwcx sequence"
 
-set bp2 [gdb_get_line_number "ldarx"]
-gdb_breakpoint "$bp2" "Breakpoint $decimal at $hex" \
-  "Set the breakpoint at the start of the ldarx/stdcx sequence"
+    set bp2 [gdb_get_line_number "ldarx"]
+    gdb_breakpoint "$bp2" "Breakpoint $decimal at $hex" \
+	"Set the breakpoint at the start of the ldarx/stdcx sequence"
 
-gdb_test continue "Continuing.*Breakpoint $decimal.*" \
-  "Continue until lwarx/stwcx start breakpoint"
+    gdb_test continue "Continuing.*Breakpoint $decimal.*" \
+	"Continue until lwarx/stwcx start breakpoint"
 
-gdb_test nexti "bne.*1b" \
-  "Step through the lwarx/stwcx sequence"
+    gdb_test nexti "bne.*1b" \
+	"Step through the lwarx/stwcx sequence"
 
-gdb_test continue "Continuing.*Breakpoint $decimal.*" \
-  "Continue until ldarx/stdcx start breakpoint"
+    gdb_test continue "Continuing.*Breakpoint $decimal.*" \
+	"Continue until ldarx/stdcx start breakpoint"
 
-gdb_test nexti "bne.*1b" \
-  "Step through the ldarx/stdcx sequence"
+    gdb_test nexti "bne.*1b" \
+	"Step through the ldarx/stdcx sequence"
+}
+
+foreach displaced { "off" "on" } {
+    with_test_prefix "displaced=$displaced" {
+	do_test $displaced
+    }
+}