gdb: fix reg corruption from displaced stepping on amd64

This commit aims to address a problem that exists with the current
approach to displaced stepping, and was identified in PR gdb/22921.

Displaced stepping is currently supported on AArch64, ARM, amd64,
i386, rs6000 (ppc), and s390.  Of these, I believe there is a problem
with the current approach which will impact amd64 and ARM, and can
lead to random register corruption when the inferior makes use of
asynchronous signals and GDB is using displaced stepping.

The problem can be found in displaced_step_buffers::finish in
displaced-stepping.c, and is this; after GDB tries to perform a
displaced step, and the inferior stops, GDB classifies the stop into
one of two states, either the displaced step succeeded, or the
displaced step failed.

If the displaced step succeeded then gdbarch_displaced_step_fixup is
called, which has the job of fixing up the state of the current
inferior as if the step had not been performed in a displaced manner.
This all seems just fine.

However, if the displaced step is considered to have not completed
then GDB doesn't call gdbarch_displaced_step_fixup, instead GDB
remains in displaced_step_buffers::finish and just performs a minimal
fixup which involves adjusting the program counter back to its
original value.

The problem here is that for amd64 and ARM setting up for a displaced
step can involve changing the values in some temporary registers.  If
the displaced step succeeds then this is fine; after the step the
temporary registers are restored to their original values in the
architecture specific code.

But if the displaced step does not succeed then the temporary
registers are never restored, and they retain their modified values.

In this context a temporary register is simply any register that is
not otherwise used by the instruction being stepped that the
architecture specific code considers safe to borrow for the lifetime
of the instruction being stepped.

In the bug PR gdb/22921, the amd64 instruction being stepped is
an rip-relative instruction like this:

  jmp    *0x2fe2(%rip)

When we displaced step this instruction we borrow a register, and
modify the instruction to something like:

  jmp    *0x2fe2(%rcx)

with %rcx having its value adjusted to contain the original %rip
value.

Now if the displaced step does not succeed, then %rcx will be left
with a corrupted value.  Obviously corrupting any register is bad; in
the bug report this problem was spotted because %rcx is used as a
function argument register.

And finally, why might a displaced step not succeed?  Asynchronous
signals provides one reason.  GDB sets up for the displaced step and,
at that precise moment, the OS delivers a signal (SIGALRM in the bug
report), the signal stops the inferior at the address of the displaced
instruction.  GDB cancels the displaced instruction, handles the
signal, and then tries again with the displaced step.  But it is that
first cancellation of the displaced step that causes the problem; in
that case GDB (correctly) sees the displaced step as having not
completed, and so does not perform the architecture specific fixup,
leaving the register corrupted.

The reason why I think AArch64, rs600, i386, and s390 are not effected
by this problem is that I don't believe these architectures make use
of any temporary registers, so when a displaced step is not completed
successfully, the minimal fix up is sufficient.

On amd64 we use at most one temporary register.

On ARM, looking at arm_displaced_step_copy_insn_closure, we could
modify up to 16 temporary registers, and the instruction being
displaced stepped could be expanded to multiple replacement
instructions, which increases the chances of this bug triggering.

This commit only aims to address the issue on amd64 for now, though I
believe that the approach I'm proposing here might be applicable for
ARM too.

What I propose is that we always call gdbarch_displaced_step_fixup.

We will now pass an extra argument to gdbarch_displaced_step_fixup,
this a boolean that indicates whether GDB thinks the displaced step
completed successfully or not.

When this flag is false this indicates that the displaced step halted
for some "other" reason.  On ARM GDB can potentially read the
inferior's program counter in order figure out how far through the
sequence of replacement instructions we got, and from that GDB can
figure out what fixup needs to be performed.

On targets like amd64 the problem is slightly easier as displaced
stepping only uses a single replacement instruction.  If the displaced
step didn't complete the GDB knows that the single instruction didn't
execute.

The point is that by always calling gdbarch_displaced_step_fixup, each
architecture can now ensure that the inferior state is fixed up
correctly in all cases, not just the success case.

On amd64 this ensures that we always restore the temporary register
value, and so bug PR gdb/22921 is resolved.

In order to move all architectures to this new API, I have moved the
minimal roll-back version of the code inside the architecture specific
fixup functions for AArch64, rs600, s390, and ARM.  For all of these
except ARM I think this is good enough, as no temporaries are used all
that's needed is the program counter restore anyway.

For ARM the minimal code is no worse than what we had before, though I
do consider this architecture's displaced-stepping broken.

I've updated the gdb.arch/amd64-disp-step.exp test to cover the
'jmpq*' instruction that was causing problems in the original bug, and
also added support for testing the displaced step in the presence of
asynchronous signal delivery.

I've also added two new tests (for amd64 and i386) that check that GDB
can correctly handle displaced stepping over a single instruction that
branches to itself.  I added these tests after a first version of this
patch relied too much on checking the program-counter value in order
to see if the displaced instruction had executed.  This works fine in
almost all cases, but when an instruction branches to itself a pure
program counter check is not sufficient.  The new tests expose this
problem.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22921

Approved-By: Pedro Alves <pedro@palves.net>

23 files changed, 567 insertions, 92 deletions

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index a1daa9a1788..d8349e4ccdb 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3371,14 +3371,21 @@ void
 aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
 			      struct displaced_step_copy_insn_closure *dsc_,
 			      CORE_ADDR from, CORE_ADDR to,
-			      struct regcache *regs)
+			      struct regcache *regs, bool completed_p)
 {
-  aarch64_displaced_step_copy_insn_closure *dsc
-    = (aarch64_displaced_step_copy_insn_closure *) dsc_;
+  CORE_ADDR pc = regcache_read_pc (regs);
 
-  ULONGEST pc;
+  /* If the displaced instruction didn't complete successfully then all we
+     need to do is restore the program counter.  */
+  if (!completed_p)
+    {
+      pc = from + (pc - to);
+      regcache_write_pc (regs, pc);
+      return;
+    }
 
-  regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc);
+  aarch64_displaced_step_copy_insn_closure *dsc
+    = (aarch64_displaced_step_copy_insn_closure *) dsc_;
 
   displaced_debug_printf ("PC after stepping: %s (was %s).",
 			  paddress (gdbarch, pc), paddress (gdbarch, to));
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index ae38327ffab..505e050ba48 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -142,7 +142,7 @@ displaced_step_copy_insn_closure_up
 void aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
 				   displaced_step_copy_insn_closure *dsc,
 				   CORE_ADDR from, CORE_ADDR to,
-				   struct regcache *regs);
+				   struct regcache *regs, bool completed_p);
 
 bool aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch);
 
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 228b7518cb0..8d345257e12 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1690,7 +1690,7 @@ void
 amd64_displaced_step_fixup (struct gdbarch *gdbarch,
 			    struct displaced_step_copy_insn_closure *dsc_,
 			    CORE_ADDR from, CORE_ADDR to,
-			    struct regcache *regs)
+			    struct regcache *regs, bool completed_p)
 {
   amd64_displaced_step_copy_insn_closure *dsc
     = (amd64_displaced_step_copy_insn_closure *) dsc_;
@@ -1725,14 +1725,14 @@ amd64_displaced_step_fixup (struct gdbarch *gdbarch,
      the displaced instruction; make it relative to the original insn.
      Well, signal handler returns don't need relocation either, but we use the
      value of %rip to recognize those; see below.  */
-  if (! amd64_absolute_jmp_p (insn_details)
-      && ! amd64_absolute_call_p (insn_details)
-      && ! amd64_ret_p (insn_details))
+  if (!completed_p
+      || (!amd64_absolute_jmp_p (insn_details)
+	  && !amd64_absolute_call_p (insn_details)
+	  && !amd64_ret_p (insn_details)))
     {
-      ULONGEST orig_rip;
       int insn_len;
 
-      regcache_cooked_read_unsigned (regs, AMD64_RIP_REGNUM, &orig_rip);
+      CORE_ADDR pc = regcache_read_pc (regs);
 
       /* A signal trampoline system call changes the %rip, resuming
 	 execution of the main program after the signal handler has
@@ -1749,24 +1749,23 @@ amd64_displaced_step_fixup (struct gdbarch *gdbarch,
 	 it unrelocated.  Goodness help us if there are PC-relative
 	 system calls.	*/
       if (amd64_syscall_p (insn_details, &insn_len)
-	  && orig_rip != to + insn_len
 	  /* GDB can get control back after the insn after the syscall.
-	     Presumably this is a kernel bug.
-	     Fixup ensures its a nop, we add one to the length for it.  */
-	  && orig_rip != to + insn_len + 1)
+	     Presumably this is a kernel bug.  Fixup ensures its a nop, we
+	     add one to the length for it.  */
+	  && (pc < to || pc > (to + insn_len + 1)))
 	displaced_debug_printf ("syscall changed %%rip; not relocating");
       else
 	{
-	  ULONGEST rip = orig_rip - insn_offset;
+	  CORE_ADDR rip = pc - insn_offset;
 
 	  /* 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);
+	  regcache_write_pc (regs, rip);
 
 	  displaced_debug_printf ("relocated %%rip from %s to %s",
-				  paddress (gdbarch, orig_rip),
+				  paddress (gdbarch, pc),
 				  paddress (gdbarch, rip));
 	}
     }
@@ -1779,7 +1778,7 @@ amd64_displaced_step_fixup (struct gdbarch *gdbarch,
   /* If the instruction was a call, the return address now atop the
      stack is the address following the copied instruction.  We need
      to make it the address following the original instruction.	 */
-  if (amd64_call_p (insn_details))
+  if (completed_p && amd64_call_p (insn_details))
     {
       ULONGEST rsp;
       ULONGEST retaddr;
diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h
index 929b4b8bdc5..31bf7f2f96f 100644
--- a/gdb/amd64-tdep.h
+++ b/gdb/amd64-tdep.h
@@ -93,7 +93,7 @@ extern displaced_step_copy_insn_closure_up amd64_displaced_step_copy_insn
    struct regcache *regs);
 extern void amd64_displaced_step_fixup
   (struct gdbarch *gdbarch, displaced_step_copy_insn_closure *closure,
-   CORE_ADDR from, CORE_ADDR to, struct regcache *regs);
+   CORE_ADDR from, CORE_ADDR to, struct regcache *regs, bool completed_p);
 
 /* Initialize the ABI for amd64.  Uses DEFAULT_TDESC as fallback
    tdesc, if INFO does not specify one.  */
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 492a71fa987..bfe7d63ed8f 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8659,8 +8659,32 @@ void
 arm_displaced_step_fixup (struct gdbarch *gdbarch,
 			  struct displaced_step_copy_insn_closure *dsc_,
 			  CORE_ADDR from, CORE_ADDR to,
-			  struct regcache *regs)
+			  struct regcache *regs, bool completed_p)
 {
+  /* The following block exists as a temporary measure while displaced
+     stepping is fixed architecture at a time within GDB.
+
+     In an earlier implementation of displaced stepping, if GDB thought the
+     displaced instruction had not been executed then this fix up function
+     was never called.  As a consequence, things that should be fixed by
+     this function were left in an unfixed state.
+
+     However, it's not as simple as always calling this function; this
+     function needs to be updated to decide what should be fixed up based
+     on whether the displaced step executed or not, which requires each
+     architecture to be considered individually.
+
+     Until this architecture is updated, this block replicates the old
+     behaviour; we just restore the program counter register, and leave
+     everything else unfixed.  */
+  if (!completed_p)
+    {
+      CORE_ADDR pc = regcache_read_pc (regs);
+      pc = from + (pc - to);
+      regcache_write_pc (regs, pc);
+      return;
+    }
+
   arm_displaced_step_copy_insn_closure *dsc
     = (arm_displaced_step_copy_insn_closure *) dsc_;
 
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index a8d21c44ba4..cb0e8ce959b 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -296,7 +296,8 @@ int arm_frame_is_thumb (frame_info_ptr frame);
 
 extern void arm_displaced_step_fixup (struct gdbarch *,
 				      displaced_step_copy_insn_closure *,
-				      CORE_ADDR, CORE_ADDR, struct regcache *);
+				      CORE_ADDR, CORE_ADDR,
+				      struct regcache *, bool);
 
 /* Return the bit mask in ARM_PS_REGNUM that indicates Thumb mode.  */
 extern int arm_psr_thumb_bit (struct gdbarch *);
diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c
index c26888404b3..bc59ef01478 100644
--- a/gdb/displaced-stepping.c
+++ b/gdb/displaced-stepping.c
@@ -263,22 +263,13 @@ displaced_step_buffers::finish (gdbarch *arch, thread_info *thread,
   bool instruction_executed_successfully
     = displaced_step_instruction_executed_successfully (arch, status);
 
-  if (instruction_executed_successfully)
-    {
-      gdbarch_displaced_step_fixup (arch, copy_insn_closure.get (),
-				    buffer->original_pc,
-				    buffer->addr, rc);
-      return DISPLACED_STEP_FINISH_STATUS_OK;
-    }
-  else
-    {
-      /* Since the instruction didn't complete, all we can do is relocate the
-	 PC.  */
-      CORE_ADDR pc = regcache_read_pc (rc);
-      pc = buffer->original_pc + (pc - buffer->addr);
-      regcache_write_pc (rc, pc);
-      return DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED;
-    }
+  gdbarch_displaced_step_fixup (arch, copy_insn_closure.get (),
+				buffer->original_pc, buffer->addr,
+				rc, instruction_executed_successfully);
+
+  return (instruction_executed_successfully
+	  ? DISPLACED_STEP_FINISH_STATUS_OK
+	  : DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED);
 }
 
 const displaced_step_copy_insn_closure *
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index bbf2376fc4b..a2bd08b8315 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -1068,9 +1068,9 @@ typedef bool (gdbarch_displaced_step_hw_singlestep_ftype) (struct gdbarch *gdbar
 extern bool gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch);
 extern void set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep);
 
-/* Fix up the state resulting from successfully single-stepping a
-   displaced instruction, to give the result we would have gotten from
-   stepping the instruction in its original location.
+/* Fix up the state after attempting to single-step a displaced
+   instruction, to give the result we would have gotten from stepping the
+   instruction in its original location.
 
    REGS is the register state resulting from single-stepping the
    displaced instruction.
@@ -1078,15 +1078,24 @@ extern void set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, g
    CLOSURE is the result from the matching call to
    gdbarch_displaced_step_copy_insn.
 
-   If you provide gdbarch_displaced_step_copy_insn.but not this
-   function, then GDB assumes that no fixup is needed after
-   single-stepping the instruction.
+   FROM is the address where the instruction was original located, TO is
+   the address of the displaced buffer where the instruction was copied
+   to for stepping.
+
+   COMPLETED_P is true if GDB stopped as a result of the requested step
+   having completed (e.g. the inferior stopped with SIGTRAP), otherwise
+   COMPLETED_P is false and GDB stopped for some other reason.  In the
+   case where a single instruction is expanded to multiple replacement
+   instructions for stepping then it may be necessary to read the current
+   program counter from REGS in order to decide how far through the
+   series of replacement instructions the inferior got before stopping,
+   this may impact what will need fixing up in this function.
 
    For a general explanation of displaced stepping and how GDB uses it,
    see the comments in infrun.c. */
 
-typedef void (gdbarch_displaced_step_fixup_ftype) (struct gdbarch *gdbarch, struct displaced_step_copy_insn_closure *closure, CORE_ADDR from, CORE_ADDR to, struct regcache *regs);
-extern void gdbarch_displaced_step_fixup (struct gdbarch *gdbarch, struct displaced_step_copy_insn_closure *closure, CORE_ADDR from, CORE_ADDR to, struct regcache *regs);
+typedef void (gdbarch_displaced_step_fixup_ftype) (struct gdbarch *gdbarch, struct displaced_step_copy_insn_closure *closure, CORE_ADDR from, CORE_ADDR to, struct regcache *regs, bool completed_p);
+extern void gdbarch_displaced_step_fixup (struct gdbarch *gdbarch, struct displaced_step_copy_insn_closure *closure, CORE_ADDR from, CORE_ADDR to, struct regcache *regs, bool completed_p);
 extern void set_gdbarch_displaced_step_fixup (struct gdbarch *gdbarch, gdbarch_displaced_step_fixup_ftype *displaced_step_fixup);
 
 /* Prepare THREAD for it to displaced step the instruction at its current PC.
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 84beb087336..995f49e525f 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -4057,13 +4057,13 @@ set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
 }
 
 void
-gdbarch_displaced_step_fixup (struct gdbarch *gdbarch, struct displaced_step_copy_insn_closure *closure, CORE_ADDR from, CORE_ADDR to, struct regcache *regs)
+gdbarch_displaced_step_fixup (struct gdbarch *gdbarch, struct displaced_step_copy_insn_closure *closure, CORE_ADDR from, CORE_ADDR to, struct regcache *regs, bool completed_p)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->displaced_step_fixup != NULL);
   if (gdbarch_debug >= 2)
     gdb_printf (gdb_stdlog, "gdbarch_displaced_step_fixup called\n");
-  gdbarch->displaced_step_fixup (gdbarch, closure, from, to, regs);
+  gdbarch->displaced_step_fixup (gdbarch, closure, from, to, regs, completed_p);
 }
 
 void
diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index 52beaeaa245..d30c537ee1c 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -1771,9 +1771,9 @@ gdbarch_software_single_step routine, and true otherwise.
 
 Method(
     comment="""
-Fix up the state resulting from successfully single-stepping a
-displaced instruction, to give the result we would have gotten from
-stepping the instruction in its original location.
+Fix up the state after attempting to single-step a displaced
+instruction, to give the result we would have gotten from stepping the
+instruction in its original location.
 
 REGS is the register state resulting from single-stepping the
 displaced instruction.
@@ -1781,9 +1781,18 @@ displaced instruction.
 CLOSURE is the result from the matching call to
 gdbarch_displaced_step_copy_insn.
 
-If you provide gdbarch_displaced_step_copy_insn.but not this
-function, then GDB assumes that no fixup is needed after
-single-stepping the instruction.
+FROM is the address where the instruction was original located, TO is
+the address of the displaced buffer where the instruction was copied
+to for stepping.
+
+COMPLETED_P is true if GDB stopped as a result of the requested step
+having completed (e.g. the inferior stopped with SIGTRAP), otherwise
+COMPLETED_P is false and GDB stopped for some other reason.  In the
+case where a single instruction is expanded to multiple replacement
+instructions for stepping then it may be necessary to read the current
+program counter from REGS in order to decide how far through the
+series of replacement instructions the inferior got before stopping,
+this may impact what will need fixing up in this function.
 
 For a general explanation of displaced stepping and how GDB uses it,
 see the comments in infrun.c.
@@ -1795,6 +1804,7 @@ see the comments in infrun.c.
         ("CORE_ADDR", "from"),
         ("CORE_ADDR", "to"),
         ("struct regcache *", "regs"),
+        ("bool", "completed_p")
     ],
     predicate=False,
     predefault="NULL",
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index e93479c35a3..1ab9fc0e87d 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -843,7 +843,7 @@ void
 i386_displaced_step_fixup (struct gdbarch *gdbarch,
 			   struct displaced_step_copy_insn_closure *closure_,
 			   CORE_ADDR from, CORE_ADDR to,
-			   struct regcache *regs)
+			   struct regcache *regs, bool completed_p)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
@@ -886,14 +886,14 @@ i386_displaced_step_fixup (struct gdbarch *gdbarch,
      the displaced instruction; make it relative.  Well, signal
      handler returns don't need relocation either, but we use the
      value of %eip to recognize those; see below.  */
-  if (! i386_absolute_jmp_p (insn)
-      && ! i386_absolute_call_p (insn)
-      && ! i386_ret_p (insn))
+  if (!completed_p
+      || (!i386_absolute_jmp_p (insn)
+	  && !i386_absolute_call_p (insn)
+	  && !i386_ret_p (insn)))
     {
-      ULONGEST orig_eip;
       int insn_len;
 
-      regcache_cooked_read_unsigned (regs, I386_EIP_REGNUM, &orig_eip);
+      CORE_ADDR pc = regcache_read_pc (regs);
 
       /* A signal trampoline system call changes the %eip, resuming
 	 execution of the main program after the signal handler has
@@ -910,25 +910,25 @@ 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 - insn_start) + insn_len
+	  && pc != to + (insn - insn_start) + insn_len
 	  /* GDB can get control back after the insn after the syscall.
 	     Presumably this is a kernel bug.
 	     i386_displaced_step_copy_insn ensures its a nop,
 	     we add one to the length for it.  */
-	  && orig_eip != to + (insn - insn_start) + insn_len + 1)
+	  && pc != to + (insn - insn_start) + insn_len + 1)
 	displaced_debug_printf ("syscall changed %%eip; not relocating");
       else
 	{
-	  ULONGEST eip = (orig_eip - insn_offset) & 0xffffffffUL;
+	  ULONGEST eip = (pc - insn_offset) & 0xffffffffUL;
 
 	  /* 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);
+	  regcache_write_pc (regs, eip);
 
 	  displaced_debug_printf ("relocated %%eip from %s to %s",
-				  paddress (gdbarch, orig_eip),
+				  paddress (gdbarch, pc),
 				  paddress (gdbarch, eip));
 	}
     }
@@ -941,7 +941,7 @@ i386_displaced_step_fixup (struct gdbarch *gdbarch,
   /* If the instruction was a call, the return address now atop the
      stack is the address following the copied instruction.  We need
      to make it the address following the original instruction.  */
-  if (i386_call_p (insn))
+  if (completed_p && i386_call_p (insn))
     {
       ULONGEST esp;
       ULONGEST retaddr;
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 371bce72369..642ac89b240 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -448,7 +448,7 @@ extern displaced_step_copy_insn_closure_up i386_displaced_step_copy_insn
    struct regcache *regs);
 extern void i386_displaced_step_fixup
   (struct gdbarch *gdbarch, displaced_step_copy_insn_closure *closure,
-   CORE_ADDR from, CORE_ADDR to, regcache *regs);
+   CORE_ADDR from, CORE_ADDR to, regcache *regs, bool completed_p);
 
 /* Initialize a basic ELF architecture variant.  */
 extern void i386_elf_init_abi (struct gdbarch_info, struct gdbarch *);
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index b071f38c960..afeea02a84d 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -952,8 +952,18 @@ static void
 ppc_displaced_step_fixup (struct gdbarch *gdbarch,
 			  struct displaced_step_copy_insn_closure *closure_,
 			  CORE_ADDR from, CORE_ADDR to,
-			  struct regcache *regs)
+			  struct regcache *regs, bool completed_p)
 {
+  /* If the displaced instruction didn't complete successfully then all we
+     need to do is restore the program counter.  */
+  if (!completed_p)
+    {
+      CORE_ADDR pc = regcache_read_pc (regs);
+      pc = from + (pc - to);
+      regcache_write_pc (regs, pc);
+      return;
+    }
+
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   /* Our closure is a copy of the instruction.  */
   ppc_displaced_step_copy_insn_closure *closure
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 081a8b68867..047ee088aed 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -482,8 +482,19 @@ static void
 s390_displaced_step_fixup (struct gdbarch *gdbarch,
 			   displaced_step_copy_insn_closure *closure_,
 			   CORE_ADDR from, CORE_ADDR to,
-			   struct regcache *regs)
+			   struct regcache *regs, bool completed_p)
 {
+  CORE_ADDR pc = regcache_read_pc (regs);
+
+  /* If the displaced instruction didn't complete successfully then all we
+     need to do is restore the program counter.  */
+  if (!completed_p)
+    {
+      pc = from + (pc - to);
+      regcache_write_pc (regs, pc);
+      return;
+    }
+
   /* Our closure is a copy of the instruction.  */
   s390_displaced_step_copy_insn_closure *closure
     = (s390_displaced_step_copy_insn_closure *) closure_;
@@ -495,10 +506,8 @@ s390_displaced_step_fixup (struct gdbarch *gdbarch,
   unsigned int b2, r1, r2, x2, r3;
   int i2, d2;
 
-  /* Get current PC and addressing mode bit.  */
-  CORE_ADDR pc = regcache_read_pc (regs);
+  /* Get addressing mode bit.  */
   ULONGEST amode = 0;
-
   if (register_size (gdbarch, S390_PSWA_REGNUM) == 4)
     {
       regcache_cooked_read_unsigned (regs, S390_PSWA_REGNUM, &amode);
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-self-call-alarm.c b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call-alarm.c
new file mode 100644
index 00000000000..aec3d294b15
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call-alarm.c
@@ -0,0 +1,24 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+
+void
+setup_alarm (void)
+{
+  alarm (300);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.S b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.S
new file mode 100644
index 00000000000..7372dc10132
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.S
@@ -0,0 +1,49 @@
+/* Copyright 2009-2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   This file is part of the gdb testsuite.
+   It tests displaced stepping over various insns that require special
+   handling.  */
+
+	.text
+
+	.global main
+main:
+	nop
+
+	callq	setup_alarm
+
+	nop
+
+/***********************************************/
+
+/* test call/ret */
+
+	.global test_call
+test_call:
+	call test_call
+	nop
+	.global test_ret_end
+test_ret_end:
+	nop
+
+/***********************************************/
+
+/* all done */
+
+done:
+	mov $0,%rdi
+	call exit
+	hlt
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.exp b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.exp
new file mode 100644
index 00000000000..db44a319a47
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step-self-call.exp
@@ -0,0 +1,81 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test amd64 displaced stepping over a call instruction that calls to
+# itself.  This is pretty unlikely to be seen in the wild, but does
+# test a corner case of our displaced step handling.
+
+require is_x86_64_m64_target
+
+set newline "\[\r\n\]*"
+
+set opts {debug nopie}
+standard_testfile .S -alarm.c
+
+if { [prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" $opts] } {
+    return -1
+}
+
+gdb_test "set displaced-stepping on" ""
+gdb_test "show displaced-stepping" ".* displaced stepping .* is on.*"
+
+if {![runto_main]} {
+    return 0
+}
+
+# Proceed to the test function.
+gdb_breakpoint "test_call"
+gdb_continue_to_breakpoint "test_call"
+
+# Get the current stack pointer value.
+set sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*"]
+
+# Get the address of the next instruction.
+set next_insn_addr ""
+gdb_test_multiple "x/2i \$pc" "get address of next insn" {
+    -re "\r\n=> $hex \[^\r\n\]+\r\n" {
+	exp_continue
+    }
+    -re "^   ($hex) \[^\r\n\]+\r\n" {
+	set next_insn_addr $expect_out(1,string)
+	exp_continue
+    }
+    -re "^$::gdb_prompt $" {
+	gdb_assert {![string equal $next_insn_addr ""]} \
+	    $gdb_test_name
+    }
+}
+
+# Clear the slot on the stack and confirm it was set to zero.
+set sp [expr $sp - 0x8]
+gdb_test_no_output "set {unsigned long long} $sp = 0"
+set zero_val 0x[format %016x 0]
+gdb_test "x/1gx 0x[format %x $sp]" "$hex:\\s+${zero_val}" \
+    "check return address slot was set to zero"
+
+# Single step.
+gdb_test "stepi" \
+    "Breakpoint $decimal, test_call \\(\\) at .*"
+
+# Check stack pointer was updated to the expected value.
+set new_sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*" \
+	       "get stack pointer after step"]
+gdb_assert {[expr $sp == $new_sp]} \
+    "check stack pointer was updated as expected"
+
+# Check the contents of the stack were updated to the expected value.
+set next_insn_addr 0x[format %016X $next_insn_addr]
+gdb_test "x/1gx 0x[format %x $sp]" "$hex:\\s+$next_insn_addr" \
+    "check return address was updated correctly"
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-signal.c b/gdb/testsuite/gdb.arch/amd64-disp-step-signal.c
new file mode 100644
index 00000000000..311e11e7afd
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step-signal.c
@@ -0,0 +1,36 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <signal.h>
+#include <stdio.h>
+
+#ifdef SIGALRM
+
+static void
+sigalrm_handler (int sig)
+{
+}
+
+#endif
+
+void
+setup_signal_handler (void)
+{
+#ifdef SIGALRM
+  signal (SIGALRM, sigalrm_handler);
+#endif
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.S b/gdb/testsuite/gdb.arch/amd64-disp-step.S
index b25e292bdf0..bf73778cf43 100644
--- a/gdb/testsuite/gdb.arch/amd64-disp-step.S
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step.S
@@ -23,6 +23,10 @@
 main:
 	nop
 
+	callq	setup_signal_handler
+
+	nop
+
 /***********************************************/
 
 /* test call/ret */
@@ -135,6 +139,14 @@ test_rip_rdi:
 test_rip_rdi_end:
 	nop
 
+	.global test_jmp
+test_jmp:
+	jmpq 	*jmp_dest(%rip)
+	nop
+	.global test_jmp_end
+test_jmp_end:
+	nop
+
 	/* skip over test data */
 	jmp done
 
@@ -142,6 +154,9 @@ test_rip_rdi_end:
 
 answer:	.8byte 42
 
+jmp_dest:
+	.8byte	test_jmp_end
+
 /***********************************************/
 
 /* all done */
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
index 2aee1e05774..0919e71d715 100644
--- a/gdb/testsuite/gdb.arch/amd64-disp-step.exp
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
@@ -17,15 +17,14 @@
 
 # Test amd64 displaced stepping.
 
-
 require is_x86_64_m64_target
 
 set newline "\[\r\n\]*"
 
 set opts {debug nopie}
-standard_testfile .S
+standard_testfile .S -signal.c
 
-if { [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] } {
+if { [prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" $opts] } {
     return -1
 }
 
@@ -154,9 +153,13 @@ proc set_regs { regs val } {
     }
 }
 
-# Verify all REGS equal VAL, except REG which equals REG_VAL.
+# Verify all REGS equal VAL, except EXCEPT_REG which equals
+# EXCEPT_REG_VAL.
+#
+# It is fine for EXCEPT_REG to be the empty string, in which case no
+# register will be checked for EXCEPT_REG_VAL.
 
-proc verify_regs { test_name regs val except_reg except_reg_val } {
+proc_with_prefix verify_regs { regs val except_reg except_reg_val } {
     global newline
 
     foreach reg ${regs} {
@@ -165,36 +168,89 @@ proc verify_regs { test_name regs val except_reg except_reg_val } {
 	    set expected ${except_reg_val}
 	}
 	# The cast to (int) is because RBP is printed as a pointer.
-	gdb_test "p (int) \$${reg}" " = ${expected}${newline}" "${test_name} ${reg} expected value"
+	gdb_test "p (int) \$${reg}" " = ${expected}${newline}" "${reg} expected value"
     }
 }
 
-proc rip_test { reg } {
+# Run the rip-relative tests.
+#
+# TEST_START_LABEL and TEST_END_LABEL are two labels that delimit the
+# test in the srcfile.
+#
+# REG is either the name of a register which is the destination
+# location (when testing the add instruction), otherwise REG should be
+# the empty string, when testing the 'jmpq*' instruction.
+#
+# SIGNAL_MODES is a list which always contains 'off' and optionally
+# might also contain 'on'.  The 'on' value is only included if the
+# target supports sending SIGALRM to the inferior.  The test is
+# repeated for each signal mode.  With signal mode 'on' we send a
+# signal to the inferior while it is performing a displaced step.
+proc rip_test { reg test_start_label test_end_label signal_modes } {
     global srcfile rip_regs
 
-    set test_start_label "test_rip_${reg}"
-    set test_end_label "test_rip_${reg}_end"
-
     gdb_test "break ${test_start_label}" \
 	"Breakpoint.*at.* file .*$srcfile, line.*"
     gdb_test "break ${test_end_label}" \
 	"Breakpoint.*at.* file .*$srcfile, line.*"
 
-    gdb_test "continue" \
-	"Continuing.*Breakpoint.*, ${test_start_label} ().*" \
-	"continue to ${test_start_label}"
+    foreach_with_prefix send_signal $signal_modes {
+	if {$send_signal eq [lindex $signal_modes 0]} {
+	    # The first time through we can just continue to the
+	    # breakpoint.
+	    gdb_test "continue" \
+		"Continuing.*Breakpoint.*, ${test_start_label} ().*" \
+		"continue to ${test_start_label}"
+	} else {
+	    # For the second time through the test we need to jump
+	    # back to the beginning.
+	    gdb_test "jump ${test_start_label}" \
+		"Breakpoint.*, ${test_start_label} ().*" \
+		"jump back to ${test_start_label}"
+	}
+
+	set_regs ${rip_regs} 0
+
+	if {$send_signal} {
+	    # The signal sending tests require that the signal appear to
+	    # arrive from an outside source, i.e. we can't use GDB's 'signal'
+	    # command to deliver it.
+	    #
+	    # The signal must arrive while GDB is processing the displaced
+	    # step instruction.
+	    #
+	    # If we use 'signal' to send the signal GDB doesn't actually do
+	    # the displaced step, but instead just delivers the signal.
+	    set inferior_pid [get_inferior_pid]
+	    remote_exec target "kill -ALRM $inferior_pid"
+	}
 
-    set_regs ${rip_regs} 0
+	gdb_test "continue" \
+	    "Continuing.*Breakpoint.*, ${test_end_label} ().*" \
+	    "continue to ${test_end_label}"
 
-    gdb_test "continue" \
-	"Continuing.*Breakpoint.*, ${test_end_label} ().*" \
-	"continue to ${test_end_label}"
+	verify_regs ${rip_regs} 0 ${reg} 42
+    }
+}
 
-    verify_regs "test rip w/${reg}" ${rip_regs} 0 ${reg} 42
+if {![target_info exists gdb,nosignals] && ![istarget "*-*-mingw*"]} {
+    # Only targets that support SIGALRM can run the signal tests.
+    set signal_modes { off on }
+} else {
+    set signal_modes { off }
 }
 
+# The rip-relative add instructions.  There's a test writing to
+# each register in RIP_REGS in turn.
 foreach reg ${rip_regs} {
-    rip_test $reg
+    with_test_prefix "add into ${reg}" {
+	rip_test $reg "test_rip_${reg}" "test_rip_${reg}_end" $signal_modes
+    }
+}
+
+# Now test the rip-relative 'jmpq*' instruction.
+with_test_prefix "rip-relative jmpq*" {
+    rip_test "" "test_jmp" "test_jmp_end" $signal_modes
 }
 
 ##########################################
diff --git a/gdb/testsuite/gdb.arch/i386-disp-step-self-call-alarm.c b/gdb/testsuite/gdb.arch/i386-disp-step-self-call-alarm.c
new file mode 100644
index 00000000000..aec3d294b15
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-disp-step-self-call-alarm.c
@@ -0,0 +1,24 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+
+void
+setup_alarm (void)
+{
+  alarm (300);
+}
diff --git a/gdb/testsuite/gdb.arch/i386-disp-step-self-call.S b/gdb/testsuite/gdb.arch/i386-disp-step-self-call.S
new file mode 100644
index 00000000000..30553d508ba
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-disp-step-self-call.S
@@ -0,0 +1,49 @@
+/* Copyright 2009-2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   This file is part of the gdb testsuite.
+   It tests displaced stepping over various insns that require special
+   handling.  */
+
+	.text
+
+	.global main
+main:
+	nop
+
+	call	setup_alarm
+
+	nop
+
+/***********************************************/
+
+/* test call/ret */
+
+	.global test_call
+test_call:
+	call test_call
+	nop
+	.global test_ret_end
+test_ret_end:
+	nop
+
+/***********************************************/
+
+/* all done */
+
+done:
+	pushl $0
+	call exit
+	hlt
diff --git a/gdb/testsuite/gdb.arch/i386-disp-step-self-call.exp b/gdb/testsuite/gdb.arch/i386-disp-step-self-call.exp
new file mode 100644
index 00000000000..7ea036fe3e6
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/i386-disp-step-self-call.exp
@@ -0,0 +1,81 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test i386 displaced stepping over a call instruction that calls to
+# itself.  This is pretty unlikely to be seen in the wild, but does
+# test a corner case of our displaced step handling.
+
+require is_x86_like_target
+
+set newline "\[\r\n\]*"
+
+set opts {debug nopie}
+standard_testfile .S -alarm.c
+
+if { [prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" $opts] } {
+    return -1
+}
+
+gdb_test "set displaced-stepping on" ""
+gdb_test "show displaced-stepping" ".* displaced stepping .* is on.*"
+
+if {![runto_main]} {
+    return 0
+}
+
+# Proceed to the test function.
+gdb_breakpoint "test_call"
+gdb_continue_to_breakpoint "test_call"
+
+# Get the current stack pointer value.
+set sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*"]
+
+# Get the address of the next instruction.
+set next_insn_addr ""
+gdb_test_multiple "x/2i \$pc" "get address of next insn" {
+    -re "\r\n=> $hex \[^\r\n\]+\r\n" {
+	exp_continue
+    }
+    -re "^   ($hex) \[^\r\n\]+\r\n" {
+	set next_insn_addr $expect_out(1,string)
+	exp_continue
+    }
+    -re "^$::gdb_prompt $" {
+	gdb_assert {![string equal $next_insn_addr ""]} \
+	    $gdb_test_name
+    }
+}
+
+# Clear the slot on the stack and confirm it was set to zero.
+set sp [expr $sp - 0x4]
+gdb_test_no_output "set {unsigned long long} $sp = 0"
+set zero_val 0x[format %08x 0]
+gdb_test "x/1wx 0x[format %x $sp]" "$hex:\\s+${zero_val}" \
+    "check return address slot was set to zero"
+
+# Single step.
+gdb_test "stepi" \
+    "Breakpoint $decimal, test_call \\(\\) at .*"
+
+# Check stack pointer was updated to the expected value.
+set new_sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*" \
+	       "get stack pointer after step"]
+gdb_assert {[expr $sp == $new_sp]} \
+    "check stack pointer was updated as expected"
+
+# Check the contents of the stack were updated to the expected value.
+set next_insn_addr 0x[format %08X $next_insn_addr]
+gdb_test "x/1wx 0x[format %x $sp]" "$hex:\\s+$next_insn_addr" \
+    "check return address was updated correctly"