Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support

Andrew Burgess
2020-12-07T15:17:10Z

* Luis Machado <luis.machado@linaro.org> [2020-12-02 15:12:26 -0300]:

> CC-ing paulmathieu@google.com and fredrik.hederstierna@verisure.com, who
> were also looking into supporting bare metal ARM core files.
>
> Just a quick note...
>
> Back in October we pondered about this and it looked reasonable, at the
> time, to put some effort into documenting the format (unlike the current
> Linux core file format, which lacks good documentation).
>
> My take on it is that we need to settle on a format that works for multiple
> architectures.

Hi Luis,

Thanks for your feedback, I'd love to better understand what you are
proposing here.

Could you expand a little on why we need a format that supports
multiple architectures (i.e. what benefits will this bring)? And how
this would be different to the approach taken here?

Thanks,
Andrew

>
> On 12/2/20 2:39 PM, Andrew Burgess wrote:
> > This commit adds the ability for bare metal RISC-V target to generate
> > core files from within GDB.
> >
> > The intended use case is that a user will connect to a remote bare
> > metal target, debug up to some error condition, then generate a core
> > file in the normal way using:
> >
> > (gdb) generate-core-file
> >
> > This core file can then be used to revisit the state of the remote
> > target without having to reconnect to the remote target.
> >
> > The core file creation is all placed into a new file
> > riscv-none-tdep.c, this follows the existing naming pattern as
> > riscv-linux-tdep.c, and riscv-fbsd-tdep.c, where 'none' is the ABI
> > name.
> >
> > Currently only the x-regs and f-regs (if present) are written out, and
> > the formatting follows the Linux layout, rather than inventing yet
> > another layout. Future commits will add support for writing out the
> > CSRs.
> >
> > gdb/ChangeLog:
> >
> > * Makefile.in (ALL_TARGET_OBS): Add riscv-none-tdep.o.
> > (ALLDEPFILES): Add riscv-none-tdep.c.
> > * configure.tgt (riscv*-*-*): Include riscv-none-tdep.c.
> > * riscv-none-tdep.c: New file.
> > ---
> > gdb/ChangeLog | 8 +
> > gdb/Makefile.in | 2 +
> > gdb/configure.tgt | 2 +-
> > gdb/riscv-none-tdep.c | 345 ++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 356 insertions(+), 1 deletion(-)
> > create mode 100644 gdb/riscv-none-tdep.c
> >
> > diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> > index a86e8d648e6..f515670d03d 100644
> > --- a/gdb/Makefile.in
> > +++ b/gdb/Makefile.in
> > @@ -807,6 +807,7 @@ ALL_TARGET_OBS = \
> > ravenscar-thread.o \
> > riscv-fbsd-tdep.o \
> > riscv-linux-tdep.o \
> > + riscv-none-tdep.o \
> > riscv-ravenscar-thread.o \
> > riscv-tdep.o \
> > rl78-tdep.o \
> > @@ -2269,6 +2270,7 @@ ALLDEPFILES = \
> > riscv-fbsd-tdep.c \
> > riscv-linux-nat.c \
> > riscv-linux-tdep.c \
> > + riscv-none-tdep.c \
> > riscv-ravenscar-thread.c \
> > riscv-tdep.c \
> > rl78-tdep.c \
> > diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> > index 6e039838748..ad88ddd9302 100644
> > --- a/gdb/configure.tgt
> > +++ b/gdb/configure.tgt
> > @@ -85,7 +85,7 @@ ia64*-*-*)
> > ;;
> > riscv*-*-*)
> > - cpu_obs="riscv-tdep.o arch/riscv.o \
> > + cpu_obs="riscv-tdep.o riscv-none-tdep.o arch/riscv.o \
> > ravenscar-thread.o riscv-ravenscar-thread.o";;
> > x86_64-*-*)
> > diff --git a/gdb/riscv-none-tdep.c b/gdb/riscv-none-tdep.c
> > new file mode 100644
> > index 00000000000..0a4215a60e9
> > --- /dev/null
> > +++ b/gdb/riscv-none-tdep.c
> > @@ -0,0 +1,345 @@
> > +/* 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/>. */
> > +
> > +/* This file contain code that is specific for bare-metal RISC-V targets. */
> > +
> > +#include "defs.h"
> > +#include "inferior.h"
> > +#include "gdbcore.h"
> > +#include "target.h"
> > +#include "arch-utils.h"
> > +#include "regcache.h"
> > +#include "osabi.h"
> > +#include "riscv-tdep.h"
> > +#include "elf-bfd.h"
> > +#include "regset.h"
> > +
> > +/* Function declarations. */
> > +
> > +static void riscv_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);
> > +
> > +/* Function Definitions. */
> > +
> > +/* Called to figure out a target description for the corefile being read.
> > + If we get here then the corefile didn't have a target description
> > + embedded inside it, so we need to figure out a default description
> > + based just on the properties of the corefile itself. */
> > +
> > +static const struct target_desc *
> > +riscv_core_read_description (struct gdbarch *gdbarch,
> > + struct target_ops *target,
> > + bfd *abfd)
> > +{
> > + error (_("unable to figure out target description for RISC-V core files"));
> > + return nullptr;
> > +}
> > +
> > +/* Determine which signal stopped execution. */
> > +
> > +static int
> > +find_signalled_thread (struct thread_info *info, void *data)
> > +{
> > + if (info->suspend.stop_signal != GDB_SIGNAL_0
> > + && info->ptid.pid () == inferior_ptid.pid ())
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > +/* Structure for passing information from riscv_corefile_thread via an
> > + iterator to riscv_collect_regset_section_cb. */
> > +
> > +struct riscv_collect_regset_section_cb_data
> > +{
> > + riscv_collect_regset_section_cb_data
> > + (struct gdbarch *gdbarch, const struct regcache *regcache,
> > + ptid_t ptid, bfd *obfd, enum gdb_signal stop_signal,
> > + gdb::unique_xmalloc_ptr<char> *note_data, int *note_size)
> > + : gdbarch (gdbarch), regcache (regcache), obfd (obfd),
> > + note_data (note_data), note_size (note_size),
> > + stop_signal (stop_signal)
> > + {
> > + /* The LWP is often not available for bare metal target, in which case
> > + use the tid instead. */
> > + if (ptid.lwp_p ())
> > + lwp = ptid.lwp ();
> > + else
> > + lwp = ptid.tid ();
> > + }
> > +
> > + struct gdbarch *gdbarch;
> > + const struct regcache *regcache;
> > + bfd *obfd;
> > + gdb::unique_xmalloc_ptr<char> *note_data;
> > + int *note_size;
> > + long lwp;
> > + enum gdb_signal stop_signal;
> > + bool abort_iteration = false;
> > +};
> > +
> > +/* Records information about the single thread INFO into *NOTE_DATA, and
> > + updates *NOTE_SIZE. OBFD is the core file being generated. GDBARCH is
> > + the architecture the core file is being created for. */
> > +
> > +static void
> > +riscv_corefile_thread (struct gdbarch *gdbarch, bfd *obfd,
> > + struct thread_info *info,
> > + gdb::unique_xmalloc_ptr<char> *note_data,
> > + int *note_size)
> > +{
> > + struct regcache *regcache
> > + = get_thread_arch_regcache (info->inf->process_target (),
> > + info->ptid, gdbarch);
> > +
> > + /* Ideally we should be able to read all of the registers known to this
> > + target. Unfortunately, sometimes targets advertise CSRs that can't be
> > + read. We don't want these registers to prevent a core file being
> > + dumped, so we fetch the registers one by one here, and ignore any
> > + errors. This does mean that the register will show up as zero in the
> > + core dump, which might be confusing, but probably better than being
> > + unable to dump a core file. */
> > + for (int regnum = 0; regnum < gdbarch_num_regs (gdbarch); ++regnum)
> > + {
> > + try
> > + {
> > + target_fetch_registers (regcache, regnum);
> > + }
> > + catch (const gdb_exception_error &e)
> > + { /* Ignore errors. */ }
> > + }
> > +
> > + /* Call riscv_collect_regset_section_cb for each regset, passing in the
> > + DATA object. Appends the core file notes to *(DATA->NOTE_DATA) to
> > + describe all the registers in this thread. */
> > + riscv_collect_regset_section_cb_data data (gdbarch, regcache, info->ptid,
> > + obfd, info->suspend.stop_signal,
> > + note_data, note_size);
> > + gdbarch_iterate_over_regset_sections (gdbarch,
> > + riscv_collect_regset_section_cb,
> > + &data, regcache);
> > +}
> > +
> > +/* Build the note section for a corefile, and return it in a malloc
> > + buffer. Currently this just dumps all available registers for each
> > + thread. */
> > +
> > +static gdb::unique_xmalloc_ptr<char>
> > +riscv_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
> > +{
> > + if (!gdbarch_iterate_over_regset_sections_p (gdbarch))
> > + return NULL;
> > +
> > + /* Data structures into which we accumulate the core file notes. */
> > + gdb::unique_xmalloc_ptr<char> note_data;
> > +
> > + /* Add note information about the executable and its arguments. */
> > + char fname[16] = {'\0'};
> > + char psargs[80] = {'\0'};
> > + if (get_exec_file (0))
> > + {
> > + strncpy (fname, lbasename (get_exec_file (0)), sizeof (fname));
> > + fname[sizeof (fname) - 1] = 0;
> > + strncpy (psargs, get_exec_file (0), sizeof (psargs));
> > + psargs[sizeof (psargs) - 1] = 0;
> > +
> > + const char *inf_args = get_inferior_args ();
> > + if (inf_args != nullptr && *inf_args != '\0'
> > + && (strlen (inf_args)
> > + < ((int) sizeof (psargs) - (int) strlen (psargs))))
> > + {
> > + strncat (psargs, " ",
> > + sizeof (psargs) - strlen (psargs));
> > + strncat (psargs, inf_args,
> > + sizeof (psargs) - strlen (psargs));
> > + }
> > +
> > + psargs[sizeof (psargs) - 1] = 0;
> > + }
> > + note_data.reset (elfcore_write_prpsinfo (obfd, note_data.release (),
> > + note_size, fname,
> > + psargs));
> > +
> > + /* Update our understanding of the available threads. */
> > + try
> > + {
> > + update_thread_list ();
> > + }
> > + catch (const gdb_exception_error &e)
> > + {
> > + exception_print (gdb_stderr, e);
> > + }
> > +
> > + /* As we do in linux-tdep.c, prefer dumping the signalled thread first.
> > + The "first thread" is what tools use to infer the signalled thread.
> > + In case there's more than one signalled thread, prefer the current
> > + thread, if it is signalled. */
> > + thread_info *signalled_thr, *curr_thr = inferior_thread ();
> > + if (curr_thr->suspend.stop_signal != GDB_SIGNAL_0)
> > + signalled_thr = curr_thr;
> > + else
> > + {
> > + signalled_thr = iterate_over_threads (find_signalled_thread, nullptr);
> > + if (signalled_thr == nullptr)
> > + signalled_thr = curr_thr;
> > + }
> > +
> > + /* First add information about the signalled thread, then add information
> > + about all the other threads, see above for the reasoning. */
> > + riscv_corefile_thread (gdbarch, obfd, signalled_thr, &note_data, note_size);
> > + for (thread_info *thr : current_inferior ()->non_exited_threads ())
> > + {
> > + if (thr == signalled_thr)
> > + continue;
> > + riscv_corefile_thread (gdbarch, obfd, signalled_thr, &note_data,
> > + note_size);
> > + }
> > +
> > + return note_data;
> > +}
> > +
> > +/* Define the general register mapping. This follows the same format as
> > + the RISC-V linux corefile. The linux kernel puts the PC at offset 0,
> > + gdb puts it at offset 32. Register x0 is always 0 and can be ignored.
> > + Registers x1 to x31 are in the same place. */
> > +
> > +static const struct regcache_map_entry riscv_gregmap[] =
> > +{
> > + { 1, RISCV_PC_REGNUM, 0 },
> > + { 31, RISCV_RA_REGNUM, 0 }, /* x1 to x31 */
> > + { 0 }
> > +};
> > +
> > +/* Define the FP register mapping. This follows the same format as the
> > + RISC-V linux corefile. The kernel puts the 32 FP regs first, and then
> > + FCSR. */
> > +
> > +static const struct regcache_map_entry riscv_fregmap[] =
> > +{
> > + { 32, RISCV_FIRST_FP_REGNUM, 0 },
> > + { 1, RISCV_CSR_FCSR_REGNUM, 0 },
> > + { 0 }
> > +};
> > +
> > +/* Define the general register regset. */
> > +
> > +static const struct regset riscv_gregset =
> > +{
> > + riscv_gregmap, riscv_supply_regset, regcache_collect_regset
> > +};
> > +
> > +/* Define the FP register regset. */
> > +
> > +static const struct regset riscv_fregset =
> > +{
> > + riscv_fregmap, riscv_supply_regset, regcache_collect_regset
> > +};
> > +
> > +/* Callback for iterate_over_regset_sections that records a single regset
> > + in the corefile note section. */
> > +
> > +static void
> > +riscv_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)
> > +{
> > + riscv_collect_regset_section_cb_data *data
> > + = (riscv_collect_regset_section_cb_data *) cb_data;
> > +
> > + /* The only flag is REGSET_VARIABLE_SIZE, and we don't use that. */
> > + gdb_assert (regset->flags == 0);
> > + gdb_assert (supply_size == collect_size);
> > +
> > + if (data->abort_iteration)
> > + return;
> > +
> > + gdb_assert (regset != nullptr && regset->collect_regset);
> > +
> > + /* This is intentionally zero-initialized by using std::vector, so that
> > + any padding bytes in the core file will show a zero. */
> > + 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->reset (elfcore_write_prstatus
> > + (data->obfd, data->note_data->release (),
> > + data->note_size, data->lwp,
> > + gdb_signal_to_host (data->stop_signal),
> > + buf.data ()));
> > + else
> > + data->note_data->reset (elfcore_write_register_note
> > + (data->obfd, data->note_data->release (),
> > + data->note_size,
> > + sect_name, buf.data (), collect_size));
> > +
> > + if (data->note_data->get () == NULL)
> > + data->abort_iteration = true;
> > +}
> > +
> > +/* Implement the "iterate_over_regset_sections" gdbarch method. */
> > +
> > +static void
> > +riscv_iterate_over_regset_sections (struct gdbarch *gdbarch,
> > + iterate_over_regset_sections_cb *cb,
> > + void *cb_data,
> > + const struct regcache *regcache)
> > +{
> > + /* Write out the GPRs. */
> > + int sz = 32 * riscv_isa_xlen (gdbarch);
> > + cb (".reg", sz, sz, &riscv_gregset, NULL, cb_data);
> > +
> > + /* Write out the FPRs, but only if present. */
> > + if (riscv_isa_flen (gdbarch) > 0)
> > + {
> > + sz = (32 * riscv_isa_flen (gdbarch)
> > + + register_size (gdbarch, RISCV_CSR_FCSR_REGNUM));
> > + cb (".reg2", sz, sz, &riscv_fregset, NULL, cb_data);
> > + }
> > +}
> > +
> > +/* Initialize RISC-V bare-metal ABI info. */
> > +
> > +static void
> > +riscv_none_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> > +{
> > + /* Find or create a target description from a core file. */
> > + set_gdbarch_core_read_description (gdbarch, riscv_core_read_description);
> > +
> > + /* How to create a core file for bare metal RISC-V. */
> > + set_gdbarch_make_corefile_notes (gdbarch, riscv_make_corefile_notes);
> > +
> > + /* Iterate over registers for reading and writing bare metal RISC-V core
> > + files. */
> > + set_gdbarch_iterate_over_regset_sections
> > + (gdbarch, riscv_iterate_over_regset_sections);
> > +
> > +}
> > +
> > +/* Initialize RISC-V bare-metal target support. */
> > +
> > +void _initialize_riscv_none_tdep ();
> > +void
> > +_initialize_riscv_none_tdep ()
> > +{
> > + gdbarch_register_osabi (bfd_arch_riscv, 0, GDB_OSABI_NONE,
> > + riscv_none_init_abi);
> > +}
> > +
> >