Re: [RFA] arm-pikeos: software single step

[Pedro Alves <palves@redhat.com>]

On 09/10/2018 04:13 PM, Joel Brobecker wrote:
> From: Jerome Guitton <guitton@adacore.com>
> 
> Hello,
> 
> On ARM, PikeOS does not support hardware single step, causing various
> semi-random errors when trying to next/step over some user code. So
> this patch changes this target to use software-single-step instead.
> 
> The challenge is that, up to now, the PikeOS target was in all respects
> identical to a baremetal target as far as GDB was concerned, meaning
> we were using the baremetal osabi for this target too. This is no longer
> possible, and we need to introduce a new OSABI variant. Unfortunately,
> there isn't anything in the object file that would allow us to
> differentiate between the two platforms. So we have to rely on a
> heuristic instead, where we look for some known symbols that are
> required in a PikeOS application (these symbols are expected to be
> defined by the default linker script, and correspond to routines used
> to allocate the application stack).

It's unfortunate to have to introduce an OSABI for something that is a
property of the target that doesn't effect anything when e.g., you're
not connected yet.

It seems to me we can take a better approach here, that eliminates
this particular problem not just for PikeOS, but for all other cases
of random RTOS's.

That is, you should be able to make your stub tell GDB that it
can or can't hardware single step, and then GDB adjusts itself.

We already have all the info we need, we're just not using it.

See target_can_do_single_step and the remote.c implementation:

 int
 remote_target::can_do_single_step ()
 {
   /* We can only tell whether target supports single step or not by
      supported s and S vCont actions if the stub supports vContSupported
      feature.  If the stub doesn't support vContSupported feature,
      we have conservatively to think target doesn't supports single
      step.  */
   if (packet_support (PACKET_vContSupported) == PACKET_ENABLE)


From the manual:

 @item vContSupported
 This feature indicates whether @value{GDBN} wants to know the
 supported actions in the reply to @samp{vCont?} packet.
 @end table

 @item vCont?
 @cindex @samp{vCont?} packet
 Request a list of actions supported by the @samp{vCont} packet.

So, if you make your stub indicate support for the "vContSupported"
feature, and make sure the the reply to "vCont?" does not include
the s/S features, then GDB knows the target does not support
hardware single step.

The only think missing then I think is moving the only call to
target_can_do_single_step out of the arm-linux-tdep.c file:

 static std::vector<CORE_ADDR>
 arm_linux_software_single_step (struct regcache *regcache)
 {
   struct gdbarch *gdbarch = regcache->arch ();
   struct arm_get_next_pcs next_pcs_ctx;
 
   /* 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 {};

into somewhere more generic, around infrun.c:maybe_software_singlestep.

Can you try that?

Thanks,
Pedro Alves

> 
> gdb/ChangeLog (Jerome Guitton  <guitton@adacore.com>):
> 
>         * arm-pikeos-tdep.c: New file.
>         * configure.tgt: Add arm-pikeos-tdep.o to the case of ARM
>         embedded system.
>         * defs.h (enum gdb_osabi): Add GDB_OSABI_PIKEOS.
>         * osabi.c (gdb_osabi_names): Add name for GDB_OSABI_PIKEOS.
> 
> Tested on arm-pikeos and arm-elf using AdaCore's testsuite.
> We also evaluated it on armhf-linux as a cross platform.
> 
> OK to apply?
> 
> Thanks!
>