Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Luis Machado <luis.machado@linaro.org>
To: gdb-patches@sourceware.org
Cc: alan.hayward@arm.com
Subject: [PATCH,v2][AArch64] Fix step-over-syscall.exp failure
Date: Mon, 13 Jan 2020 18:21:00 -0000	[thread overview]
Message-ID: <20200113174239.7819-3-luis.machado@linaro.org> (raw)
In-Reply-To: <20200113174239.7819-1-luis.machado@linaro.org>

New in v2:

- Reverted to using pc_adjust as bool/offset and added more comments to explain
how it is being used.

--

In particular, this one:

FAIL: gdb.base/step-over-syscall.exp: fork: displaced=on: check_pc_after_cross_syscall: single step over fork final pc

When ptrace fork event reporting is enabled, GDB gets a PTRACE_EVENT_FORK
event whenever the inferior executes the fork syscall.

Then the logic is that GDB needs to step the inferior yet again in order to
receive a predetermined SIGTRAP, but no execution takes place because the
signal was already queued for delivery. That means the PC should stay the same.

I noticed the aarch64 code is currently adjusting the PC in this situation,
making the inferior skip an instruction without executing it.

The following change checks if we did not execute the instruction
(pc - to == 0), making proper adjustments for such case.

Regression tested on aarch64-linux-gnu.

gdb/ChangeLog:

2020-01-13  Luis Machado  <luis.machado@linaro.org>

	* aarch64-tdep.c (struct aarch64_displaced_step_closure )
	<pc_adjust>: Adjust the documentation.
	(aarch64_displaced_step_fixup): Check if PC really moved before
	adjusting it.
---
 gdb/aarch64-tdep.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index da41e22130..6a9d34dc67 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2737,7 +2737,8 @@ struct aarch64_displaced_step_closure : public displaced_step_closure
      is being displaced stepping.  */
   int cond = 0;
 
-  /* PC adjustment offset after displaced stepping.  */
+  /* PC adjustment offset after displaced stepping.  If 0, then we don't
+     write the PC back, assuming the PC is already the right address.  */
   int32_t pc_adjust = 0;
 };
 
@@ -3032,11 +3033,12 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
 {
   aarch64_displaced_step_closure *dsc = (aarch64_displaced_step_closure *) dsc_;
 
+  ULONGEST pc;
+
+  regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc);
+
   if (dsc->cond)
     {
-      ULONGEST pc;
-
-      regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc);
       if (pc - to == 8)
 	{
 	  /* Condition is true.  */
@@ -3052,6 +3054,13 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
 
   if (dsc->pc_adjust != 0)
     {
+      /* Make sure the previous instruction was executed (that is, the PC
+	 has changed).  If the PC didn't change, then discard the adjustment
+	 offset.  Otherwise we may skip an instruction before its execution
+	 took place.  */
+      if ((pc - to) == 0)
+	dsc->pc_adjust = 0;
+
       if (debug_displaced)
 	{
 	  debug_printf ("displaced: fixup: set PC to %s:%d\n",
-- 
2.17.1


  parent reply	other threads:[~2020-01-13 17:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-13 17:42 [PATCH,v2][AArch64] Add more debugging output to aarch64_displaced_step_fixup Luis Machado
2020-01-13 17:42 ` [PATCH] [AArch64] Convert an int flag variable to bool Luis Machado
2020-01-14  4:25   ` Simon Marchi
2020-01-13 18:21 ` Luis Machado [this message]
2020-01-21 11:32   ` [PATCH,v2][AArch64] Fix step-over-syscall.exp failure Alan Hayward
2020-01-21 13:30     ` Luis Machado
2020-01-21 11:24 ` [PATCH,v2][AArch64] Add more debugging output to aarch64_displaced_step_fixup Alan Hayward

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200113174239.7819-3-luis.machado@linaro.org \
    --to=luis.machado@linaro.org \
    --cc=alan.hayward@arm.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox