Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi

Simon Marchi
2020-10-03T03:57:07Z

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