[RFA] arm-pikeos: software single step

[RFA] arm-pikeos: software single step

[Joel Brobecker]

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).

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!
-- 
Joel

---
 gdb/arm-pikeos-tdep.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.tgt     |    1 +
 gdb/defs.h            |    1 +
 gdb/osabi.c           |    1 +
 4 files changed, 98 insertions(+)
 create mode 100644 gdb/arm-pikeos-tdep.c

diff --git a/gdb/arm-pikeos-tdep.c b/gdb/arm-pikeos-tdep.c
new file mode 100644
index 0000000..1df3379
--- /dev/null
+++ b/gdb/arm-pikeos-tdep.c
@@ -0,0 +1,95 @@
+/* Copyright (C) 2016-2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 "defs.h"
+#include "objfiles.h"
+#include "arm-tdep.h"
+#include "osabi.h"
+
+static void
+arm_pikeos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  /* Single stepping.  */
+  set_gdbarch_software_single_step (gdbarch, arm_software_single_step);
+}
+
+static enum gdb_osabi
+arm_pikeos_osabi_sniffer (bfd *abfd)
+{
+  long storage_needed;
+  asymbol **symbol_table;
+  long number_of_symbols;
+  long i;
+  int pikeos_stack_found = 0;
+  int pikeos_stack_size_found = 0;
+  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+
+  /* The BFD target of PikeOS is really just standard elf, so we
+     cannot use it to detect this variant.  The only common thing that
+     may be found in PikeOS modules are symbols _vm_stack/__p4_stack and
+     _vm_stack_size/__p4_stack_end. They are used to specify the stack
+     location and size; and defined by the default linker script.
+
+     OS ABI sniffers are called before the minimal symtabs are
+     created. So inspect the symbol table using BFD.  */
+
+  storage_needed = bfd_get_symtab_upper_bound (abfd);
+
+  if (storage_needed <= 0)
+    return GDB_OSABI_UNKNOWN;
+
+  symbol_table = (asymbol **) xmalloc (storage_needed);
+  make_cleanup (xfree, symbol_table);
+
+  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
+
+  if (number_of_symbols < 0)
+    {
+      do_cleanups (old_chain);
+      return GDB_OSABI_UNKNOWN;
+    }
+
+  for (i = 0; i < number_of_symbols; i++)
+    {
+      const char *name = bfd_asymbol_name (symbol_table[i]);
+
+      if (strcmp (name, "_vm_stack") == 0
+	  || strcmp (name, "__p4_stack") == 0)
+	pikeos_stack_found = 1;
+
+      if (strcmp (name, "_vm_stack_size") == 0
+	  || strcmp (name, "__p4_stack_end") == 0)
+	pikeos_stack_size_found = 1;
+    }
+
+  do_cleanups (old_chain);
+
+  if (pikeos_stack_found && pikeos_stack_size_found)
+    return GDB_OSABI_PIKEOS;
+  else
+    return GDB_OSABI_UNKNOWN;
+}
+
+void
+_initialize_arm_pikeos_tdep (void)
+{
+  /* Register the sniffer for the PikeOS targets.  */
+  gdbarch_register_osabi_sniffer (bfd_arch_arm, bfd_target_elf_flavour,
+                                  arm_pikeos_osabi_sniffer);
+  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_PIKEOS,
+                          arm_pikeos_init_abi);
+}
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 6d1a4df..a1f703f 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -180,6 +180,7 @@ arm*-*-symbianelf*)
 	;;
 arm*-*-*)
 	# Target: ARM embedded system
+        gdb_target_obs="arm-pikeos-tdep.o"
 	gdb_sim=../sim/arm/libsim.a
 	;;
 
diff --git a/gdb/defs.h b/gdb/defs.h
index fc42170..28f7a11 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -495,6 +495,7 @@ enum gdb_osabi
   GDB_OSABI_LYNXOS178,
   GDB_OSABI_NEWLIB,
   GDB_OSABI_SDE,
+  GDB_OSABI_PIKEOS,
 
   GDB_OSABI_INVALID		/* keep this last */
 };
diff --git a/gdb/osabi.c b/gdb/osabi.c
index 7d0540b..7405ff1 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -80,6 +80,7 @@ static const struct osabi_names gdb_osabi_names[] =
   { "LynxOS178", NULL },
   { "Newlib", NULL },
   { "SDE", NULL },
+  { "Pikeos", NULL },
 
   { "<invalid>", NULL }
 };
-- 
1.7.10.4

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

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> The challenge is that, up to now, the PikeOS target was in all respects
Joel> identical to a baremetal target as far as GDB was concerned, meaning
Joel> we were using the baremetal osabi for this target too. This is no longer
Joel> possible, and we need to introduce a new OSABI variant.

I don't really know anything about this area, but...

Is there any chance of introducing some flag into the object files so
that the heuristic could be removed at some future point?

Joel> +
Joel> +static void
Joel> +arm_pikeos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)

The usual thing about functions needing an intro comment.

Joel> +  { "Pikeos", NULL },

The email spelled it "PikeOS", so maybe it should read that way here as
well.

Tom

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

[Joel Brobecker]

> Joel> The challenge is that, up to now, the PikeOS target was in all respects
> Joel> identical to a baremetal target as far as GDB was concerned, meaning
> Joel> we were using the baremetal osabi for this target too. This is no longer
> Joel> possible, and we need to introduce a new OSABI variant.
> 
> I don't really know anything about this area, but...
> 
> Is there any chance of introducing some flag into the object files so
> that the heuristic could be removed at some future point?

I don't really see this, because we don't really control the platform.

> Joel> +
> Joel> +static void
> Joel> +arm_pikeos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> 
> The usual thing about functions needing an intro comment.

OK, I will do that. I saw it, and grepped around, saw that it wasn't
really done for the few spot checks I did, so thought it was just
so obvious that it was deliberately omitted.

> Joel> +  { "Pikeos", NULL },
> 
> The email spelled it "PikeOS", so maybe it should read that way here as
> well.

Indeed, that's a good point.

Thanks for the review, Tom. New version of the patch coming up.
-- 
Joel

[RFA v2] arm-pikeos: software single step

[y]

From: Jerome Guitton <guitton@adacore.com>

Hello,

Below is the updated version following Tom's comments. The only
changes are made are to fix the issues pointed out by Tom:
  - Missing function documentation;
  - Wrong casing in "PikeOS".

Thank you!

----------------------------------------------------------------------

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).

gdb/ChangeLog:

        * 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.
---
 gdb/arm-pikeos-tdep.c |  102 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.tgt     |    1 +
 gdb/defs.h            |    1 +
 gdb/osabi.c           |    1 +
 4 files changed, 105 insertions(+)
 create mode 100644 gdb/arm-pikeos-tdep.c

diff --git a/gdb/arm-pikeos-tdep.c b/gdb/arm-pikeos-tdep.c
new file mode 100644
index 0000000..9640f1f
--- /dev/null
+++ b/gdb/arm-pikeos-tdep.c
@@ -0,0 +1,102 @@
+/* Copyright (C) 2016-2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 "defs.h"
+#include "objfiles.h"
+#include "arm-tdep.h"
+#include "osabi.h"
+
+/* The gdbarch_register_osabi handler for ARM PikeOS; it performs
+   the gdbarch initialization for that platform.  */
+
+static void
+arm_pikeos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  /* Single stepping.  */
+  set_gdbarch_software_single_step (gdbarch, arm_software_single_step);
+}
+
+/* The ARM PikeOS OSABI sniffer (see gdbarch_register_osabi_sniffer).
+   Returns GDB_OSABI_PIKEOS if the given BFD is a PikeOS binary,
+   GDB_OSABI_UNKNOWN otherwise.  */
+
+static enum gdb_osabi
+arm_pikeos_osabi_sniffer (bfd *abfd)
+{
+  long storage_needed;
+  asymbol **symbol_table;
+  long number_of_symbols;
+  long i;
+  int pikeos_stack_found = 0;
+  int pikeos_stack_size_found = 0;
+  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+
+  /* The BFD target of PikeOS is really just standard elf, so we
+     cannot use it to detect this variant.  The only common thing that
+     may be found in PikeOS modules are symbols _vm_stack/__p4_stack and
+     _vm_stack_size/__p4_stack_end. They are used to specify the stack
+     location and size; and defined by the default linker script.
+
+     OS ABI sniffers are called before the minimal symtabs are
+     created. So inspect the symbol table using BFD.  */
+
+  storage_needed = bfd_get_symtab_upper_bound (abfd);
+
+  if (storage_needed <= 0)
+    return GDB_OSABI_UNKNOWN;
+
+  symbol_table = (asymbol **) xmalloc (storage_needed);
+  make_cleanup (xfree, symbol_table);
+
+  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
+
+  if (number_of_symbols < 0)
+    {
+      do_cleanups (old_chain);
+      return GDB_OSABI_UNKNOWN;
+    }
+
+  for (i = 0; i < number_of_symbols; i++)
+    {
+      const char *name = bfd_asymbol_name (symbol_table[i]);
+
+      if (strcmp (name, "_vm_stack") == 0
+	  || strcmp (name, "__p4_stack") == 0)
+	pikeos_stack_found = 1;
+
+      if (strcmp (name, "_vm_stack_size") == 0
+	  || strcmp (name, "__p4_stack_end") == 0)
+	pikeos_stack_size_found = 1;
+    }
+
+  do_cleanups (old_chain);
+
+  if (pikeos_stack_found && pikeos_stack_size_found)
+    return GDB_OSABI_PIKEOS;
+  else
+    return GDB_OSABI_UNKNOWN;
+}
+
+void
+_initialize_arm_pikeos_tdep (void)
+{
+  /* Register the sniffer for the PikeOS targets.  */
+  gdbarch_register_osabi_sniffer (bfd_arch_arm, bfd_target_elf_flavour,
+                                  arm_pikeos_osabi_sniffer);
+  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_PIKEOS,
+                          arm_pikeos_init_abi);
+}
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 6d1a4df..a1f703f 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -180,6 +180,7 @@ arm*-*-symbianelf*)
 	;;
 arm*-*-*)
 	# Target: ARM embedded system
+        gdb_target_obs="arm-pikeos-tdep.o"
 	gdb_sim=../sim/arm/libsim.a
 	;;
 
diff --git a/gdb/defs.h b/gdb/defs.h
index fc42170..28f7a11 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -495,6 +495,7 @@ enum gdb_osabi
   GDB_OSABI_LYNXOS178,
   GDB_OSABI_NEWLIB,
   GDB_OSABI_SDE,
+  GDB_OSABI_PIKEOS,
 
   GDB_OSABI_INVALID		/* keep this last */
 };
diff --git a/gdb/osabi.c b/gdb/osabi.c
index 7d0540b..68f4665 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -80,6 +80,7 @@ static const struct osabi_names gdb_osabi_names[] =
   { "LynxOS178", NULL },
   { "Newlib", NULL },
   { "SDE", NULL },
+  { "PikeOS", NULL },
 
   { "<invalid>", NULL }
 };
-- 
1.7.10.4

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

[Simon Marchi]

(not sure why the From address is y@eu.adacore.com, I'll replace it with 
Joel's).

> +  symbol_table = (asymbol **) xmalloc (storage_needed);
> +  make_cleanup (xfree, symbol_table);

It would make Tom really happy if you could avoid introducing a cleanup 
:).  I would suggest using an std::vector<asymbol *>.  If you'd rather 
not do it it's not a big deal, we can change it after.

> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index 6d1a4df..a1f703f 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -180,6 +180,7 @@ arm*-*-symbianelf*)
>  	;;
>  arm*-*-*)
>  	# Target: ARM embedded system
> +        gdb_target_obs="arm-pikeos-tdep.o"
>  	gdb_sim=../sim/arm/libsim.a
>  	;;

The object should be added to ALL_TARGETS_OBS in the Makefile, so it's 
included in an enable-targets=all build.

Just to understand better why the object file needs to be added here, 
what is the target triplet? arm-pikeos-something, arm-none-something?

Just wondering: when you connect to the target, you talk to a gdb stub 
that is built right into the application/rtos?

Simon

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

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> (not sure why the From address is y@eu.adacore.com, I'll replace it
Simon> with Joel's).

>> +  symbol_table = (asymbol **) xmalloc (storage_needed);
>> +  make_cleanup (xfree, symbol_table);

Simon> It would make Tom really happy if you could avoid introducing a
Simon> cleanup :).

Wow, thanks.  I sometimes check patches for new cleanups but I totally
missed it in this one :)

Tom

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

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]

> (not sure why the From address is y@eu.adacore.com, I'll replace it with
> Joel's).

It was my mistake answering "y" rather than just typing answer
to use my email address as the default when "git send-email"
asked me about it. Sorry, and thanks for fixing!

> > +  symbol_table = (asymbol **) xmalloc (storage_needed);
> > +  make_cleanup (xfree, symbol_table);
> 
> It would make Tom really happy if you could avoid introducing a cleanup :).
> I would suggest using an std::vector<asymbol *>.  If you'd rather not do it
> it's not a big deal, we can change it after.

Attached is the patch I am testing. I am wondering if this is the best
way, though. What do you think?

One question I asked myself is whether we needed the std::vector at
all, as the building of the vector is a bit clunky in this situation.
As I understand it, this is mostly to automate the destruction of
the array. I was wondering whether we could do without the std::vector
entirely, and just handle the array without a cleanup, since the code
is simple enough that we can make sure it doesn't throw (I was hoping
that eg marking the function noexcept would help guaranty that). But
at the end of the day, although it's manageable in this case, I felt
it was better to go with the safer approach.

Same remark with resizing the array: In practice, we don't need to do
it since we know the bounds and iterate over the elements without
accessing the them from the vector; but it's clearer and safer this way.

> > diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> > index 6d1a4df..a1f703f 100644
> > --- a/gdb/configure.tgt
> > +++ b/gdb/configure.tgt
> > @@ -180,6 +180,7 @@ arm*-*-symbianelf*)
> >  	;;
> >  arm*-*-*)
> >  	# Target: ARM embedded system
> > +        gdb_target_obs="arm-pikeos-tdep.o"
> >  	gdb_sim=../sim/arm/libsim.a
> >  	;;
> 
> The object should be added to ALL_TARGETS_OBS in the Makefile, so it's
> included in an enable-targets=all build.

Argh, yes, true. Added.

> Just to understand better why the object file needs to be added here, what
> is the target triplet? arm-pikeos-something, arm-none-something?

It is arm-*-pikeos*, which config.sub then translates to arm-*-eabi.

> Just wondering: when you connect to the target, you talk to a gdb stub
> that is built right into the application/rtos?

Something like that. We have to use a tool provided by the vendor
called muxa; the tool connects to the system running PikeOS, and
we connect to that tool using the remote protocol.

Thanks for the review!
-- 
Joel

[-- Attachment #2: 0001-arm-pikeos-software-single-step.patch --]
[-- Type: text/x-diff, Size: 6427 bytes --]

From 5e98b8cb6fefc1419e3357675e3c5c6d375cc461 Mon Sep 17 00:00:00 2001
From: Jerome Guitton <guitton@adacore.com>
Date: Mon, 10 Sep 2018 13:04:41 +0200
Subject: [PATCH] arm-pikeos: software single step

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).

gdb/ChangeLog:

        * defs.h (enum gdb_osabi): Add GDB_OSABI_PIKEOS.
        * osabi.c (gdb_osabi_names): Add name for GDB_OSABI_PIKEOS.
        * arm-pikeos-tdep.c: New file.
        * configure.tgt: Add arm-pikeos-tdep.o to the case of ARM
        embedded system.
        * Makefile.in (ALL_TARGET_OBS): Add arm-pikeos-tdep.o.

Tested on arm-pikeos and arm-elf using AdaCore's testsuite.
We also evaluated it on armhf-linux as a cross platform.
---
 gdb/Makefile.in       |    1 +
 gdb/arm-pikeos-tdep.c |   93 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.tgt     |    1 +
 gdb/defs.h            |    1 +
 gdb/osabi.c           |    1 +
 5 files changed, 97 insertions(+)
 create mode 100644 gdb/arm-pikeos-tdep.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d49f3ee..5d6e217 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -670,6 +670,7 @@ ALL_TARGET_OBS = \
 	arm-linux-tdep.o \
 	arm-nbsd-tdep.o \
 	arm-obsd-tdep.o \
+	arm-pikeos-tdep.o \
 	arm-symbian-tdep.o \
 	arm-tdep.o \
 	arm-wince-tdep.o \
diff --git a/gdb/arm-pikeos-tdep.c b/gdb/arm-pikeos-tdep.c
new file mode 100644
index 0000000..786f67a
--- /dev/null
+++ b/gdb/arm-pikeos-tdep.c
@@ -0,0 +1,93 @@
+/* Copyright (C) 2016-2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 "defs.h"
+#include "objfiles.h"
+#include "arm-tdep.h"
+#include "osabi.h"
+
+/* The gdbarch_register_osabi handler for ARM PikeOS; it performs
+   the gdbarch initialization for that platform.  */
+
+static void
+arm_pikeos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  /* Single stepping.  */
+  set_gdbarch_software_single_step (gdbarch, arm_software_single_step);
+}
+
+/* The ARM PikeOS OSABI sniffer (see gdbarch_register_osabi_sniffer).
+   Returns GDB_OSABI_PIKEOS if the given BFD is a PikeOS binary,
+   GDB_OSABI_UNKNOWN otherwise.  */
+
+static enum gdb_osabi
+arm_pikeos_osabi_sniffer (bfd *abfd)
+{
+  long number_of_symbols;
+  long i;
+  int pikeos_stack_found = 0;
+  int pikeos_stack_size_found = 0;
+
+  /* The BFD target of PikeOS is really just standard elf, so we
+     cannot use it to detect this variant.  The only common thing that
+     may be found in PikeOS modules are symbols _vm_stack/__p4_stack and
+     _vm_stack_size/__p4_stack_end. They are used to specify the stack
+     location and size; and defined by the default linker script.
+
+     OS ABI sniffers are called before the minimal symtabs are
+     created. So inspect the symbol table using BFD.  */
+
+  long max_number_of_symbols
+    = bfd_get_symtab_upper_bound (abfd) / sizeof (asymbol *);
+  if (max_number_of_symbols <= 0)
+    return GDB_OSABI_UNKNOWN;
+
+  std::vector<asymbol *> symbol_table (max_number_of_symbols);
+  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table.data ());
+  if (number_of_symbols <= 0)
+    return GDB_OSABI_UNKNOWN;
+  gdb_assert (number_of_symbols <= max_number_of_symbols);
+  symbol_table.resize (number_of_symbols);
+
+  for (i = 0; i < number_of_symbols; i++)
+    {
+      const char *name = bfd_asymbol_name (symbol_table[i]);
+
+      if (strcmp (name, "_vm_stack") == 0
+	  || strcmp (name, "__p4_stack") == 0)
+	pikeos_stack_found = 1;
+
+      if (strcmp (name, "_vm_stack_size") == 0
+	  || strcmp (name, "__p4_stack_end") == 0)
+	pikeos_stack_size_found = 1;
+    }
+
+  if (pikeos_stack_found && pikeos_stack_size_found)
+    return GDB_OSABI_PIKEOS;
+  else
+    return GDB_OSABI_UNKNOWN;
+}
+
+void
+_initialize_arm_pikeos_tdep (void)
+{
+  /* Register the sniffer for the PikeOS targets.  */
+  gdbarch_register_osabi_sniffer (bfd_arch_arm, bfd_target_elf_flavour,
+                                  arm_pikeos_osabi_sniffer);
+  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_PIKEOS,
+                          arm_pikeos_init_abi);
+}
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 6d1a4df..03c0268 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -180,6 +180,7 @@ arm*-*-symbianelf*)
 	;;
 arm*-*-*)
 	# Target: ARM embedded system
+	gdb_target_obs="arm-pikeos-tdep.o"
 	gdb_sim=../sim/arm/libsim.a
 	;;
 
diff --git a/gdb/defs.h b/gdb/defs.h
index fc42170..28f7a11 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -495,6 +495,7 @@ enum gdb_osabi
   GDB_OSABI_LYNXOS178,
   GDB_OSABI_NEWLIB,
   GDB_OSABI_SDE,
+  GDB_OSABI_PIKEOS,
 
   GDB_OSABI_INVALID		/* keep this last */
 };
diff --git a/gdb/osabi.c b/gdb/osabi.c
index 7d0540b..68f4665 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -80,6 +80,7 @@ static const struct osabi_names gdb_osabi_names[] =
   { "LynxOS178", NULL },
   { "Newlib", NULL },
   { "SDE", NULL },
+  { "PikeOS", NULL },
 
   { "<invalid>", NULL }
 };
-- 
1.7.10.4

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

[Pedro Alves]

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!
> 

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

[Simon Marchi]

On 2018-09-11 09:56, Joel Brobecker wrote:
>> > +  symbol_table = (asymbol **) xmalloc (storage_needed);
>> > +  make_cleanup (xfree, symbol_table);
>> 
>> It would make Tom really happy if you could avoid introducing a 
>> cleanup :).
>> I would suggest using an std::vector<asymbol *>.  If you'd rather not 
>> do it
>> it's not a big deal, we can change it after.
> 
> Attached is the patch I am testing. I am wondering if this is the best
> way, though. What do you think?

It looks good to me.  You could use a range-for, because it's cool:

   for (asymbol *sym : symbol_table)

but what you have is also fine.

> One question I asked myself is whether we needed the std::vector at
> all, as the building of the vector is a bit clunky in this situation.
> As I understand it, this is mostly to automate the destruction of
> the array. I was wondering whether we could do without the std::vector
> entirely, and just handle the array without a cleanup, since the code
> is simple enough that we can make sure it doesn't throw (I was hoping
> that eg marking the function noexcept would help guaranty that). But
> at the end of the day, although it's manageable in this case, I felt
> it was better to go with the safer approach.

Well, it would be fine to not use a vector, but in any case I would 
recommend the use of an object that ensures the memory is de-allocated 
in any case, whether it is an std::vector, an std::unique_ptr or a 
gdb::unique_xmalloc_ptr.  I don't see any advantage of doing a manual 
free over using one of those.

An alternative would be to use a VLA:

   asymbol *symbol_table[max_number_of_symbols];

which I think ends up being like an alloca.  But in this case there 
could be a huge number of symbols I suppose, so I would avoid that.

> Same remark with resizing the array: In practice, we don't need to do
> it since we know the bounds and iterate over the elements without
> accessing the them from the vector; but it's clearer and safer this 
> way.

Right, with the range-for you would need the resize though.

Simon

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

[Joel Brobecker]

Hi Pedro,

> 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.

Unfortunately, the problem is that we do not control the stub (muxa),
it is a tool that the vendor provides.

-- 
Joel

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

[Pedro Alves]

On 09/11/2018 12:04 PM, Joel Brobecker wrote:
> Hi Pedro,
> 
>> 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.
> 
> Unfortunately, the problem is that we do not control the stub (muxa),
> it is a tool that the vendor provides.
Did you check whether it is already reporting the RSP packets as I
had suggested?  We wouldn't be adding new packets, but instead using
some that are already defined.

If the stub really needs modification, I'm not opposed to your patch
as stop gap.  Would there be any chance to forward the information to
the sysgo folks, see if they're willing to tweak the stub?  It should
be a trivial change.

IWBN to fix the infrastructure, so that the next time a similar
patch comes in we could just "no, fix your stub".  :-)

Thanks,
Pedro Alves

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

[Joel Brobecker]

> > Attached is the patch I am testing. I am wondering if this is the best
> > way, though. What do you think?
> 
> It looks good to me.  You could use a range-for, because it's cool:
> 
>   for (asymbol *sym : symbol_table)
> 
> but what you have is also fine.

Unfortunately, it looks like the change is causing regressions, and
it is a bit obscure at the moment, which is not entirely surprising
when one deals with memory.

> > One question I asked myself is whether we needed the std::vector at
> > all, as the building of the vector is a bit clunky in this situation.
> > As I understand it, this is mostly to automate the destruction of
> > the array. I was wondering whether we could do without the std::vector
> > entirely, and just handle the array without a cleanup, since the code
> > is simple enough that we can make sure it doesn't throw (I was hoping
> > that eg marking the function noexcept would help guaranty that). But
> > at the end of the day, although it's manageable in this case, I felt
> > it was better to go with the safer approach.
> 
> Well, it would be fine to not use a vector, but in any case I would
> recommend the use of an object that ensures the memory is de-allocated in
> any case, whether it is an std::vector, an std::unique_ptr or a
> gdb::unique_xmalloc_ptr.  I don't see any advantage of doing a manual free
> over using one of those.
> 
> An alternative would be to use a VLA:
> 
>   asymbol *symbol_table[max_number_of_symbols];
> 
> which I think ends up being like an alloca.  But in this case there could be
> a huge number of symbols I suppose, so I would avoid that.

Yes, the table might be too large and blow the stack. All things
considered, I think the better model here seems to be to use a
gdb::unique_xmalloc_ptr. I tried that, and the code looks simpler,
but I get the same kind of regressions as above.

And unfortunately at this point, I have to hold off for a while,
because I am out of license, and then I will be traveling home.
But thanks a lot for all the feedback received so far; when I pick
things up again, I will not be starting from scratch.

-- 
Joel

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

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> +  long max_number_of_symbols
Joel> +    = bfd_get_symtab_upper_bound (abfd) / sizeof (asymbol *);
Joel> +  if (max_number_of_symbols <= 0)
Joel> +    return GDB_OSABI_UNKNOWN;

Joel> +  std::vector<asymbol *> symbol_table (max_number_of_symbols);
Joel> +  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table.data ());
Joel> +  if (number_of_symbols <= 0)
Joel> +    return GDB_OSABI_UNKNOWN;
Joel> +  gdb_assert (number_of_symbols <= max_number_of_symbols);
Joel> +  symbol_table.resize (number_of_symbols);

I looked, and some spots doing this just use xmalloc and manage it
manually.  machoread though uses gdb::def_vector; which is nice since it
doesn't clear the memory.

Joel> +  for (i = 0; i < number_of_symbols; i++)

If you have an explicit bound on the loop then you don't need to resize
the vector to be smaller.

No idea why it isn't working for you, the patch looks ok to me.
Sorry about this.  If you'd rather get it in, I can remove the cleanup later.

Tom

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

[Joel Brobecker]

Hi again Pedro,

> > Unfortunately, the problem is that we do not control the stub (muxa),
> > it is a tool that the vendor provides.
>
> Did you check whether it is already reporting the RSP packets as I
> had suggested?  We wouldn't be adding new packets, but instead using
> some that are already defined.
> 
> If the stub really needs modification, I'm not opposed to your patch
> as stop gap.  Would there be any chance to forward the information to
> the sysgo folks, see if they're willing to tweak the stub?  It should
> be a trivial change.

So, here is what I could gather (full log at the end of this email [1]).
GDB sends the $qSupported packet, equiring about support...

    | Sending packet: $qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+#df...Ack

... and the list returned is fairly small, as I suspected:

    | Packet received: qXfer:features:read+

After that, I don't see very much happening at the vCont level,
so fast-forward to the first "cont" command (after having inserted
a breakpoint), and we see some interesting stuff. In particular,
GDB asks the target what vCont support it provides, and here is
what we receive:

    | Sending packet: $vCont?#49...Ack
    | Packet received: vCont;c;s;C;S
    | Packet vCont (verbose-resume) is supported

Hah! So, on the one end, the stub says stepping is supported
('s' and 'S'), but on the other hand we were told that this
feature is not supported on ARM.

So, if I understand correctly the current situation, we do have
one small infrastructure adjustment to do in GDB, but then once
done, if the stub on advertised support for "vCont;c;C", then
GDB would automatically be switching to software single step.
Do I understand correctly?

If that's correct, I'll make sure AdaCore forwards the suggestion
to Sysgo.

Thanks!
-- 
Joel

[1] For the record, complete copy/paste of a short "target remote"
    + "break xxx" + "cont" + "next" session, just for the record.

    | (gdb) tar rem :1502
    | Remote debugging using :1502
    | Sending packet: $qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+#df...Ack
    | Packet received: qXfer:features:read+
    | Packet qSupported (supported-packets) is supported
    | Sending packet: $vMustReplyEmpty#3a...Ack
    | Packet received:
    | Sending packet: $Hg0#df...Ack
    | Packet received: OK
    | Sending packet: $qXfer:features:read:target.xml:0,18a#15...Ack
    | Packet received: m<?xml version="1.0"?>\n<!-- Copyright (C) 2009-2015 Free Software Foundation, Inc.\n\n     Copying and distribution of this file, with or without modification,\n     are permitted in any medium without royalty provided the copyright\n     notice and this notice are preserved.  -->\n\n<!DOCTYPE target SYSTEM "gdb-target.dtd">\n<target version="1.0">\n\n<feature name="org.gnu.gdb.arm.core">\n  <reg name
    | Sending packet: $qXfer:features:read:target.xml:189,18a#87...Ack
    | Packet received: m="r0" bitsize="32" type="uint32"/>\n  <reg name="r1" bitsize="32" type="uint32"/>\n  <reg name="r2" bitsize="32" type="uint32"/>\n  <reg name="r3" bitsize="32" type="uint32"/>\n  <reg name="r4" bitsize="32" type="uint32"/>\n  <reg name="r5" bitsize="32" type="uint32"/>\n  <reg name="r6" bitsize="32" type="uint32"/>\n  <reg name="r7" bitsize="32" type="uint32"/>\n  <reg name="r8" bitsize="32" type="
    | Sending packet: $qXfer:features:read:target.xml:312,18a#7b...Ack
    | Packet received: muint32"/>\n  <reg name="r9" bitsize="32" type="uint32"/>\n  <reg name="r10" bitsize="32" type="uint32"/>\n  <reg name="r11" bitsize="32" type="uint32"/>\n  <reg name="r12" bitsize="32" type="uint32"/>\n  <reg name="sp" bitsize="32" type="data_ptr"/>\n  <reg name="lr" bitsize="32" type="code_ptr"/>\n  <reg name="pc" bitsize="32" type="code_ptr"/>\n\n  <!-- The CPSR is register 25, rather than registe
    | Sending packet: $qXfer:features:read:target.xml:49b,18a#b4...Ack
    | Packet received: mr 16, because\n       the FPA registers historically were placed between the PC\n       and the CPSR in the "g" packet.  -->\n  <reg name="cpsr" bitsize="32" regnum="25"/>\n</feature>\n\n<feature name="org.gnu.gdb.arm.vfp">\n  <reg name="d0" bitsize="64" type="ieee_double"/>\n  <reg name="d1" bitsize="64" type="ieee_double"/>\n  <reg name="d2" bitsize="64" type="ieee_double"/>\n  <reg name="d3" bitsi
    | Sending packet: $qXfer:features:read:target.xml:624,18a#81...Ack
    | Packet received: mze="64" type="ieee_double"/>\n  <reg name="d4" bitsize="64" type="ieee_double"/>\n  <reg name="d5" bitsize="64" type="ieee_double"/>\n  <reg name="d6" bitsize="64" type="ieee_double"/>\n  <reg name="d7" bitsize="64" type="ieee_double"/>\n  <reg name="d8" bitsize="64" type="ieee_double"/>\n  <reg name="d9" bitsize="64" type="ieee_double"/>\n  <reg name="d10" bitsize="64" type="ieee_double"/>\n  <reg
    | Sending packet: $qXfer:features:read:target.xml:7ad,18a#e1...Ack
    | Packet received: m name="d11" bitsize="64" type="ieee_double"/>\n  <reg name="d12" bitsize="64" type="ieee_double"/>\n  <reg name="d13" bitsize="64" type="ieee_double"/>\n  <reg name="d14" bitsize="64" type="ieee_double"/>\n  <reg name="d15" bitsize="64" type="ieee_double"/>\n  <reg name="d16" bitsize="64" type="ieee_double"/>\n  <reg name="d17" bitsize="64" type="ieee_double"/>\n  <reg name="d18" bitsize="64" type
    | Sending packet: $qXfer:features:read:target.xml:936,18a#87...Ack
    | Packet received: m="ieee_double"/>\n  <reg name="d19" bitsize="64" type="ieee_double"/>\n  <reg name="d20" bitsize="64" type="ieee_double"/>\n  <reg name="d21" bitsize="64" type="ieee_double"/>\n  <reg name="d22" bitsize="64" type="ieee_double"/>\n  <reg name="d23" bitsize="64" type="ieee_double"/>\n  <reg name="d24" bitsize="64" type="ieee_double"/>\n  <reg name="d25" bitsize="64" type="ieee_double"/>\n  <reg name=
    | Sending packet: $qXfer:features:read:target.xml:abf,18a#0e...Ack
    | Packet received: m"d26" bitsize="64" type="ieee_double"/>\n  <reg name="d27" bitsize="64" type="ieee_double"/>\n  <reg name="d28" bitsize="64" type="ieee_double"/>\n  <reg name="d29" bitsize="64" type="ieee_double"/>\n  <reg name="d30" bitsize="64" type="ieee_double"/>\n  <reg name="d31" bitsize="64" type="ieee_double"/>\n\n  <reg name="fpscr" bitsize="32" type="int" group="float"/>\n</feature>\n\n<feature name="org.g
    | Sending packet: $qXfer:features:read:target.xml:c48,18a#b4...Ack
    | Packet received: lnu.gdb.arm.neon"/>\n\n</target>\n
    | Sending packet: $qTStatus#49...Ack
    | Packet received:
    | Packet qTStatus (trace-status) is NOT supported
    | Sending packet: $?#3f...Ack
    | Packet received: S05
    | Sending packet: $qfThreadInfo#bb...Ack
    | Packet received: m1
    | Sending packet: $qsThreadInfo#c8...Ack
    | Packet received: l
    | Sending packet: $qAttached#8f...Ack
    | Packet received:
    | Packet qAttached (query-attached) is NOT supported
    | Sending packet: $Hc-1#09...Ack
    | Packet received: OK
    | Sending packet: $qC#b4...Ack
    | Packet received: QC1
    | Sending packet: $qOffsets#4b...Ack
    | Packet received:
    | Sending packet: $g#67...Ack
    | Packet received: 01000000e07b0f08c47b0f082014030800000000000000000000000000000000000000000000000000000000fc7f0f0800000000987f0f083c040108502a0108100000a00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000[144 bytes omitted]
    | Sending packet: $m8018748,4#41...Ack
    | Packet received: 1eff2fe1
    | Sending packet: $qfThreadInfo#bb...Ack
    | Packet received: m1
    | Sending packet: $qsThreadInfo#c8...Ack
    | Packet received: l
    | Sending packet: $m8012a50,4#5e...Ack
    | Packet received: 700020e1
    | gdbarch_breakpoint () at arm-gdbstub.c:290
    | 290	arm-gdbstub.c: No such file or directory.
    | Sending packet: $qSymbol::#5b...Ack
    | Packet received:
    | Packet qSymbol (symbol-lookup) is NOT supported
    | (gdb) b test_simple
    | Sending packet: $m8010500,40#5b...Ack
    | Packet received: 00b08de200f020e300d08be204b09de41eff2fe104b02de500b08de200f020e300d08be204b09de41eff2fe100482de904b08de230d04de20030a0e308300be5
    | Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | Breakpoint 1 at 0x8010538: file simple.adb, line 16.
    | (gdb) cont
    | Continuing.
    | Sending packet: $Z0,8010538,4#7f...Ack
    | Packet received:
    | Packet Z0 (software-breakpoint) is NOT supported
    | Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | Sending packet: $X8010538,0:#57...Ack
    | Packet received:
    | binary downloading NOT supported by target
    | Sending packet: $M8010538,4:fedeffe7#4c...Ack
    | Packet received: OK
    | Sending packet: $vCont?#49...Ack
    | Packet received: vCont;c;s;C;S
    | Packet vCont (verbose-resume) is supported
    | Sending packet: $vCont;c#a8...Ack
    | Packet received: T050f:38050108;0d:407f0f08;thread:1;
    | Sending packet: $qfThreadInfo#bb...Ack
    | Packet received: m1
    | Sending packet: $qsThreadInfo#c8...Ack
    | Packet received: l
    | Sending packet: $M8010538,4:0030a0e3#3c...Ack
    | Packet received: OK
    | 
    | Breakpoint 1, Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | simple.test_simple () at simple.adb:16
    | 16	      A : Integer   := 0;
    | (gdb) n
    | Sending packet: $g#67...Ack
    | Packet received: 4851020860510208010000004450020800000000000000000000000000000000000000000000000000000000747f0f0825000000407f0f08b405010838050108100000200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000[144 bytes omitted]
    | Sending packet: $m8018748,4#41...Ack
    | Packet received: 1eff2fe1
    | Sending packet: $M8018748,4:fedeffe7#57...Ack
    | Packet received: OK
    | Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | Sending packet: $m801053c,4#61...Ack
    | Packet received: 08300be5
    | Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | Sending packet: $m801053c,4#61...Ack
    | Packet received: 08300be5
    | Sending packet: $M801053c,4:fedeffe7#77...Ack
    | Packet received: OK
    | Sending packet: $vCont;c:1#13...Ack
    | Packet received: T050f:3c050108;0d:407f0f08;thread:1;
    | Sending packet: $M801053c,4:08300be5#72...Ack
    | Packet received: OK
    | Sending packet: $m8010538,4#36...Ack
    | Packet received: 0030a0e3
    | Sending packet: $M8010538,4:fedeffe7#4c...Ack
    | Packet received: OK
    | Sending packet: $g#67...Ack
    | Packet received: 4851020860510208010000000000000000000000000000000000000000000000000000000000000000000000747f0f0825000000407f0f08b40501083c050108100000200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000[144 bytes omitted]
    | Sending packet: $m801053c,4#61...Ack
    | Packet received: 08300be5
    | Sending packet: $m801053c,4#61...Ack
    | Packet received: 08300be5
    | Sending packet: $m8010540,4#2f...Ack
    | Packet received: 0030a0e3
    | Sending packet: $m801053c,4#61...Ack
    | Packet received: 08300be5
    | Sending packet: $m801053c,4#61...Ack
    | Packet received: 08300be5
    | Sending packet: $m801053c,4#61...Ack
    | Packet received: 08300be5
    | Sending packet: $m8010540,4#2f...Ack
    | Packet received: 0030a0e3
    | Sending packet: $M8010540,4:fedeffe7#45...Ack
    | Packet received: OK
    | Sending packet: $vCont;c#a8...Ack
    | Packet received: T050f:40050108;0d:407f0f08;thread:1;
    | Sending packet: $M8010540,4:0030a0e3#35...Ack
    | Packet received: OK
    | Sending packet: $m8010540,4#2f...Ack
    | Packet received: 0030a0e3
    | Sending packet: $g#67...Ack
    | Packet received: 4851020860510208010000000000000000000000000000000000000000000000000000000000000000000000747f0f0825000000407f0f08b405010840050108100000200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000[144 bytes omitted]
    | Sending packet: $M8018748,4:1eff2fe1#eb...Ack
    | Packet received: OK
    | Sending packet: $qfThreadInfo#bb...Ack
    | Packet received: m1
    | Sending packet: $qsThreadInfo#c8...Ack
    | Packet received: l
    | Sending packet: $M8010538,4:0030a0e3#3c...Ack
    | Packet received: OK
    | 17	      B : Float     := 0.0;

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

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2968 bytes --]

Hi 
On Tue, Sep 11, 2018 at 09:40:57AM -0600, Tom Tromey wrote:
> >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> 
> Joel> +  long max_number_of_symbols
> Joel> +    = bfd_get_symtab_upper_bound (abfd) / sizeof (asymbol *);
> Joel> +  if (max_number_of_symbols <= 0)
> Joel> +    return GDB_OSABI_UNKNOWN;
> 
> Joel> +  std::vector<asymbol *> symbol_table (max_number_of_symbols);
> Joel> +  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table.data ());
> Joel> +  if (number_of_symbols <= 0)
> Joel> +    return GDB_OSABI_UNKNOWN;
> Joel> +  gdb_assert (number_of_symbols <= max_number_of_symbols);
> Joel> +  symbol_table.resize (number_of_symbols);
> 
> I looked, and some spots doing this just use xmalloc and manage it
> manually.  machoread though uses gdb::def_vector; which is nice since it
> doesn't clear the memory.

Indeed; I think the std::vector approach I had also had that property,
if I understand the spec correctly. Before I could see this suggestion,
I thought gdb::unique_xmalloc_ptr kind of conveyed the idea that
this was more of a C object (due to the use with BFD), than a C++
object, so used that instead and found the code to be slightly
smaller. The approach has its drawbacks as well, so maybe if people
have a preference for using the gdb::def_vector, I can do that too.

> No idea why it isn't working for you, the patch looks ok to me.
> Sorry about this.  If you'd rather get it in, I can remove the cleanup later.

Thanks. I didn't want to do that, because it would have put the burden
on you, and new code shouldn't add constructs we are actively trying
to remove.

Luckily, I found that the issue was due to some instability of
the target, and could repeat the same errors with AdaCore version
of GDB. Our testsuite framework is capable of dealing with that,
by shutting down and restarting the development environment before
re-trying the testcase. This is actually activated by default, but
like the idiot that I am, I had explicitly turned it off!

The testing is running as we speak, but because it's one testcase
at a time, I will not have the results until tomorrow. But so far,
so good, so fingers crossed...

In the menatime, attached is the patch I am testing.

gdb/ChangeLog:

        * defs.h (enum gdb_osabi): Add GDB_OSABI_PIKEOS.
        * osabi.c (gdb_osabi_names): Add name for GDB_OSABI_PIKEOS.
        * arm-pikeos-tdep.c: New file.
        * configure.tgt: Add arm-pikeos-tdep.o to the case of ARM
        embedded system.
        * Makefile.in (ALL_TARGET_OBS): Add arm-pikeos-tdep.o.

Note the discussion with Pedro about getting the vendor to enhance
the stub to tell GDB that hardware single stepping is not supported
(https://www.sourceware.org/ml/gdb-patches/2018-09/msg00345.html).
We'll work with the vendor, and so this patch would be applied as
a stop gap measure. But it's unclear if and when the stub would be
enhanced, allowing us to remove this patch.

Thanks!
-- 
Joel

[-- Attachment #2: 0001-arm-pikeos-software-single-step.patch --]
[-- Type: text/x-diff, Size: 6324 bytes --]

From bfa3fa1394b92b6651127f7ca6f7f1b68db6a9ca Mon Sep 17 00:00:00 2001
From: Jerome Guitton <guitton@adacore.com>
Date: Mon, 10 Sep 2018 13:04:41 +0200
Subject: [PATCH] arm-pikeos: software single step

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).

gdb/ChangeLog:

        * defs.h (enum gdb_osabi): Add GDB_OSABI_PIKEOS.
        * osabi.c (gdb_osabi_names): Add name for GDB_OSABI_PIKEOS.
        * arm-pikeos-tdep.c: New file.
        * configure.tgt: Add arm-pikeos-tdep.o to the case of ARM
        embedded system.
        * Makefile.in (ALL_TARGET_OBS): Add arm-pikeos-tdep.o.

Tested on arm-pikeos and arm-elf using AdaCore's testsuite.
We also evaluated it on armhf-linux as a cross platform.
---
 gdb/Makefile.in       |    1 +
 gdb/arm-pikeos-tdep.c |   92 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.tgt     |    1 +
 gdb/defs.h            |    1 +
 gdb/osabi.c           |    1 +
 5 files changed, 96 insertions(+)
 create mode 100644 gdb/arm-pikeos-tdep.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d49f3ee..5d6e217 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -670,6 +670,7 @@ ALL_TARGET_OBS = \
 	arm-linux-tdep.o \
 	arm-nbsd-tdep.o \
 	arm-obsd-tdep.o \
+	arm-pikeos-tdep.o \
 	arm-symbian-tdep.o \
 	arm-tdep.o \
 	arm-wince-tdep.o \
diff --git a/gdb/arm-pikeos-tdep.c b/gdb/arm-pikeos-tdep.c
new file mode 100644
index 0000000..4662a28
--- /dev/null
+++ b/gdb/arm-pikeos-tdep.c
@@ -0,0 +1,92 @@
+/* Copyright (C) 2016-2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 "defs.h"
+#include "objfiles.h"
+#include "arm-tdep.h"
+#include "osabi.h"
+
+/* The gdbarch_register_osabi handler for ARM PikeOS; it performs
+   the gdbarch initialization for that platform.  */
+
+static void
+arm_pikeos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  /* Single stepping.  */
+  set_gdbarch_software_single_step (gdbarch, arm_software_single_step);
+}
+
+/* The ARM PikeOS OSABI sniffer (see gdbarch_register_osabi_sniffer).
+   Returns GDB_OSABI_PIKEOS if the given BFD is a PikeOS binary,
+   GDB_OSABI_UNKNOWN otherwise.  */
+
+static enum gdb_osabi
+arm_pikeos_osabi_sniffer (bfd *abfd)
+{
+  long number_of_symbols;
+  long i;
+  int pikeos_stack_found = 0;
+  int pikeos_stack_size_found = 0;
+
+  /* The BFD target of PikeOS is really just standard elf, so we
+     cannot use it to detect this variant.  The only common thing that
+     may be found in PikeOS modules are symbols _vm_stack/__p4_stack and
+     _vm_stack_size/__p4_stack_end. They are used to specify the stack
+     location and size; and defined by the default linker script.
+
+     OS ABI sniffers are called before the minimal symtabs are
+     created. So inspect the symbol table using BFD.  */
+
+  long storage_needed = bfd_get_symtab_upper_bound (abfd);
+  if (storage_needed <= 0)
+    return GDB_OSABI_UNKNOWN;
+
+  gdb::unique_xmalloc_ptr<asymbol *> symbol_table
+    ((asymbol **) xmalloc (storage_needed));
+  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table.get ());
+
+  if (number_of_symbols <= 0)
+    return GDB_OSABI_UNKNOWN;
+
+  for (i = 0; i < number_of_symbols; i++)
+    {
+      const char *name = bfd_asymbol_name (symbol_table.get ()[i]);
+
+      if (strcmp (name, "_vm_stack") == 0
+	  || strcmp (name, "__p4_stack") == 0)
+	pikeos_stack_found = 1;
+
+      if (strcmp (name, "_vm_stack_size") == 0
+	  || strcmp (name, "__p4_stack_end") == 0)
+	pikeos_stack_size_found = 1;
+    }
+
+  if (pikeos_stack_found && pikeos_stack_size_found)
+    return GDB_OSABI_PIKEOS;
+  else
+    return GDB_OSABI_UNKNOWN;
+}
+
+void
+_initialize_arm_pikeos_tdep (void)
+{
+  /* Register the sniffer for the PikeOS targets.  */
+  gdbarch_register_osabi_sniffer (bfd_arch_arm, bfd_target_elf_flavour,
+                                  arm_pikeos_osabi_sniffer);
+  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_PIKEOS,
+                          arm_pikeos_init_abi);
+}
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 6d1a4df..03c0268 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -180,6 +180,7 @@ arm*-*-symbianelf*)
 	;;
 arm*-*-*)
 	# Target: ARM embedded system
+	gdb_target_obs="arm-pikeos-tdep.o"
 	gdb_sim=../sim/arm/libsim.a
 	;;
 
diff --git a/gdb/defs.h b/gdb/defs.h
index fc42170..28f7a11 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -495,6 +495,7 @@ enum gdb_osabi
   GDB_OSABI_LYNXOS178,
   GDB_OSABI_NEWLIB,
   GDB_OSABI_SDE,
+  GDB_OSABI_PIKEOS,
 
   GDB_OSABI_INVALID		/* keep this last */
 };
diff --git a/gdb/osabi.c b/gdb/osabi.c
index 7d0540b..68f4665 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -80,6 +80,7 @@ static const struct osabi_names gdb_osabi_names[] =
   { "LynxOS178", NULL },
   { "Newlib", NULL },
   { "SDE", NULL },
+  { "PikeOS", NULL },
 
   { "<invalid>", NULL }
 };
-- 
1.7.10.4

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

[Pedro Alves]

On 09/11/2018 09:57 PM, Joel Brobecker wrote:
> Hi again Pedro,
> 
>>> Unfortunately, the problem is that we do not control the stub (muxa),
>>> it is a tool that the vendor provides.
>>
>> Did you check whether it is already reporting the RSP packets as I
>> had suggested?  We wouldn't be adding new packets, but instead using
>> some that are already defined.
>>
>> If the stub really needs modification, I'm not opposed to your patch
>> as stop gap.  Would there be any chance to forward the information to
>> the sysgo folks, see if they're willing to tweak the stub?  It should
>> be a trivial change.
> 
> So, here is what I could gather (full log at the end of this email [1]).
> GDB sends the $qSupported packet, equiring about support...
> 
>     | Sending packet: $qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+#df...Ack
> 
> ... and the list returned is fairly small, as I suspected:
> 
>     | Packet received: qXfer:features:read+
> 
> After that, I don't see very much happening at the vCont level,
> so fast-forward to the first "cont" command (after having inserted
> a breakpoint), and we see some interesting stuff. In particular,
> GDB asks the target what vCont support it provides, and here is
> what we receive:
> 
>     | Sending packet: $vCont?#49...Ack
>     | Packet received: vCont;c;s;C;S
>     | Packet vCont (verbose-resume) is supported
> 
> Hah! So, on the one end, the stub says stepping is supported
> ('s' and 'S'), but on the other hand we were told that this
> feature is not supported on ARM.
> 
> So, if I understand correctly the current situation, we do have
> one small infrastructure adjustment to do in GDB, but then once
> done, if the stub on advertised support for "vCont;c;C", then
> GDB would automatically be switching to software single step.
> Do I understand correctly?

Almost.  GDB can't trust "vCont;c;C" alone, because for a long
while GDBserver would send "vCont;c;s;C;S" even if the target
did not support hardware stepping.  So what a stub needs to do
is:

Return "vCont;c;C" to "vCont?" _AND_ include "vContSupported"
in the reported "qSupported" features.  The latter tells GDB
to trust that the actions included in "vCont?" are really the
supported ones.  (I wish we had implemented this a little bit
differently, but that ship has sailed, and although a bit
cumbersome, it works.)

> 
> If that's correct, I'll make sure AdaCore forwards the suggestion
> to Sysgo.
> 
> Thanks!
Thanks,
Pedro Alves

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

[Joel Brobecker]

> Almost.  GDB can't trust "vCont;c;C" alone, because for a long
> while GDBserver would send "vCont;c;s;C;S" even if the target
> did not support hardware stepping.  So what a stub needs to do
> is:
> 
> Return "vCont;c;C" to "vCont?" _AND_ include "vContSupported"
> in the reported "qSupported" features.  The latter tells GDB
> to trust that the actions included in "vCont?" are really the
> supported ones.  (I wish we had implemented this a little bit
> differently, but that ship has sailed, and although a bit
> cumbersome, it works.)

Excellent. I've passed that information in what is hopefully
a concise and clear message that JeromeG can pass to Sysgo.
Hopefully something will come out of it.

Thanks Pedro!

-- 
Joel

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

[Joel Brobecker]

> The testing is running as we speak, but because it's one testcase
> at a time, I will not have the results until tomorrow. But so far,
> so good, so fingers crossed...
> 
> In the menatime, attached is the patch I am testing.
> 
> gdb/ChangeLog:
> 
>         * defs.h (enum gdb_osabi): Add GDB_OSABI_PIKEOS.
>         * osabi.c (gdb_osabi_names): Add name for GDB_OSABI_PIKEOS.
>         * arm-pikeos-tdep.c: New file.
>         * configure.tgt: Add arm-pikeos-tdep.o to the case of ARM
>         embedded system.
>         * Makefile.in (ALL_TARGET_OBS): Add arm-pikeos-tdep.o.

The testing was clean (pfew!).

> Note the discussion with Pedro about getting the vendor to enhance
> the stub to tell GDB that hardware single stepping is not supported
> (https://www.sourceware.org/ml/gdb-patches/2018-09/msg00345.html).
> We'll work with the vendor, and so this patch would be applied as
> a stop gap measure. But it's unclear if and when the stub would be
> enhanced, allowing us to remove this patch.

For the record, as mentioned later in the thread above, the enhancement
request should be passed to Sysgo soon.

In the meantime, is it OK to push this patch as a stopgap measure?
Or would people rather I use a different idiom for managing the memory
where the symbols are stored?

Thank you!

> >From bfa3fa1394b92b6651127f7ca6f7f1b68db6a9ca Mon Sep 17 00:00:00 2001
> From: Jerome Guitton <guitton@adacore.com>
> Date: Mon, 10 Sep 2018 13:04:41 +0200
> Subject: [PATCH] arm-pikeos: software single step
> 
> 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).
> 
> gdb/ChangeLog:
> 
>         * defs.h (enum gdb_osabi): Add GDB_OSABI_PIKEOS.
>         * osabi.c (gdb_osabi_names): Add name for GDB_OSABI_PIKEOS.
>         * arm-pikeos-tdep.c: New file.
>         * configure.tgt: Add arm-pikeos-tdep.o to the case of ARM
>         embedded system.
>         * Makefile.in (ALL_TARGET_OBS): Add arm-pikeos-tdep.o.
> 
> Tested on arm-pikeos and arm-elf using AdaCore's testsuite.
> We also evaluated it on armhf-linux as a cross platform.
> ---
>  gdb/Makefile.in       |    1 +
>  gdb/arm-pikeos-tdep.c |   92 +++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/configure.tgt     |    1 +
>  gdb/defs.h            |    1 +
>  gdb/osabi.c           |    1 +
>  5 files changed, 96 insertions(+)
>  create mode 100644 gdb/arm-pikeos-tdep.c
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index d49f3ee..5d6e217 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -670,6 +670,7 @@ ALL_TARGET_OBS = \
>  	arm-linux-tdep.o \
>  	arm-nbsd-tdep.o \
>  	arm-obsd-tdep.o \
> +	arm-pikeos-tdep.o \
>  	arm-symbian-tdep.o \
>  	arm-tdep.o \
>  	arm-wince-tdep.o \
> diff --git a/gdb/arm-pikeos-tdep.c b/gdb/arm-pikeos-tdep.c
> new file mode 100644
> index 0000000..4662a28
> --- /dev/null
> +++ b/gdb/arm-pikeos-tdep.c
> @@ -0,0 +1,92 @@
> +/* Copyright (C) 2016-2018 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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 "defs.h"
> +#include "objfiles.h"
> +#include "arm-tdep.h"
> +#include "osabi.h"
> +
> +/* The gdbarch_register_osabi handler for ARM PikeOS; it performs
> +   the gdbarch initialization for that platform.  */
> +
> +static void
> +arm_pikeos_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  /* Single stepping.  */
> +  set_gdbarch_software_single_step (gdbarch, arm_software_single_step);
> +}
> +
> +/* The ARM PikeOS OSABI sniffer (see gdbarch_register_osabi_sniffer).
> +   Returns GDB_OSABI_PIKEOS if the given BFD is a PikeOS binary,
> +   GDB_OSABI_UNKNOWN otherwise.  */
> +
> +static enum gdb_osabi
> +arm_pikeos_osabi_sniffer (bfd *abfd)
> +{
> +  long number_of_symbols;
> +  long i;
> +  int pikeos_stack_found = 0;
> +  int pikeos_stack_size_found = 0;
> +
> +  /* The BFD target of PikeOS is really just standard elf, so we
> +     cannot use it to detect this variant.  The only common thing that
> +     may be found in PikeOS modules are symbols _vm_stack/__p4_stack and
> +     _vm_stack_size/__p4_stack_end. They are used to specify the stack
> +     location and size; and defined by the default linker script.
> +
> +     OS ABI sniffers are called before the minimal symtabs are
> +     created. So inspect the symbol table using BFD.  */
> +
> +  long storage_needed = bfd_get_symtab_upper_bound (abfd);
> +  if (storage_needed <= 0)
> +    return GDB_OSABI_UNKNOWN;
> +
> +  gdb::unique_xmalloc_ptr<asymbol *> symbol_table
> +    ((asymbol **) xmalloc (storage_needed));
> +  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table.get ());
> +
> +  if (number_of_symbols <= 0)
> +    return GDB_OSABI_UNKNOWN;
> +
> +  for (i = 0; i < number_of_symbols; i++)
> +    {
> +      const char *name = bfd_asymbol_name (symbol_table.get ()[i]);
> +
> +      if (strcmp (name, "_vm_stack") == 0
> +	  || strcmp (name, "__p4_stack") == 0)
> +	pikeos_stack_found = 1;
> +
> +      if (strcmp (name, "_vm_stack_size") == 0
> +	  || strcmp (name, "__p4_stack_end") == 0)
> +	pikeos_stack_size_found = 1;
> +    }
> +
> +  if (pikeos_stack_found && pikeos_stack_size_found)
> +    return GDB_OSABI_PIKEOS;
> +  else
> +    return GDB_OSABI_UNKNOWN;
> +}
> +
> +void
> +_initialize_arm_pikeos_tdep (void)
> +{
> +  /* Register the sniffer for the PikeOS targets.  */
> +  gdbarch_register_osabi_sniffer (bfd_arch_arm, bfd_target_elf_flavour,
> +                                  arm_pikeos_osabi_sniffer);
> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_PIKEOS,
> +                          arm_pikeos_init_abi);
> +}
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index 6d1a4df..03c0268 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -180,6 +180,7 @@ arm*-*-symbianelf*)
>  	;;
>  arm*-*-*)
>  	# Target: ARM embedded system
> +	gdb_target_obs="arm-pikeos-tdep.o"
>  	gdb_sim=../sim/arm/libsim.a
>  	;;
>  
> diff --git a/gdb/defs.h b/gdb/defs.h
> index fc42170..28f7a11 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -495,6 +495,7 @@ enum gdb_osabi
>    GDB_OSABI_LYNXOS178,
>    GDB_OSABI_NEWLIB,
>    GDB_OSABI_SDE,
> +  GDB_OSABI_PIKEOS,
>  
>    GDB_OSABI_INVALID		/* keep this last */
>  };
> diff --git a/gdb/osabi.c b/gdb/osabi.c
> index 7d0540b..68f4665 100644
> --- a/gdb/osabi.c
> +++ b/gdb/osabi.c
> @@ -80,6 +80,7 @@ static const struct osabi_names gdb_osabi_names[] =
>    { "LynxOS178", NULL },
>    { "Newlib", NULL },
>    { "SDE", NULL },
> +  { "PikeOS", NULL },
>  
>    { "<invalid>", NULL }
>  };
> -- 
> 1.7.10.4
> 


-- 
Joel

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

[Simon Marchi]

On 2018-09-13 20:47, Joel Brobecker wrote:
> Or would people rather I use a different idiom for managing the memory
> where the symbols are stored?

What you have here is fine, I didn't want to focus the discussion on 
this detail :).

Simon

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

[Joel Brobecker]

> > gdb/ChangeLog:
> > 
> >         * defs.h (enum gdb_osabi): Add GDB_OSABI_PIKEOS.
> >         * osabi.c (gdb_osabi_names): Add name for GDB_OSABI_PIKEOS.
> >         * arm-pikeos-tdep.c: New file.
> >         * configure.tgt: Add arm-pikeos-tdep.o to the case of ARM
> >         embedded system.
> >         * Makefile.in (ALL_TARGET_OBS): Add arm-pikeos-tdep.o.

I just pushed this patch to master...

-- 
Joel