[PATCH 1/2] [AArch64] Fix step-over-syscall.exp failure
Luis Machado
luis.machado@linaro.org
Mon Dec 30 16:25:00 GMT 2019
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 existing code abuses the pc_adjust variable to contain both an offset and
also a bool telling GDB when to adjust the PC (pc_adjust != 0).
This patch fixes this case by adding a new bool that tells us when we're
supposed to adjust the PC, and then proceeding to check if we did not execute
the instruction (pc - to == 0), making proper adjustments for such case.
Regression tested on aarch64-linux-gnu on the tryserver.
gdb/ChangeLog:
2019-12-30 Luis Machado <luis.machado@linaro.org>
* aarch64-tdep.c (struct aarch64_displaced_step_closure )
<should_adjust_pc>: New member.
<pc_adjust>: Adjust the documentation.
(aarch64_displaced_step_b): Set should_adjust_pc.
(aarch64_displaced_step_b_cond): Likewise.
(aarch64_displaced_step_cb): Likewise.
(aarch64_displaced_step_tb): Likewise.
(aarch64_displaced_step_adr): Likewise.
(aarch64_displaced_step_ldr_literal): Likewise.
(aarch64_displaced_step_others): Likewise.
(aarch64_displaced_step_fixup): Check if PC really moved before
adjusting it.
Change-Id: I828b7b7f2726f42ce107708f9692f07c63bf728c
---
gdb/aarch64-tdep.c | 46 +++++++++++++++++++++++++++++++++++++++-------
1 file changed, 39 insertions(+), 7 deletions(-)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 1d5fb2001d..a639b753cd 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2737,7 +2737,12 @@ struct aarch64_displaced_step_closure : public displaced_step_closure
is being displaced stepping. */
int cond = 0;
- /* PC adjustment offset after displaced stepping. */
+ /* True if we should adjust the PC after displaced stepping, false
+ otherwise. */
+ bool should_adjust_pc = false;
+
+ /* PC adjustment offset after displaced stepping, if should_adjust_pc
+ is true. */
int32_t pc_adjust = 0;
};
@@ -2783,6 +2788,9 @@ aarch64_displaced_step_b (const int is_bl, const int32_t offset,
emit_nop (dsd->insn_buf);
dsd->insn_count++;
dsd->dsc->pc_adjust = offset;
+
+ if (offset != 0)
+ dsd->dsc->should_adjust_pc = true;
}
if (is_bl)
@@ -2818,6 +2826,9 @@ aarch64_displaced_step_b_cond (const unsigned cond, const int32_t offset,
dsd->dsc->cond = 1;
dsd->dsc->pc_adjust = offset;
dsd->insn_count = 1;
+
+ if (offset != 0)
+ dsd->dsc->should_adjust_pc = true;
}
/* Dynamically allocate a new register. If we know the register
@@ -2852,6 +2863,9 @@ aarch64_displaced_step_cb (const int32_t offset, const int is_cbnz,
dsd->insn_count = 1;
dsd->dsc->cond = 1;
dsd->dsc->pc_adjust = offset;
+
+ if (offset != 0)
+ dsd->dsc->should_adjust_pc = true;
}
/* Implementation of aarch64_insn_visitor method "tb". */
@@ -2877,6 +2891,9 @@ aarch64_displaced_step_tb (const int32_t offset, int is_tbnz,
dsd->insn_count = 1;
dsd->dsc->cond = 1;
dsd->dsc->pc_adjust = offset;
+
+ if (offset != 0)
+ dsd->dsc->should_adjust_pc = true;
}
/* Implementation of aarch64_insn_visitor method "adr". */
@@ -2902,6 +2919,7 @@ aarch64_displaced_step_adr (const int32_t offset, const unsigned rd,
address);
dsd->dsc->pc_adjust = 4;
+ dsd->dsc->should_adjust_pc = true;
emit_nop (dsd->insn_buf);
dsd->insn_count = 1;
}
@@ -2929,6 +2947,7 @@ aarch64_displaced_step_ldr_literal (const int32_t offset, const int is_sw,
aarch64_register (rt, 1), zero);
dsd->dsc->pc_adjust = 4;
+ dsd->dsc->should_adjust_pc = true;
}
/* Implementation of aarch64_insn_visitor method "others". */
@@ -2946,10 +2965,12 @@ aarch64_displaced_step_others (const uint32_t insn,
if ((insn & 0xfffffc1f) == 0xd65f0000)
{
/* RET */
- dsd->dsc->pc_adjust = 0;
}
else
- dsd->dsc->pc_adjust = 4;
+ {
+ dsd->dsc->pc_adjust = 4;
+ dsd->dsc->should_adjust_pc = true;
+ }
}
static const struct aarch64_insn_visitor visitor =
@@ -3030,13 +3051,15 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
CORE_ADDR from, CORE_ADDR to,
struct regcache *regs)
{
+ ULONGEST pc;
+
aarch64_displaced_step_closure *dsc = (aarch64_displaced_step_closure *) dsc_;
+ /* Fetch the current PC, after the displaced execution took place. */
+ 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. */
@@ -3045,13 +3068,22 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
{
/* Condition is false. */
dsc->pc_adjust = 4;
+ dsc->should_adjust_pc = true;
}
else
gdb_assert_not_reached ("Unexpected PC value after displaced stepping");
}
- if (dsc->pc_adjust != 0)
+ if (dsc->should_adjust_pc)
{
+
+ /* Sometimes we may get a SIGTRAP even before executing an instruction.
+ Such is the case when we are stepping over a fork/vfork/clone syscall
+ and the instruction after the syscall instruction. Make sure we don't
+ adjust the PC when we did not really move. */
+ if ((pc - to) == 0)
+ dsc->pc_adjust = 0;
+
if (debug_displaced)
{
debug_printf ("displaced: fixup: set PC to %s:%d\n",
--
2.17.1
More information about the Gdb-patches
mailing list