Re: [PATCH 00/18] Remote all-stop on top of non-stop

[Yao Qi <qiyaoltc at gmail dot com>]

Yao Qi <qiyaoltc@gmail.com> writes:

> There is one fail on multi-arch aarch64-linux in
> gdb.base/range-stepping.exp,
>
>   FAIL: gdb.base/range-stepping.exp: multi insns: next: vCont;s=1 vCont;r=1
>
> in the testing GDB and GDBserver is configured for aarch64-linux, but
> the program is compiled for arm-linux.  I checked gdb.log that there is
> vCont;r but no vCont;s.  I suspect that GDB does software single
> step, but arm-linux-tdep.c:arm_linux_software_single_step has already disable
> software single step if GDBserver can do single step (AArch64 GDBserver
> can do hardware single step).

Hi Pedro,
This fail above isn't caused by your patch series, but this series
exposes something we need to think about here.

In the test, after command "n" is issued, the test expects to see
vCont;s and vCont;r, because GDB first steps over the breakpoint and
then do range-stepping across the line of code.  Here is an assumption
that the remote target can do range stepping must support single step
(either by hardware or by software done by remote target itself), and
that is why I check the number vCont;r and vCont;s in the tests.  This
assumption is true for x86-linux and aarch64-linux.

However, it isn't the case when aarch64-linux GDBservers debugs
arm-linux program.  Aarch64-linux GDBserver claims supporting
range-stepping by defining aarch64_supports_range_stepping in
linux-aarch64-low.c, gdb.base/range-stepping.exp is tested.
(Note that this test is skipped on pure arm-linux testing, because
arm-linux GDBserver doesn't support range-stepping).  GDB will still
emit vCont;r to do range stepping, that is fine.

Before range-stepping, GDB needs to step over the breakpoint by in-line
stepping, GDB uses the right gdbarch (for arm-linux) to do that, so the
right decision on hardware single step vs software single step can be
made according to target_can_do_single_step ...

static int
arm_linux_software_single_step (struct frame_info *frame)
{
  struct gdbarch *gdbarch = get_frame_arch (frame);
  struct address_space *aspace = get_frame_address_space (frame);
  CORE_ADDR next_pc;

  if (arm_deal_with_atomic_sequence (frame))
    return 1;

  /* If the target does have hardware single step, GDB doesn't have
     to bother software single step.  */
  if (target_can_do_single_step () == 1)
    return 0;

in the multi-arch case, GDB stills emit vCont;s because it knows the
remote target can do single step.  That is why these tests pass before.

With your patch applied, GDB prefers to step over the breakpoint by
out-of-line stepping, and nowadays gdbarch (for arm-linux) decides to
resume the instructions in scratchpad rather than single step them,

in infrun.c:resume:

	  /* Update pc to reflect the new address from which we will
	     execute instructions due to displaced stepping.  */
	  pc = regcache_read_pc (get_thread_regcache (inferior_ptid));

	  displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
	  step = gdbarch_displaced_step_hw_singlestep (gdbarch,
						       displaced->step_closure);

(gdbarch_displaced_step_hw_singlestep returns zero on arm-linux), so GDB
emits vCont;c rather than vCont;s.  There are some ways fixing this
problem,

 1. stop checking vCont;s packet anymore in range-stepping tests.
 2. let gdbarch_displaced_step_hw_singlestep returns true for arm-linux
 in the multi-arch case like this,

int
arm_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
				  struct displaced_step_closure *closure)
{
  if (target_can_do_single_step () == 1)
    return 1;

  return 0;
}
then further, we need to either,

  2.1 teach GDB core to support single stepping multiple instructions in
  scratch pad.  Nowadays, GDB only expects one stop event when executing
  instructions in the scratchpad.  ARM is the only target that GDB
  copies more than one instructions to the scratchpad, and resume
  program there instead of single step.  Other targets, like x86,
  aarch64, GDB only copies *one* instruction to the scratchpad and
  single step.
  2.2 rewrite arm displaced stepping code to be aware that the target
  may be able to do single step, so that each time GDB has only to copy
  one instruction to the scratchpad, do single step and fix up if necessary.

Fix #1 looks reasonable and ideal to me, and the easiest one.  Fix #2.1
and #2.2 will need much work, at least #2.2, and I don't know how useful
#2.1 is.

-- 
Yao (éå)