public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb: add support for handling core dumps on arm-none-eabi
@ 2020-10-02 17:32 Paul Mathieu
  2020-10-02 17:51 ` Luis Machado
  2020-10-02 23:55 ` [PATCH] " Simon Marchi
  0 siblings, 2 replies; 44+ messages in thread
From: Paul Mathieu @ 2020-10-02 17:32 UTC (permalink / raw)
  To: gdb-patches

Core dump files really help debugging crashes.
It is not uncommon for embedded targets to have the ability to generate
memory and CPU register dumps, which can easily be converted into core dump
files.

This patch adds support for loading core files into gdb on arm-none-eabi
targets.
The patch was originally written by Robin Haberkorn <
robin.haberkorn@googlemail.com>

gdb/ChangeLog:
2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
2020-10-02  Paul Mathieu <paulmathieu@google.com>

* arm-none-tdep.c: Added. Provide CPU registers from a core file
* floating point registers not yet supported (FIXME)


---
 gdb/Makefile.in     |   2 +
 gdb/arm-none-tdep.c | 140 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.tgt   |   2 +-
 3 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 gdb/arm-none-tdep.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index dbede7a9cf..7f0e3ea0b0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
  arm-obsd-tdep.o \
  arm-pikeos-tdep.o \
  arm-symbian-tdep.o \
+ arm-none-tdep.o \
  arm-tdep.o \
  arm-wince-tdep.o \
  avr-tdep.o \
@@ -2150,6 +2151,7 @@ ALLDEPFILES = \
  arm-nbsd-tdep.c \
  arm-obsd-tdep.c \
  arm-symbian-tdep.c \
+ arm-none-tdep.c \
  arm-tdep.c \
  avr-tdep.c \
  bfin-linux-tdep.c \
diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
new file mode 100644
index 0000000000..7641a9f7f0
--- /dev/null
+++ b/gdb/arm-none-tdep.c
@@ -0,0 +1,140 @@
+/* Native-dependent code for GDB targetting embedded ARM.
+
+   Copyright (C) 2020 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 "command.h"
+#include "gdbarch.h"
+#include "gdbcore.h"
+#include "inferior.h"
+#include "target.h"
+#include "regcache.h"
+
+#include "arch/arm.h"
+
+#if 0
+#include <fcntl.h>
+#include <time.h>
+#ifdef HAVE_SYS_PROCFS_H
+#include <sys/procfs.h>
+#endif
+#endif
+
+typedef struct {
+  uint32_t reg[18];
+} gdb_gregset_t;
+
+#define ARM_CPSR_GREGNUM 16
+
+extern int arm_apcs_32;
+
+static void
+arm_supply_gregset (struct regcache *regcache, const gdb_gregset_t *gregs)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  enum bfd_endian byte_order = type_byte_order (register_type(gdbarch, 0));
+  int regno;
+  CORE_ADDR reg_pc;
+  gdb_byte pc_buf[ARM_INT_REGISTER_SIZE];
+
+  for (regno = ARM_A1_REGNUM; regno < ARM_PC_REGNUM; regno++)
+    regcache->raw_supply (regno, gregs->reg + regno);
+
+  if (arm_apcs_32)
+    regcache->raw_supply (ARM_PS_REGNUM, gregs->reg + ARM_CPSR_GREGNUM);
+  else
+    regcache->raw_supply (ARM_PS_REGNUM, gregs->reg + ARM_PC_REGNUM);
+
+  reg_pc = extract_unsigned_integer ((const gdb_byte*)(gregs->reg +
ARM_PC_REGNUM),
+ ARM_INT_REGISTER_SIZE, byte_order);
+  reg_pc = gdbarch_addr_bits_remove (gdbarch, reg_pc);
+  store_unsigned_integer (pc_buf, ARM_INT_REGISTER_SIZE, byte_order,
reg_pc);
+  regcache->raw_supply (ARM_PC_REGNUM, pc_buf);
+}
+
+/* Provide registers to GDB from a core file.
+
+   CORE_REG_SECT points to an array of bytes, which are the contents
+   of a `note' from a core file which BFD thinks might contain
+   register contents.  CORE_REG_SIZE is its size.
+
+   WHICH says which register set corelow suspects this is:
+     0 --- the general-purpose register set, in gregset_t format
+     2 --- the floating-point register set, in fpregset_t format
+
+   REG_ADDR is ignored.  */
+
+static void
+fetch_core_registers (struct regcache *regcache,
+      char *core_reg_sect,
+      unsigned core_reg_size,
+      int which,
+      CORE_ADDR reg_addr)
+{
+  switch (which)
+    {
+    case 0:
+      if (core_reg_size != sizeof (gdb_gregset_t))
+ warning (_("Wrong size gregset in core file."));
+      else
+ {
+  gdb_gregset_t gregset;
+  memcpy (&gregset, core_reg_sect, sizeof (gregset));
+  arm_supply_gregset (regcache, &gregset);
+ }
+      break;
+
+#if 0 // TODO
+    case 2:
+      if (core_reg_size != sizeof (gdb_fpregset_t))
+ warning (_("Wrong size fpregset in core file."));
+      else
+ {
+  gdb_fpregset_t fpregset;
+  memcpy (&fpregset, core_reg_sect, sizeof (fpregset));
+  if (gdbarch_fp0_regnum (regcache->arch ()) >= 0)
+    arm_supply_fpregset (regcache, &fpregset);
+ }
+      break;
+#endif
+
+    default:
+      /* We've covered all the kinds of registers we know about here,
+         so this must be something we wouldn't know what to do with
+         anyway.  Just ignore it.  */
+      break;
+    }
+}
+
+/* Register that we are able to handle ELF core file formats using
+   standard procfs "regset" structures.  */
+
+static struct core_fns arm_none_core_fns =
+{
+  bfd_target_elf_flavour, /* core_flavour */
+  default_check_format, /* check_format */
+  default_core_sniffer, /* core_sniffer */
+  fetch_core_registers, /* core_read_registers */
+  NULL /* next */
+};
+
+void
+_initialize_arm_none_tdep (void)
+{
+  deprecated_add_core_fns (&arm_none_core_fns);
+}
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index a3e11c4b9b..0cf05efdd1 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -189,7 +189,7 @@ arm*-*-symbianelf*)
  ;;
 arm*-*-*)
  # Target: ARM embedded system
- gdb_target_obs="arm-pikeos-tdep.o"
+ gdb_target_obs="arm-pikeos-tdep.o arm-none-tdep.o"
  gdb_sim=../sim/arm/libsim.a
  ;;

--

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-02 17:32 [PATCH] gdb: add support for handling core dumps on arm-none-eabi Paul Mathieu
@ 2020-10-02 17:51 ` Luis Machado
  2020-10-02 21:54   ` Paul Mathieu
  2020-10-02 23:55 ` [PATCH] " Simon Marchi
  1 sibling, 1 reply; 44+ messages in thread
From: Luis Machado @ 2020-10-02 17:51 UTC (permalink / raw)
  To: Paul Mathieu, gdb-patches

Hi,

It seems the e-mail client has messed up the proper formatting 
(spaces/tabs) of the patch. I'll defer formatting review until the patch 
comes through clean of e-mail client interferences.

Also, it would be nice to get rid of dead code (like "#if 0" blocks). If 
it is not functional, then there is little point in including such code.

Who is generating such core files? Is there some way I can access the 
tool to give it a try? How can I test the patch itself?

Some general comments below...

On 10/2/20 2:32 PM, Paul Mathieu via Gdb-patches wrote:
> Core dump files really help debugging crashes.
> It is not uncommon for embedded targets to have the ability to generate
> memory and CPU register dumps, which can easily be converted into core dump
> files.
> 
> This patch adds support for loading core files into gdb on arm-none-eabi
> targets.
> The patch was originally written by Robin Haberkorn <
> robin.haberkorn@googlemail.com>
> 
> gdb/ChangeLog:
> 2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
> 2020-10-02  Paul Mathieu <paulmathieu@google.com>
> 
> * arm-none-tdep.c: Added. Provide CPU registers from a core file
> * floating point registers not yet supported (FIXME)
> 
> 
> ---
>   gdb/Makefile.in     |   2 +
>   gdb/arm-none-tdep.c | 140 ++++++++++++++++++++++++++++++++++++++++++++
>   gdb/configure.tgt   |   2 +-
>   3 files changed, 143 insertions(+), 1 deletion(-)
>   create mode 100644 gdb/arm-none-tdep.c
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index dbede7a9cf..7f0e3ea0b0 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
>    arm-obsd-tdep.o \
>    arm-pikeos-tdep.o \
>    arm-symbian-tdep.o \
> + arm-none-tdep.o \
>    arm-tdep.o \
>    arm-wince-tdep.o \
>    avr-tdep.o \
> @@ -2150,6 +2151,7 @@ ALLDEPFILES = \
>    arm-nbsd-tdep.c \
>    arm-obsd-tdep.c \
>    arm-symbian-tdep.c \
> + arm-none-tdep.c \
>    arm-tdep.c \
>    avr-tdep.c \
>    bfin-linux-tdep.c \
> diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
> new file mode 100644
> index 0000000000..7641a9f7f0
> --- /dev/null
> +++ b/gdb/arm-none-tdep.c
> @@ -0,0 +1,140 @@
> +/* Native-dependent code for GDB targetting embedded ARM.
> +
> +   Copyright (C) 2020 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 "command.h"
> +#include "gdbarch.h"
> +#include "gdbcore.h"
> +#include "inferior.h"
> +#include "target.h"
> +#include "regcache.h"
> +
> +#include "arch/arm.h"
> +
> +#if 0
> +#include <fcntl.h>
> +#include <time.h>
> +#ifdef HAVE_SYS_PROCFS_H
> +#include <sys/procfs.h>
> +#endif
> +#endif
> +
> +typedef struct {
> +  uint32_t reg[18];
> +} gdb_gregset_t;
> +
> +#define ARM_CPSR_GREGNUM 16
> +
> +extern int arm_apcs_32;
> +
> +static void
> +arm_supply_gregset (struct regcache *regcache, const gdb_gregset_t *gregs)
> +{
> +  struct gdbarch *gdbarch = regcache->arch ();
> +  enum bfd_endian byte_order = type_byte_order (register_type(gdbarch, 0));
> +  int regno;
> +  CORE_ADDR reg_pc;
> +  gdb_byte pc_buf[ARM_INT_REGISTER_SIZE];
> +
> +  for (regno = ARM_A1_REGNUM; regno < ARM_PC_REGNUM; regno++)
> +    regcache->raw_supply (regno, gregs->reg + regno);
> +
> +  if (arm_apcs_32)
> +    regcache->raw_supply (ARM_PS_REGNUM, gregs->reg + ARM_CPSR_GREGNUM);
> +  else
> +    regcache->raw_supply (ARM_PS_REGNUM, gregs->reg + ARM_PC_REGNUM);
> +
> +  reg_pc = extract_unsigned_integer ((const gdb_byte*)(gregs->reg +
> ARM_PC_REGNUM),
> + ARM_INT_REGISTER_SIZE, byte_order);
> +  reg_pc = gdbarch_addr_bits_remove (gdbarch, reg_pc);
> +  store_unsigned_integer (pc_buf, ARM_INT_REGISTER_SIZE, byte_order,
> reg_pc);
> +  regcache->raw_supply (ARM_PC_REGNUM, pc_buf);
> +}
> +
> +/* Provide registers to GDB from a core file.
> +
> +   CORE_REG_SECT points to an array of bytes, which are the contents
> +   of a `note' from a core file which BFD thinks might contain
> +   register contents.  CORE_REG_SIZE is its size.
> +
> +   WHICH says which register set corelow suspects this is:
> +     0 --- the general-purpose register set, in gregset_t format
> +     2 --- the floating-point register set, in fpregset_t format
> +
> +   REG_ADDR is ignored.  */
> +
> +static void
> +fetch_core_registers (struct regcache *regcache,
> +      char *core_reg_sect,
> +      unsigned core_reg_size,
> +      int which,
> +      CORE_ADDR reg_addr)
> +{
> +  switch (which)
> +    {
> +    case 0:
> +      if (core_reg_size != sizeof (gdb_gregset_t))
> + warning (_("Wrong size gregset in core file."));
> +      else
> + {
> +  gdb_gregset_t gregset;
> +  memcpy (&gregset, core_reg_sect, sizeof (gregset));
> +  arm_supply_gregset (regcache, &gregset);
> + }

We usually go for one of the extract_* functions instead of memcpy, to 
account for differences in endianness.

It is not safe to assume the host has the same endianness as the target. 
Same thing for other uses of memcpy in the patch.

> +      break;
> +
> +#if 0 // TODO
> +    case 2:
> +      if (core_reg_size != sizeof (gdb_fpregset_t))
> + warning (_("Wrong size fpregset in core file."));
> +      else
> + {
> +  gdb_fpregset_t fpregset;
> +  memcpy (&fpregset, core_reg_sect, sizeof (fpregset));
> +  if (gdbarch_fp0_regnum (regcache->arch ()) >= 0)
> +    arm_supply_fpregset (regcache, &fpregset);
> + }
> +      break;
> +#endif
> +
> +    default:
> +      /* We've covered all the kinds of registers we know about here,
> +         so this must be something we wouldn't know what to do with
> +         anyway.  Just ignore it.  */
> +      break;
> +    }
> +}
> +
> +/* Register that we are able to handle ELF core file formats using
> +   standard procfs "regset" structures.  */
> +
> +static struct core_fns arm_none_core_fns =
> +{
> +  bfd_target_elf_flavour, /* core_flavour */
> +  default_check_format, /* check_format */
> +  default_core_sniffer, /* core_sniffer */
> +  fetch_core_registers, /* core_read_registers */
> +  NULL /* next */
> +};
> +
> +void
> +_initialize_arm_none_tdep (void)
> +{
> +  deprecated_add_core_fns (&arm_none_core_fns);
> +}

This particular function was removed in commit 
6ba0a321037b748f0b1285c44af762021a380bd3. I'm guessing this won't build.

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-02 17:51 ` Luis Machado
@ 2020-10-02 21:54   ` Paul Mathieu
  2020-10-02 21:59     ` Simon Marchi
  2020-10-03  3:57     ` Simon Marchi
  0 siblings, 2 replies; 44+ messages in thread
From: Paul Mathieu @ 2020-10-02 21:54 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches, Alan Hayward

Hi Luis,

Thanks for the quick feedback!

It seems the e-mail client has messed up the proper formatting
> (spaces/tabs) of the patch. I'll defer formatting review until the patch
> comes through clean of e-mail client interferences.


Woops, sorry about this. I'm submitting a new patch, hopefully it will be
treated well by the email gods.


> Also, it would be nice to get rid of dead code (like "#if 0" blocks). If
> it is not functional, then there is little point in including such code.
>

Done.


> Who is generating such core files? Is there some way I can access the
> tool to give it a try? How can I test the patch itself?
>

I made one myself, with a python script that I wrote.
I have a live embedded target that I can use to dump memory regions and CPU
registers over a serial line. My python script then formats the memory
dumps into PT_LOAD segments, and the CPU registers into a NT_PRSTATUS note
segment, and stores it as an ELF32 core file.
I'm happy to share any material that would help testing this, including the
python script, an example of firmware and core file and some embedded
source code.

I addressed the rest of your feedback through essentially re-writing the
patch to not use `deprecated_add_core_fns`
In the new patch, I had to use the GDB_OSABI_LINUX, otherwise my core
wouldn't be recognized. I'm not sure if this can potentially interfere with
the arm-linux implementation. Since it has its own entry in configure.tgt,
I imagine not?

Thanks!

gdb/ChangeLog:
2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
2020-10-02  Paul Mathieu <paulmathieu@google.com>
* arm-none-tdep.c: Source file added. Provide CPU registers from a core
file
* floating point registers not yet supported (FIXME)
---
 gdb/Makefile.in     |   2 +
 gdb/arm-none-tdep.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.tgt   |   2 +-
 3 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 gdb/arm-none-tdep.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index dbede7a9cf..7f0e3ea0b0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
  arm-obsd-tdep.o \
  arm-pikeos-tdep.o \
  arm-symbian-tdep.o \
+ arm-none-tdep.o \
  arm-tdep.o \
  arm-wince-tdep.o \
  avr-tdep.o \
@@ -2150,6 +2151,7 @@ ALLDEPFILES = \
  arm-nbsd-tdep.c \
  arm-obsd-tdep.c \
  arm-symbian-tdep.c \
+ arm-none-tdep.c \
  arm-tdep.c \
  avr-tdep.c \
  bfin-linux-tdep.c \
diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
new file mode 100644
index 0000000000..bdf346bdc3
--- /dev/null
+++ b/gdb/arm-none-tdep.c
@@ -0,0 +1,107 @@
+/* Native-dependent code for GDB targetting embedded ARM.
+
+   Copyright (C) 2020 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 "osabi.h"
+
+#include "arch/arm.h"
+#include "gdbarch.h"
+#include "regcache.h"
+#include "regset.h"
+
+struct arm_none_reg
+{
+  uint32_t reg[13];
+  uint32_t sp;
+  uint32_t lr;
+  uint32_t pc;
+  uint32_t cpsr;
+  uint32_t orig_r0;
+};
+
+extern int arm_apcs_32;
+
+static void
+arm_none_supply_gregset (const struct regset *regset,
+                         struct regcache *regcache, int regnum,
+                         const void *gregs, size_t len)
+{
+  const arm_none_reg *gregset = static_cast<const arm_none_reg *> (gregs);
+  gdb_assert (len >= sizeof (arm_none_reg));
+
+  /* Integer registers.  */
+  for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++)
+    if (regnum == -1 || regnum == i)
+      regcache->raw_supply (i, (char *)&gregset->reg[i]);
+
+  if (regnum == -1 || regnum == ARM_SP_REGNUM)
+    regcache->raw_supply (ARM_SP_REGNUM, (char *)&gregset->sp);
+
+  if (regnum == -1 || regnum == ARM_LR_REGNUM)
+    regcache->raw_supply (ARM_LR_REGNUM, (char *)&gregset->lr);
+
+  if (regnum == -1 || regnum == ARM_PC_REGNUM)
+    {
+      CORE_ADDR r_pc
+          = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc);
+      regcache->raw_supply (ARM_PC_REGNUM, (char *)&r_pc);
+    }
+
+  if (regnum == -1 || regnum == ARM_PS_REGNUM)
+    {
+      if (arm_apcs_32)
+        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->cpsr);
+      else
+        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->pc);
+    }
+}
+
+static const struct regset arm_none_regset
+    = { nullptr, arm_none_supply_gregset,
+        /* We don't need a collect function because we only use this
reading
+           registers (via iterate_over_regset_sections and
+           fetch_regs/fetch_register).  */
+        nullptr, 0 };
+
+static void
+arm_none_iterate_over_regset_sections (struct gdbarch *gdbarch,
+                                       iterate_over_regset_sections_cb *cb,
+                                       void *cb_data,
+                                       const struct regcache *regcache)
+{
+  cb (".reg", sizeof (arm_none_reg), sizeof (arm_none_reg),
&arm_none_regset,
+      NULL, cb_data);
+}
+
+static void arm_none_elf_init_abi (struct gdbarch_info info,
+                                   struct gdbarch *gdbarch);
+static void
+arm_none_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  set_gdbarch_iterate_over_regset_sections (
+      gdbarch, arm_none_iterate_over_regset_sections);
+}
+
+void _initialize_arm_none_tdep (void);
+void
+_initialize_arm_none_tdep (void)
+{
+  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
+                          arm_none_elf_init_abi);
+}
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index a3e11c4b9b..0cf05efdd1 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -189,7 +189,7 @@ arm*-*-symbianelf*)
  ;;
 arm*-*-*)
  # Target: ARM embedded system
- gdb_target_obs="arm-pikeos-tdep.o"
+ gdb_target_obs="arm-pikeos-tdep.o arm-none-tdep.o"
  gdb_sim=../sim/arm/libsim.a
  ;;

--

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-02 21:54   ` Paul Mathieu
@ 2020-10-02 21:59     ` Simon Marchi
  2020-10-03  3:57     ` Simon Marchi
  1 sibling, 0 replies; 44+ messages in thread
From: Simon Marchi @ 2020-10-02 21:59 UTC (permalink / raw)
  To: Paul Mathieu, Luis Machado; +Cc: gdb-patches

On 2020-10-02 5:54 p.m., Paul Mathieu via Gdb-patches wrote:
> Woops, sorry about this. I'm submitting a new patch, hopefully it will be
> treated well by the email gods.

Make sure you send it using git-send-email, and we'll all be happy!

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-02 17:32 [PATCH] gdb: add support for handling core dumps on arm-none-eabi Paul Mathieu
  2020-10-02 17:51 ` Luis Machado
@ 2020-10-02 23:55 ` Simon Marchi
  2020-10-03  0:35   ` Paul Mathieu
  1 sibling, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-02 23:55 UTC (permalink / raw)
  To: Paul Mathieu, gdb-patches


Some quick comments, just skimming the patch.  I won't comment about
indentation issue, because I don't know if it has been modified by the
email client.

I support what Luis has already said, it needs to be documented what
this format is, where it is defined/specified, who produces it, etc.

On 2020-10-02 1:32 p.m., Paul Mathieu via Gdb-patches wrote:
> Core dump files really help debugging crashes.
> It is not uncommon for embedded targets to have the ability to generate
> memory and CPU register dumps, which can easily be converted into core dump
> files.
>
> This patch adds support for loading core files into gdb on arm-none-eabi
> targets.
> The patch was originally written by Robin Haberkorn <
> robin.haberkorn@googlemail.com>
>
> gdb/ChangeLog:
> 2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
> 2020-10-02  Paul Mathieu <paulmathieu@google.com>
>
> * arm-none-tdep.c: Added. Provide CPU registers from a core file
> * floating point registers not yet supported (FIXME)
>
>
> ---
>  gdb/Makefile.in     |   2 +
>  gdb/arm-none-tdep.c | 140 ++++++++++++++++++++++++++++++++++++++++++++
>  gdb/configure.tgt   |   2 +-
>  3 files changed, 143 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/arm-none-tdep.c
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index dbede7a9cf..7f0e3ea0b0 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
>   arm-obsd-tdep.o \
>   arm-pikeos-tdep.o \
>   arm-symbian-tdep.o \
> + arm-none-tdep.o \
>   arm-tdep.o \
>   arm-wince-tdep.o \
>   avr-tdep.o \
> @@ -2150,6 +2151,7 @@ ALLDEPFILES = \
>   arm-nbsd-tdep.c \
>   arm-obsd-tdep.c \
>   arm-symbian-tdep.c \
> + arm-none-tdep.c \
>   arm-tdep.c \
>   avr-tdep.c \
>   bfin-linux-tdep.c \
> diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
> new file mode 100644
> index 0000000000..7641a9f7f0
> --- /dev/null
> +++ b/gdb/arm-none-tdep.c
> @@ -0,0 +1,140 @@
> +/* Native-dependent code for GDB targetting embedded ARM.

Maybe "bare-metal ARM" would be more precise?  Embedded doesn't mean "no
OS", which I think is what you want to mean here.

> +
> +   Copyright (C) 2020 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 "command.h"
> +#include "gdbarch.h"
> +#include "gdbcore.h"
> +#include "inferior.h"
> +#include "target.h"
> +#include "regcache.h"
> +
> +#include "arch/arm.h"
> +
> +#if 0
> +#include <fcntl.h>
> +#include <time.h>
> +#ifdef HAVE_SYS_PROCFS_H
> +#include <sys/procfs.h>
> +#endif
> +#endif
> +
> +typedef struct {
> +  uint32_t reg[18];
> +} gdb_gregset_t;

Use:

struct gdb_gregset_t
{
  ...
};

We are in C++, so when you use it you can still omit the "struct" keyword.

> +
> +#define ARM_CPSR_GREGNUM 16
> +
> +extern int arm_apcs_32;

Don't declare this here, there's a suitable declaration in arm-tdep.h.

> +
> +static void
> +arm_supply_gregset (struct regcache *regcache, const gdb_gregset_t *gregs)
> +{
> +  struct gdbarch *gdbarch = regcache->arch ();
> +  enum bfd_endian byte_order = type_byte_order (register_type(gdbarch, 0));
> +  int regno;
> +  CORE_ADDR reg_pc;
> +  gdb_byte pc_buf[ARM_INT_REGISTER_SIZE];
> +
> +  for (regno = ARM_A1_REGNUM; regno < ARM_PC_REGNUM; regno++)
> +    regcache->raw_supply (regno, gregs->reg + regno);
> +
> +  if (arm_apcs_32)
> +    regcache->raw_supply (ARM_PS_REGNUM, gregs->reg + ARM_CPSR_GREGNUM);
> +  else
> +    regcache->raw_supply (ARM_PS_REGNUM, gregs->reg + ARM_PC_REGNUM);
> +
> +  reg_pc = extract_unsigned_integer ((const gdb_byte*)(gregs->reg +
> ARM_PC_REGNUM),

Space before *, and after the cast:

 (const gdb_byte *) (gregs...)

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-02 23:55 ` [PATCH] " Simon Marchi
@ 2020-10-03  0:35   ` Paul Mathieu
  2020-10-03  2:24     ` Simon Marchi
  0 siblings, 1 reply; 44+ messages in thread
From: Paul Mathieu @ 2020-10-03  0:35 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Thanks for the feedback Simon!
I did send a new version of that patch as a reply to Luis' feedback, you
seem to have commented on the first one.
Should I have sent that patch in a different thread?

Some quick comments, just skimming the patch.  I won't comment about
> indentation issue, because I don't know if it has been modified by the
> email client.
>
> I support what Luis has already said, it needs to be documented what
> this format is, where it is defined/specified, who produces it, etc.
>

I'm not sure how (and where) this could be documented, are there examples
of such documentation?
Since I couldn't find any prior art for bare metal targets, I went for
something that I assumed was a standard core file format.


> gdb/ChangeLog:
> > 2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
> > 2020-10-02  Paul Mathieu <paulmathieu@google.com>
> >
> > * arm-none-tdep.c: Added. Provide CPU registers from a core file
> > * floating point registers not yet supported (FIXME)
> >
> >
> > ---
> >  gdb/Makefile.in     |   2 +
> >  gdb/arm-none-tdep.c | 140 ++++++++++++++++++++++++++++++++++++++++++++
> >  gdb/configure.tgt   |   2 +-
> >  3 files changed, 143 insertions(+), 1 deletion(-)
> >  create mode 100644 gdb/arm-none-tdep.c
> >
> > diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> > index dbede7a9cf..7f0e3ea0b0 100644
> > --- a/gdb/Makefile.in
> > +++ b/gdb/Makefile.in
> > @@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
> >   arm-obsd-tdep.o \
> >   arm-pikeos-tdep.o \
> >   arm-symbian-tdep.o \
> > + arm-none-tdep.o \
> >   arm-tdep.o \
> >   arm-wince-tdep.o \
> >   avr-tdep.o \
> > @@ -2150,6 +2151,7 @@ ALLDEPFILES = \
> >   arm-nbsd-tdep.c \
> >   arm-obsd-tdep.c \
> >   arm-symbian-tdep.c \
> > + arm-none-tdep.c \
> >   arm-tdep.c \
> >   avr-tdep.c \
> >   bfin-linux-tdep.c \
> > diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
> > new file mode 100644
> > index 0000000000..7641a9f7f0
> > --- /dev/null
> > +++ b/gdb/arm-none-tdep.c
> > @@ -0,0 +1,140 @@
> > +/* Native-dependent code for GDB targetting embedded ARM.
>
> Maybe "bare-metal ARM" would be more precise?  Embedded doesn't mean "no
> OS", which I think is what you want to mean here.
>

Sure, bare-metal ARM works.


>
> > +
> > +   Copyright (C) 2020 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 "command.h"
> > +#include "gdbarch.h"
> > +#include "gdbcore.h"
> > +#include "inferior.h"
> > +#include "target.h"
> > +#include "regcache.h"
> > +
> > +#include "arch/arm.h"
> > +
> > +#if 0
> > +#include <fcntl.h>
> > +#include <time.h>
> > +#ifdef HAVE_SYS_PROCFS_H
> > +#include <sys/procfs.h>
> > +#endif
> > +#endif
> > +
> > +typedef struct {
> > +  uint32_t reg[18];
> > +} gdb_gregset_t;
>
> Use:
>
> struct gdb_gregset_t
> {
>   ...
> };
>
> We are in C++, so when you use it you can still omit the "struct" keyword.
>

Done in the patch I sent after Luis' review.


>
> > +
> > +#define ARM_CPSR_GREGNUM 16
> > +
> > +extern int arm_apcs_32;
>
> Don't declare this here, there's a suitable declaration in arm-tdep.h.
>

Done.


>
> > +
> > +static void
> > +arm_supply_gregset (struct regcache *regcache, const gdb_gregset_t
> *gregs)
> > +{
> > +  struct gdbarch *gdbarch = regcache->arch ();
> > +  enum bfd_endian byte_order = type_byte_order (register_type(gdbarch,
> 0));
> > +  int regno;
> > +  CORE_ADDR reg_pc;
> > +  gdb_byte pc_buf[ARM_INT_REGISTER_SIZE];
> > +
> > +  for (regno = ARM_A1_REGNUM; regno < ARM_PC_REGNUM; regno++)
> > +    regcache->raw_supply (regno, gregs->reg + regno);
> > +
> > +  if (arm_apcs_32)
> > +    regcache->raw_supply (ARM_PS_REGNUM, gregs->reg + ARM_CPSR_GREGNUM);
> > +  else
> > +    regcache->raw_supply (ARM_PS_REGNUM, gregs->reg + ARM_PC_REGNUM);
> > +
> > +  reg_pc = extract_unsigned_integer ((const gdb_byte*)(gregs->reg +
> > ARM_PC_REGNUM),
>
> Space before *, and after the cast:
>
>  (const gdb_byte *) (gregs...)
>

Done.


>
> Simon
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-03  0:35   ` Paul Mathieu
@ 2020-10-03  2:24     ` Simon Marchi
  0 siblings, 0 replies; 44+ messages in thread
From: Simon Marchi @ 2020-10-03  2:24 UTC (permalink / raw)
  To: Paul Mathieu; +Cc: gdb-patches

On 2020-10-02 8:35 p.m., Paul Mathieu wrote:
> Thanks for the feedback Simon!
> I did send a new version of that patch as a reply to Luis' feedback, you seem to have commented on the first one.
> Should I have sent that patch in a different thread?

Ah sorry, I didn't see it buried in your response.  It's fine, although
doing proper version helps tracking the different versions.

>
>     Some quick comments, just skimming the patch.  I won't comment about
>     indentation issue, because I don't know if it has been modified by the
>     email client.
>
>     I support what Luis has already said, it needs to be documented what
>     this format is, where it is defined/specified, who produces it, etc.
>
>
> I'm not sure how (and where) this could be documented, are there examples of such documentation?
> Since I couldn't find any prior art for bare metal targets, I went for something that I assumed was a standard core file format.

I'm not very well-versed in the domain of core files, but it is my
understanding that the format is quite platform dependent.  The
container is often an ELF file with notes.  But the format of those
notes' content varies from one platform to another.  For example, the
layout of how registers are saved in a Linux core file differs from the
layout of registers in a FreeBSD core.  Similarly, for bare-metal, the
tool you are using to generate the core decides of a certain layout,
which could be different from the layout chosen by another tool.

Therefore, I think it's important to describe what the layout expected
by your code is (what tool/platform produces it).

It is a good start to document it somewhere in the code.  We can then
think if it would be relevant to document it in the user manual as well
(we'll have a better idea when we understand better where these cores
come from).

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-02 21:54   ` Paul Mathieu
  2020-10-02 21:59     ` Simon Marchi
@ 2020-10-03  3:57     ` Simon Marchi
  2020-10-03 18:14       ` [PATCH v2] " Paul Mathieu
  1 sibling, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-03  3:57 UTC (permalink / raw)
  To: Paul Mathieu, Luis Machado; +Cc: gdb-patches

On 2020-10-02 5:54 p.m., Paul Mathieu via Gdb-patches wrote:
> Woops, sorry about this. I'm submitting a new patch, hopefully it will be
> treated well by the email gods.

Sorry, I replied to this message but I completely missed the rest of
your reply below.

Note that I am unable to apply this patch either because long lines were
again wrapped by your email client.  Please use git-send-email.  You can
still reply in the thread by using --in-reply-to.

>> Who is generating such core files? Is there some way I can access the
>> tool to give it a try? How can I test the patch itself?
>>
>
> I made one myself, with a python script that I wrote.
> I have a live embedded target that I can use to dump memory regions and CPU
> registers over a serial line. My python script then formats the memory
> dumps into PT_LOAD segments, and the CPU registers into a NT_PRSTATUS note
> segment, and stores it as an ELF32 core file.
> I'm happy to share any material that would help testing this, including the
> python script, an example of firmware and core file and some embedded
> source code.

Ok, so that answers the question in my other email, about which tool
produces this format of core.  The answer is this specific python
script, and the format is "Paul's ARM core format" :).

I don't know what Luis thinks about this, but I would be a bit hesitant
to add support in GDB for a core format that's not standard nor the
output of some "well-known" tool (which would be a de facto standard).

Is there a format that already exists in the ARM ecosystem for core
dumps of bare-metal systems, we could base our stuff on?

Alternatively, do you think you could implement GDB's generate-core-file
command for bare-metal ARM?  First, it would make sense for GDB to be
able to produce a core in some format and be able to ingest it again.
And it would act as some kind of documentation / reference
implementation of what GDB expects, and that could become some de facto
standard.  People who would like to have their core readable by GDB
would produce it in this format.

> I addressed the rest of your feedback through essentially re-writing the
> patch to not use `deprecated_add_core_fns`
> In the new patch, I had to use the GDB_OSABI_LINUX, otherwise my core
> wouldn't be recognized. I'm not sure if this can potentially interfere with
> the arm-linux implementation. Since it has its own entry in configure.tgt,
> I imagine not?

Aren't you able to use GDB_OSABI_NONE?  But see comments below.

> gdb/ChangeLog:
> 2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
> 2020-10-02  Paul Mathieu <paulmathieu@google.com>
> * arm-none-tdep.c: Source file added. Provide CPU registers from a core
> file
> * floating point registers not yet supported (FIXME)
> ---
>  gdb/Makefile.in     |   2 +
>  gdb/arm-none-tdep.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
>  gdb/configure.tgt   |   2 +-
>  3 files changed, 110 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/arm-none-tdep.c
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index dbede7a9cf..7f0e3ea0b0 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
>   arm-obsd-tdep.o \
>   arm-pikeos-tdep.o \
>   arm-symbian-tdep.o \
> + arm-none-tdep.o \
>   arm-tdep.o \
>   arm-wince-tdep.o \
>   avr-tdep.o \
> @@ -2150,6 +2151,7 @@ ALLDEPFILES = \
>   arm-nbsd-tdep.c \
>   arm-obsd-tdep.c \
>   arm-symbian-tdep.c \
> + arm-none-tdep.c \
>   arm-tdep.c \
>   avr-tdep.c \
>   bfin-linux-tdep.c \

Please keep the lists sorted.

Note that so far, everything related to bare-metal for an architecture
was simply in <arch>-tdep.c.  So perhaps it should just be there?  I'll
defer to Luis to decide what is the best organization for the ARM code.

> +static void
> +arm_none_supply_gregset (const struct regset *regset,
> +                         struct regcache *regcache, int regnum,
> +                         const void *gregs, size_t len)
> +{
> +  const arm_none_reg *gregset = static_cast<const arm_none_reg *> (gregs);
> +  gdb_assert (len >= sizeof (arm_none_reg));
> +
> +  /* Integer registers.  */
> +  for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++)
> +    if (regnum == -1 || regnum == i)
> +      regcache->raw_supply (i, (char *)&gregset->reg[i]);
> +
> +  if (regnum == -1 || regnum == ARM_SP_REGNUM)
> +    regcache->raw_supply (ARM_SP_REGNUM, (char *)&gregset->sp);
> +
> +  if (regnum == -1 || regnum == ARM_LR_REGNUM)
> +    regcache->raw_supply (ARM_LR_REGNUM, (char *)&gregset->lr);
> +
> +  if (regnum == -1 || regnum == ARM_PC_REGNUM)
> +    {
> +      CORE_ADDR r_pc
> +          = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc);

We'd typically use one ident (two spaces) before the equal sign.  But
otherwise that's spot on.

> +      regcache->raw_supply (ARM_PC_REGNUM, (char *)&r_pc);
> +    }
> +
> +  if (regnum == -1 || regnum == ARM_PS_REGNUM)
> +    {
> +      if (arm_apcs_32)
> +        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->cpsr);
> +      else
> +        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->pc);
> +    }
> +}
> +
> +static const struct regset arm_none_regset
> +    = { nullptr, arm_none_supply_gregset,
> +        /* We don't need a collect function because we only use this
> reading
> +           registers (via iterate_over_regset_sections and
> +           fetch_regs/fetch_register).  */
> +        nullptr, 0 };

See above where `reading` is on a separate line, that's an example of
the patch getting mangled by the email client.

> +void _initialize_arm_none_tdep (void);
> +void
> +_initialize_arm_none_tdep (void)
> +{
> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
> +                          arm_none_elf_init_abi);

I don't think this is what you want to do.  This "declares" that
combination of architecture ARM and osabi Linux exists, and that
arm_none_elf_init_abi should be called when such a combo is detected to
initialize the gdbarch object.

However, such a combo is already registered somewhere else, as you can
guess:

  https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/arm-linux-tdep.c;h=f60cb51763f1c5b14811494b646acf54b62d1f10;hb=HEAD#l2010

So I presume that if you build a GDB configured with
--enable-targets=all (which will include arm-linux-tdep.c), you will hit
this assertion:

  https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/osabi.c;h=e8a813b62f268417edf5f0ad164c9c942bd43b1e;hb=HEAD#l179

What I think you want is to register an osabi sniffer, whose job is to
try to guess an osabi given a bfd.  Apparently that bfd can sometimes
represent the ELF core file, so you can look up sections and see if that
file looks like a core file you can handle.  And if so, return
GDB_OSABI_NONE.

And then, you'd need to register (with gdbarch_register_osabi) osabi
GDB_OSABI_NONE with the arm architecture (same as you did, but with
GDB_OSABI_NONE instead of GDB_OSABI_LINUX) with the callback to call
when this combo is detected:

  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_NONE,
                          arm_none_elf_init_abi);

I'm not sure about that part, but I think it's the right direction.

Now, the question is how to recognize one of these cores?  As an
example, to recognize Cygwin core dumps (which use ELF files as a
container without a recognizable ABI tag, so it's kind of the same
situation as you), we use the .reg section size to determine if the BFD
is a Cygwin core:

  https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/amd64-windows-tdep.c;h=e427c20538961bda2a1a0f033207eebce64c4729;hb=HEAD#l1387

So, you can do that, although I think we all agree that it's not ideal.
If there was some way of identifying that the core is in the format we
recognize (say, if there was some special section with a tag), it would
make it much easier to identify it without having to guess.

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-03  3:57     ` Simon Marchi
@ 2020-10-03 18:14       ` Paul Mathieu
  2020-10-04 17:30         ` Paul Mathieu
                           ` (2 more replies)
  0 siblings, 3 replies; 44+ messages in thread
From: Paul Mathieu @ 2020-10-03 18:14 UTC (permalink / raw)
  To: gdb-patches

Thanks Simon for the super helpful feedback!
A few answers inline, and hopefully a properly formatted patch at the
bottom.

> Note that I am unable to apply this patch either because long lines were
> again wrapped by your email client.  Please use git-send-email.  You can
> still reply in the thread by using --in-reply-to.

Couldn't get git send-email to work with my SMTP, but I got some version
of mutt to comply. Hopefully you can apply the patch this time.


> Ok, so that answers the question in my other email, about which tool
> produces this format of core.  The answer is this specific python
> script, and the format is "Paul's ARM core format" :).

It was some team work between what gdb understood "natively" and what
the patch does.
More specifically, this patch only seems to tell gdb to get the
registers from the core file the way it would normally do, and then
supply them almost verbatim to the regcache.
The struct for these registers is nothing special either, you can find
it in the kernel code as well:
https://elixir.bootlin.com/linux/latest/source/arch/arm/include/uapi/asm/ptrace.h#L135


> I don't know what Luis thinks about this, but I would be a bit hesitant
> to add support in GDB for a core format that's not standard nor the
> output of some "well-known" tool (which would be a de facto standard).
>
> Is there a format that already exists in the ARM ecosystem for core
> dumps of bare-metal systems, we could base our stuff on?

I'm not aware of anything that does that. There are tools that generate
memory dumps and CPU register dumps into their own proprietary format,
but nothing that I know that you could call a 'core file'.

Are there other tools besides gdb that deal with core dumps? Maybe that
could help me find other formats.

That being said, the format used here is "well-known" in the sense that
it's the exact same format the linux kernel would use to dump cores on
arm-linux-*


> Alternatively, do you think you could implement GDB's generate-core-file
> command for bare-metal ARM?  First, it would make sense for GDB to be
> able to produce a core in some format and be able to ingest it again.
> And it would act as some kind of documentation / reference
> implementation of what GDB expects, and that could become some de facto
> standard.  People who would like to have their core readable by GDB
> would produce it in this format.

I had never cloned the gdb source repo until 2 days ago. I have no idea
how much work that would be, given that I know next to nothing about the
gdb codebase.

I do agree that this seems like a better way to do it, since there are
already many integrations with a gdb server talking to a live bare-metal
target over a physical debugger.

As long as gdb already supports primitives to grab memory dumps and cpu
registers of a remote target, I'd imagine generating a core file
shouldn't be too much work. I'm quite sure most of that functionality
already exists for extremely similar targets.


>> +void _initialize_arm_none_tdep (void);
>> +void
>> +_initialize_arm_none_tdep (void)
>> +{
>> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
>> +                          arm_none_elf_init_abi);
>
> I don't think this is what you want to do.  This "declares" that
> combination of architecture ARM and osabi Linux exists, and that
> arm_none_elf_init_abi should be called when such a combo is detected to
> initialize the gdbarch object.
>
> However, such a combo is already registered somewhere else, as you can
> guess:
>
>   https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/arm-linux-tdep.c;h=f60cb51763f1c5b14811494b646acf54b62d1f10;hb=HEAD#l2010
>
> So I presume that if you build a GDB configured with
> --enable-targets=all (which will include arm-linux-tdep.c), you will hit
> this assertion:
>
>   https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/osabi.c;h=e8a813b62f268417edf5f0ad164c9c942bd43b1e;hb=HEAD#l179

Ah, I was afraid of that. Thanks for the pointers.


> What I think you want is to register an osabi sniffer, whose job is to
> try to guess an osabi given a bfd.  Apparently that bfd can sometimes
> represent the ELF core file, so you can look up sections and see if that
> file looks like a core file you can handle.  And if so, return
> GDB_OSABI_NONE.
>
> And then, you'd need to register (with gdbarch_register_osabi) osabi
> GDB_OSABI_NONE with the arm architecture (same as you did, but with
> GDB_OSABI_NONE instead of GDB_OSABI_LINUX) with the callback to call
> when this combo is detected:
>
>   gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_NONE,
>                           arm_none_elf_init_abi);
>
> I'm not sure about that part, but I think it's the right direction.
>
> Now, the question is how to recognize one of these cores?  As an
> example, to recognize Cygwin core dumps (which use ELF files as a
> container without a recognizable ABI tag, so it's kind of the same
> situation as you), we use the .reg section size to determine if the BFD
> is a Cygwin core:
>
>   https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/amd64-windows-tdep.c;h=e427c20538961bda2a1a0f033207eebce64c4729;hb=HEAD#l1387
>
> So, you can do that, although I think we all agree that it's not ideal.
> If there was some way of identifying that the core is in the format we
> recognize (say, if there was some special section with a tag), it would
> make it much easier to identify it without having to guess.

Having a sniffer for GDB_OSABI_NONE seems like the right way to do this.

Since the format isn't specified (yet), I can still freely control it. I
imagine I could add some sort of OSABI marker in the core file to mark
it as an arm-none-eabi core.o

My first guess was to set the EI_OSABI e_ident elf header field to
ELFOSABI_NONE, but that happens to be the same enum value as
ELFOSABI_SYSV, which is afaik broadly used by the linux kernel for core
files. So it wouldn't be a good marker.

Not sure what a better way would be to not abuse the ELF structure and
produce reasonable ELF core dumps (since they already work so well with
gdb).


Paul



gdb/Changelog
2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
2020-10-02  Paul Mathieu <paulmathieu@google.com>
	* arm-none-tdep.c: Source file added. Provide CPU registers from a core
	file
	* floating point registers not yet supported (FIXME)
---
 gdb/Makefile.in     |   2 +
 gdb/arm-none-tdep.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.tgt   |   2 +-
 3 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 gdb/arm-none-tdep.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index dbede7a9cf..7f0e3ea0b0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
 	arm-obsd-tdep.o \
 	arm-pikeos-tdep.o \
 	arm-symbian-tdep.o \
+	arm-none-tdep.o \
 	arm-tdep.o \
 	arm-wince-tdep.o \
 	avr-tdep.o \
@@ -2150,6 +2151,7 @@ ALLDEPFILES = \
 	arm-nbsd-tdep.c \
 	arm-obsd-tdep.c \
 	arm-symbian-tdep.c \
+	arm-none-tdep.c \
 	arm-tdep.c \
 	avr-tdep.c \
 	bfin-linux-tdep.c \
diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
new file mode 100644
index 0000000000..21743c40a0
--- /dev/null
+++ b/gdb/arm-none-tdep.c
@@ -0,0 +1,106 @@
+/* Native-dependent code for GDB targetting bare-metal ARM.
+
+   Copyright (C) 2020 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 "osabi.h"
+
+#include "arch/arm.h"
+#include "arm-tdep.h"
+#include "gdbarch.h"
+#include "regcache.h"
+#include "regset.h"
+
+struct arm_none_reg
+{
+  uint32_t reg[13];
+  uint32_t sp;
+  uint32_t lr;
+  uint32_t pc;
+  uint32_t cpsr;
+  uint32_t orig_r0;
+};
+
+static void
+arm_none_supply_gregset (const struct regset *regset,
+                         struct regcache *regcache, int regnum,
+                         const void *gregs, size_t len)
+{
+  const arm_none_reg *gregset = static_cast<const arm_none_reg *> (gregs);
+  gdb_assert (len >= sizeof (arm_none_reg));
+
+  /* Integer registers.  */
+  for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++)
+    if (regnum == -1 || regnum == i)
+      regcache->raw_supply (i, (char *)&gregset->reg[i]);
+
+  if (regnum == -1 || regnum == ARM_SP_REGNUM)
+    regcache->raw_supply (ARM_SP_REGNUM, (char *)&gregset->sp);
+
+  if (regnum == -1 || regnum == ARM_LR_REGNUM)
+    regcache->raw_supply (ARM_LR_REGNUM, (char *)&gregset->lr);
+
+  if (regnum == -1 || regnum == ARM_PC_REGNUM)
+    {
+      CORE_ADDR r_pc
+        = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc);
+      regcache->raw_supply (ARM_PC_REGNUM, (char *)&r_pc);
+    }
+
+  if (regnum == -1 || regnum == ARM_PS_REGNUM)
+    {
+      if (arm_apcs_32)
+        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->cpsr);
+      else
+        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->pc);
+    }
+}
+
+static const struct regset arm_none_regset
+    = { nullptr, arm_none_supply_gregset,
+        /* We don't need a collect function because we only use this reading
+           registers (via iterate_over_regset_sections and
+           fetch_regs/fetch_register).  */
+        nullptr, 0 };
+
+static void
+arm_none_iterate_over_regset_sections (struct gdbarch *gdbarch,
+                                       iterate_over_regset_sections_cb *cb,
+                                       void *cb_data,
+                                       const struct regcache *regcache)
+{
+  cb (".reg", sizeof (arm_none_reg), sizeof (arm_none_reg), &arm_none_regset,
+      NULL, cb_data);
+}
+
+static void arm_none_elf_init_abi (struct gdbarch_info info,
+                                   struct gdbarch *gdbarch);
+static void
+arm_none_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  set_gdbarch_iterate_over_regset_sections (
+      gdbarch, arm_none_iterate_over_regset_sections);
+}
+
+void _initialize_arm_none_tdep (void);
+void
+_initialize_arm_none_tdep (void)
+{
+  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
+                          arm_none_elf_init_abi);
+}
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index a3e11c4b9b..0cf05efdd1 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -189,7 +189,7 @@ arm*-*-symbianelf*)
 	;;
 arm*-*-*)
 	# Target: ARM embedded system
-	gdb_target_obs="arm-pikeos-tdep.o"
+	gdb_target_obs="arm-pikeos-tdep.o arm-none-tdep.o"
 	gdb_sim=../sim/arm/libsim.a
 	;;

--
2.28.0.806.g8561365e88-goog


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-03 18:14       ` [PATCH v2] " Paul Mathieu
@ 2020-10-04 17:30         ` Paul Mathieu
  2020-10-04 23:41         ` Simon Marchi
  2020-10-05 12:58         ` Luis Machado
  2 siblings, 0 replies; 44+ messages in thread
From: Paul Mathieu @ 2020-10-04 17:30 UTC (permalink / raw)
  To: gdb-patches, Luis Machado, Simon Marchi, Robin Haberkorn

Cc'ing Robin, the original author of the patch, who can maybe help us
answer a couple questions.

> > Ok, so that answers the question in my other email, about which tool
> > produces this format of core.  The answer is this specific python
> > script, and the format is "Paul's ARM core format" :).
>
> It was some team work between what gdb understood "natively" and what
> the patch does.
> More specifically, this patch only seems to tell gdb to get the
> registers from the core file the way it would normally do, and then
> supply them almost verbatim to the regcache.
> The struct for these registers is nothing special either, you can find
> it in the kernel code as well:
> https://elixir.bootlin.com/linux/latest/source/arch/arm/include/uapi/asm/ptrace.h#L135
>
>
> > I don't know what Luis thinks about this, but I would be a bit hesitant
> > to add support in GDB for a core format that's not standard nor the
> > output of some "well-known" tool (which would be a de facto standard).
> >
> > Is there a format that already exists in the ARM ecosystem for core
> > dumps of bare-metal systems, we could base our stuff on?
>
> I'm not aware of anything that does that. There are tools that generate
> memory dumps and CPU register dumps into their own proprietary format,
> but nothing that I know that you could call a 'core file'.
>
> Are there other tools besides gdb that deal with core dumps? Maybe that
> could help me find other formats.
>
> That being said, the format used here is "well-known" in the sense that
> it's the exact same format the linux kernel would use to dump cores on
> arm-linux-*

Robin, what was the format you used for your core dumps? Did you use a
tool that's already out there?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-03 18:14       ` [PATCH v2] " Paul Mathieu
  2020-10-04 17:30         ` Paul Mathieu
@ 2020-10-04 23:41         ` Simon Marchi
  2020-10-06  4:32           ` Paul Mathieu
  2020-10-05 12:58         ` Luis Machado
  2 siblings, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-04 23:41 UTC (permalink / raw)
  To: Paul Mathieu, gdb-patches

On 2020-10-03 2:14 p.m., Paul Mathieu via Gdb-patches wrote:
> Thanks Simon for the super helpful feedback!
> A few answers inline, and hopefully a properly formatted patch at the
> bottom.
>
>> Note that I am unable to apply this patch either because long lines were
>> again wrapped by your email client.  Please use git-send-email.  You can
>> still reply in the thread by using --in-reply-to.
>
> Couldn't get git send-email to work with my SMTP, but I got some version
> of mutt to comply. Hopefully you can apply the patch this time.

Thanks, I am now able to apply the patch.  However, you would it to
export the patch using "git-format-patch", so that we get the full patch
including the commit message (since we review the commit messages to
make sure they include all the relevant information).

>> Ok, so that answers the question in my other email, about which tool
>> produces this format of core.  The answer is this specific python
>> script, and the format is "Paul's ARM core format" :).
>
> It was some team work between what gdb understood "natively" and what
> the patch does.
> More specifically, this patch only seems to tell gdb to get the
> registers from the core file the way it would normally do, and then
> supply them almost verbatim to the regcache.
> The struct for these registers is nothing special either, you can find
> it in the kernel code as well:
> https://elixir.bootlin.com/linux/latest/source/arch/arm/include/uapi/asm/ptrace.h#L135

Ok, so that is some useful information to include in the code comments
and / or in the commit log, that the register layout used for bare-metal
core dumps was chosen to be the same as that of the Linux kernel.

>> I don't know what Luis thinks about this, but I would be a bit hesitant
>> to add support in GDB for a core format that's not standard nor the
>> output of some "well-known" tool (which would be a de facto standard).
>>
>> Is there a format that already exists in the ARM ecosystem for core
>> dumps of bare-metal systems, we could base our stuff on?
>
> I'm not aware of anything that does that. There are tools that generate
> memory dumps and CPU register dumps into their own proprietary format,
> but nothing that I know that you could call a 'core file'.
>
> Are there other tools besides gdb that deal with core dumps? Maybe that
> could help me find other formats.

I don't know any, but I don't have much experience in embedded development.

> That being said, the format used here is "well-known" in the sense that
> it's the exact same format the linux kernel would use to dump cores on
> arm-linux-*

That sounds like a good choice to me.  As I said above, I think it
should be mentioned in some comments.

>> Alternatively, do you think you could implement GDB's generate-core-file
>> command for bare-metal ARM?  First, it would make sense for GDB to be
>> able to produce a core in some format and be able to ingest it again.
>> And it would act as some kind of documentation / reference
>> implementation of what GDB expects, and that could become some de facto
>> standard.  People who would like to have their core readable by GDB
>> would produce it in this format.
>
> I had never cloned the gdb source repo until 2 days ago. I have no idea
> how much work that would be, given that I know next to nothing about the
> gdb codebase.

>
> I do agree that this seems like a better way to do it, since there are
> already many integrations with a gdb server talking to a live bare-metal
> target over a physical debugger.
>
> As long as gdb already supports primitives to grab memory dumps and cpu
> registers of a remote target, I'd imagine generating a core file
> shouldn't be too much work. I'm quite sure most of that functionality
> already exists for extremely similar targets.

You can start at function "gcore_command" and pull on that thread.

It seems like you could implement the gdbarch_make_corefile_notes method
for the gdbarch for bare-metal ARM.  To see examples, look for uses of
set_gdbarch_make_corefile_notes.

> Having a sniffer for GDB_OSABI_NONE seems like the right way to do this.
>
> Since the format isn't specified (yet), I can still freely control it. I
> imagine I could add some sort of OSABI marker in the core file to mark
> it as an arm-none-eabi core.o
>
> My first guess was to set the EI_OSABI e_ident elf header field to
> ELFOSABI_NONE, but that happens to be the same enum value as
> ELFOSABI_SYSV, which is afaik broadly used by the linux kernel for core
> files. So it wouldn't be a good marker.
>
> Not sure what a better way would be to not abuse the ELF structure and
> produce reasonable ELF core dumps (since they already work so well with
> gdb).

I really don't know either.  Technically, it could be as simple as
adding a new section / note of your choice, but I don't know if that
would be an acceptable thing to do.

I sent comments on the second patch you sent (let's call it 1.1), but
not all of them seem to have been fixed, so here they are again.

> gdb/Changelog
> 2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
> 2020-10-02  Paul Mathieu <paulmathieu@google.com>
> 	* arm-none-tdep.c: Source file added. Provide CPU registers from a core
> 	file
> 	* floating point registers not yet supported (FIXME)
> ---
>  gdb/Makefile.in     |   2 +
>  gdb/arm-none-tdep.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
>  gdb/configure.tgt   |   2 +-
>  3 files changed, 109 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/arm-none-tdep.c
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index dbede7a9cf..7f0e3ea0b0 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
>  	arm-obsd-tdep.o \
>  	arm-pikeos-tdep.o \
>  	arm-symbian-tdep.o \
> +	arm-none-tdep.o \

Please keep the lists sorted.

Note that so far, everything related to bare-metal for an architecture
was simply in <arch>-tdep.c.  So perhaps it should just be there?  I'll
defer to Luis to decide what is the best organization for the ARM code.

>  	arm-tdep.o \
>  	arm-wince-tdep.o \
>  	avr-tdep.o \
> @@ -2150,6 +2151,7 @@ ALLDEPFILES = \
>  	arm-nbsd-tdep.c \
>  	arm-obsd-tdep.c \
>  	arm-symbian-tdep.c \
> +	arm-none-tdep.c \
>  	arm-tdep.c \
>  	avr-tdep.c \
>  	bfin-linux-tdep.c \
> diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
> new file mode 100644
> index 0000000000..21743c40a0
> --- /dev/null
> +++ b/gdb/arm-none-tdep.c
> @@ -0,0 +1,106 @@
> +/* Native-dependent code for GDB targetting bare-metal ARM.

Actually, this should say "Target-dependent", not "native-dependent".
"native" in GDB means things used when GDB runs directly on the target
platform.

> +
> +   Copyright (C) 2020 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 "osabi.h"
> +
> +#include "arch/arm.h"
> +#include "arm-tdep.h"
> +#include "gdbarch.h"
> +#include "regcache.h"
> +#include "regset.h"
> +
> +struct arm_none_reg
> +{
> +  uint32_t reg[13];
> +  uint32_t sp;
> +  uint32_t lr;
> +  uint32_t pc;
> +  uint32_t cpsr;
> +  uint32_t orig_r0;
> +};

So this is the layout of the registers found in the core?  I would add a
comment on top of it, saying just that.  And that there's no standard
layout for register layouts in ARM bare-metal cores, but it was chosen
to use the same layout as the Linux kernel uses.

> +
> +static void
> +arm_none_supply_gregset (const struct regset *regset,
> +                         struct regcache *regcache, int regnum,
> +                         const void *gregs, size_t len)
> +{
> +  const arm_none_reg *gregset = static_cast<const arm_none_reg *> (gregs);
> +  gdb_assert (len >= sizeof (arm_none_reg));
> +
> +  /* Integer registers.  */
> +  for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++)
> +    if (regnum == -1 || regnum == i)
> +      regcache->raw_supply (i, (char *)&gregset->reg[i]);

Space after the cast (apply everywhere).

> +static const struct regset arm_none_regset
> +    = { nullptr, arm_none_supply_gregset,
> +        /* We don't need a collect function because we only use this reading
> +           registers (via iterate_over_regset_sections and
> +           fetch_regs/fetch_register).  */
> +        nullptr, 0 };
> +
> +static void
> +arm_none_iterate_over_regset_sections (struct gdbarch *gdbarch,
> +                                       iterate_over_regset_sections_cb *cb,
> +                                       void *cb_data,
> +                                       const struct regcache *regcache)
> +{
> +  cb (".reg", sizeof (arm_none_reg), sizeof (arm_none_reg), &arm_none_regset,
> +      NULL, cb_data);
> +}
> +
> +static void arm_none_elf_init_abi (struct gdbarch_info info,
> +                                   struct gdbarch *gdbarch);

You don't need this declaration.

> +static void
> +arm_none_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  set_gdbarch_iterate_over_regset_sections (
> +      gdbarch, arm_none_iterate_over_regset_sections);
> +}
> +
> +void _initialize_arm_none_tdep (void);
> +void
> +_initialize_arm_none_tdep (void)

Remove the "void" in the lines above.

> +{
> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
> +                          arm_none_elf_init_abi);

As mentioned previously, this should be GDB_OSABI_NONE, not
GDB_OSABI_LINUX.  Like this, it GDB aborts on startup when configured
with --enable-targets=all.

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-03 18:14       ` [PATCH v2] " Paul Mathieu
  2020-10-04 17:30         ` Paul Mathieu
  2020-10-04 23:41         ` Simon Marchi
@ 2020-10-05 12:58         ` Luis Machado
  2020-10-05 13:24           ` Alan Hayward
  2 siblings, 1 reply; 44+ messages in thread
From: Luis Machado @ 2020-10-05 12:58 UTC (permalink / raw)
  To: Paul Mathieu, gdb-patches

Hi,

I'll go through the new patch soon. I think this was has already been 
said, but it would be nice to have the core format as close as the Linux 
Kernel one as possible, but then have a different ABI marker or some 
other identifiable tag so we can tell those apart.

If we are adding support for this, we should have in mind it should be 
flexible enough to accommodate other targets and future changes, not 
just a one-off.

On 10/3/20 3:14 PM, Paul Mathieu via Gdb-patches wrote:
> Thanks Simon for the super helpful feedback!
> A few answers inline, and hopefully a properly formatted patch at the
> bottom.
> 
>> Note that I am unable to apply this patch either because long lines were
>> again wrapped by your email client.  Please use git-send-email.  You can
>> still reply in the thread by using --in-reply-to.
> 
> Couldn't get git send-email to work with my SMTP, but I got some version
> of mutt to comply. Hopefully you can apply the patch this time.
> 
> 
>> Ok, so that answers the question in my other email, about which tool
>> produces this format of core.  The answer is this specific python
>> script, and the format is "Paul's ARM core format" :).
> 
> It was some team work between what gdb understood "natively" and what
> the patch does.
> More specifically, this patch only seems to tell gdb to get the
> registers from the core file the way it would normally do, and then
> supply them almost verbatim to the regcache.
> The struct for these registers is nothing special either, you can find
> it in the kernel code as well:
> https://elixir.bootlin.com/linux/latest/source/arch/arm/include/uapi/asm/ptrace.h#L135
> 
> 
>> I don't know what Luis thinks about this, but I would be a bit hesitant
>> to add support in GDB for a core format that's not standard nor the
>> output of some "well-known" tool (which would be a de facto standard).
>>
>> Is there a format that already exists in the ARM ecosystem for core
>> dumps of bare-metal systems, we could base our stuff on?
> 
> I'm not aware of anything that does that. There are tools that generate
> memory dumps and CPU register dumps into their own proprietary format,
> but nothing that I know that you could call a 'core file'.
> 
> Are there other tools besides gdb that deal with core dumps? Maybe that
> could help me find other formats.
> 
> That being said, the format used here is "well-known" in the sense that
> it's the exact same format the linux kernel would use to dump cores on
> arm-linux-*
> 
> 
>> Alternatively, do you think you could implement GDB's generate-core-file
>> command for bare-metal ARM?  First, it would make sense for GDB to be
>> able to produce a core in some format and be able to ingest it again.
>> And it would act as some kind of documentation / reference
>> implementation of what GDB expects, and that could become some de facto
>> standard.  People who would like to have their core readable by GDB
>> would produce it in this format.
> 
> I had never cloned the gdb source repo until 2 days ago. I have no idea
> how much work that would be, given that I know next to nothing about the
> gdb codebase.
> 
> I do agree that this seems like a better way to do it, since there are
> already many integrations with a gdb server talking to a live bare-metal
> target over a physical debugger.
> 
> As long as gdb already supports primitives to grab memory dumps and cpu
> registers of a remote target, I'd imagine generating a core file
> shouldn't be too much work. I'm quite sure most of that functionality
> already exists for extremely similar targets.
> 
> 
>>> +void _initialize_arm_none_tdep (void);
>>> +void
>>> +_initialize_arm_none_tdep (void)
>>> +{
>>> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
>>> +                          arm_none_elf_init_abi);
>>
>> I don't think this is what you want to do.  This "declares" that
>> combination of architecture ARM and osabi Linux exists, and that
>> arm_none_elf_init_abi should be called when such a combo is detected to
>> initialize the gdbarch object.
>>
>> However, such a combo is already registered somewhere else, as you can
>> guess:
>>
>>    https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/arm-linux-tdep.c;h=f60cb51763f1c5b14811494b646acf54b62d1f10;hb=HEAD#l2010
>>
>> So I presume that if you build a GDB configured with
>> --enable-targets=all (which will include arm-linux-tdep.c), you will hit
>> this assertion:
>>
>>    https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/osabi.c;h=e8a813b62f268417edf5f0ad164c9c942bd43b1e;hb=HEAD#l179
> 
> Ah, I was afraid of that. Thanks for the pointers.
> 
> 
>> What I think you want is to register an osabi sniffer, whose job is to
>> try to guess an osabi given a bfd.  Apparently that bfd can sometimes
>> represent the ELF core file, so you can look up sections and see if that
>> file looks like a core file you can handle.  And if so, return
>> GDB_OSABI_NONE.
>>
>> And then, you'd need to register (with gdbarch_register_osabi) osabi
>> GDB_OSABI_NONE with the arm architecture (same as you did, but with
>> GDB_OSABI_NONE instead of GDB_OSABI_LINUX) with the callback to call
>> when this combo is detected:
>>
>>    gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_NONE,
>>                            arm_none_elf_init_abi);
>>
>> I'm not sure about that part, but I think it's the right direction.
>>
>> Now, the question is how to recognize one of these cores?  As an
>> example, to recognize Cygwin core dumps (which use ELF files as a
>> container without a recognizable ABI tag, so it's kind of the same
>> situation as you), we use the .reg section size to determine if the BFD
>> is a Cygwin core:
>>
>>    https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/amd64-windows-tdep.c;h=e427c20538961bda2a1a0f033207eebce64c4729;hb=HEAD#l1387
>>
>> So, you can do that, although I think we all agree that it's not ideal.
>> If there was some way of identifying that the core is in the format we
>> recognize (say, if there was some special section with a tag), it would
>> make it much easier to identify it without having to guess.
> 
> Having a sniffer for GDB_OSABI_NONE seems like the right way to do this.
> 
> Since the format isn't specified (yet), I can still freely control it. I
> imagine I could add some sort of OSABI marker in the core file to mark
> it as an arm-none-eabi core.o
> 
> My first guess was to set the EI_OSABI e_ident elf header field to
> ELFOSABI_NONE, but that happens to be the same enum value as
> ELFOSABI_SYSV, which is afaik broadly used by the linux kernel for core
> files. So it wouldn't be a good marker.
> 
> Not sure what a better way would be to not abuse the ELF structure and
> produce reasonable ELF core dumps (since they already work so well with
> gdb).
> 
> 
> Paul
> 
> 
> 
> gdb/Changelog
> 2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
> 2020-10-02  Paul Mathieu <paulmathieu@google.com>
> 	* arm-none-tdep.c: Source file added. Provide CPU registers from a core
> 	file
> 	* floating point registers not yet supported (FIXME)
> ---
>   gdb/Makefile.in     |   2 +
>   gdb/arm-none-tdep.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
>   gdb/configure.tgt   |   2 +-
>   3 files changed, 109 insertions(+), 1 deletion(-)
>   create mode 100644 gdb/arm-none-tdep.c
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index dbede7a9cf..7f0e3ea0b0 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
>   	arm-obsd-tdep.o \
>   	arm-pikeos-tdep.o \
>   	arm-symbian-tdep.o \
> +	arm-none-tdep.o \
>   	arm-tdep.o \
>   	arm-wince-tdep.o \
>   	avr-tdep.o \
> @@ -2150,6 +2151,7 @@ ALLDEPFILES = \
>   	arm-nbsd-tdep.c \
>   	arm-obsd-tdep.c \
>   	arm-symbian-tdep.c \
> +	arm-none-tdep.c \
>   	arm-tdep.c \
>   	avr-tdep.c \
>   	bfin-linux-tdep.c \
> diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
> new file mode 100644
> index 0000000000..21743c40a0
> --- /dev/null
> +++ b/gdb/arm-none-tdep.c
> @@ -0,0 +1,106 @@
> +/* Native-dependent code for GDB targetting bare-metal ARM.
> +
> +   Copyright (C) 2020 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 "osabi.h"
> +
> +#include "arch/arm.h"
> +#include "arm-tdep.h"
> +#include "gdbarch.h"
> +#include "regcache.h"
> +#include "regset.h"
> +
> +struct arm_none_reg
> +{
> +  uint32_t reg[13];
> +  uint32_t sp;
> +  uint32_t lr;
> +  uint32_t pc;
> +  uint32_t cpsr;
> +  uint32_t orig_r0;
> +};
> +
> +static void
> +arm_none_supply_gregset (const struct regset *regset,
> +                         struct regcache *regcache, int regnum,
> +                         const void *gregs, size_t len)
> +{
> +  const arm_none_reg *gregset = static_cast<const arm_none_reg *> (gregs);
> +  gdb_assert (len >= sizeof (arm_none_reg));
> +
> +  /* Integer registers.  */
> +  for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++)
> +    if (regnum == -1 || regnum == i)
> +      regcache->raw_supply (i, (char *)&gregset->reg[i]);
> +
> +  if (regnum == -1 || regnum == ARM_SP_REGNUM)
> +    regcache->raw_supply (ARM_SP_REGNUM, (char *)&gregset->sp);
> +
> +  if (regnum == -1 || regnum == ARM_LR_REGNUM)
> +    regcache->raw_supply (ARM_LR_REGNUM, (char *)&gregset->lr);
> +
> +  if (regnum == -1 || regnum == ARM_PC_REGNUM)
> +    {
> +      CORE_ADDR r_pc
> +        = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc);
> +      regcache->raw_supply (ARM_PC_REGNUM, (char *)&r_pc);
> +    }
> +
> +  if (regnum == -1 || regnum == ARM_PS_REGNUM)
> +    {
> +      if (arm_apcs_32)
> +        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->cpsr);
> +      else
> +        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->pc);
> +    }
> +}
> +
> +static const struct regset arm_none_regset
> +    = { nullptr, arm_none_supply_gregset,
> +        /* We don't need a collect function because we only use this reading
> +           registers (via iterate_over_regset_sections and
> +           fetch_regs/fetch_register).  */
> +        nullptr, 0 };
> +
> +static void
> +arm_none_iterate_over_regset_sections (struct gdbarch *gdbarch,
> +                                       iterate_over_regset_sections_cb *cb,
> +                                       void *cb_data,
> +                                       const struct regcache *regcache)
> +{
> +  cb (".reg", sizeof (arm_none_reg), sizeof (arm_none_reg), &arm_none_regset,
> +      NULL, cb_data);
> +}
> +
> +static void arm_none_elf_init_abi (struct gdbarch_info info,
> +                                   struct gdbarch *gdbarch);
> +static void
> +arm_none_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  set_gdbarch_iterate_over_regset_sections (
> +      gdbarch, arm_none_iterate_over_regset_sections);
> +}
> +
> +void _initialize_arm_none_tdep (void);
> +void
> +_initialize_arm_none_tdep (void)
> +{
> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
> +                          arm_none_elf_init_abi);
> +}
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index a3e11c4b9b..0cf05efdd1 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -189,7 +189,7 @@ arm*-*-symbianelf*)
>   	;;
>   arm*-*-*)
>   	# Target: ARM embedded system
> -	gdb_target_obs="arm-pikeos-tdep.o"
> +	gdb_target_obs="arm-pikeos-tdep.o arm-none-tdep.o"
>   	gdb_sim=../sim/arm/libsim.a
>   	;;
> 
> --
> 2.28.0.806.g8561365e88-goog
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-05 12:58         ` Luis Machado
@ 2020-10-05 13:24           ` Alan Hayward
  0 siblings, 0 replies; 44+ messages in thread
From: Alan Hayward @ 2020-10-05 13:24 UTC (permalink / raw)
  To: Luis Machado; +Cc: Paul Mathieu, gdb-patches\@sourceware.org, nd



> On 5 Oct 2020, at 13:58, Luis Machado via Gdb-patches <gdb-patches@sourceware.org> wrote:
> 
> Hi,
> 
> I'll go through the new patch soon. I think this was has already been said, but it would be nice to have the core format as close as the Linux Kernel one as possible, but then have a different ABI marker or some other identifiable tag so we can tell those apart.
> 
> If we are adding support for this, we should have in mind it should be flexible enough to accommodate other targets and future changes, not just a one-off.
> 

Been going back and too on my opinion here.
Should we be defining a format ourselves here - And if we do, who else would
use it. But having something is better than nothing. And if it’s simple and
obvious, based on existing standards, then maybe that’s all that’s needed.

+1 for GDB being able to generate the files. That’s fairly crucial.

+1 for Documentation. Maybe even outside of GDB. But I’m not entirely sure
where that would be.

Where it makes sense, sharing code between arm-linux and arm-none would be
good too.


Alan.

> On 10/3/20 3:14 PM, Paul Mathieu via Gdb-patches wrote:
>> Thanks Simon for the super helpful feedback!
>> A few answers inline, and hopefully a properly formatted patch at the
>> bottom.
>>> Note that I am unable to apply this patch either because long lines were
>>> again wrapped by your email client.  Please use git-send-email.  You can
>>> still reply in the thread by using --in-reply-to.
>> Couldn't get git send-email to work with my SMTP, but I got some version
>> of mutt to comply. Hopefully you can apply the patch this time.
>>> Ok, so that answers the question in my other email, about which tool
>>> produces this format of core.  The answer is this specific python
>>> script, and the format is "Paul's ARM core format" :).
>> It was some team work between what gdb understood "natively" and what
>> the patch does.
>> More specifically, this patch only seems to tell gdb to get the
>> registers from the core file the way it would normally do, and then
>> supply them almost verbatim to the regcache.
>> The struct for these registers is nothing special either, you can find
>> it in the kernel code as well:
>> https://elixir.bootlin.com/linux/latest/source/arch/arm/include/uapi/asm/ptrace.h#L135
>>> I don't know what Luis thinks about this, but I would be a bit hesitant
>>> to add support in GDB for a core format that's not standard nor the
>>> output of some "well-known" tool (which would be a de facto standard).
>>> 
>>> Is there a format that already exists in the ARM ecosystem for core
>>> dumps of bare-metal systems, we could base our stuff on?
>> I'm not aware of anything that does that. There are tools that generate
>> memory dumps and CPU register dumps into their own proprietary format,
>> but nothing that I know that you could call a 'core file'.
>> Are there other tools besides gdb that deal with core dumps? Maybe that
>> could help me find other formats.
>> That being said, the format used here is "well-known" in the sense that
>> it's the exact same format the linux kernel would use to dump cores on
>> arm-linux-*
>>> Alternatively, do you think you could implement GDB's generate-core-file
>>> command for bare-metal ARM?  First, it would make sense for GDB to be
>>> able to produce a core in some format and be able to ingest it again.
>>> And it would act as some kind of documentation / reference
>>> implementation of what GDB expects, and that could become some de facto
>>> standard.  People who would like to have their core readable by GDB
>>> would produce it in this format.
>> I had never cloned the gdb source repo until 2 days ago. I have no idea
>> how much work that would be, given that I know next to nothing about the
>> gdb codebase.
>> I do agree that this seems like a better way to do it, since there are
>> already many integrations with a gdb server talking to a live bare-metal
>> target over a physical debugger.
>> As long as gdb already supports primitives to grab memory dumps and cpu
>> registers of a remote target, I'd imagine generating a core file
>> shouldn't be too much work. I'm quite sure most of that functionality
>> already exists for extremely similar targets.
>>>> +void _initialize_arm_none_tdep (void);
>>>> +void
>>>> +_initialize_arm_none_tdep (void)
>>>> +{
>>>> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
>>>> +                          arm_none_elf_init_abi);
>>> 
>>> I don't think this is what you want to do.  This "declares" that
>>> combination of architecture ARM and osabi Linux exists, and that
>>> arm_none_elf_init_abi should be called when such a combo is detected to
>>> initialize the gdbarch object.
>>> 
>>> However, such a combo is already registered somewhere else, as you can
>>> guess:
>>> 
>>>   https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/arm-linux-tdep.c;h=f60cb51763f1c5b14811494b646acf54b62d1f10;hb=HEAD#l2010
>>> 
>>> So I presume that if you build a GDB configured with
>>> --enable-targets=all (which will include arm-linux-tdep.c), you will hit
>>> this assertion:
>>> 
>>>   https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/osabi.c;h=e8a813b62f268417edf5f0ad164c9c942bd43b1e;hb=HEAD#l179
>> Ah, I was afraid of that. Thanks for the pointers.
>>> What I think you want is to register an osabi sniffer, whose job is to
>>> try to guess an osabi given a bfd.  Apparently that bfd can sometimes
>>> represent the ELF core file, so you can look up sections and see if that
>>> file looks like a core file you can handle.  And if so, return
>>> GDB_OSABI_NONE.
>>> 
>>> And then, you'd need to register (with gdbarch_register_osabi) osabi
>>> GDB_OSABI_NONE with the arm architecture (same as you did, but with
>>> GDB_OSABI_NONE instead of GDB_OSABI_LINUX) with the callback to call
>>> when this combo is detected:
>>> 
>>>   gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_NONE,
>>>                           arm_none_elf_init_abi);
>>> 
>>> I'm not sure about that part, but I think it's the right direction.
>>> 
>>> Now, the question is how to recognize one of these cores?  As an
>>> example, to recognize Cygwin core dumps (which use ELF files as a
>>> container without a recognizable ABI tag, so it's kind of the same
>>> situation as you), we use the .reg section size to determine if the BFD
>>> is a Cygwin core:
>>> 
>>>   https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/amd64-windows-tdep.c;h=e427c20538961bda2a1a0f033207eebce64c4729;hb=HEAD#l1387
>>> 
>>> So, you can do that, although I think we all agree that it's not ideal.
>>> If there was some way of identifying that the core is in the format we
>>> recognize (say, if there was some special section with a tag), it would
>>> make it much easier to identify it without having to guess.
>> Having a sniffer for GDB_OSABI_NONE seems like the right way to do this.
>> Since the format isn't specified (yet), I can still freely control it. I
>> imagine I could add some sort of OSABI marker in the core file to mark
>> it as an arm-none-eabi core.o
>> My first guess was to set the EI_OSABI e_ident elf header field to
>> ELFOSABI_NONE, but that happens to be the same enum value as
>> ELFOSABI_SYSV, which is afaik broadly used by the linux kernel for core
>> files. So it wouldn't be a good marker.
>> Not sure what a better way would be to not abuse the ELF structure and
>> produce reasonable ELF core dumps (since they already work so well with
>> gdb).
>> Paul
>> gdb/Changelog
>> 2018-09-29  Robin Haberkorn <robin.haberkorn@googlemail.com>
>> 2020-10-02  Paul Mathieu <paulmathieu@google.com>
>> 	* arm-none-tdep.c: Source file added. Provide CPU registers from a core
>> 	file
>> 	* floating point registers not yet supported (FIXME)
>> ---
>>  gdb/Makefile.in     |   2 +
>>  gdb/arm-none-tdep.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
>>  gdb/configure.tgt   |   2 +-
>>  3 files changed, 109 insertions(+), 1 deletion(-)
>>  create mode 100644 gdb/arm-none-tdep.c
>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index dbede7a9cf..7f0e3ea0b0 100644
>> --- a/gdb/Makefile.in
>> +++ b/gdb/Makefile.in
>> @@ -720,6 +720,7 @@ ALL_TARGET_OBS = \
>>  	arm-obsd-tdep.o \
>>  	arm-pikeos-tdep.o \
>>  	arm-symbian-tdep.o \
>> +	arm-none-tdep.o \
>>  	arm-tdep.o \
>>  	arm-wince-tdep.o \
>>  	avr-tdep.o \
>> @@ -2150,6 +2151,7 @@ ALLDEPFILES = \
>>  	arm-nbsd-tdep.c \
>>  	arm-obsd-tdep.c \
>>  	arm-symbian-tdep.c \
>> +	arm-none-tdep.c \
>>  	arm-tdep.c \
>>  	avr-tdep.c \
>>  	bfin-linux-tdep.c \
>> diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
>> new file mode 100644
>> index 0000000000..21743c40a0
>> --- /dev/null
>> +++ b/gdb/arm-none-tdep.c
>> @@ -0,0 +1,106 @@
>> +/* Native-dependent code for GDB targetting bare-metal ARM.
>> +
>> +   Copyright (C) 2020 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 "osabi.h"
>> +
>> +#include "arch/arm.h"
>> +#include "arm-tdep.h"
>> +#include "gdbarch.h"
>> +#include "regcache.h"
>> +#include "regset.h"
>> +
>> +struct arm_none_reg
>> +{
>> +  uint32_t reg[13];
>> +  uint32_t sp;
>> +  uint32_t lr;
>> +  uint32_t pc;
>> +  uint32_t cpsr;
>> +  uint32_t orig_r0;
>> +};
>> +
>> +static void
>> +arm_none_supply_gregset (const struct regset *regset,
>> +                         struct regcache *regcache, int regnum,
>> +                         const void *gregs, size_t len)
>> +{
>> +  const arm_none_reg *gregset = static_cast<const arm_none_reg *> (gregs);
>> +  gdb_assert (len >= sizeof (arm_none_reg));
>> +
>> +  /* Integer registers.  */
>> +  for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++)
>> +    if (regnum == -1 || regnum == i)
>> +      regcache->raw_supply (i, (char *)&gregset->reg[i]);
>> +
>> +  if (regnum == -1 || regnum == ARM_SP_REGNUM)
>> +    regcache->raw_supply (ARM_SP_REGNUM, (char *)&gregset->sp);
>> +
>> +  if (regnum == -1 || regnum == ARM_LR_REGNUM)
>> +    regcache->raw_supply (ARM_LR_REGNUM, (char *)&gregset->lr);
>> +
>> +  if (regnum == -1 || regnum == ARM_PC_REGNUM)
>> +    {
>> +      CORE_ADDR r_pc
>> +        = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc);
>> +      regcache->raw_supply (ARM_PC_REGNUM, (char *)&r_pc);
>> +    }
>> +
>> +  if (regnum == -1 || regnum == ARM_PS_REGNUM)
>> +    {
>> +      if (arm_apcs_32)
>> +        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->cpsr);
>> +      else
>> +        regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->pc);
>> +    }
>> +}
>> +
>> +static const struct regset arm_none_regset
>> +    = { nullptr, arm_none_supply_gregset,
>> +        /* We don't need a collect function because we only use this reading
>> +           registers (via iterate_over_regset_sections and
>> +           fetch_regs/fetch_register).  */
>> +        nullptr, 0 };
>> +
>> +static void
>> +arm_none_iterate_over_regset_sections (struct gdbarch *gdbarch,
>> +                                       iterate_over_regset_sections_cb *cb,
>> +                                       void *cb_data,
>> +                                       const struct regcache *regcache)
>> +{
>> +  cb (".reg", sizeof (arm_none_reg), sizeof (arm_none_reg), &arm_none_regset,
>> +      NULL, cb_data);
>> +}
>> +
>> +static void arm_none_elf_init_abi (struct gdbarch_info info,
>> +                                   struct gdbarch *gdbarch);
>> +static void
>> +arm_none_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> +{
>> +  set_gdbarch_iterate_over_regset_sections (
>> +      gdbarch, arm_none_iterate_over_regset_sections);
>> +}
>> +
>> +void _initialize_arm_none_tdep (void);
>> +void
>> +_initialize_arm_none_tdep (void)
>> +{
>> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX,
>> +                          arm_none_elf_init_abi);
>> +}
>> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
>> index a3e11c4b9b..0cf05efdd1 100644
>> --- a/gdb/configure.tgt
>> +++ b/gdb/configure.tgt
>> @@ -189,7 +189,7 @@ arm*-*-symbianelf*)
>>  	;;
>>  arm*-*-*)
>>  	# Target: ARM embedded system
>> -	gdb_target_obs="arm-pikeos-tdep.o"
>> +	gdb_target_obs="arm-pikeos-tdep.o arm-none-tdep.o"
>>  	gdb_sim=../sim/arm/libsim.a
>>  	;;
>> --
>> 2.28.0.806.g8561365e88-goog


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-04 23:41         ` Simon Marchi
@ 2020-10-06  4:32           ` Paul Mathieu
  2020-10-06 12:45             ` Luis Machado
  0 siblings, 1 reply; 44+ messages in thread
From: Paul Mathieu @ 2020-10-06  4:32 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

> > Having a sniffer for GDB_OSABI_NONE seems like the right way to do this.
> >
> > Since the format isn't specified (yet), I can still freely control it. I
> > imagine I could add some sort of OSABI marker in the core file to mark
> > it as an arm-none-eabi core.o
> >
> > My first guess was to set the EI_OSABI e_ident elf header field to
> > ELFOSABI_NONE, but that happens to be the same enum value as
> > ELFOSABI_SYSV, which is afaik broadly used by the linux kernel for core
> > files. So it wouldn't be a good marker.
> >
> > Not sure what a better way would be to not abuse the ELF structure and
> > produce reasonable ELF core dumps (since they already work so well with
> > gdb).
>
> I really don't know either.  Technically, it could be as simple as
> adding a new section / note of your choice, but I don't know if that
> would be an acceptable thing to do.

So, I implemented a basic sniffer, and it seems like the core file is
being recognized as GDB_OSABI_NONE, but gdb crashes while loading it.
The assert & stack trace are as follows:

gdbarch.c:3684: internal-error: void
gdbarch_iterate_over_regset_sections(gdbarch*, void (*)(const char*,
int, int, const regset*, const char*, void*), void*, const regcache*):
Assertion `gdbarch->iterate_over_regset_sections != NULL' failed.

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007fd93a9a4537 in __GI_abort () at abort.c:79
#2  0x000055e524ab29b7 in dump_core () at utils.c:204
#3  0x000055e524ab770d in internal_vproblem(internal_problem *, const
char *, int, const char *, typedef __va_list_tag __va_list_tag *) (
    problem=0x55e524f5a3a0 <internal_error_problem>, file=<optimized
out>, line=<optimized out>, fmt=<optimized out>, ap=<optimized out>)
    at utils.c:414
#4  0x000055e524ab78eb in internal_verror (file=<optimized out>,
line=<optimized out>, fmt=<optimized out>, ap=ap@entry=0x7fffb8c40628)
    at utils.c:439
#5  0x000055e524be45c2 in internal_error
(file=file@entry=0x55e524c7e484 "gdbarch.c", line=line@entry=3684,
fmt=<optimized out>)
    at errors.cc:55
#6  0x000055e5248d8c48 in gdbarch_iterate_over_regset_sections
(gdbarch=0x55e5261014f0,
    cb=0x55e5248354e0 <get_core_registers_cb(char const*, int, int,
regset const*, char const*, void*)>, cb_data=0x7fffb8c40730,
    regcache=0x0) at gdbarch.c:3687
#7  0x000055e524834f07 in core_target::fetch_registers
(regno=<optimized out>, regcache=0x55e5261625c0, this=<optimized out>)
    at corelow.c:735
#8  core_target::fetch_registers (this=<optimized out>,
regcache=0x55e5261625c0, regno=<optimized out>) at corelow.c:723
#9  0x000055e524a6ee8d in target_fetch_registers
(regcache=regcache@entry=0x55e5261625c0, regno=regno@entry=15) at
target.h:1334
#10 0x000055e5249df14a in regcache::raw_update (regnum=15,
this=0x55e5261625c0) at regcache.c:583
#11 regcache::raw_update (this=0x55e5261625c0, regnum=<optimized out>)
at regcache.c:572
#12 0x000055e5249df1ea in readable_regcache::raw_read
(this=0x55e5261625c0, regnum=15, buf=0x7fffb8c407c0 "\001") at
regcache.c:597
#13 0x000055e5249e4edf in readable_regcache::cooked_read<unsigned
long, void> (this=this@entry=0x55e5261625c0, regnum=15,
    val=val@entry=0x7fffb8c40818) at regcache.c:769
#14 0x000055e5249e1ce3 in regcache_cooked_read_unsigned
(val=0x7fffb8c40818, regnum=<optimized out>, regcache=0x55e5261625c0)
    at regcache.c:790
#15 regcache_read_pc (regcache=0x55e5261625c0) at regcache.c:1294
#16 0x000055e524907e2e in post_create_inferior
(target=target@entry=0x55e52615b480, from_tty=from_tty@entry=1) at
infcmd.c:303
#17 0x000055e5248360ff in core_target_open (arg=<error reading
variable: value has been optimized out>,
    from_tty=<error reading variable: value has been optimized out>)
at corelow.c:521
#18 0x000055e5249571e0 in catch_command_errors (command=<optimized
out>, arg=<optimized out>, from_tty=<optimized out>) at main.c:457
#19 0x000055e524958ecf in captured_main_1 (context=<optimized out>) at
main.c:1167
#20 0x000055e52495904b in captured_main (data=<optimized out>) at main.c:1268
#21 gdb_main (args=<optimized out>) at main.c:1268
#22 0x000055e524765f0c in main (argc=<optimized out>, argv=<optimized
out>) at gdb.c:32

Seems to me like current_inferior()->gdbarch is returning the
arm-linux gdbarch. Maybe this is because the main .elf executable
itself is recognized as arm-linux? I have virtually no control over
the executable image, so I would not be able to insert the same kind
of note section as I would for a core file to differentiate it from
arm-linux executables.

I'm not sure how to proceed forward, would you have any pointers?

Thanks!
Paul

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-06  4:32           ` Paul Mathieu
@ 2020-10-06 12:45             ` Luis Machado
  2020-10-06 14:29               ` Simon Marchi
  0 siblings, 1 reply; 44+ messages in thread
From: Luis Machado @ 2020-10-06 12:45 UTC (permalink / raw)
  To: Paul Mathieu, Simon Marchi; +Cc: gdb-patches

On 10/6/20 1:32 AM, Paul Mathieu via Gdb-patches wrote:
>>> Having a sniffer for GDB_OSABI_NONE seems like the right way to do this.
>>>
>>> Since the format isn't specified (yet), I can still freely control it. I
>>> imagine I could add some sort of OSABI marker in the core file to mark
>>> it as an arm-none-eabi core.o
>>>
>>> My first guess was to set the EI_OSABI e_ident elf header field to
>>> ELFOSABI_NONE, but that happens to be the same enum value as
>>> ELFOSABI_SYSV, which is afaik broadly used by the linux kernel for core
>>> files. So it wouldn't be a good marker.
>>>
>>> Not sure what a better way would be to not abuse the ELF structure and
>>> produce reasonable ELF core dumps (since they already work so well with
>>> gdb).
>>
>> I really don't know either.  Technically, it could be as simple as
>> adding a new section / note of your choice, but I don't know if that
>> would be an acceptable thing to do.
> 
> So, I implemented a basic sniffer, and it seems like the core file is
> being recognized as GDB_OSABI_NONE, but gdb crashes while loading it.
> The assert & stack trace are as follows:
> 
> gdbarch.c:3684: internal-error: void
> gdbarch_iterate_over_regset_sections(gdbarch*, void (*)(const char*,
> int, int, const regset*, const char*, void*), void*, const regcache*):
> Assertion `gdbarch->iterate_over_regset_sections != NULL' failed.
> 
> #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007fd93a9a4537 in __GI_abort () at abort.c:79
> #2  0x000055e524ab29b7 in dump_core () at utils.c:204
> #3  0x000055e524ab770d in internal_vproblem(internal_problem *, const
> char *, int, const char *, typedef __va_list_tag __va_list_tag *) (
>      problem=0x55e524f5a3a0 <internal_error_problem>, file=<optimized
> out>, line=<optimized out>, fmt=<optimized out>, ap=<optimized out>)
>      at utils.c:414
> #4  0x000055e524ab78eb in internal_verror (file=<optimized out>,
> line=<optimized out>, fmt=<optimized out>, ap=ap@entry=0x7fffb8c40628)
>      at utils.c:439
> #5  0x000055e524be45c2 in internal_error
> (file=file@entry=0x55e524c7e484 "gdbarch.c", line=line@entry=3684,
> fmt=<optimized out>)
>      at errors.cc:55
> #6  0x000055e5248d8c48 in gdbarch_iterate_over_regset_sections
> (gdbarch=0x55e5261014f0,
>      cb=0x55e5248354e0 <get_core_registers_cb(char const*, int, int,
> regset const*, char const*, void*)>, cb_data=0x7fffb8c40730,
>      regcache=0x0) at gdbarch.c:3687
> #7  0x000055e524834f07 in core_target::fetch_registers
> (regno=<optimized out>, regcache=0x55e5261625c0, this=<optimized out>)
>      at corelow.c:735
> #8  core_target::fetch_registers (this=<optimized out>,
> regcache=0x55e5261625c0, regno=<optimized out>) at corelow.c:723
> #9  0x000055e524a6ee8d in target_fetch_registers
> (regcache=regcache@entry=0x55e5261625c0, regno=regno@entry=15) at
> target.h:1334
> #10 0x000055e5249df14a in regcache::raw_update (regnum=15,
> this=0x55e5261625c0) at regcache.c:583
> #11 regcache::raw_update (this=0x55e5261625c0, regnum=<optimized out>)
> at regcache.c:572
> #12 0x000055e5249df1ea in readable_regcache::raw_read
> (this=0x55e5261625c0, regnum=15, buf=0x7fffb8c407c0 "\001") at
> regcache.c:597
> #13 0x000055e5249e4edf in readable_regcache::cooked_read<unsigned
> long, void> (this=this@entry=0x55e5261625c0, regnum=15,
>      val=val@entry=0x7fffb8c40818) at regcache.c:769
> #14 0x000055e5249e1ce3 in regcache_cooked_read_unsigned
> (val=0x7fffb8c40818, regnum=<optimized out>, regcache=0x55e5261625c0)
>      at regcache.c:790
> #15 regcache_read_pc (regcache=0x55e5261625c0) at regcache.c:1294
> #16 0x000055e524907e2e in post_create_inferior
> (target=target@entry=0x55e52615b480, from_tty=from_tty@entry=1) at
> infcmd.c:303
> #17 0x000055e5248360ff in core_target_open (arg=<error reading
> variable: value has been optimized out>,
>      from_tty=<error reading variable: value has been optimized out>)
> at corelow.c:521
> #18 0x000055e5249571e0 in catch_command_errors (command=<optimized
> out>, arg=<optimized out>, from_tty=<optimized out>) at main.c:457
> #19 0x000055e524958ecf in captured_main_1 (context=<optimized out>) at
> main.c:1167
> #20 0x000055e52495904b in captured_main (data=<optimized out>) at main.c:1268
> #21 gdb_main (args=<optimized out>) at main.c:1268
> #22 0x000055e524765f0c in main (argc=<optimized out>, argv=<optimized
> out>) at gdb.c:32
> 
> Seems to me like current_inferior()->gdbarch is returning the
> arm-linux gdbarch. Maybe this is because the main .elf executable
> itself is recognized as arm-linux? I have virtually no control over
> the executable image, so I would not be able to insert the same kind
> of note section as I would for a core file to differentiate it from
> arm-linux executables.

I think there may be enough hints here and there to be able to tell 
those apart via an heuristic. Isn't there a particular note section or 
flag that you can fetch and tell it is not a Linux executable but a 
bare-metal one?

> 
> I'm not sure how to proceed forward, would you have any pointers?

See the code in gdb/arm-tdep.c:arm_gdbarch_init, around the BFD flag 
checks and the ABI checks. Is that something usable?

> 
> Thanks!
> Paul
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-06 12:45             ` Luis Machado
@ 2020-10-06 14:29               ` Simon Marchi
  2020-10-06 16:59                 ` Paul Mathieu
  0 siblings, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-06 14:29 UTC (permalink / raw)
  To: Luis Machado, Paul Mathieu; +Cc: gdb-patches

On 2020-10-06 8:45 a.m., Luis Machado wrote:
> On 10/6/20 1:32 AM, Paul Mathieu via Gdb-patches wrote:
>> Seems to me like current_inferior()->gdbarch is returning the
>> arm-linux gdbarch. Maybe this is because the main .elf executable
>> itself is recognized as arm-linux? I have virtually no control over
>> the executable image, so I would not be able to insert the same kind
>> of note section as I would for a core file to differentiate it from
>> arm-linux executables.
>
> I think there may be enough hints here and there to be able to tell
> those apart via an heuristic. Isn't there a particular note section or
> flag that you can fetch and tell it is not a Linux executable but a
> bare-metal one?

Just so we are on the same page as to why this is happening, can you try
to step into function "gdbarch_info_fill" while you're executable is
loaded?

What I think is happening is that gdbarch_lookup_osabi returns
GDB_OSABI_UNKNOWN, because no sniffer identified it.

So the default osabi of your build gets used instead:

    /* From the configured default.  */
    #ifdef GDB_OSABI_DEFAULT
      if (info->osabi == GDB_OSABI_UNKNOWN)
        info->osabi = GDB_OSABI_DEFAULT;
    #endif

So indeed, if there was a sniffer identifying the binary as a "none"
osabi, it would probably work.  The problem is, how do you write such a
sniffer?  It's easy to check for a Linux binary, you check for the os
abi ELF note.  But how do you identify a "none" binary?  You would have
to check that's it's not a Linux binary, not FreeBSD binary, not a
Windows binary, etc.

I don't really like the idea of having a "default" osabi that we fall
back on, because it can just silently get it wrong, like in this case.
I think it would make more sense for the fall back to be "none".
Because it's hard to detect a "none" binary, as explained above.

In the mean time, for your testing, you can force the osabi to none by
using "set osabi none" prior to loading anything.

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-06 14:29               ` Simon Marchi
@ 2020-10-06 16:59                 ` Paul Mathieu
  2020-10-06 17:37                   ` Simon Marchi
  0 siblings, 1 reply; 44+ messages in thread
From: Paul Mathieu @ 2020-10-06 16:59 UTC (permalink / raw)
  To: Simon Marchi, Alan Hayward; +Cc: Luis Machado, gdb-patches

On Tue, Oct 6, 2020 at 7:29 AM Simon Marchi <simark@simark.ca> wrote:
> On 2020-10-06 8:45 a.m., Luis Machado wrote:
> > On 10/6/20 1:32 AM, Paul Mathieu via Gdb-patches wrote:
> >> Seems to me like current_inferior()->gdbarch is returning the
> >> arm-linux gdbarch. Maybe this is because the main .elf executable
> >> itself is recognized as arm-linux? I have virtually no control over
> >> the executable image, so I would not be able to insert the same kind
> >> of note section as I would for a core file to differentiate it from
> >> arm-linux executables.
> >
> > I think there may be enough hints here and there to be able to tell
> > those apart via an heuristic. Isn't there a particular note section or
> > flag that you can fetch and tell it is not a Linux executable but a
> > bare-metal one?

I did a quick comparison between two executables.
The first one is taken from the armhf debian distribution.
The second one is for my bare-metal cortex-m4 target.

➜  bin readelf -h screen
ELF Header:
  Magic:   7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00
  Class:                             ELF32
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              DYN (Shared object file)
  Machine:                           ARM
  Version:                           0x1
  Entry point address:               0x6271
  Start of program headers:          52 (bytes into file)
  Start of section headers:          310736 (bytes into file)
  Flags:                             0x5000400, Version5 EABI, hard-float ABI
  Size of this header:               52 (bytes)
  Size of program headers:           32 (bytes)
  Number of program headers:         9
  Size of section headers:           40 (bytes)
  Number of section headers:         28
  Section header string table index: 27

➜  bin readelf -h my_bare_metal_stuff.elf
ELF Header:
  Magic:   7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00
  Class:                             ELF32
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              EXEC (Executable file)
  Machine:                           ARM
  Version:                           0x1
  Entry point address:               0xc018011
  Start of program headers:          52 (bytes into file)
  Start of section headers:          49269716 (bytes into file)
  Flags:                             0x5000400, Version5 EABI, hard-float ABI
  Size of this header:               52 (bytes)
  Size of program headers:           32 (bytes)
  Number of program headers:         39
  Size of section headers:           40 (bytes)
  Number of section headers:         76
  Section header string table index: 75

The ELF header does not help us much, except that the linux executable
is marked as a DYN object, probably because it's dynamically
linked(?). I assume that statically linked executables will then use
the EXEC ELF type, which means I can't use that.
In particular, the OS/ABI section is identical. That's a quirk of the
enum, where SYSV and NONE have the same value. Yikes.

There are also some notes in the linux binary that look nice:

➜  bin readelf --notes screen

Displaying notes found in: .note.gnu.build-id
  Owner                Data size Description
  GNU                  0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: 76c649229b0402d6a0d1f65c3925e85fcb18e79d

Displaying notes found in: .note.ABI-tag
  Owner                Data size Description
  GNU                  0x00000010 NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 3.2.0

My own bare-metal executable contains no notes at all.
Again, here, I'm not sure how widespread these notes are, and if linux
really requires them to run the binary or not.
As the note name suggests, it looks like a GNU extension, which
implies that it's not mandatory?

I found one more thing, trying to find differences between the 2 files.

➜  bin readelf --arch-specific screen
Attribute Section: aeabi
File Attributes
  Tag_CPU_name: "7-A"
  Tag_CPU_arch: v7
  Tag_CPU_arch_profile: Application
  Tag_ARM_ISA_use: Yes
  Tag_THUMB_ISA_use: Thumb-2
  Tag_FP_arch: VFPv3-D16
  Tag_ABI_PCS_wchar_t: 4
  Tag_ABI_FP_rounding: Needed
  Tag_ABI_FP_denormal: Needed
  Tag_ABI_FP_exceptions: Needed
  Tag_ABI_FP_number_model: IEEE 754
  Tag_ABI_align_needed: 8-byte
  Tag_ABI_align_preserved: 8-byte, except leaf SP
  Tag_ABI_enum_size: int
  Tag_ABI_VFP_args: VFP registers
  Tag_CPU_unaligned_access: v6

➜  bin readelf --arch-specific my_bare_metal_stuff.elf
Attribute Section: aeabi
File Attributes
  Tag_CPU_name: "8-M.MAIN"
  Tag_CPU_arch: v8-M.mainline
  Tag_CPU_arch_profile: Microcontroller
  Tag_THUMB_ISA_use: Yes
  Tag_FP_arch: FPv5/FP-D16 for ARMv8
  Tag_ABI_PCS_wchar_t: 4
  Tag_ABI_FP_denormal: Needed
  Tag_ABI_FP_exceptions: Needed
  Tag_ABI_FP_number_model: IEEE 754
  Tag_ABI_align_needed: 8-byte
  Tag_ABI_enum_size: small
  Tag_ABI_VFP_args: VFP registers
  Tag_CPU_unaligned_access: v6
  Tag_DSP_extension: Allowed

The Tag_CPU_arch_profile value is different, and looks like I could use that.
Alan, would you say this tag is meaningful & reliable?


> Just so we are on the same page as to why this is happening, can you try
> to step into function "gdbarch_info_fill" while you're executable is
> loaded?
>
> What I think is happening is that gdbarch_lookup_osabi returns
> GDB_OSABI_UNKNOWN, because no sniffer identified it.
>
> So the default osabi of your build gets used instead:
>
>     /* From the configured default.  */
>     #ifdef GDB_OSABI_DEFAULT
>       if (info->osabi == GDB_OSABI_UNKNOWN)
>         info->osabi = GDB_OSABI_DEFAULT;
>     #endif

That looks right. `gdbarch_lookup_osabi` indeed returns GDB_OSABI_UNKNOWN.
Here's what I get in gdb after loading the main file:

(gdb) show osabi
The current OS ABI is "auto" (currently "GNU/Linux").
The default OS ABI is "GNU/Linux".



> So indeed, if there was a sniffer identifying the binary as a "none"
> osabi, it would probably work.  The problem is, how do you write such a
> sniffer?  It's easy to check for a Linux binary, you check for the os
> abi ELF note.

As mentioned above, I looked into ELF file differences between
arm-linux and arm-none and couldn't find anything of substance. Am I
missing something?

> But how do you identify a "none" binary?  You would have
> to check that's it's not a Linux binary, not FreeBSD binary, not a
> Windows binary, etc.

Agreed. There seems to be a sniffer for PikeOS, which looks for a
specific symbol in the symbol table. There is a comment in there
saying that the BFD target is otherwise just standard ELF. I'm afraid
that if I was to use a trick, such as the Tag_CPU_arch_profile
mentioned above, I would prevent some other targets from working
properly.

> I don't really like the idea of having a "default" osabi that we fall
> back on, because it can just silently get it wrong, like in this case.
> I think it would make more sense for the fall back to be "none".
> Because it's hard to detect a "none" binary, as explained above.

I looked into configure.tgt, and it looks like default osabi is only
set for targets that have -linux in them. I'm building gdb with
`./configure --enable-targets=arm-none-eabi`, should that help? In my
case, the resulting gdb program still crashes with the same message.
Interestingly, it also prints out a warning:

warning: A handler for the OS ABI "GNU/Linux" is not built into this
configuration
of GDB.  Attempting to continue with the default armv8-m.main settings.

I think in this case, the fact that it uses linux as a default osabi
could be a bug?

In the general case, a multiarch gdb would require `set osabi none` to
work, but that's already better than nothing, IMO.

> In the mean time, for your testing, you can force the osabi to none by
> using "set osabi none" prior to loading anything.

Yes, that works fine. Thanks!
I'll send a revised patch that uses GDB_OSABI_NONE shortly.

Super thanks for the great feedback!
Paul

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-06 16:59                 ` Paul Mathieu
@ 2020-10-06 17:37                   ` Simon Marchi
  0 siblings, 0 replies; 44+ messages in thread
From: Simon Marchi @ 2020-10-06 17:37 UTC (permalink / raw)
  To: Paul Mathieu, Alan Hayward; +Cc: gdb-patches

On 2020-10-06 12:59 p.m., Paul Mathieu via Gdb-patches wrote:
> There are also some notes in the linux binary that look nice:
>
> ➜  bin readelf --notes screen
>
> Displaying notes found in: .note.gnu.build-id
>   Owner                Data size Description
>   GNU                  0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring)
>     Build ID: 76c649229b0402d6a0d1f65c3925e85fcb18e79d
>
> Displaying notes found in: .note.ABI-tag
>   Owner                Data size Description
>   GNU                  0x00000010 NT_GNU_ABI_TAG (ABI version tag)
>     OS: Linux, ABI: 3.2.0
>
> My own bare-metal executable contains no notes at all.
> Again, here, I'm not sure how widespread these notes are, and if linux
> really requires them to run the binary or not.
> As the note name suggests, it looks like a GNU extension, which
> implies that it's not mandatory?

I thought that the ABI version tag was ubiquitous enough, because of:

  https://refspecs.linuxfoundation.org/LSB_1.2.0/gLSB/noteabitag.html

But I just checked in an Alpine system (which uses musl as the libc,
busybox for the core utils, and pretty much no GNU stuff):

    $ docker run -it alpine
    / # apk add binutils
    fetch http://dl-cdn.alpinelinux.org/alpine/v3.12/main/x86_64/APKINDEX.tar.gz
    fetch http://dl-cdn.alpinelinux.org/alpine/v3.12/community/x86_64/APKINDEX.tar.gz
    (1/3) Installing libgcc (9.3.0-r2)
    (2/3) Installing libstdc++ (9.3.0-r2)
    (3/3) Installing binutils (2.34-r1)
    Executing busybox-1.31.1-r16.trigger
    OK: 17 MiB in 17 packages
    / # readelf -n /bin/busybox
    / #

So the question is not arm-specific.  Here, how would we differentiate
this busybox executable from a bare-metal x86-64 executable, and know
that the busybox one is made to run on Linux and not the other one.

> I found one more thing, trying to find differences between the 2 files.
>
> ➜  bin readelf --arch-specific screen
> Attribute Section: aeabi
> File Attributes
>   Tag_CPU_name: "7-A"
>   Tag_CPU_arch: v7
>   Tag_CPU_arch_profile: Application
>   Tag_ARM_ISA_use: Yes
>   Tag_THUMB_ISA_use: Thumb-2
>   Tag_FP_arch: VFPv3-D16
>   Tag_ABI_PCS_wchar_t: 4
>   Tag_ABI_FP_rounding: Needed
>   Tag_ABI_FP_denormal: Needed
>   Tag_ABI_FP_exceptions: Needed
>   Tag_ABI_FP_number_model: IEEE 754
>   Tag_ABI_align_needed: 8-byte
>   Tag_ABI_align_preserved: 8-byte, except leaf SP
>   Tag_ABI_enum_size: int
>   Tag_ABI_VFP_args: VFP registers
>   Tag_CPU_unaligned_access: v6
>
> ➜  bin readelf --arch-specific my_bare_metal_stuff.elf
> Attribute Section: aeabi
> File Attributes
>   Tag_CPU_name: "8-M.MAIN"
>   Tag_CPU_arch: v8-M.mainline
>   Tag_CPU_arch_profile: Microcontroller
>   Tag_THUMB_ISA_use: Yes
>   Tag_FP_arch: FPv5/FP-D16 for ARMv8
>   Tag_ABI_PCS_wchar_t: 4
>   Tag_ABI_FP_denormal: Needed
>   Tag_ABI_FP_exceptions: Needed
>   Tag_ABI_FP_number_model: IEEE 754
>   Tag_ABI_align_needed: 8-byte
>   Tag_ABI_enum_size: small
>   Tag_ABI_VFP_args: VFP registers
>   Tag_CPU_unaligned_access: v6
>   Tag_DSP_extension: Allowed
>
> The Tag_CPU_arch_profile value is different, and looks like I could use that.
> Alan, would you say this tag is meaningful & reliable?

From what I read, this is just the "ARM profile", meaning the letter in
the ARM processor model number.  Cortex M0 == M (Microcontroller).
Cortex A53 -> A (Application).  So not very reliable, it doesn't
indicate whether or not you're running an OS.

>> Just so we are on the same page as to why this is happening, can you try
>> to step into function "gdbarch_info_fill" while you're executable is
>> loaded?
>>
>> What I think is happening is that gdbarch_lookup_osabi returns
>> GDB_OSABI_UNKNOWN, because no sniffer identified it.
>>
>> So the default osabi of your build gets used instead:
>>
>>     /* From the configured default.  */
>>     #ifdef GDB_OSABI_DEFAULT
>>       if (info->osabi == GDB_OSABI_UNKNOWN)
>>         info->osabi = GDB_OSABI_DEFAULT;
>>     #endif
>
> That looks right. `gdbarch_lookup_osabi` indeed returns GDB_OSABI_UNKNOWN.
> Here's what I get in gdb after loading the main file:
>
> (gdb) show osabi
> The current OS ABI is "auto" (currently "GNU/Linux").
> The default OS ABI is "GNU/Linux".

Thanks.

>> So indeed, if there was a sniffer identifying the binary as a "none"
>> osabi, it would probably work.  The problem is, how do you write such a
>> sniffer?  It's easy to check for a Linux binary, you check for the os
>> abi ELF note.
>
> As mentioned above, I looked into ELF file differences between
> arm-linux and arm-none and couldn't find anything of substance. Am I
> missing something?

I don't think so.  Well, the closest thing is .note.ABI-tag, but since
that it's not _always_ there... note sure we can rely on it.

>> But how do you identify a "none" binary?  You would have
>> to check that's it's not a Linux binary, not FreeBSD binary, not a
>> Windows binary, etc.
>
> Agreed. There seems to be a sniffer for PikeOS, which looks for a
> specific symbol in the symbol table. There is a comment in there
> saying that the BFD target is otherwise just standard ELF. I'm afraid
> that if I was to use a trick, such as the Tag_CPU_arch_profile
> mentioned above, I would prevent some other targets from working
> properly.

Indeed.  I don't think sniffers have a concept of priority at the
moment, but we could think of adding that.  Since the PikeOS sniffer is
more specific, it would have a higher priority than yours, so would run
first.  So you would catch whatever was not identified as PikeOS.

>> I don't really like the idea of having a "default" osabi that we fall
>> back on, because it can just silently get it wrong, like in this case.
>> I think it would make more sense for the fall back to be "none".
>> Because it's hard to detect a "none" binary, as explained above.
>
> I looked into configure.tgt, and it looks like default osabi is only
> set for targets that have -linux in them. I'm building gdb with
> `./configure --enable-targets=arm-none-eabi`, should that help? In my
> case, the resulting gdb program still crashes with the same message.
> Interestingly, it also prints out a warning:
>
> warning: A handler for the OS ABI "GNU/Linux" is not built into this
> configuration
> of GDB.  Attempting to continue with the default armv8-m.main settings.
>
> I think in this case, the fact that it uses linux as a default osabi
> could be a bug?

Not sure.  When you use --enable-targets=arm-none-eabi, I think that it
implicitly still uses --target=x86_64-pc-linux-gnu or something
(assuming you work on x86-64 GNU/Linux).  --enable-targets is used to
add more targets than the default one provided by --target.

You end up with this message because:

1. You built GDB with arm/none and x86-64/linux.  You did not include
   arm/linux.
2. The "default osabi", as set in configure.tgt, is Linux, because your
   --target is something-something-linux.
3. The architecture of your executable is ARM, the osabi GDB tries to
   use is "Linux" (because it's the fall back), and it proceeds to
   complain that you don't have support for arm/linux.

So, not really a bug, it's kind of expected due to your configuration
and how the default fall back osabi is decided.

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-25 21:06                         ` Fredrik Hederstierna
  2020-10-26 11:24                           ` Luis Machado
@ 2021-06-22  2:16                           ` Mike Frysinger
  1 sibling, 0 replies; 44+ messages in thread
From: Mike Frysinger @ 2021-06-22  2:16 UTC (permalink / raw)
  To: Fredrik Hederstierna; +Cc: Simon Marchi, Paul Mathieu, gdb-patches

On 25 Oct 2020 21:06, Fredrik Hederstierna via Gdb-patches wrote:
> From: Simon Marchi <simark@simark.ca>
> > Luis and Alan (both GDB ARM maintainers) also expressed the desire to
> > have the format documented.  Alan suggested a section in the GDB manual,
> > in the ARM-specific section.  I think it is a good idea.  This is
> > important, so we have something to point to when people ask "what format
> > should I generate so GDB can read it".
> 
> Yes, it was suggested by someone to adding an arm-none section in "G.5.3 ARM Features".
> How to proceed on this? I guess the current design is same as *ix minus all specific *ix parts that could be stripped out from documentation.

good use for the wiki ?
-mike

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2021-01-18 11:09                                     ` Andrew Burgess
@ 2021-01-18 14:01                                       ` Luis Machado
  0 siblings, 0 replies; 44+ messages in thread
From: Luis Machado @ 2021-01-18 14:01 UTC (permalink / raw)
  To: Andrew Burgess
  Cc: Fredrik Hederstierna, Paul Mathieu, Simon Marchi, gdb-patches

Hi Andrew,

On 1/18/21 8:09 AM, Andrew Burgess wrote:
> * Luis Machado via Gdb-patches <gdb-patches@sourceware.org> [2021-01-14 09:50:42 -0300]:
> 
>> Hi,
>>
>> On 1/14/21 9:36 AM, Fredrik Hederstierna wrote:
>>> Ping, do anyone have any more input how to proceed on this?
>>> I think I have made what I can do, to the limits of my knowledge and understanding.
>>>
>>> I read recently about RISCV now seems to have merged corefile support for their arch?
>>
>> The idea was to establish a common ground that could be used for both RISCV
>> and ARM, and prevent design issues from getting in the way.
>>
>> If RISCV bare metal corefile was merged, does that mean we can reuse the
>> same strategy to pursue bare metal ARM core file support now? And does that
>> mean we just need to come up with a patch using the same design?
> 
> We covered most of this ground in the discussion on the RISC-V patch,
> but if we assume that the plan is ELF + NOTES then most of the
> "design" is actually dictated by GDB and BFD already.  All we actually
> end up providing on the GDB side is glue to funnel the GDB state over
> to BFD.
> 
> Most of the work is in laying out the registers, something which is (I
> hope we can agree) something that is target specific.
> 
> My goal for this week is to get an updated revision of the RISC-V bare
> metal patches on to the list, with as much code as possible moved into
> a common file.  Honestly, I don't think there's going to be a huge
> amount of reuse, but I'll do as much as I can.

I'm sure you will. I can write some documentation based on my 
understanding so far. Maybe that will be a good start and then folks can 
add to it.


> 
> The ARM patch that was proposed was already ELF + NOTES, and already
> had some code in a common file.  So I'll probably borrow some of that.
> 
>>
>>> why can RISCV corefile be merged but not this? I guess ARM-Cortex users is magnitude amount higher and the benefit of this feature is huge.
>>> And it would be really good if synergy could be used to share code, since alot functions I guess are same.
>>> If documentation is the issue, do we have an issue ticket on that?
>>
>>  From my end, the acceptance of ARM bare metal core files was dependent on
>> having a documented design of what the bare metal core files should look
>> like. Was that documentation included in the RISCV work? If not, that wasn't
>> the agreement when I send comments to that patch series.
> 
> The RISC-V work has not been merged, and writing some documentation is
> also on my goal list for this week.

That's what I noticed when I went looking for the commits. I think there 
was a misunderstanding.

> 
> I take all feedback I get on this list seriously, and though mistakes
> can happen, I'll not going to merge a patch without fixing the review
> issues, at least not without having a good discussion with the
> reviewer first.

No doubt about that, really. I think there was some expectation that 
merging changes into GDB was quicker than it really is in practice.

> 
> Thanks,
> Andrew
> 
>>
>>>
>>> Can we just merge this patch-v4 and set target GDB-11, and solve the doc-issue-ticket, then we just force ourselves to solve docs before the release,
>>> or how can we 'make it happen'? It seems to be about to fail again if time just goes and none try push it further forward.
>>
>> I'd love to have such support, but I'm not actively working on it. I can
>> commit to review the changes and not let it be forgotten, but the
>> development work must be pursued by someone else.
> 
> 
> 
>>
>>>
>>> Thanks! Kindly,
>>> Fredrik
>>>
>>> From: Paul Mathieu <paulmathieu@google.com>
>>> Sent: Tuesday, October 27, 2020 5:53 PM
>>> To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>
>>> Cc: Luis Machado <luis.machado@linaro.org>; Simon Marchi <simark@simark.ca>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
>>> Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
>>> Hi Fredrik,
>>>
>>>> This is the current format when trying from ARM simulator:
>>>>
>>>> fredrik@legion ~/src/armv4t_coretest$ readelf -aA test.core
>>>> ELF Header:
>>>>      Magic:   7f 45 4c 46 01 01 01 61 00 00 00 00 00 00 00 00
>>>>      Class:                             ELF32
>>>>      Data:                              2's complement, little endian
>>>>      Version:                           1 (current)
>>>>      OS/ABI:                            ARM
>>>>      ABI Version:                       0
>>>>      Type:                              CORE (Core file)
>>>>      Machine:                           ARM
>>>>      Version:                           0x1
>>>>      Entry point address:               0x0
>>>>      Start of program headers:          52 (bytes into file)
>>>>      Start of section headers:          8084 (bytes into file)
>>>>      Flags:                             0x0
>>>>      Size of this header:               52 (bytes)
>>>>      Size of program headers:           32 (bytes)
>>>>      Number of program headers:         5
>>>>      Size of section headers:           40 (bytes)
>>>>      Number of section headers:         7
>>>>      Section header string table index: 6
>>>>
>>>> Section Headers:
>>>>      [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>>>>      [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>>>>      [ 1] note0             NOTE            00000000 001e44 000138 00   A  0   0  1
>>>>      [ 2] load              PROGBITS        00010000 0000d4 000100 00  AX  0   0  1
>>>>      [ 3] load              PROGBITS        00080000 0001d4 000000 00  WA  0   0  1
>>>>      [ 4] load              PROGBITS        00080000 0001d4 000400 00  WA  0   0  1
>>>>      [ 5] load              PROGBITS        000fe790 0005d4 001870 00  WA  0   0  1
>>>>      [ 6] .shstrtab         STRTAB          00000000 001f7c 000016 00      0   0  1
>>>> Key to Flags:
>>>>      W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
>>>>      L (link order), O (extra OS processing required), G (group), T (TLS),
>>>>      C (compressed), x (unknown), o (OS specific), E (exclude),
>>>>      y (purecode), p (processor specific)
>>>>
>>>> There are no section groups in this file.
>>>>
>>>> Program Headers:
>>>>      Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>>>>      NOTE           0x001e44 0x00000000 0x00000000 0x00138 0x00000 R   0x1
>>>>      LOAD           0x0000d4 0x00010000 0x00000000 0x00100 0x00100 R E 0x1
>>>>      LOAD           0x0001d4 0x00080000 0x00000000 0x00000 0x00000 RW  0x1
>>>>      LOAD           0x0001d4 0x00080000 0x00000000 0x00400 0x00400 RW  0x1
>>>>      LOAD           0x0005d4 0x000fe790 0x00000000 0x01870 0x01870 RW  0x1
>>>>
>>>>     Section to Segment mapping:
>>>>      Segment Sections...
>>>>       00
>>>>       01     load
>>>>       02     load
>>>>       03     load load
>>>>       04     load
>>>>
>>>> There is no dynamic section in this file.
>>>> There are no relocations in this file.
>>>> There are no unwind sections in this file.
>>>> No version information found in this file.
>>>>
>>>> Displaying notes found at file offset 0x00001e44 with length 0x00000138:
>>>>      Owner                 Data size       Description
>>>>      CORE                 0x0000007c       NT_PRPSINFO (prpsinfo structure)
>>>>      CORE                 0x00000094       NT_PRSTATUS (prstatus structure)
>>>
>>> Does this support `.reg/xxx` notes for RTOS that support multiple tasks?
>>> It would be really nice to have `info threads` "just work"
>>>
>>> Paul
>>>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2021-01-14 12:50                                   ` Luis Machado
@ 2021-01-18 11:09                                     ` Andrew Burgess
  2021-01-18 14:01                                       ` Luis Machado
  0 siblings, 1 reply; 44+ messages in thread
From: Andrew Burgess @ 2021-01-18 11:09 UTC (permalink / raw)
  To: Luis Machado
  Cc: Fredrik Hederstierna, Paul Mathieu, Simon Marchi, gdb-patches

* Luis Machado via Gdb-patches <gdb-patches@sourceware.org> [2021-01-14 09:50:42 -0300]:

> Hi,
> 
> On 1/14/21 9:36 AM, Fredrik Hederstierna wrote:
> > Ping, do anyone have any more input how to proceed on this?
> > I think I have made what I can do, to the limits of my knowledge and understanding.
> > 
> > I read recently about RISCV now seems to have merged corefile support for their arch?
> 
> The idea was to establish a common ground that could be used for both RISCV
> and ARM, and prevent design issues from getting in the way.
> 
> If RISCV bare metal corefile was merged, does that mean we can reuse the
> same strategy to pursue bare metal ARM core file support now? And does that
> mean we just need to come up with a patch using the same design?

We covered most of this ground in the discussion on the RISC-V patch,
but if we assume that the plan is ELF + NOTES then most of the
"design" is actually dictated by GDB and BFD already.  All we actually
end up providing on the GDB side is glue to funnel the GDB state over
to BFD.

Most of the work is in laying out the registers, something which is (I
hope we can agree) something that is target specific.

My goal for this week is to get an updated revision of the RISC-V bare
metal patches on to the list, with as much code as possible moved into
a common file.  Honestly, I don't think there's going to be a huge
amount of reuse, but I'll do as much as I can.

The ARM patch that was proposed was already ELF + NOTES, and already
had some code in a common file.  So I'll probably borrow some of that.

> 
> > why can RISCV corefile be merged but not this? I guess ARM-Cortex users is magnitude amount higher and the benefit of this feature is huge.
> > And it would be really good if synergy could be used to share code, since alot functions I guess are same.
> > If documentation is the issue, do we have an issue ticket on that?
> 
> From my end, the acceptance of ARM bare metal core files was dependent on
> having a documented design of what the bare metal core files should look
> like. Was that documentation included in the RISCV work? If not, that wasn't
> the agreement when I send comments to that patch series.

The RISC-V work has not been merged, and writing some documentation is
also on my goal list for this week.

I take all feedback I get on this list seriously, and though mistakes
can happen, I'll not going to merge a patch without fixing the review
issues, at least not without having a good discussion with the
reviewer first.

Thanks,
Andrew

> 
> > 
> > Can we just merge this patch-v4 and set target GDB-11, and solve the doc-issue-ticket, then we just force ourselves to solve docs before the release,
> > or how can we 'make it happen'? It seems to be about to fail again if time just goes and none try push it further forward.
> 
> I'd love to have such support, but I'm not actively working on it. I can
> commit to review the changes and not let it be forgotten, but the
> development work must be pursued by someone else.



> 
> > 
> > Thanks! Kindly,
> > Fredrik
> > 
> > From: Paul Mathieu <paulmathieu@google.com>
> > Sent: Tuesday, October 27, 2020 5:53 PM
> > To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>
> > Cc: Luis Machado <luis.machado@linaro.org>; Simon Marchi <simark@simark.ca>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
> > Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
> > Hi Fredrik,
> > 
> > > This is the current format when trying from ARM simulator:
> > > 
> > > fredrik@legion ~/src/armv4t_coretest$ readelf -aA test.core
> > > ELF Header:
> > >     Magic:   7f 45 4c 46 01 01 01 61 00 00 00 00 00 00 00 00
> > >     Class:                             ELF32
> > >     Data:                              2's complement, little endian
> > >     Version:                           1 (current)
> > >     OS/ABI:                            ARM
> > >     ABI Version:                       0
> > >     Type:                              CORE (Core file)
> > >     Machine:                           ARM
> > >     Version:                           0x1
> > >     Entry point address:               0x0
> > >     Start of program headers:          52 (bytes into file)
> > >     Start of section headers:          8084 (bytes into file)
> > >     Flags:                             0x0
> > >     Size of this header:               52 (bytes)
> > >     Size of program headers:           32 (bytes)
> > >     Number of program headers:         5
> > >     Size of section headers:           40 (bytes)
> > >     Number of section headers:         7
> > >     Section header string table index: 6
> > > 
> > > Section Headers:
> > >     [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> > >     [ 0]                   NULL            00000000 000000 000000 00      0   0  0
> > >     [ 1] note0             NOTE            00000000 001e44 000138 00   A  0   0  1
> > >     [ 2] load              PROGBITS        00010000 0000d4 000100 00  AX  0   0  1
> > >     [ 3] load              PROGBITS        00080000 0001d4 000000 00  WA  0   0  1
> > >     [ 4] load              PROGBITS        00080000 0001d4 000400 00  WA  0   0  1
> > >     [ 5] load              PROGBITS        000fe790 0005d4 001870 00  WA  0   0  1
> > >     [ 6] .shstrtab         STRTAB          00000000 001f7c 000016 00      0   0  1
> > > Key to Flags:
> > >     W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
> > >     L (link order), O (extra OS processing required), G (group), T (TLS),
> > >     C (compressed), x (unknown), o (OS specific), E (exclude),
> > >     y (purecode), p (processor specific)
> > > 
> > > There are no section groups in this file.
> > > 
> > > Program Headers:
> > >     Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
> > >     NOTE           0x001e44 0x00000000 0x00000000 0x00138 0x00000 R   0x1
> > >     LOAD           0x0000d4 0x00010000 0x00000000 0x00100 0x00100 R E 0x1
> > >     LOAD           0x0001d4 0x00080000 0x00000000 0x00000 0x00000 RW  0x1
> > >     LOAD           0x0001d4 0x00080000 0x00000000 0x00400 0x00400 RW  0x1
> > >     LOAD           0x0005d4 0x000fe790 0x00000000 0x01870 0x01870 RW  0x1
> > > 
> > >    Section to Segment mapping:
> > >     Segment Sections...
> > >      00
> > >      01     load
> > >      02     load
> > >      03     load load
> > >      04     load
> > > 
> > > There is no dynamic section in this file.
> > > There are no relocations in this file.
> > > There are no unwind sections in this file.
> > > No version information found in this file.
> > > 
> > > Displaying notes found at file offset 0x00001e44 with length 0x00000138:
> > >     Owner                 Data size       Description
> > >     CORE                 0x0000007c       NT_PRPSINFO (prpsinfo structure)
> > >     CORE                 0x00000094       NT_PRSTATUS (prstatus structure)
> > 
> > Does this support `.reg/xxx` notes for RTOS that support multiple tasks?
> > It would be really nice to have `info threads` "just work"
> > 
> > Paul
> > 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2021-01-14 12:36                                 ` Fredrik Hederstierna
  2021-01-14 12:50                                   ` Luis Machado
@ 2021-01-18 11:01                                   ` Andrew Burgess
  1 sibling, 0 replies; 44+ messages in thread
From: Andrew Burgess @ 2021-01-18 11:01 UTC (permalink / raw)
  To: Fredrik Hederstierna; +Cc: Paul Mathieu, Simon Marchi, gdb-patches

* Fredrik Hederstierna via Gdb-patches <gdb-patches@sourceware.org> [2021-01-14 12:36:35 +0000]:

> Ping, do anyone have any more input how to proceed on this?
> I think I have made what I can do, to the limits of my knowledge and understanding.
> 
> I read recently about RISCV now seems to have merged corefile
> support for their arch?

This is not correct, I posted some patches for RISC-V bare metal core
file support here:

  https://sourceware.org/pipermail/gdb-patches/2020-December/173714.html

They have not yet been merged.

> why can RISCV corefile be merged but not this? I guess ARM-Cortex
> users is magnitude amount higher and the benefit of this feature is
> huge.

Patches are not merged based on the popularity of a target, but on the
commitment of an engineer to see the patch merged.  You need to follow
up to review comments and convince other members of the list that the
patch is worth adding to GDB.

> And it would be really good if synergy could be used to share code,
> since alot functions I guess are same.

That requires people to do the work, right?

> If documentation is the issue, do we have an issue ticket on that?

You're welcome to create one.  Luis has made it clear he's going to
object to the patch until he sees some documentation.  If you want to
get your patch merged you either need to write the documentation,
convince someone else to write it, or convince Luis to drop that
requirement.  If you think having a ticket will help with any of the
above steps, then go for it.  But creating a ticket in itself doesn't
get the docs written.

> 
> Can we just merge this patch-v4 and set target GDB-11, and solve the
> doc-issue-ticket, then we just force ourselves to solve docs before
> the release, or how can we 'make it happen'?

The problem is nobody really wants to write documentation, so if we do
things that way, there's a pretty good chance the docs wont get
written.  Then either we (a) leave the feature in with no docs, or (b)
rip the feature out just to make some kind of point.

I think we should just decide up front if we are happy for it to be
undocumented, or write the docs.

>                                              It seems to be about to
> fail again if time just goes and none try push it further forward.

GDB is maintained with volunteer effort.  Nothing happens unless
someone wants to it happen.  So don't think of it as "failing",
instead think that if the patch stalls its simply a reflection that
demand for this feature is actually pretty low.

Thanks,
Andrew








> 
> Thanks! Kindly,
> Fredrik
> 
> From: Paul Mathieu <paulmathieu@google.com>
> Sent: Tuesday, October 27, 2020 5:53 PM
> To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>
> Cc: Luis Machado <luis.machado@linaro.org>; Simon Marchi <simark@simark.ca>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
> Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi 
>  
> Hi Fredrik,
> 
> > This is the current format when trying from ARM simulator:
> >
> > fredrik@legion ~/src/armv4t_coretest$ readelf -aA test.core
> > ELF Header:
> >   Magic:   7f 45 4c 46 01 01 01 61 00 00 00 00 00 00 00 00
> >   Class:                             ELF32
> >   Data:                              2's complement, little endian
> >   Version:                           1 (current)
> >   OS/ABI:                            ARM
> >   ABI Version:                       0
> >   Type:                              CORE (Core file)
> >   Machine:                           ARM
> >   Version:                           0x1
> >   Entry point address:               0x0
> >   Start of program headers:          52 (bytes into file)
> >   Start of section headers:          8084 (bytes into file)
> >   Flags:                             0x0
> >   Size of this header:               52 (bytes)
> >   Size of program headers:           32 (bytes)
> >   Number of program headers:         5
> >   Size of section headers:           40 (bytes)
> >   Number of section headers:         7
> >   Section header string table index: 6
> >
> > Section Headers:
> >   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> >   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
> >   [ 1] note0             NOTE            00000000 001e44 000138 00   A  0   0  1
> >   [ 2] load              PROGBITS        00010000 0000d4 000100 00  AX  0   0  1
> >   [ 3] load              PROGBITS        00080000 0001d4 000000 00  WA  0   0  1
> >   [ 4] load              PROGBITS        00080000 0001d4 000400 00  WA  0   0  1
> >   [ 5] load              PROGBITS        000fe790 0005d4 001870 00  WA  0   0  1
> >   [ 6] .shstrtab         STRTAB          00000000 001f7c 000016 00      0   0  1
> > Key to Flags:
> >   W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
> >   L (link order), O (extra OS processing required), G (group), T (TLS),
> >   C (compressed), x (unknown), o (OS specific), E (exclude),
> >   y (purecode), p (processor specific)
> >
> > There are no section groups in this file.
> >
> > Program Headers:
> >   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
> >   NOTE           0x001e44 0x00000000 0x00000000 0x00138 0x00000 R   0x1
> >   LOAD           0x0000d4 0x00010000 0x00000000 0x00100 0x00100 R E 0x1
> >   LOAD           0x0001d4 0x00080000 0x00000000 0x00000 0x00000 RW  0x1
> >   LOAD           0x0001d4 0x00080000 0x00000000 0x00400 0x00400 RW  0x1
> >   LOAD           0x0005d4 0x000fe790 0x00000000 0x01870 0x01870 RW  0x1
> >
> >  Section to Segment mapping:
> >   Segment Sections...
> >    00
> >    01     load
> >    02     load
> >    03     load load
> >    04     load
> >
> > There is no dynamic section in this file.
> > There are no relocations in this file.
> > There are no unwind sections in this file.
> > No version information found in this file.
> >
> > Displaying notes found at file offset 0x00001e44 with length 0x00000138:
> >   Owner                 Data size       Description
> >   CORE                 0x0000007c       NT_PRPSINFO (prpsinfo structure)
> >   CORE                 0x00000094       NT_PRSTATUS (prstatus structure)
> 
> Does this support `.reg/xxx` notes for RTOS that support multiple tasks?
> It would be really nice to have `info threads` "just work"
> 
> Paul

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2021-01-14 12:36                                 ` Fredrik Hederstierna
@ 2021-01-14 12:50                                   ` Luis Machado
  2021-01-18 11:09                                     ` Andrew Burgess
  2021-01-18 11:01                                   ` Andrew Burgess
  1 sibling, 1 reply; 44+ messages in thread
From: Luis Machado @ 2021-01-14 12:50 UTC (permalink / raw)
  To: Fredrik Hederstierna, Paul Mathieu; +Cc: Simon Marchi, gdb-patches

Hi,

On 1/14/21 9:36 AM, Fredrik Hederstierna wrote:
> Ping, do anyone have any more input how to proceed on this?
> I think I have made what I can do, to the limits of my knowledge and understanding.
> 
> I read recently about RISCV now seems to have merged corefile support for their arch?

The idea was to establish a common ground that could be used for both 
RISCV and ARM, and prevent design issues from getting in the way.

If RISCV bare metal corefile was merged, does that mean we can reuse the 
same strategy to pursue bare metal ARM core file support now? And does 
that mean we just need to come up with a patch using the same design?

> why can RISCV corefile be merged but not this? I guess ARM-Cortex users is magnitude amount higher and the benefit of this feature is huge.
> And it would be really good if synergy could be used to share code, since alot functions I guess are same.
> If documentation is the issue, do we have an issue ticket on that?

 From my end, the acceptance of ARM bare metal core files was dependent 
on having a documented design of what the bare metal core files should 
look like. Was that documentation included in the RISCV work? If not, 
that wasn't the agreement when I send comments to that patch series.

> 
> Can we just merge this patch-v4 and set target GDB-11, and solve the doc-issue-ticket, then we just force ourselves to solve docs before the release,
> or how can we 'make it happen'? It seems to be about to fail again if time just goes and none try push it further forward.

I'd love to have such support, but I'm not actively working on it. I can 
commit to review the changes and not let it be forgotten, but the 
development work must be pursued by someone else.

> 
> Thanks! Kindly,
> Fredrik
> 
> From: Paul Mathieu <paulmathieu@google.com>
> Sent: Tuesday, October 27, 2020 5:53 PM
> To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>
> Cc: Luis Machado <luis.machado@linaro.org>; Simon Marchi <simark@simark.ca>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
> Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
>   
> Hi Fredrik,
> 
>> This is the current format when trying from ARM simulator:
>>
>> fredrik@legion ~/src/armv4t_coretest$ readelf -aA test.core
>> ELF Header:
>>     Magic:   7f 45 4c 46 01 01 01 61 00 00 00 00 00 00 00 00
>>     Class:                             ELF32
>>     Data:                              2's complement, little endian
>>     Version:                           1 (current)
>>     OS/ABI:                            ARM
>>     ABI Version:                       0
>>     Type:                              CORE (Core file)
>>     Machine:                           ARM
>>     Version:                           0x1
>>     Entry point address:               0x0
>>     Start of program headers:          52 (bytes into file)
>>     Start of section headers:          8084 (bytes into file)
>>     Flags:                             0x0
>>     Size of this header:               52 (bytes)
>>     Size of program headers:           32 (bytes)
>>     Number of program headers:         5
>>     Size of section headers:           40 (bytes)
>>     Number of section headers:         7
>>     Section header string table index: 6
>>
>> Section Headers:
>>     [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>>     [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>>     [ 1] note0             NOTE            00000000 001e44 000138 00   A  0   0  1
>>     [ 2] load              PROGBITS        00010000 0000d4 000100 00  AX  0   0  1
>>     [ 3] load              PROGBITS        00080000 0001d4 000000 00  WA  0   0  1
>>     [ 4] load              PROGBITS        00080000 0001d4 000400 00  WA  0   0  1
>>     [ 5] load              PROGBITS        000fe790 0005d4 001870 00  WA  0   0  1
>>     [ 6] .shstrtab         STRTAB          00000000 001f7c 000016 00      0   0  1
>> Key to Flags:
>>     W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
>>     L (link order), O (extra OS processing required), G (group), T (TLS),
>>     C (compressed), x (unknown), o (OS specific), E (exclude),
>>     y (purecode), p (processor specific)
>>
>> There are no section groups in this file.
>>
>> Program Headers:
>>     Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>>     NOTE           0x001e44 0x00000000 0x00000000 0x00138 0x00000 R   0x1
>>     LOAD           0x0000d4 0x00010000 0x00000000 0x00100 0x00100 R E 0x1
>>     LOAD           0x0001d4 0x00080000 0x00000000 0x00000 0x00000 RW  0x1
>>     LOAD           0x0001d4 0x00080000 0x00000000 0x00400 0x00400 RW  0x1
>>     LOAD           0x0005d4 0x000fe790 0x00000000 0x01870 0x01870 RW  0x1
>>
>>    Section to Segment mapping:
>>     Segment Sections...
>>      00
>>      01     load
>>      02     load
>>      03     load load
>>      04     load
>>
>> There is no dynamic section in this file.
>> There are no relocations in this file.
>> There are no unwind sections in this file.
>> No version information found in this file.
>>
>> Displaying notes found at file offset 0x00001e44 with length 0x00000138:
>>     Owner                 Data size       Description
>>     CORE                 0x0000007c       NT_PRPSINFO (prpsinfo structure)
>>     CORE                 0x00000094       NT_PRSTATUS (prstatus structure)
> 
> Does this support `.reg/xxx` notes for RTOS that support multiple tasks?
> It would be really nice to have `info threads` "just work"
> 
> Paul
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-27 16:53                               ` Paul Mathieu
@ 2021-01-14 12:36                                 ` Fredrik Hederstierna
  2021-01-14 12:50                                   ` Luis Machado
  2021-01-18 11:01                                   ` Andrew Burgess
  0 siblings, 2 replies; 44+ messages in thread
From: Fredrik Hederstierna @ 2021-01-14 12:36 UTC (permalink / raw)
  To: Paul Mathieu; +Cc: Luis Machado, Simon Marchi, gdb-patches

Ping, do anyone have any more input how to proceed on this?
I think I have made what I can do, to the limits of my knowledge and understanding.

I read recently about RISCV now seems to have merged corefile support for their arch?
why can RISCV corefile be merged but not this? I guess ARM-Cortex users is magnitude amount higher and the benefit of this feature is huge.
And it would be really good if synergy could be used to share code, since alot functions I guess are same.
If documentation is the issue, do we have an issue ticket on that?

Can we just merge this patch-v4 and set target GDB-11, and solve the doc-issue-ticket, then we just force ourselves to solve docs before the release,
or how can we 'make it happen'? It seems to be about to fail again if time just goes and none try push it further forward.

Thanks! Kindly,
Fredrik

From: Paul Mathieu <paulmathieu@google.com>
Sent: Tuesday, October 27, 2020 5:53 PM
To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>
Cc: Luis Machado <luis.machado@linaro.org>; Simon Marchi <simark@simark.ca>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi 
 
Hi Fredrik,

> This is the current format when trying from ARM simulator:
>
> fredrik@legion ~/src/armv4t_coretest$ readelf -aA test.core
> ELF Header:
>   Magic:   7f 45 4c 46 01 01 01 61 00 00 00 00 00 00 00 00
>   Class:                             ELF32
>   Data:                              2's complement, little endian
>   Version:                           1 (current)
>   OS/ABI:                            ARM
>   ABI Version:                       0
>   Type:                              CORE (Core file)
>   Machine:                           ARM
>   Version:                           0x1
>   Entry point address:               0x0
>   Start of program headers:          52 (bytes into file)
>   Start of section headers:          8084 (bytes into file)
>   Flags:                             0x0
>   Size of this header:               52 (bytes)
>   Size of program headers:           32 (bytes)
>   Number of program headers:         5
>   Size of section headers:           40 (bytes)
>   Number of section headers:         7
>   Section header string table index: 6
>
> Section Headers:
>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>   [ 1] note0             NOTE            00000000 001e44 000138 00   A  0   0  1
>   [ 2] load              PROGBITS        00010000 0000d4 000100 00  AX  0   0  1
>   [ 3] load              PROGBITS        00080000 0001d4 000000 00  WA  0   0  1
>   [ 4] load              PROGBITS        00080000 0001d4 000400 00  WA  0   0  1
>   [ 5] load              PROGBITS        000fe790 0005d4 001870 00  WA  0   0  1
>   [ 6] .shstrtab         STRTAB          00000000 001f7c 000016 00      0   0  1
> Key to Flags:
>   W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
>   L (link order), O (extra OS processing required), G (group), T (TLS),
>   C (compressed), x (unknown), o (OS specific), E (exclude),
>   y (purecode), p (processor specific)
>
> There are no section groups in this file.
>
> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   NOTE           0x001e44 0x00000000 0x00000000 0x00138 0x00000 R   0x1
>   LOAD           0x0000d4 0x00010000 0x00000000 0x00100 0x00100 R E 0x1
>   LOAD           0x0001d4 0x00080000 0x00000000 0x00000 0x00000 RW  0x1
>   LOAD           0x0001d4 0x00080000 0x00000000 0x00400 0x00400 RW  0x1
>   LOAD           0x0005d4 0x000fe790 0x00000000 0x01870 0x01870 RW  0x1
>
>  Section to Segment mapping:
>   Segment Sections...
>    00
>    01     load
>    02     load
>    03     load load
>    04     load
>
> There is no dynamic section in this file.
> There are no relocations in this file.
> There are no unwind sections in this file.
> No version information found in this file.
>
> Displaying notes found at file offset 0x00001e44 with length 0x00000138:
>   Owner                 Data size       Description
>   CORE                 0x0000007c       NT_PRPSINFO (prpsinfo structure)
>   CORE                 0x00000094       NT_PRSTATUS (prstatus structure)

Does this support `.reg/xxx` notes for RTOS that support multiple tasks?
It would be really nice to have `info threads` "just work"

Paul

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-26 15:49                             ` Fredrik Hederstierna
@ 2020-10-27 16:53                               ` Paul Mathieu
  2021-01-14 12:36                                 ` Fredrik Hederstierna
  0 siblings, 1 reply; 44+ messages in thread
From: Paul Mathieu @ 2020-10-27 16:53 UTC (permalink / raw)
  To: Fredrik Hederstierna; +Cc: Luis Machado, Simon Marchi, gdb-patches

Hi Fredrik,

> This is the current format when trying from ARM simulator:
>
> fredrik@legion ~/src/armv4t_coretest$ readelf -aA test.core
> ELF Header:
>   Magic:   7f 45 4c 46 01 01 01 61 00 00 00 00 00 00 00 00
>   Class:                             ELF32
>   Data:                              2's complement, little endian
>   Version:                           1 (current)
>   OS/ABI:                            ARM
>   ABI Version:                       0
>   Type:                              CORE (Core file)
>   Machine:                           ARM
>   Version:                           0x1
>   Entry point address:               0x0
>   Start of program headers:          52 (bytes into file)
>   Start of section headers:          8084 (bytes into file)
>   Flags:                             0x0
>   Size of this header:               52 (bytes)
>   Size of program headers:           32 (bytes)
>   Number of program headers:         5
>   Size of section headers:           40 (bytes)
>   Number of section headers:         7
>   Section header string table index: 6
>
> Section Headers:
>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>   [ 1] note0             NOTE            00000000 001e44 000138 00   A  0   0  1
>   [ 2] load              PROGBITS        00010000 0000d4 000100 00  AX  0   0  1
>   [ 3] load              PROGBITS        00080000 0001d4 000000 00  WA  0   0  1
>   [ 4] load              PROGBITS        00080000 0001d4 000400 00  WA  0   0  1
>   [ 5] load              PROGBITS        000fe790 0005d4 001870 00  WA  0   0  1
>   [ 6] .shstrtab         STRTAB          00000000 001f7c 000016 00      0   0  1
> Key to Flags:
>   W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
>   L (link order), O (extra OS processing required), G (group), T (TLS),
>   C (compressed), x (unknown), o (OS specific), E (exclude),
>   y (purecode), p (processor specific)
>
> There are no section groups in this file.
>
> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   NOTE           0x001e44 0x00000000 0x00000000 0x00138 0x00000 R   0x1
>   LOAD           0x0000d4 0x00010000 0x00000000 0x00100 0x00100 R E 0x1
>   LOAD           0x0001d4 0x00080000 0x00000000 0x00000 0x00000 RW  0x1
>   LOAD           0x0001d4 0x00080000 0x00000000 0x00400 0x00400 RW  0x1
>   LOAD           0x0005d4 0x000fe790 0x00000000 0x01870 0x01870 RW  0x1
>
>  Section to Segment mapping:
>   Segment Sections...
>    00
>    01     load
>    02     load
>    03     load load
>    04     load
>
> There is no dynamic section in this file.
> There are no relocations in this file.
> There are no unwind sections in this file.
> No version information found in this file.
>
> Displaying notes found at file offset 0x00001e44 with length 0x00000138:
>   Owner                 Data size       Description
>   CORE                 0x0000007c       NT_PRPSINFO (prpsinfo structure)
>   CORE                 0x00000094       NT_PRSTATUS (prstatus structure)

Does this support `.reg/xxx` notes for RTOS that support multiple tasks?
It would be really nice to have `info threads` "just work"

Paul

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-26 11:24                           ` Luis Machado
@ 2020-10-26 15:49                             ` Fredrik Hederstierna
  2020-10-27 16:53                               ` Paul Mathieu
  0 siblings, 1 reply; 44+ messages in thread
From: Fredrik Hederstierna @ 2020-10-26 15:49 UTC (permalink / raw)
  To: Luis Machado, Simon Marchi, Paul Mathieu; +Cc: gdb-patches

> From: Luis Machado <luis.machado@linaro.org>
> Sent: Monday, October 26, 2020 12:24 PM
> 
>>> Luis and Alan (both GDB ARM maintainers) also expressed the desire to
>>> have the format documented.  Alan suggested a section in the GDB manual,
>>> in the ARM-specific section.  I think it is a good idea.  This is
>>> important, so we have something to point to when people ask "what format
>>> should I generate so GDB can read it".
>> 
>> Yes, it was suggested by someone to adding an arm-none section in "G.5.3 ARM Features".
>> How to proceed on this? I guess the current design is same as *ix minus all specific *ix parts that could be stripped out from documentation.
>
> I recall also suggesting discussing the core file format out in the open 
> through the gdb@sourcware.org mailing list (here 
> https://urldefense.com/v3/__https://sourceware.org/pipermail/gdb-patches/2020-October/172721.html__;!!BFCLnRDDbM3FOmw!sNi8byoH5SSID20XZ9bMNX2DOjXF4ZJcrHvYbpgAnuXp0WCSVEiMdf2v2kNX1j4uVrbATpFt$ ). 
> That would make it easier to others interested in this to contribute and 
> provide their thoughts.
>
> It makes it a bit harder to design things based on patches directly.

Yes, agree, re-engineering is not preferred design,
and the Linux format is far more complex that what a none-corefile would need probably.
I tried to strip out everything but 'memory' and 'registers',
so the none corefile should be a regular valid ELF file, with memory and registers only I guess.

I tried to find a good specification/documentation source for corefiles, but it is not very clear where to look...
Found this as a starting point
https://stackoverflow.com/questions/5986366/elf-core-file-format

This is the current format when trying from ARM simulator:

fredrik@legion ~/src/armv4t_coretest$ readelf -aA test.core 
ELF Header:
  Magic:   7f 45 4c 46 01 01 01 61 00 00 00 00 00 00 00 00 
  Class:                             ELF32
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            ARM
  ABI Version:                       0
  Type:                              CORE (Core file)
  Machine:                           ARM
  Version:                           0x1
  Entry point address:               0x0
  Start of program headers:          52 (bytes into file)
  Start of section headers:          8084 (bytes into file)
  Flags:                             0x0
  Size of this header:               52 (bytes)
  Size of program headers:           32 (bytes)
  Number of program headers:         5
  Size of section headers:           40 (bytes)
  Number of section headers:         7
  Section header string table index: 6

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] note0             NOTE            00000000 001e44 000138 00   A  0   0  1
  [ 2] load              PROGBITS        00010000 0000d4 000100 00  AX  0   0  1
  [ 3] load              PROGBITS        00080000 0001d4 000000 00  WA  0   0  1
  [ 4] load              PROGBITS        00080000 0001d4 000400 00  WA  0   0  1
  [ 5] load              PROGBITS        000fe790 0005d4 001870 00  WA  0   0  1
  [ 6] .shstrtab         STRTAB          00000000 001f7c 000016 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  y (purecode), p (processor specific)

There are no section groups in this file.

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  NOTE           0x001e44 0x00000000 0x00000000 0x00138 0x00000 R   0x1
  LOAD           0x0000d4 0x00010000 0x00000000 0x00100 0x00100 R E 0x1
  LOAD           0x0001d4 0x00080000 0x00000000 0x00000 0x00000 RW  0x1
  LOAD           0x0001d4 0x00080000 0x00000000 0x00400 0x00400 RW  0x1
  LOAD           0x0005d4 0x000fe790 0x00000000 0x01870 0x01870 RW  0x1

 Section to Segment mapping:
  Segment Sections...
   00     
   01     load 
   02     load 
   03     load load 
   04     load 

There is no dynamic section in this file.
There are no relocations in this file.
There are no unwind sections in this file.
No version information found in this file.

Displaying notes found at file offset 0x00001e44 with length 0x00000138:
  Owner                 Data size       Description
  CORE                 0x0000007c       NT_PRPSINFO (prpsinfo structure)
  CORE                 0x00000094       NT_PRSTATUS (prstatus structure)

/Fredrik

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-25 21:06                         ` Fredrik Hederstierna
@ 2020-10-26 11:24                           ` Luis Machado
  2020-10-26 15:49                             ` Fredrik Hederstierna
  2021-06-22  2:16                           ` Mike Frysinger
  1 sibling, 1 reply; 44+ messages in thread
From: Luis Machado @ 2020-10-26 11:24 UTC (permalink / raw)
  To: Fredrik Hederstierna, Simon Marchi, Paul Mathieu; +Cc: gdb-patches

On 10/25/20 6:06 PM, Fredrik Hederstierna via Gdb-patches wrote:
>> From: Simon Marchi <simark@simark.ca>
>> Sent: Friday, October 23, 2020 2:37 AM
>>
>> On my side, from the comments I gave earlier and other observations:
>>
>> - there area clearly some unnecessary includes, cut it down to what's
>>   necessary
>> - don't put things in the header file if they are only used in the
>>    source file (the macros in arm-none-tdep.h, for example)
>> - none_init_corefile has an unused parameter
>> - some unnecessary forward declarations here and there
>> - is the handling of inferior arguments relevant for bare-metal?
>> - if it works with the GDB simulator (target sim), I'd really like if we
>>    could have a test for this: generate a core, read it back, make sure
>>    you can print stuff
> 
> Thanks for feedback, I think the new [Patch v4] address all your bullets above.
> 
> I have tested the patch on ARM simulator in GDB, and it worked,
> though some observations:
> 
> - The ARM simulator does not support ARM Cortex architecture?
>    I had to try it out on ARMv4T arch code, though needed to make some patchwork.
> - The ARM simulator does enumerate registers differently, and remapping is done,
> though if trying to fetch all registers by passing -1, then eg D0-D15 was not handled.
> - The ARM simulator expect that target have a stack frame and tried to put that in separate section,
>    though this needs to be setup, if just running with SP=0, then SP will soon be 0xFFFFFFFxx, and
>    then when trying to make corefile and stack should be saved, all 0-0xFFFFFFFFF will be saved 4GByte!
>    Though by explicitly setting SP (either by code or from console) it seems to work.
>    Cortex handles initial stack pointer differently by having an entry for SP in start vectors table.
> 
> This worked for me in the ARM simulator on test application compiled for ARMv4T thumb.
> 
> (gdb) target sim
> (gdb) load
> (gdb) break main
> (gdb) run
> (gdb) set $sp = 0xFFFFFF
> (gdb) cont
> <CTRL-C>
> (gdb) gcore test.core
> 
> Exiting and restarting GDB with elf-file and adding corefile 'test.core', same registers and call-stack could be reviewed, so my simple example worked.
> 
>> Luis and Alan (both GDB ARM maintainers) also expressed the desire to
>> have the format documented.  Alan suggested a section in the GDB manual,
>> in the ARM-specific section.  I think it is a good idea.  This is
>> important, so we have something to point to when people ask "what format
>> should I generate so GDB can read it".
> 
> Yes, it was suggested by someone to adding an arm-none section in "G.5.3 ARM Features".
> How to proceed on this? I guess the current design is same as *ix minus all specific *ix parts that could be stripped out from documentation.

I recall also suggesting discussing the core file format out in the open 
through the gdb@sourcware.org mailing list (here 
https://sourceware.org/pipermail/gdb-patches/2020-October/172721.html). 
That would make it easier to others interested in this to contribute and 
provide their thoughts.

It makes it a bit harder to design things based on patches directly.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-23  0:37                       ` Simon Marchi
@ 2020-10-25 21:06                         ` Fredrik Hederstierna
  2020-10-26 11:24                           ` Luis Machado
  2021-06-22  2:16                           ` Mike Frysinger
  0 siblings, 2 replies; 44+ messages in thread
From: Fredrik Hederstierna @ 2020-10-25 21:06 UTC (permalink / raw)
  To: Simon Marchi, Paul Mathieu; +Cc: gdb-patches

> From: Simon Marchi <simark@simark.ca>
> Sent: Friday, October 23, 2020 2:37 AM
> 
> On my side, from the comments I gave earlier and other observations:
> 
> - there area clearly some unnecessary includes, cut it down to what's
>  necessary
> - don't put things in the header file if they are only used in the
>   source file (the macros in arm-none-tdep.h, for example)
> - none_init_corefile has an unused parameter
> - some unnecessary forward declarations here and there
> - is the handling of inferior arguments relevant for bare-metal?
> - if it works with the GDB simulator (target sim), I'd really like if we
>   could have a test for this: generate a core, read it back, make sure
>   you can print stuff

Thanks for feedback, I think the new [Patch v4] address all your bullets above.

I have tested the patch on ARM simulator in GDB, and it worked,
though some observations:

- The ARM simulator does not support ARM Cortex architecture?
  I had to try it out on ARMv4T arch code, though needed to make some patchwork.
- The ARM simulator does enumerate registers differently, and remapping is done,
though if trying to fetch all registers by passing -1, then eg D0-D15 was not handled.
- The ARM simulator expect that target have a stack frame and tried to put that in separate section,
  though this needs to be setup, if just running with SP=0, then SP will soon be 0xFFFFFFFxx, and
  then when trying to make corefile and stack should be saved, all 0-0xFFFFFFFFF will be saved 4GByte!
  Though by explicitly setting SP (either by code or from console) it seems to work.
  Cortex handles initial stack pointer differently by having an entry for SP in start vectors table.

This worked for me in the ARM simulator on test application compiled for ARMv4T thumb.

(gdb) target sim
(gdb) load
(gdb) break main
(gdb) run
(gdb) set $sp = 0xFFFFFF
(gdb) cont
<CTRL-C>
(gdb) gcore test.core

Exiting and restarting GDB with elf-file and adding corefile 'test.core', same registers and call-stack could be reviewed, so my simple example worked.

> Luis and Alan (both GDB ARM maintainers) also expressed the desire to
> have the format documented.  Alan suggested a section in the GDB manual,
> in the ARM-specific section.  I think it is a good idea.  This is
> important, so we have something to point to when people ask "what format
> should I generate so GDB can read it".

Yes, it was suggested by someone to adding an arm-none section in "G.5.3 ARM Features".
How to proceed on this? I guess the current design is same as *ix minus all specific *ix parts that could be stripped out from documentation.

Thanks! BR Fredrik

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-22 22:32                     ` Fredrik Hederstierna
@ 2020-10-23  0:37                       ` Simon Marchi
  2020-10-25 21:06                         ` Fredrik Hederstierna
  0 siblings, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-23  0:37 UTC (permalink / raw)
  To: Fredrik Hederstierna, Paul Mathieu; +Cc: gdb-patches

On 2020-10-22 6:32 p.m., Fredrik Hederstierna via Gdb-patches wrote:
>> From: Simon Marchi <simark@simark.ca>
>> Sent: Thursday, October 22, 2020 3:49 AM
>> I've rebased your patch on top of master and made the necessary changes
>> to make it build (the equivalent changes that I did to linux-tdep.c and
>> fbsd-tdep.c).
>>
>> You can find it on the users/simark/arm-none-core-file branch upstream:
>
> Looks great, thanks Simon, I have no futher changes to be made,
> what is next step, are we near for this to being mergable?
> The features regarding eg FreeRTOS could be added later on top of this I guess.
> The format of the corefile right now is exactly as Linux, minus all '*ix' stuff,
> only threads, registers and memory I think. What is next step on this?

On my side, from the comments I gave earlier and other observations:

- there area clearly some unnecessary includes, cut it down to what's
  necessary
- don't put things in the header file if they are only used in the
  source file (the macros in arm-none-tdep.h, for example)
- none_init_corefile has an unused parameter
- some unnecessary forward declarations here and there
- is the handling of inferior arguments relevant for bare-metal?
- if it works with the GDB simulator (target sim), I'd really like if we
  could have a test for this: generate a core, read it back, make sure
  you can print stuff

Luis and Alan (both GDB ARM maintainers) also expressed the desire to
have the format documented.  Alan suggested a section in the GDB manual,
in the ARM-specific section.  I think it is a good idea.  This is
important, so we have something to point to when people ask "what format
should I generate so GDB can read it".

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-22  1:49                   ` Simon Marchi
@ 2020-10-22 22:32                     ` Fredrik Hederstierna
  2020-10-23  0:37                       ` Simon Marchi
  0 siblings, 1 reply; 44+ messages in thread
From: Fredrik Hederstierna @ 2020-10-22 22:32 UTC (permalink / raw)
  To: Simon Marchi, Paul Mathieu; +Cc: gdb-patches

> From: Simon Marchi <simark@simark.ca>
> Sent: Thursday, October 22, 2020 3:49 AM
> I've rebased your patch on top of master and made the necessary changes
> to make it build (the equivalent changes that I did to linux-tdep.c and
> fbsd-tdep.c).
>
> You can find it on the users/simark/arm-none-core-file branch upstream:

Looks great, thanks Simon, I have no futher changes to be made,
what is next step, are we near for this to being mergable?
The features regarding eg FreeRTOS could be added later on top of this I guess.
The format of the corefile right now is exactly as Linux, minus all '*ix' stuff,
only threads, registers and memory I think. What is next step on this?

Thanks, BR Fredrik

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-22  0:52                 ` Fredrik Hederstierna
  2020-10-22  1:24                   ` Simon Marchi
@ 2020-10-22  1:49                   ` Simon Marchi
  2020-10-22 22:32                     ` Fredrik Hederstierna
  1 sibling, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-22  1:49 UTC (permalink / raw)
  To: Fredrik Hederstierna, Paul Mathieu; +Cc: gdb-patches


On 2020-10-21 8:52 p.m., Fredrik Hederstierna via Gdb-patches wrote:
> Hi,
> I did a new patch v3 where I try address all your comments,
> stripped alot of more "*ix" stuff, and hopefully the remaining parts are all good for bare metal none core files.
> Still I have not done the xmalloc/xfree transform to newer variant, but maybe that can be in another future commit.

I've rebased your patch on top of master and made the necessary changes
to make it build (the equivalent changes that I did to linux-tdep.c and
fbsd-tdep.c).

You can find it on the users/simark/arm-none-core-file branch upstream:

https://sourceware.org/git/?p=binutils-gdb.git;a=shortlog;h=refs/heads/users/simark/arm-none-core-file

Feel free to pick it up.

I've also removed an xmalloc/xfree to use an std::string instead, but
other than that I haven't made any other change.

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-22  0:52                 ` Fredrik Hederstierna
@ 2020-10-22  1:24                   ` Simon Marchi
  2020-10-22  1:49                   ` Simon Marchi
  1 sibling, 0 replies; 44+ messages in thread
From: Simon Marchi @ 2020-10-22  1:24 UTC (permalink / raw)
  To: Fredrik Hederstierna, Paul Mathieu
  Cc: gdb-patches, Luis Machado, Alan Hayward


On 2020-10-21 8:52 p.m., Fredrik Hederstierna wrote:
> Hi,
> I did a new patch v3 where I try address all your comments,
> stripped alot of more "*ix" stuff, and hopefully the remaining parts are all good for bare metal none core files.
> Still I have not done the xmalloc/xfree transform to newer variant, but maybe that can be in another future commit.
> And sorry for the git-email mess, I'm still learning to use this feature..
> Any comments or feedback are welcome!
> Thanks, BR Fredrik

Do you know why your patches are sent as two emails, one with (what
appears to be) the commit log, and another one with the diff?  If you
put the description in the git commit/log message itself, it will be
sent as part of the patch, all in one message.

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-20 23:06               ` Simon Marchi
@ 2020-10-22  0:52                 ` Fredrik Hederstierna
  2020-10-22  1:24                   ` Simon Marchi
  2020-10-22  1:49                   ` Simon Marchi
  0 siblings, 2 replies; 44+ messages in thread
From: Fredrik Hederstierna @ 2020-10-22  0:52 UTC (permalink / raw)
  To: Simon Marchi, Paul Mathieu; +Cc: gdb-patches, Luis Machado, Alan Hayward

Hi,
I did a new patch v3 where I try address all your comments,
stripped alot of more "*ix" stuff, and hopefully the remaining parts are all good for bare metal none core files.
Still I have not done the xmalloc/xfree transform to newer variant, but maybe that can be in another future commit.
And sorry for the git-email mess, I'm still learning to use this feature..
Any comments or feedback are welcome!
Thanks, BR Fredrik

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-20 22:05             ` Fredrik Hederstierna
@ 2020-10-20 23:06               ` Simon Marchi
  2020-10-22  0:52                 ` Fredrik Hederstierna
  0 siblings, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-20 23:06 UTC (permalink / raw)
  To: Fredrik Hederstierna, Paul Mathieu
  Cc: gdb-patches, Luis Machado, Alan Hayward

On 2020-10-20 6:05 p.m., Fredrik Hederstierna wrote:
>
>
>
>> From: Simon Marchi <simark@simark.ca>
>> Sent: Tuesday, October 20, 2020 5:04 PM
>> To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>; Paul Mathieu <paulmathieu@google.com>
>> Cc: gdb-patches@sourceware.org <gdb-patches@sourceware.org>; Luis Machado <luis.machado@linaro.org>; Alan Hayward > <Alan.Hayward@arm.com>
>> Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
>
>> Ok, thanks for trying.  Is Outlook's SMTP not really SMTP?
>
> I think git-email now works with the SMTP server, but need to learn how to embed message etc. sorry for beginners mistakes..

Well, that's good news!

>> One thing that doesn't look right is this, in none_make_corefile_notes:
>>
>>       int global_id = 1;
>>       struct thread_info *info = find_thread_global_id (global_id);
>>
>> As you might have noticed, GDB now supports being connected to multiple
>> targets at the same time.  So, inferior 1 could be a local GNU/Linux
>> executable, while inferior 2 could be a remote bare-metal ARM program.
>
>
>> If so, I guess GDB should just dump all the current inferior's threads
>> in the core dump.
>
> I've tried to modify code so it should dump all threads,
> created a [PATCH v2] which includes this (sent with git-email).

Thanks, I'll take a look.

>
>> I see you have this commented out:
>>
>>     /* make_cleanup (xfree, note_data); */
>>
>> The way to do it now would be to make note_data a
>> gdb::unique_xmalloc_ptr<char>, and do "return note_data.release ();"
>> when returning.  This way, it gets automatically freed if something bad
>> happens.  And ideally, gdbarch_make_corefile_notes should be changed to
>> return a gdb::unique_xmalloc_ptr<char>, but that's out of the scope of
>> this patch (I'll give it a quick try).
>
> I tried to copy linux-tdep.c, and I could not find where data is unallocated, but maybe its a later problem.

If everything works well, your function returns allocated data to the
caller and gives up ownership.  So it becomes the caller's problem to
de-allocate it.  However, you need to make sure that your function
doesn't leak the data if an exception is thrown.

If you do:

  char *data = (char *) xmalloc (1234);
  call_function_that_may_throw ();
  return data;

... and call_function_that_may_throw throws, the allocated buffer leaks.

If you do:

  gdb::unique_xmalloc_ptr<char> data ((char *) xmalloc (1234));
  call_function_that_may_throw ();
  return data.release ();

... then data gets freed if the function throws.

I'll look at changing the callback type to make it return a
gdb::unique_xmalloc_ptr<char> instead of a `char *`, in which case it
would become:

  gdb::unique_xmalloc_ptr<char> data ((char *) xmalloc (1234));
  call_function_that_may_throw ();
  return data;

>> You don't need an _initialize_none_tdep if you don't do anything in it.
>
> Ok, I kept if for now, if something pops up that needs to be put there along the road..

If you don't have anything, just remove it.  We can easily add it later.

>> One last question: I see that you deal with AUXV stuff.  Will bare-metal
>> arm programs really have an auxiliary vector?
>
> I'm not sure what Auxv contains, but seems to be eg some system info that is handy, I do not know what/if its filled with something by default on a bare-metal arm-none target/arch?

On "standard" OSes, the auxiliary vector (auxv) is set up by the kernel
when it loads the executable in memory, to provide some information
about the ELF file how it was loaded.  I don't think it applies to bare
metal targets.  If not (I'd be happy to be proven wrong), you shouldn't
include it, it just makes the code unnecessarily more complicated.

Simon


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-20 15:04           ` Simon Marchi
@ 2020-10-20 22:05             ` Fredrik Hederstierna
  2020-10-20 23:06               ` Simon Marchi
  0 siblings, 1 reply; 44+ messages in thread
From: Fredrik Hederstierna @ 2020-10-20 22:05 UTC (permalink / raw)
  To: Simon Marchi, Paul Mathieu; +Cc: gdb-patches, Luis Machado, Alan Hayward




> From: Simon Marchi <simark@simark.ca>
> Sent: Tuesday, October 20, 2020 5:04 PM
> To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>; Paul Mathieu <paulmathieu@google.com>
> Cc: gdb-patches@sourceware.org <gdb-patches@sourceware.org>; Luis Machado <luis.machado@linaro.org>; Alan Hayward > <Alan.Hayward@arm.com>
> Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi 
 
> Ok, thanks for trying.  Is Outlook's SMTP not really SMTP?

I think git-email now works with the SMTP server, but need to learn how to embed message etc. sorry for beginners mistakes..

> One warning to fix we get when applying:
>
> Applying: gdb: Support corefiles for arm-none-eabi
> .git/rebase-apply/patch:703: trailing whitespace.

I found one place with trailing whitespace that I missed, should be fixed.

> One thing that doesn't look right is this, in none_make_corefile_notes:
>
>      int global_id = 1;
>      struct thread_info *info = find_thread_global_id (global_id);
>
> As you might have noticed, GDB now supports being connected to multiple
> targets at the same time.  So, inferior 1 could be a local GNU/Linux
> executable, while inferior 2 could be a remote bare-metal ARM program.


> If so, I guess GDB should just dump all the current inferior's threads
> in the core dump.

I've tried to modify code so it should dump all threads,
created a [PATCH v2] which includes this (sent with git-email).

> I see you have this commented out:
>
>    /* make_cleanup (xfree, note_data); */
>
> The way to do it now would be to make note_data a
> gdb::unique_xmalloc_ptr<char>, and do "return note_data.release ();"
> when returning.  This way, it gets automatically freed if something bad
> happens.  And ideally, gdbarch_make_corefile_notes should be changed to
> return a gdb::unique_xmalloc_ptr<char>, but that's out of the scope of
> this patch (I'll give it a quick try).

I tried to copy linux-tdep.c, and I could not find where data is unallocated, but maybe its a later problem.

> You don't need an _initialize_none_tdep if you don't do anything in it.

Ok, I kept if for now, if something pops up that needs to be put there along the road..

> One last question: I see that you deal with AUXV stuff.  Will bare-metal
> arm programs really have an auxiliary vector?

I'm not sure what Auxv contains, but seems to be eg some system info that is handy, I do not know what/if its filled with something by default on a bare-metal arm-none target/arch?

Thanks! BR Fredrik

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-20 14:00         ` Fredrik Hederstierna
@ 2020-10-20 15:04           ` Simon Marchi
  2020-10-20 22:05             ` Fredrik Hederstierna
  0 siblings, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-20 15:04 UTC (permalink / raw)
  To: Fredrik Hederstierna, Paul Mathieu
  Cc: gdb-patches, Luis Machado, Alan Hayward

> Sorry my crappy corporate Outlook SMTP doesnt support me sending through git, and I had problems to using eg Google gmail,
> I try to attach the patch here and in the meantime try to solve this in a better way, sorry for the inconvenience.

Ok, thanks for trying.  Is Outlook's SMTP not really SMTP?

One warning to fix we get when applying:

Applying: gdb: Support corefiles for arm-none-eabi
.git/rebase-apply/patch:703: trailing whitespace.

I'm leaving stylistic comments for later, it will be easier if you
succeed using git-send-email.

One thing that doesn't look right is this, in none_make_corefile_notes:

      int global_id = 1;
      struct thread_info *info = find_thread_global_id (global_id);

As you might have noticed, GDB now supports being connected to multiple
targets at the same time.  So, inferior 1 could be a local GNU/Linux
executable, while inferior 2 could be a remote bare-metal ARM program.

So you can't assume that the thread you want to dump has global id 1.
You should get the current inferior with the current_inferior function,
and access its threads.

Also, if GDB connects to a remote target that is able to report the OS's
(FreeRTOS, for example) tasks as threads, is it possible to see them in
GDB?  If I understand correctly this section of the OpenOCD manual,
OpenOCD can do it:

    http://openocd.org/doc/html/GDB-and-OpenOCD.html#RTOS-Support

If so, I guess GDB should just dump all the current inferior's threads
in the core dump.

I see you have this commented out:

    /* make_cleanup (xfree, note_data); */

The way to do it now would be to make note_data a
gdb::unique_xmalloc_ptr<char>, and do "return note_data.release ();"
when returning.  This way, it gets automatically freed if something bad
happens.  And ideally, gdbarch_make_corefile_notes should be changed to
return a gdb::unique_xmalloc_ptr<char>, but that's out of the scope of
this patch (I'll give it a quick try).

You don't need an _initialize_none_tdep if you don't do anything in it.

One last question: I see that you deal with AUXV stuff.  Will bare-metal
arm programs really have an auxiliary vector?

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-20 12:39       ` Simon Marchi
@ 2020-10-20 14:00         ` Fredrik Hederstierna
  2020-10-20 15:04           ` Simon Marchi
  0 siblings, 1 reply; 44+ messages in thread
From: Fredrik Hederstierna @ 2020-10-20 14:00 UTC (permalink / raw)
  To: Simon Marchi, Paul Mathieu; +Cc: gdb-patches, Luis Machado, Alan Hayward

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




From: Simon Marchi <simark@simark.ca>
Sent: Tuesday, October 20, 2020 2:39 PM
To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>; Paul Mathieu <paulmathieu@google.com>
Cc: gdb-patches@sourceware.org <gdb-patches@sourceware.org>; Luis Machado <luis.machado@linaro.org>; Alan Hayward <Alan.Hayward@arm.com>
Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi 
 

On 2020-10-20 7:41 a.m., Fredrik Hederstierna wrote:
> Hi all,
>
> I rebased and update my patch and it not run against GDB 10 / git-master, available at
>
> https://urldefense.com/v3/__https://sourceware.org/bugzilla/show_bug.cgi?id=14383__;!!BFCLnRDDbM3FOmw!sFHQSRGODtw_TucPLlwMNAMob6sJKx4DCgz3ryd9VjcmJNdXXPz78keCE4GRXIlJ9HDNheH5$ 
>
> I debugged my remote arm-none bare-metal Cortex-M4 target app over openocd/JTAG,
> and generate core-file:
> (gdb) gcore bare_metal.core
>
> Then later off-line on my host PC, I successfully debugged it with register, call-stack etc:
> $ arm-none-eabi-gdb my_bare_metal_app.elf bare_metal.core
>
> So this patch 'works', but I'm not sure if this is the correct direction to go,
> but could be a start.
> Besides this, next step could be to do this 'core-generator' tool that could be used for embedded targets, running on host PC.
>
> Please check the patch out, it builds fine with GDB-10/master on my machine.
> Any comments are appreciated!
>
> Thanks, BR Fredrik


> Can you please send it on this list using git-send-email?  We don't do
> patch review on bugzilla (and it's not really practical).
> 
> You can send it as a reply to your message (so it's threaded correctly)
> by mentioning the Message-Id of your message:
> 
>   git send-email --in-reply-to AM6PR10MB21504CBD56291D9C255739E1EF1F0@AM6PR10MB2150.EURPRD10.PROD.OUTLOOK.COM <other args>
> 
> Simon

Sorry my crappy corporate Outlook SMTP doesnt support me sending through git, and I had problems to using eg Google gmail,
I try to attach the patch here and in the meantime try to solve this in a better way, sorry for the inconvenience.

Kindly, Fredrik

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-corefiles-for-arm-none-eabi.patch --]
[-- Type: text/x-patch; name="0001-corefiles-for-arm-none-eabi.patch", Size: 24757 bytes --]

From 48f7f72a4d50b697483cb9cde56496f07c7aec3e Mon Sep 17 00:00:00 2001
From: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>
Date: Tue, 20 Oct 2020 09:06:55 +0200
Subject: [PATCH] gdb: Support corefiles for arm-none-eabi

---
 gdb/arm-none-tdep.c | 368 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/arm-none-tdep.h |  60 ++++++++
 gdb/arm-tdep.c      |   8 +
 gdb/configure.tgt   |   6 +-
 gdb/none-tdep.c     | 231 +++++++++++++++++++++++++++
 gdb/none-tdep.h     |  39 +++++
 6 files changed, 711 insertions(+), 1 deletion(-)
 create mode 100644 gdb/arm-none-tdep.c
 create mode 100644 gdb/arm-none-tdep.h
 create mode 100644 gdb/none-tdep.c
 create mode 100644 gdb/none-tdep.h

diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
new file mode 100644
index 0000000000..9d83a8b913
--- /dev/null
+++ b/gdb/arm-none-tdep.c
@@ -0,0 +1,368 @@
+/* none on ARM target support.
+
+   Copyright (C) 2020 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 "target.h"
+#include "value.h"
+#include "gdbtypes.h"
+#include "gdbcore.h"
+#include "frame.h"
+#include "regcache.h"
+#include "solib-svr4.h"
+#include "osabi.h"
+#include "regset.h"
+#include "trad-frame.h"
+#include "tramp-frame.h"
+#include "breakpoint.h"
+#include "auxv.h"
+
+#include "aarch32-tdep.h"
+#include "arch/arm.h"
+#include "arm-tdep.h"
+#include "arm-none-tdep.h"
+#include "glibc-tdep.h"
+#include "arch-utils.h"
+#include "inferior.h"
+#include "gdbthread.h"
+#include "symfile.h"
+
+#include "cli/cli-utils.h"
+#include "stap-probe.h"
+#include "parser-defs.h"
+#include "user-regs.h"
+#include <ctype.h>
+
+#include "elf-bfd.h"
+#include "coff/internal.h"
+#include "elf/arm.h"
+
+#include "elf/common.h"
+
+/* ---- based on arm-linux-tdep.h ---- */
+
+#define ARM_NONE_SIZEOF_NWFPE (8 * ARM_FP_REGISTER_SIZE \
+				+ 2 * ARM_INT_REGISTER_SIZE \
+				+ 8 + ARM_INT_REGISTER_SIZE)
+
+/* The index to access CSPR in user_regs defined in GLIBC.  */
+#define ARM_CPSR_GREGNUM 16
+
+/* Support for register format used by the NWFPE FPA emulator.  Each
+   register takes three words, where either the first one, two, or
+   three hold a single, double, or extended precision value (depending
+   on the corresponding tag).  The register set is eight registers,
+   followed by the fpsr and fpcr, followed by eight tag bytes, and a
+   final word flag which indicates whether NWFPE has been
+   initialized.  */
+
+#define NWFPE_FPSR_OFFSET (8 * ARM_FP_REGISTER_SIZE)
+#define NWFPE_FPCR_OFFSET (NWFPE_FPSR_OFFSET + ARM_INT_REGISTER_SIZE)
+#define NWFPE_TAGS_OFFSET (NWFPE_FPCR_OFFSET + ARM_INT_REGISTER_SIZE)
+#define NWFPE_INITFLAG_OFFSET (NWFPE_TAGS_OFFSET + 8)
+
+/* Reuse ARM GNU/Linux HWCAP values for none.  These are in defined in
+   <asm/elf.h> in current kernels.  */
+#define HWCAP_VFP       64
+#define HWCAP_IWMMXT    512
+#define HWCAP_NEON      4096
+#define HWCAP_VFPv3     8192
+#define HWCAP_VFPv3D16  16384
+
+/* Core file and register set support.  */
+
+#define ARM_NONE_SIZEOF_GREGSET (18 * ARM_INT_REGISTER_SIZE)
+
+/* ---- based on arm-linux-tdep.c ---- */
+
+void
+arm_none_supply_gregset (const struct regset *regset,
+			  struct regcache *regcache,
+			  int regnum, const void *gregs_buf, size_t len)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  const gdb_byte *gregs = (const gdb_byte *) gregs_buf;
+  int regno;
+  CORE_ADDR reg_pc;
+  gdb_byte pc_buf[ARM_INT_REGISTER_SIZE];
+
+  for (regno = ARM_A1_REGNUM; regno < ARM_PC_REGNUM; regno++)
+    if (regnum == -1 || regnum == regno)
+      regcache->raw_supply (regno, gregs + ARM_INT_REGISTER_SIZE * regno);
+
+  if (regnum == ARM_PS_REGNUM || regnum == -1)
+    {
+      if (arm_apcs_32)
+	regcache->raw_supply (ARM_PS_REGNUM,
+			      gregs + ARM_INT_REGISTER_SIZE * ARM_CPSR_GREGNUM);
+      else
+	regcache->raw_supply (ARM_PS_REGNUM,
+			     gregs + ARM_INT_REGISTER_SIZE * ARM_PC_REGNUM);
+    }
+
+  if (regnum == ARM_PC_REGNUM || regnum == -1)
+    {
+      reg_pc = extract_unsigned_integer (
+		 gregs + ARM_INT_REGISTER_SIZE * ARM_PC_REGNUM,
+		 ARM_INT_REGISTER_SIZE, byte_order);
+      reg_pc = gdbarch_addr_bits_remove (gdbarch, reg_pc);
+      store_unsigned_integer (pc_buf, ARM_INT_REGISTER_SIZE, byte_order,
+			      reg_pc);
+      regcache->raw_supply (ARM_PC_REGNUM, pc_buf);
+    }
+}
+
+void
+arm_none_collect_gregset (const struct regset *regset,
+			   const struct regcache *regcache,
+			   int regnum, void *gregs_buf, size_t len)
+{
+  gdb_byte *gregs = (gdb_byte *) gregs_buf;
+  int regno;
+
+  for (regno = ARM_A1_REGNUM; regno < ARM_PC_REGNUM; regno++)
+    if (regnum == -1 || regnum == regno)
+      regcache->raw_collect (regno,
+			    gregs + ARM_INT_REGISTER_SIZE * regno);
+
+  if (regnum == ARM_PS_REGNUM || regnum == -1)
+    {
+      if (arm_apcs_32)
+	regcache->raw_collect (ARM_PS_REGNUM,
+			      gregs + ARM_INT_REGISTER_SIZE * ARM_CPSR_GREGNUM);
+      else
+	regcache->raw_collect (ARM_PS_REGNUM,
+			      gregs + ARM_INT_REGISTER_SIZE * ARM_PC_REGNUM);
+    }
+
+  if (regnum == ARM_PC_REGNUM || regnum == -1)
+    regcache->raw_collect (ARM_PC_REGNUM,
+			   gregs + ARM_INT_REGISTER_SIZE * ARM_PC_REGNUM);
+}
+
+/* Support for register format used by the NWFPE FPA emulator.  */
+
+#define typeNone		0x00
+#define typeSingle		0x01
+#define typeDouble		0x02
+#define typeExtended		0x03
+
+void
+arm_none_supply_nwfpe_register (struct regcache *regcache, int regno,
+				const gdb_byte *regs)
+{
+  const gdb_byte *reg_data;
+  gdb_byte reg_tag;
+  gdb_byte buf[ARM_FP_REGISTER_SIZE];
+
+  reg_data = regs + (regno - ARM_F0_REGNUM) * ARM_FP_REGISTER_SIZE;
+  reg_tag = regs[(regno - ARM_F0_REGNUM) + NWFPE_TAGS_OFFSET];
+  memset (buf, 0, ARM_FP_REGISTER_SIZE);
+
+  switch (reg_tag)
+    {
+    case typeSingle:
+      memcpy (buf, reg_data, 4);
+      break;
+    case typeDouble:
+      memcpy (buf, reg_data + 4, 4);
+      memcpy (buf + 4, reg_data, 4);
+      break;
+    case typeExtended:
+      /* We want sign and exponent, then least significant bits,
+	 then most significant.  NWFPE does sign, most, least.  */
+      memcpy (buf, reg_data, 4);
+      memcpy (buf + 4, reg_data + 8, 4);
+      memcpy (buf + 8, reg_data + 4, 4);
+      break;
+    default:
+      break;
+    }
+
+  regcache->raw_supply (regno, buf);
+}
+
+void
+arm_none_collect_nwfpe_register (const struct regcache *regcache, int regno,
+				 gdb_byte *regs)
+{
+  gdb_byte *reg_data;
+  gdb_byte reg_tag;
+  gdb_byte buf[ARM_FP_REGISTER_SIZE];
+
+  regcache->raw_collect (regno, buf);
+
+  /* NOTE drow/2006-06-07: This code uses the tag already in the
+     register buffer.  I've preserved that when moving the code
+     from the native file to the target file.  But this doesn't
+     always make sense.  */
+
+  reg_data = regs + (regno - ARM_F0_REGNUM) * ARM_FP_REGISTER_SIZE;
+  reg_tag = regs[(regno - ARM_F0_REGNUM) + NWFPE_TAGS_OFFSET];
+
+  switch (reg_tag)
+    {
+    case typeSingle:
+      memcpy (reg_data, buf, 4);
+      break;
+    case typeDouble:
+      memcpy (reg_data, buf + 4, 4);
+      memcpy (reg_data + 4, buf, 4);
+      break;
+    case typeExtended:
+      memcpy (reg_data, buf, 4);
+      memcpy (reg_data + 4, buf + 8, 4);
+      memcpy (reg_data + 8, buf + 4, 4);
+      break;
+    default:
+      break;
+    }
+}
+
+void
+arm_none_supply_nwfpe (const struct regset *regset,
+			struct regcache *regcache,
+			int regnum, const void *regs_buf, size_t len)
+{
+  const gdb_byte *regs = (const gdb_byte *) regs_buf;
+  int regno;
+
+  if (regnum == ARM_FPS_REGNUM || regnum == -1)
+    regcache->raw_supply (ARM_FPS_REGNUM,
+			 regs + NWFPE_FPSR_OFFSET);
+
+  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
+    if (regnum == -1 || regnum == regno)
+      arm_none_supply_nwfpe_register (regcache, regno, regs);
+}
+
+void
+arm_none_collect_nwfpe (const struct regset *regset,
+			 const struct regcache *regcache,
+			 int regnum, void *regs_buf, size_t len)
+{
+  gdb_byte *regs = (gdb_byte *) regs_buf;
+  int regno;
+
+  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
+    if (regnum == -1 || regnum == regno)
+      arm_none_collect_nwfpe_register (regcache, regno, regs);
+
+  if (regnum == ARM_FPS_REGNUM || regnum == -1)
+    regcache->raw_collect (ARM_FPS_REGNUM,
+			   regs + ARM_INT_REGISTER_SIZE * ARM_FPS_REGNUM);
+}
+
+/* Support VFP register format.  */
+
+#define ARM_NONE_SIZEOF_VFP (32 * 8 + 4)
+
+static void
+arm_none_supply_vfp (const struct regset *regset,
+		      struct regcache *regcache,
+		      int regnum, const void *regs_buf, size_t len)
+{
+  const gdb_byte *regs = (const gdb_byte *) regs_buf;
+  int regno;
+
+  if (regnum == ARM_FPSCR_REGNUM || regnum == -1)
+    regcache->raw_supply (ARM_FPSCR_REGNUM, regs + 32 * 8);
+
+  for (regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
+    if (regnum == -1 || regnum == regno)
+      regcache->raw_supply (regno, regs + (regno - ARM_D0_REGNUM) * 8);
+}
+
+static void
+arm_none_collect_vfp (const struct regset *regset,
+			 const struct regcache *regcache,
+			 int regnum, void *regs_buf, size_t len)
+{
+  gdb_byte *regs = (gdb_byte *) regs_buf;
+  int regno;
+
+  if (regnum == ARM_FPSCR_REGNUM || regnum == -1)
+    regcache->raw_collect (ARM_FPSCR_REGNUM, regs + 32 * 8);
+
+  for (regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
+    if (regnum == -1 || regnum == regno)
+      regcache->raw_collect (regno, regs + (regno - ARM_D0_REGNUM) * 8);
+}
+
+static const struct regset arm_none_gregset =
+  {
+    NULL, arm_none_supply_gregset, arm_none_collect_gregset
+  };
+
+static const struct regset arm_none_fpregset =
+  {
+    NULL, arm_none_supply_nwfpe, arm_none_collect_nwfpe
+  };
+
+static const struct regset arm_none_vfpregset =
+  {
+    NULL, arm_none_supply_vfp, arm_none_collect_vfp
+  };
+
+/* Iterate over core file register note sections.  */
+
+void
+arm_none_iterate_over_regset_sections (struct gdbarch *gdbarch,
+					iterate_over_regset_sections_cb *cb,
+					void *cb_data,
+					const struct regcache *regcache)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  cb (".reg", ARM_NONE_SIZEOF_GREGSET, ARM_NONE_SIZEOF_GREGSET,
+	&arm_none_gregset, NULL, cb_data);
+
+  if (tdep->vfp_register_count > 0)
+    cb (".reg-arm-vfp", ARM_NONE_SIZEOF_VFP, ARM_NONE_SIZEOF_VFP,
+	&arm_none_vfpregset, "VFP floating-point", cb_data);
+  else if (tdep->have_fpa_registers)
+    cb (".reg2", ARM_NONE_SIZEOF_NWFPE, ARM_NONE_SIZEOF_NWFPE,
+	&arm_none_fpregset, "FPA floating-point", cb_data);
+}
+
+/* Determine target description from core file.  */
+
+const struct target_desc *
+arm_none_core_read_description (struct gdbarch *gdbarch,
+				struct target_ops *target,
+				bfd *abfd)
+{
+  CORE_ADDR arm_hwcap = 0;
+  if (target_auxv_search (target, AT_HWCAP, &arm_hwcap) != 1)
+    return nullptr;
+
+  if (arm_hwcap & HWCAP_VFP)
+    {
+      /* NEON implies VFPv3-D32 or no-VFP unit.  Say that we only support
+         Neon with VFPv3-D32.  */
+      if (arm_hwcap & HWCAP_NEON)
+	return aarch32_read_description ();
+      else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3)
+	return arm_read_description (ARM_FP_TYPE_VFPV3);
+
+      return arm_read_description (ARM_FP_TYPE_VFPV2);
+    }
+
+  return nullptr;
+}
diff --git a/gdb/arm-none-tdep.h b/gdb/arm-none-tdep.h
new file mode 100644
index 0000000000..7927c0aff3
--- /dev/null
+++ b/gdb/arm-none-tdep.h
@@ -0,0 +1,60 @@
+/* none on ARM target support, prototypes.
+
+   Copyright (C) 2020 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/>.  */
+
+#ifndef ARM_NONE_TDEP_H
+#define ARM_NONE_TDEP_H
+
+#include "gdbarch.h"
+#include "target.h"
+#include "bfd.h"
+
+struct regset;
+struct regcache;
+
+void arm_none_supply_gregset (const struct regset *regset,
+			       struct regcache *regcache,
+			       int regnum, const void *gregs_buf, size_t len);
+void arm_none_collect_gregset (const struct regset *regset,
+				const struct regcache *regcache,
+				int regnum, void *gregs_buf, size_t len);
+
+void arm_none_supply_nwfpe_register (struct regcache *regcache, int regno,
+				     const gdb_byte *regs);
+void arm_none_collect_nwfpe_register (const struct regcache *regcache, int regno,
+				      gdb_byte *regs);
+
+void arm_none_supply_nwfpe (const struct regset *regset,
+			     struct regcache *regcache,
+			     int regnum, const void *regs_buf, size_t len);
+void arm_none_collect_nwfpe (const struct regset *regset,
+			      const struct regcache *regcache,
+			      int regnum, void *regs_buf, size_t len);
+
+void
+arm_none_iterate_over_regset_sections (struct gdbarch *gdbarch,
+                                       iterate_over_regset_sections_cb *cb,
+                                       void *cb_data,
+                                       const struct regcache *regcache);
+
+const struct target_desc *
+arm_none_core_read_description (struct gdbarch *gdbarch,
+				struct target_ops *target,
+				bfd *abfd);
+
+#endif /* ARM_NONE_TDEP_H */
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index a214f22d7a..be47cc2228 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -50,6 +50,8 @@
 #include "arch/arm.h"
 #include "arch/arm-get-next-pcs.h"
 #include "arm-tdep.h"
+#include "none-tdep.h"
+#include "arm-none-tdep.h"
 #include "gdb/sim-arm.h"
 
 #include "elf-bfd.h"
@@ -9453,6 +9455,12 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Virtual tables.  */
   set_gdbarch_vbit_in_delta (gdbarch, 1);
 
+  /* Default none core file support, can be overridden by osabi. */
+  none_init_corefile (info, gdbarch);
+  set_gdbarch_iterate_over_regset_sections (gdbarch,
+                                            arm_none_iterate_over_regset_sections);
+  set_gdbarch_core_read_description (gdbarch, arm_none_core_read_description);
+
   /* Hook in the ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
 
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index d865ecdcb6..f4a1540464 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -64,7 +64,8 @@ arc*-*-*)
 
 arm*-*-*)
 	cpu_obs="aarch32-tdep.o arch/aarch32.o arch/arm.o \
-		 arch/arm-get-next-pcs.o arm-tdep.o";;
+		 arch/arm-get-next-pcs.o arm-tdep.o \
+		 none-tdep.o arm-none-tdep.o";;
 
 hppa*-*-*)
 	# Target: HP PA-RISC
@@ -798,4 +799,7 @@ for t in x ${gdb_target_obs}; do
   if test "$t" = linux-tdep.o; then
     gdb_have_gcore=true
   fi
+  if test "$t" = arm-none-tdep.o; then
+    gdb_have_gcore=true
+  fi
 done
diff --git a/gdb/none-tdep.c b/gdb/none-tdep.c
new file mode 100644
index 0000000000..8cd139c500
--- /dev/null
+++ b/gdb/none-tdep.c
@@ -0,0 +1,231 @@
+/* Target-dependent code for none, architecture independent.
+
+   Copyright (C) 2020 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 "gdbtypes.h"
+#include "none-tdep.h"
+#include "auxv.h"
+#include "target.h"
+#include "gdbthread.h"
+#include "gdbcore.h"
+#include "regcache.h"
+#include "regset.h"
+#include "elf/common.h"
+#include "elf-bfd.h"            /* for elfcore_write_* */
+#include "inferior.h"
+#include "cli/cli-utils.h"
+#include "arch-utils.h"
+#include "gdb_obstack.h"
+#include "observable.h"
+#include "objfiles.h"
+#include "infcall.h"
+#include "gdbcmd.h"
+#include "gdb_regex.h"
+#include "gdbsupport/enum-flags.h"
+#include "gdbsupport/gdb_optional.h"
+
+#include <ctype.h>
+
+/* ---- based on linux-tdep.c ---- */
+
+/* Structure for passing information from
+   none_collect_thread_registers via an iterator to
+   none_collect_regset_section_cb. */
+
+struct none_collect_regset_section_cb_data
+{
+  struct gdbarch *gdbarch;
+  const struct regcache *regcache;
+  bfd *obfd;
+  char *note_data;
+  int *note_size;
+  unsigned long lwp;
+  enum gdb_signal stop_signal;
+  int abort_iteration;
+};
+
+/* Callback for iterate_over_regset_sections that records a single
+   regset in the corefile note section.  */
+
+static void
+none_collect_regset_section_cb (const char *sect_name, int supply_size,
+                                int collect_size, const struct regset *regset,
+                                const char *human_name, void *cb_data)
+{
+  struct none_collect_regset_section_cb_data *data
+    = (struct none_collect_regset_section_cb_data *) cb_data;
+  bool variable_size_section = (regset != NULL
+				&& regset->flags & REGSET_VARIABLE_SIZE);
+
+  if (!variable_size_section)
+    gdb_assert (supply_size == collect_size);
+
+  if (data->abort_iteration)
+    return;
+
+  gdb_assert (regset && regset->collect_regset);
+
+  /* This is intentionally zero-initialized by using std::vector, so
+     that any padding bytes in the core file will show as 0.  */
+  std::vector<gdb_byte> buf (collect_size);
+
+  regset->collect_regset (regset, data->regcache, -1, buf.data (),
+			  collect_size);
+
+  /* PRSTATUS still needs to be treated specially.  */
+  if (strcmp (sect_name, ".reg") == 0)
+    data->note_data = (char *) elfcore_write_prstatus
+      (data->obfd, data->note_data, data->note_size, data->lwp,
+       gdb_signal_to_host (data->stop_signal), buf.data ());
+  else
+    data->note_data = (char *) elfcore_write_register_note
+      (data->obfd, data->note_data, data->note_size,
+       sect_name, buf.data (), collect_size);
+
+  if (data->note_data == NULL)
+    data->abort_iteration = 1;
+}
+
+/* Records the thread's register state for the corefile note
+   section.  */
+
+static char *
+none_collect_thread_registers (const struct regcache *regcache,
+                               ptid_t ptid, bfd *obfd,
+                               char *note_data, int *note_size,
+                               enum gdb_signal stop_signal)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+  struct none_collect_regset_section_cb_data data;
+
+  data.gdbarch = gdbarch;
+  data.regcache = regcache;
+  data.obfd = obfd;
+  data.note_data = note_data;
+  data.note_size = note_size;
+  data.stop_signal = stop_signal;
+  data.abort_iteration = 0;
+
+  /* For remote targets the LWP may not be available, so use the TID.  */
+  data.lwp = ptid.lwp ();
+  if (!data.lwp)
+    data.lwp = ptid.tid ();
+
+  gdbarch_iterate_over_regset_sections (gdbarch,
+					none_collect_regset_section_cb,
+					&data, regcache);
+  return data.note_data;
+}
+
+/* Fills the "to_make_corefile_note" target vector.  Builds the note
+   section for a corefile, and returns it in a malloc buffer.  */
+
+char *
+none_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
+                          none_collect_thread_registers_ftype collect)
+{
+  char *note_data = NULL;
+
+  /* Process information.  */
+  if (get_exec_file (0))
+    {
+      const char *fname = lbasename (get_exec_file (0));
+      char *psargs = xstrdup (fname);
+
+      if (get_inferior_args ())
+        psargs = reconcat (psargs, psargs, " ", get_inferior_args (),
+			   (char *) NULL);
+
+      note_data = elfcore_write_prpsinfo (obfd, note_data, note_size,
+                                          fname, psargs);
+      xfree (psargs);
+    }
+
+  if (!note_data)
+    return NULL;
+
+  if (note_data)
+    {
+      ptid_t ptid;
+      struct regcache *regcache;
+      process_stratum_target *targ = NULL;
+      int global_id = 1;
+      struct thread_info *info = find_thread_global_id (global_id);
+      if (info)
+        {
+          ptid = info->ptid;
+          targ = info->inf->process_target ();
+        }
+      else
+        ptid = minus_one_ptid;
+      regcache = get_thread_arch_regcache (targ, ptid, gdbarch);
+
+      note_data = collect (regcache,
+			   ptid,
+			   obfd,
+			   note_data,
+			   note_size,
+			   GDB_SIGNAL_0);
+      if (!note_data)
+	return NULL;
+
+      /* Auxillary vector.  */
+      gdb::optional<gdb::byte_vector> auxv =
+        target_read_alloc (current_top_target (), TARGET_OBJECT_AUXV, NULL);
+      if (auxv && !auxv->empty ())
+        {
+          note_data = elfcore_write_note (obfd, note_data, note_size,
+                                          "CORE", NT_AUXV, auxv->data (),
+                                          auxv->size ());
+          
+          if (!note_data)
+            return NULL;
+        }
+    }
+
+  /* make_cleanup (xfree, note_data); */
+  return note_data;
+}
+
+static char *
+none_make_corefile_notes_1 (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
+{
+  return none_make_corefile_notes (gdbarch, obfd, note_size,
+                                   none_collect_thread_registers);
+}
+
+/* Setup default core file support for none targets.
+   Can be overridden later by OSABI.  */
+
+void
+none_init_corefile (struct gdbarch_info info,
+                    struct gdbarch *gdbarch)
+{
+  /* Default core file support.  */
+  set_gdbarch_make_corefile_notes (gdbarch, none_make_corefile_notes_1);
+}
+
+/* Provide a prototype to silence -Wmissing-prototypes.  */
+extern initialize_file_ftype _initialize_none_tdep;
+
+void
+_initialize_none_tdep (void)
+{
+  /* No special actions.  */
+}
diff --git a/gdb/none-tdep.h b/gdb/none-tdep.h
new file mode 100644
index 0000000000..4aea7004d8
--- /dev/null
+++ b/gdb/none-tdep.h
@@ -0,0 +1,39 @@
+/* Target-dependent code for none, architecture independent.
+
+   Copyright (C) 2020 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/>.  */
+
+#ifndef NONE_TDEP_H
+#define NONE_TDEP_H
+
+#include "bfd.h"
+
+struct regcache;
+
+typedef char *(*none_collect_thread_registers_ftype) (const struct regcache *,
+                                                      ptid_t,
+                                                      bfd *, char *, int *,
+                                                      enum gdb_signal);
+char *
+none_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
+                          none_collect_thread_registers_ftype collect);
+
+void
+none_init_corefile (struct gdbarch_info info,
+                    struct gdbarch *gdbarch);
+
+#endif /* none-tdep.h */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-20 11:41     ` Fredrik Hederstierna
@ 2020-10-20 12:39       ` Simon Marchi
  2020-10-20 14:00         ` Fredrik Hederstierna
  0 siblings, 1 reply; 44+ messages in thread
From: Simon Marchi @ 2020-10-20 12:39 UTC (permalink / raw)
  To: Fredrik Hederstierna, Paul Mathieu
  Cc: gdb-patches, Luis Machado, Alan Hayward


On 2020-10-20 7:41 a.m., Fredrik Hederstierna wrote:
> Hi all,
>
> I rebased and update my patch and it not run against GDB 10 / git-master, available at
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=14383
>
> I debugged my remote arm-none bare-metal Cortex-M4 target app over openocd/JTAG,
> and generate core-file:
> (gdb) gcore bare_metal.core
>
> Then later off-line on my host PC, I successfully debugged it with register, call-stack etc:
> $ arm-none-eabi-gdb my_bare_metal_app.elf bare_metal.core
>
> So this patch 'works', but I'm not sure if this is the correct direction to go,
> but could be a start.
> Besides this, next step could be to do this 'core-generator' tool that could be used for embedded targets, running on host PC.
>
> Please check the patch out, it builds fine with GDB-10/master on my machine.
> Any comments are appreciated!
>
> Thanks, BR Fredrik


Can you please send it on this list using git-send-email?  We don't do
patch review on bugzilla (and it's not really practical).

You can send it as a reply to your message (so it's threaded correctly)
by mentioning the Message-Id of your message:

  git send-email --in-reply-to AM6PR10MB21504CBD56291D9C255739E1EF1F0@AM6PR10MB2150.EURPRD10.PROD.OUTLOOK.COM <other args>

Simon

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-19 15:25   ` Paul Mathieu
@ 2020-10-20 11:41     ` Fredrik Hederstierna
  2020-10-20 12:39       ` Simon Marchi
  0 siblings, 1 reply; 44+ messages in thread
From: Fredrik Hederstierna @ 2020-10-20 11:41 UTC (permalink / raw)
  To: Paul Mathieu, Simon Marchi; +Cc: gdb-patches, Luis Machado, Alan Hayward

Hi all,

I rebased and update my patch and it not run against GDB 10 / git-master, available at

https://sourceware.org/bugzilla/show_bug.cgi?id=14383

I debugged my remote arm-none bare-metal Cortex-M4 target app over openocd/JTAG,
and generate core-file:
(gdb) gcore bare_metal.core

Then later off-line on my host PC, I successfully debugged it with register, call-stack etc:
$ arm-none-eabi-gdb my_bare_metal_app.elf bare_metal.core

So this patch 'works', but I'm not sure if this is the correct direction to go,
but could be a start.
Besides this, next step could be to do this 'core-generator' tool that could be used for embedded targets, running on host PC.

Please check the patch out, it builds fine with GDB-10/master on my machine.
Any comments are appreciated!

Thanks, BR Fredrik


From: Paul Mathieu <paulmathieu@google.com>
Sent: Monday, October 19, 2020 5:25 PM
To: Simon Marchi <simark@simark.ca>
Cc: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>; Luis Machado <luis.machado@linaro.org>
Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi 
 
Hi Simon, thanks for keeping this moving!


> > The idea then is to have some PC host supporting tool to convert/generate corefiles from some custom memory dump formats.
> > The FreeRTOS (or any other bare-metal alike OS) could maintain this supporting tool.
>
> Indeed.
>
> One question for you: when making GDB generate the core, I presume it
> would always have a single thread, as when debugging a bare-metal ARM
> processor with GDB, you see a single thread.
>
> Assuming you make that tool to convert a memory dump of a FreeRTOS
> system to a core GDB can read, would you make each FreeRTOS task appear
> as a separate thread in the core?

I assumed that we could use .reg/xxx note sections to dump task
information, haven't tested it though.
That would be something that generate-core would not be able to do
without support for the underlying OS. I imagine that we should be
able to add some fort of support for FreeRTOS?
My target runs RTX5, I'm not sure how widespread it is and if it is
worth adding support for. That said, as long as the core format is
specified and has support for multiple tasks (at least CPU regs),
collecting that information and formatting it shouldn't be too much
trouble on my end.


> Can you and Paul maybe sync up (and with Luis as well, probably) on what
> are the next steps?  I think your patch was a great start, but it would
> need to be rebased.  You could also look at Paul's patch to see if
> there's anything you'd like to pick from it.

Happy to. The patch that I proposed is quite minimal, the only nice
thing about it is that it builds on top of master.

Paul

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-19  2:08 ` Simon Marchi
  2020-10-19 13:13   ` Luis Machado
  2020-10-19 13:15   ` Alan Hayward
@ 2020-10-19 15:25   ` Paul Mathieu
  2020-10-20 11:41     ` Fredrik Hederstierna
  2 siblings, 1 reply; 44+ messages in thread
From: Paul Mathieu @ 2020-10-19 15:25 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Fredrik Hederstierna, gdb-patches, Luis Machado

Hi Simon, thanks for keeping this moving!


> > The idea then is to have some PC host supporting tool to convert/generate corefiles from some custom memory dump formats.
> > The FreeRTOS (or any other bare-metal alike OS) could maintain this supporting tool.
>
> Indeed.
>
> One question for you: when making GDB generate the core, I presume it
> would always have a single thread, as when debugging a bare-metal ARM
> processor with GDB, you see a single thread.
>
> Assuming you make that tool to convert a memory dump of a FreeRTOS
> system to a core GDB can read, would you make each FreeRTOS task appear
> as a separate thread in the core?

I assumed that we could use .reg/xxx note sections to dump task
information, haven't tested it though.
That would be something that generate-core would not be able to do
without support for the underlying OS. I imagine that we should be
able to add some fort of support for FreeRTOS?
My target runs RTX5, I'm not sure how widespread it is and if it is
worth adding support for. That said, as long as the core format is
specified and has support for multiple tasks (at least CPU regs),
collecting that information and formatting it shouldn't be too much
trouble on my end.


> Can you and Paul maybe sync up (and with Luis as well, probably) on what
> are the next steps?  I think your patch was a great start, but it would
> need to be rebased.  You could also look at Paul's patch to see if
> there's anything you'd like to pick from it.

Happy to. The patch that I proposed is quite minimal, the only nice
thing about it is that it builds on top of master.

Paul

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-19  2:08 ` Simon Marchi
  2020-10-19 13:13   ` Luis Machado
@ 2020-10-19 13:15   ` Alan Hayward
  2020-10-19 15:25   ` Paul Mathieu
  2 siblings, 0 replies; 44+ messages in thread
From: Alan Hayward @ 2020-10-19 13:15 UTC (permalink / raw)
  To: Simon Marchi
  Cc: Fredrik Hederstierna, gdb-patches, Paul Mathieu, Luis Machado



> On 19 Oct 2020, at 03:08, Simon Marchi <simark@simark.ca> wrote:
> 
> On 2020-10-16 8:02 p.m., Fredrik Hederstierna via Gdb-patches wrote:
>> Hi
>> 
>> I saw that recently there was new interest of corefile support for arm-none-eabi.
> 
> For others, Fredrik is referring to this other thread with the same subject:
> 
>  https://sourceware.org/pipermail/gdb-patches/2020-October/172258.html
> 
> In this other thread, we deviated from the main subject trying to figure
> out how GDB can differentiate a Linux executable from a bare-metal
> executable.  Maybe we can just ignore that for the moment and get
> something simple but useful merged.  This problem won't occur if someone
> is using a toolchain built with --target=arm-none-eabi.  And if GDB is
> built with support for the Linux osabi, then the user will just need to
> do "set osabi none" to override it.  It seems to me like it's better to
> have that than no core support at all for arm-none-eabi.
> 
>> In the past I have tried to raise interest of this several times, but with limited success unfortunately,
>> so I am happy that possible there could be an opening to get this support into GDB,
>> and I would like to take to opportunity to also try push some more for GDB maintainers to try get support for this very useful feature.
> 
> Indeed, it's apparently something that comes up often, I'm all for
> it.  Let's try to get it to work this time :).

+1

> 
>> I already tried to push in the past for my own patch that also support eg floating-point support, and gcore etc.
>> The patch is using linux core file format as starting point but has stripped out Linux specific parts.
>> 
>> See
>> https://sourceware.org/bugzilla/show_bug.cgi?id=14383
>> 
>> The GDB verision at the time was GDB-7.11.1 so it may be out-of-date.
>> (The post in mail-thread:  https://sourceware.org/pipermail/gdb/2014-September/044559.html)
> 
> I skimmed your patch, and I see that you implemented the
> support for "generate-core-file", which is awesome.  It's one of the
> comments I had on Paul's patch.

I would just add that arm_none_core_read_description will need updating
to use the newer tdesc code. Possibly identical to arm_linux_core_read_description.


> 
>> If there is interest of adding this feature now, I could also try help to get this feature into GDB.
>> 
>> I also believe that there is some need to 'formalize' the format, and my best idea so far is to try adding corefile to some popular 'bare metal' target RTOS.
>> I've been thinking of defining a format for FreeRTOS, but basically being a bare-metal target.
> 
> I think it would make sense first to make GDB able to generate cores
> (with generate-core-file) and consume them.  It makes it much easier to
> test than if we have to rely on an external tool (plus, it's useful).
> 
> In theory, it should be able to generate a core while connected to the
> GDB sim target, which means that everyone can do it, no special hardware
> or tool required.
> 
> Once this is done, it should be easy to go to projects like FreeRTOS and
> suggest adding things like that.
> 
>> The idea then is to have some PC host supporting tool to convert/generate corefiles from some custom memory dump formats.
>> The FreeRTOS (or any other bare-metal alike OS) could maintain this supporting tool.
> 
> Indeed.
> 
> One question for you: when making GDB generate the core, I presume it
> would always have a single thread, as when debugging a bare-metal ARM
> processor with GDB, you see a single thread.
> 
> Assuming you make that tool to convert a memory dump of a FreeRTOS
> system to a core GDB can read, would you make each FreeRTOS task appear
> as a separate thread in the core?
> 
>> Here is one example what I investigated, a similar PC host conversion app that could possibly be basis of such tool, example:
>> https://github.com/rogercollins/bare_core
>> 
>> I think next step is to define/decide a format that would be accepted by GDB maintainers, eg FreeRTOS-bare-metal or something,
>> then work in parallel with some supporting host PC tool, but maybe this should not be part of GDB itself?
>> Any comments or ideas are most welcome!
> 
> As I said, I think the first logical step is to make GDB able to
> generate cores and consume them.  The "Linux format minus the
> Linux-specific parts" format sounds good to me, but you would need to
> specify what are those Linux-specific parts that you removed, for
> clarity.

I’d like to request the core file format is documented somewhere.

Best place I think would be in the GDB manual.
Probably by adding an arm-none section in G.5.3 ARM Features.

(no need to do this until something is agreed on though)

> 
> Can you and Paul maybe sync up (and with Luis as well, probably) on what
> are the next steps?  I think your patch was a great start, but it would
> need to be rebased.  You could also look at Paul's patch to see if
> there's anything you'd like to pick from it.
> 
> I'll be happy to give it a try with my NorthSec conference badge [1].
> It runs a Cortex-M0 that we can debug using a Black Magic probe [2], so
> I think it's a good real-life example.  Plus, I made it run FreeRTOS, so
> it would be a good candidate for that too.
> 
> Simon
> 
> [1] https://montrehack.ca/images/19-09_nsec_badge.jpg
> [2] https://github.com/blacksphere/blackmagic/wiki


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-19  2:08 ` Simon Marchi
@ 2020-10-19 13:13   ` Luis Machado
  2020-10-19 13:15   ` Alan Hayward
  2020-10-19 15:25   ` Paul Mathieu
  2 siblings, 0 replies; 44+ messages in thread
From: Luis Machado @ 2020-10-19 13:13 UTC (permalink / raw)
  To: Simon Marchi, Fredrik Hederstierna, gdb-patches, Paul Mathieu

Hi Simon, Fredrik,

As was discussed in a private thread, it would be ideal for you 
(Fredrik) and Paul to sync things up and have a draft of a format that 
works for both, then we can go back to trying to fit it in GDB.

I'm guessing we'll want to make it generic enough to accomodate other 
architectures etc, just like the Linux core file format.

Alan and I can provide some feedback from ARM's side, and others can 
chime in for other architectures if needed.

If you'd like to discuss the format itself, I guess the gdb@ mailing 
list would be the place to have a chat before we turn it into a proper 
patch.

On 10/18/20 11:08 PM, Simon Marchi wrote:
> On 2020-10-16 8:02 p.m., Fredrik Hederstierna via Gdb-patches wrote:
>> Hi
>>
>> I saw that recently there was new interest of corefile support for arm-none-eabi.
> 
> For others, Fredrik is referring to this other thread with the same subject:
> 
>    https://sourceware.org/pipermail/gdb-patches/2020-October/172258.html
> 
> In this other thread, we deviated from the main subject trying to figure
> out how GDB can differentiate a Linux executable from a bare-metal
> executable.  Maybe we can just ignore that for the moment and get
> something simple but useful merged.  This problem won't occur if someone
> is using a toolchain built with --target=arm-none-eabi.  And if GDB is
> built with support for the Linux osabi, then the user will just need to
> do "set osabi none" to override it.  It seems to me like it's better to
> have that than no core support at all for arm-none-eabi.
> 
>> In the past I have tried to raise interest of this several times, but with limited success unfortunately,
>> so I am happy that possible there could be an opening to get this support into GDB,
>> and I would like to take to opportunity to also try push some more for GDB maintainers to try get support for this very useful feature.
> 
> Indeed, it's apparently something that comes up often, I'm all for
> it.  Let's try to get it to work this time :).
> 
>> I already tried to push in the past for my own patch that also support eg floating-point support, and gcore etc.
>> The patch is using linux core file format as starting point but has stripped out Linux specific parts.
>>
>> See
>> https://sourceware.org/bugzilla/show_bug.cgi?id=14383
>>
>> The GDB verision at the time was GDB-7.11.1 so it may be out-of-date.
>> (The post in mail-thread:  https://sourceware.org/pipermail/gdb/2014-September/044559.html)
> 
> I skimmed your patch, and I see that you implemented the
> support for "generate-core-file", which is awesome.  It's one of the
> comments I had on Paul's patch.
> 
>> If there is interest of adding this feature now, I could also try help to get this feature into GDB.
>>
>> I also believe that there is some need to 'formalize' the format, and my best idea so far is to try adding corefile to some popular 'bare metal' target RTOS.
>> I've been thinking of defining a format for FreeRTOS, but basically being a bare-metal target.
> 
> I think it would make sense first to make GDB able to generate cores
> (with generate-core-file) and consume them.  It makes it much easier to
> test than if we have to rely on an external tool (plus, it's useful).
> 
> In theory, it should be able to generate a core while connected to the
> GDB sim target, which means that everyone can do it, no special hardware
> or tool required.
> 
> Once this is done, it should be easy to go to projects like FreeRTOS and
> suggest adding things like that.
> 
>> The idea then is to have some PC host supporting tool to convert/generate corefiles from some custom memory dump formats.
>> The FreeRTOS (or any other bare-metal alike OS) could maintain this supporting tool.
> 
> Indeed.
> 
> One question for you: when making GDB generate the core, I presume it
> would always have a single thread, as when debugging a bare-metal ARM
> processor with GDB, you see a single thread.
> 
> Assuming you make that tool to convert a memory dump of a FreeRTOS
> system to a core GDB can read, would you make each FreeRTOS task appear
> as a separate thread in the core?
> 
>> Here is one example what I investigated, a similar PC host conversion app that could possibly be basis of such tool, example:
>> https://github.com/rogercollins/bare_core
>>
>> I think next step is to define/decide a format that would be accepted by GDB maintainers, eg FreeRTOS-bare-metal or something,
>> then work in parallel with some supporting host PC tool, but maybe this should not be part of GDB itself?
>> Any comments or ideas are most welcome!
> 
> As I said, I think the first logical step is to make GDB able to
> generate cores and consume them.  The "Linux format minus the
> Linux-specific parts" format sounds good to me, but you would need to
> specify what are those Linux-specific parts that you removed, for
> clarity.
> 
> Can you and Paul maybe sync up (and with Luis as well, probably) on what
> are the next steps?  I think your patch was a great start, but it would
> need to be rebased.  You could also look at Paul's patch to see if
> there's anything you'd like to pick from it.
> 
> I'll be happy to give it a try with my NorthSec conference badge [1].
> It runs a Cortex-M0 that we can debug using a Black Magic probe [2], so
> I think it's a good real-life example.  Plus, I made it run FreeRTOS, so
> it would be a good candidate for that too.
> 
> Simon
> 
> [1] https://montrehack.ca/images/19-09_nsec_badge.jpg
> [2] https://github.com/blacksphere/blackmagic/wiki
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
  2020-10-17  0:02 Fredrik Hederstierna
@ 2020-10-19  2:08 ` Simon Marchi
  2020-10-19 13:13   ` Luis Machado
                     ` (2 more replies)
  0 siblings, 3 replies; 44+ messages in thread
From: Simon Marchi @ 2020-10-19  2:08 UTC (permalink / raw)
  To: Fredrik Hederstierna, gdb-patches, Paul Mathieu, Luis Machado

On 2020-10-16 8:02 p.m., Fredrik Hederstierna via Gdb-patches wrote:
> Hi
>
> I saw that recently there was new interest of corefile support for arm-none-eabi.

For others, Fredrik is referring to this other thread with the same subject:

  https://sourceware.org/pipermail/gdb-patches/2020-October/172258.html

In this other thread, we deviated from the main subject trying to figure
out how GDB can differentiate a Linux executable from a bare-metal
executable.  Maybe we can just ignore that for the moment and get
something simple but useful merged.  This problem won't occur if someone
is using a toolchain built with --target=arm-none-eabi.  And if GDB is
built with support for the Linux osabi, then the user will just need to
do "set osabi none" to override it.  It seems to me like it's better to
have that than no core support at all for arm-none-eabi.

> In the past I have tried to raise interest of this several times, but with limited success unfortunately,
> so I am happy that possible there could be an opening to get this support into GDB,
> and I would like to take to opportunity to also try push some more for GDB maintainers to try get support for this very useful feature.

Indeed, it's apparently something that comes up often, I'm all for
it.  Let's try to get it to work this time :).

> I already tried to push in the past for my own patch that also support eg floating-point support, and gcore etc.
> The patch is using linux core file format as starting point but has stripped out Linux specific parts.
>
> See
> https://sourceware.org/bugzilla/show_bug.cgi?id=14383
>
> The GDB verision at the time was GDB-7.11.1 so it may be out-of-date.
> (The post in mail-thread:  https://sourceware.org/pipermail/gdb/2014-September/044559.html)

I skimmed your patch, and I see that you implemented the
support for "generate-core-file", which is awesome.  It's one of the
comments I had on Paul's patch.

> If there is interest of adding this feature now, I could also try help to get this feature into GDB.
>
> I also believe that there is some need to 'formalize' the format, and my best idea so far is to try adding corefile to some popular 'bare metal' target RTOS.
> I've been thinking of defining a format for FreeRTOS, but basically being a bare-metal target.

I think it would make sense first to make GDB able to generate cores
(with generate-core-file) and consume them.  It makes it much easier to
test than if we have to rely on an external tool (plus, it's useful).

In theory, it should be able to generate a core while connected to the
GDB sim target, which means that everyone can do it, no special hardware
or tool required.

Once this is done, it should be easy to go to projects like FreeRTOS and
suggest adding things like that.

> The idea then is to have some PC host supporting tool to convert/generate corefiles from some custom memory dump formats.
> The FreeRTOS (or any other bare-metal alike OS) could maintain this supporting tool.

Indeed.

One question for you: when making GDB generate the core, I presume it
would always have a single thread, as when debugging a bare-metal ARM
processor with GDB, you see a single thread.

Assuming you make that tool to convert a memory dump of a FreeRTOS
system to a core GDB can read, would you make each FreeRTOS task appear
as a separate thread in the core?

> Here is one example what I investigated, a similar PC host conversion app that could possibly be basis of such tool, example:
> https://github.com/rogercollins/bare_core
>
> I think next step is to define/decide a format that would be accepted by GDB maintainers, eg FreeRTOS-bare-metal or something,
> then work in parallel with some supporting host PC tool, but maybe this should not be part of GDB itself?
> Any comments or ideas are most welcome!

As I said, I think the first logical step is to make GDB able to
generate cores and consume them.  The "Linux format minus the
Linux-specific parts" format sounds good to me, but you would need to
specify what are those Linux-specific parts that you removed, for
clarity.

Can you and Paul maybe sync up (and with Luis as well, probably) on what
are the next steps?  I think your patch was a great start, but it would
need to be rebased.  You could also look at Paul's patch to see if
there's anything you'd like to pick from it.

I'll be happy to give it a try with my NorthSec conference badge [1].
It runs a Cortex-M0 that we can debug using a Black Magic probe [2], so
I think it's a good real-life example.  Plus, I made it run FreeRTOS, so
it would be a good candidate for that too.

Simon

[1] https://montrehack.ca/images/19-09_nsec_badge.jpg
[2] https://github.com/blacksphere/blackmagic/wiki

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] gdb: add support for handling core dumps on arm-none-eabi
@ 2020-10-17  0:02 Fredrik Hederstierna
  2020-10-19  2:08 ` Simon Marchi
  0 siblings, 1 reply; 44+ messages in thread
From: Fredrik Hederstierna @ 2020-10-17  0:02 UTC (permalink / raw)
  To: gdb-patches\@sourceware.org

Hi

I saw that recently there was new interest of corefile support for arm-none-eabi.

In the past I have tried to raise interest of this several times, but with limited success unfortunately,
so I am happy that possible there could be an opening to get this support into GDB,
and I would like to take to opportunity to also try push some more for GDB maintainers to try get support for this very useful feature.

I already tried to push in the past for my own patch that also support eg floating-point support, and gcore etc.
The patch is using linux core file format as starting point but has stripped out Linux specific parts.

See
https://sourceware.org/bugzilla/show_bug.cgi?id=14383

The GDB verision at the time was GDB-7.11.1 so it may be out-of-date.
(The post in mail-thread:  https://sourceware.org/pipermail/gdb/2014-September/044559.html)

If there is interest of adding this feature now, I could also try help to get this feature into GDB.

I also believe that there is some need to 'formalize' the format, and my best idea so far is to try adding corefile to some popular 'bare metal' target RTOS.
I've been thinking of defining a format for FreeRTOS, but basically being a bare-metal target.

The idea then is to have some PC host supporting tool to convert/generate corefiles from some custom memory dump formats.
The FreeRTOS (or any other bare-metal alike OS) could maintain this supporting tool.

Here is one example what I investigated, a similar PC host conversion app that could possibly be basis of such tool, example:
https://github.com/rogercollins/bare_core

I think next step is to define/decide a format that would be accepted by GDB maintainers, eg FreeRTOS-bare-metal or something,
then work in parallel with some supporting host PC tool, but maybe this should not be part of GDB itself?
Any comments or ideas are most welcome!

Thanks, Kindly,
Fredrik Hederstierna

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2021-06-22  2:16 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-02 17:32 [PATCH] gdb: add support for handling core dumps on arm-none-eabi Paul Mathieu
2020-10-02 17:51 ` Luis Machado
2020-10-02 21:54   ` Paul Mathieu
2020-10-02 21:59     ` Simon Marchi
2020-10-03  3:57     ` Simon Marchi
2020-10-03 18:14       ` [PATCH v2] " Paul Mathieu
2020-10-04 17:30         ` Paul Mathieu
2020-10-04 23:41         ` Simon Marchi
2020-10-06  4:32           ` Paul Mathieu
2020-10-06 12:45             ` Luis Machado
2020-10-06 14:29               ` Simon Marchi
2020-10-06 16:59                 ` Paul Mathieu
2020-10-06 17:37                   ` Simon Marchi
2020-10-05 12:58         ` Luis Machado
2020-10-05 13:24           ` Alan Hayward
2020-10-02 23:55 ` [PATCH] " Simon Marchi
2020-10-03  0:35   ` Paul Mathieu
2020-10-03  2:24     ` Simon Marchi
2020-10-17  0:02 Fredrik Hederstierna
2020-10-19  2:08 ` Simon Marchi
2020-10-19 13:13   ` Luis Machado
2020-10-19 13:15   ` Alan Hayward
2020-10-19 15:25   ` Paul Mathieu
2020-10-20 11:41     ` Fredrik Hederstierna
2020-10-20 12:39       ` Simon Marchi
2020-10-20 14:00         ` Fredrik Hederstierna
2020-10-20 15:04           ` Simon Marchi
2020-10-20 22:05             ` Fredrik Hederstierna
2020-10-20 23:06               ` Simon Marchi
2020-10-22  0:52                 ` Fredrik Hederstierna
2020-10-22  1:24                   ` Simon Marchi
2020-10-22  1:49                   ` Simon Marchi
2020-10-22 22:32                     ` Fredrik Hederstierna
2020-10-23  0:37                       ` Simon Marchi
2020-10-25 21:06                         ` Fredrik Hederstierna
2020-10-26 11:24                           ` Luis Machado
2020-10-26 15:49                             ` Fredrik Hederstierna
2020-10-27 16:53                               ` Paul Mathieu
2021-01-14 12:36                                 ` Fredrik Hederstierna
2021-01-14 12:50                                   ` Luis Machado
2021-01-18 11:09                                     ` Andrew Burgess
2021-01-18 14:01                                       ` Luis Machado
2021-01-18 11:01                                   ` Andrew Burgess
2021-06-22  2:16                           ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).