diff mbox

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

Message ID 1536604744-3814-1-git-send-email-y
State New
Headers show

Commit Message

y@eu.adacore.com Sept. 10, 2018, 6:39 p.m. UTC
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

Comments

Simon Marchi Sept. 10, 2018, 8:01 p.m. UTC | #1
(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
Tom Tromey Sept. 10, 2018, 8:52 p.m. UTC | #2
>>>>> "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
diff mbox

Patch

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 }
 };