[PATCHv2 5/9] gdb/riscv: introduce bare metal core dump support

Palmer Dabbelt palmer@dabbelt.com
Wed Feb 3 03:04:47 GMT 2021


On Mon, 01 Feb 2021 06:05:51 PST (-0800), gdb-patches@sourceware.org wrote:
> Andrew,
>
> On 1/20/21 5:23 PM, Andrew Burgess wrote:
>> The commit message below includes a description of the core file
>> format.  Though this description could exist in the commit message, I
>> don't imagine this is really the right place to document something
>> like this.
>
> Thanks for writing this down.
>
>>
>> For RISC-V I imagine the correct place would be here:
>>
>>    https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md
>>
>> And I have a commit for this repository that replicates the
>> description below.  However, I wanted to post this to the binutils/gdb
>> list first, and see if this description was good enough for all
>> stakeholders.
>>
>> If people here are happy with this commit and the documentation, then
>> I would image creating a pull request against the RISC-V ABI document,
>> and then if there's no negative feedback, merge this patch at that
>> point.
>>
>> All feedback, on docs or implementation, is welcome,

Thanks, this is great.  I have some minor comments in line, but regardless the 
psABI is probably the right place to put this sort of thing as it'll be more 
visible than just having it in GDB (though having it here seems fine as well).

>>
>> Thanks,
>> Andrew
>>
>>
>>
>>
>>
>> ----
>>
>> 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 code is split between two new files.  In
>> none-tdep.c is code designed to support the architecture agnostic
>> parts of a bare metal core dump.
>>
>> In riscv-none-tdep.c are the RISC-V specific parts, this is where the
>> regset and regcache_map_entry structures are defined that control how
>> registers are laid out in the core file.
>>
>> Currently for RISC-V only the x-regs and f-regs (if present) are
>> written out.  In future commits I plan to add support for writing out
>> the RISC-V CSRs.
>>
>> The cores dump format is based around generating an ELF containing
>
> cores -> core
>
>> sections for the writable regions of memory that a user could be
>> using.  Which regions are dumped rely on GDB's existing common core
>> dumping code, GDB will attempt to figure out the stack and heap as
>> well as copying out writable data sections as identified by the
>> original ELF.
>>
>> Register information is added to the core dump using notes, just as it
>> is for Linux of FreeBSD core dumps.  The note types used consist of
>> the 3 basic types you would expect in a OS based core dump,
>> NT_PRPSINFO, NT_PRSTATUS, NT_FPREGSET.
>>
>> The layout of these notes differs slightly (due to field sizes)
>> between RV32 and RV64.  Below I describe the data layout for each
>> note.  In all case, all padding fields should be set to zero.

I always find "should" kind of mushy, if we're really saying this is reserved 
for future use by debuggers then we should just be explicit about that.

>
> case -> cases
>
>>
>> Note NT_PRPSINFO is optional.  Its data layout is:
>>
>>    struct prpsinfo32_t		/* For RV32.  */
>>    {
>>      uint8_t padding[32];
>>      char fname[16];
>>      char psargs[80];
>>    }
>>
>>    struct prpsinfo64_t		/* For RV64.  */
>>    {
>>      uint8_t padding[40];
>>      char fname[16];
>>      char psargs[80];
>>    }
>>
>> Field 'fname' - null terminated string consisting of the basename of
>>      (up to the fist 15 characters of) the executable.  Any additional
>>      space should be set to zero.  If there's no executable name then
>>      this field can be set to all zero.
>>
>> Field 'psargs' - a null terminated string up to 80 characters in
>>      length.  Any additional space should be filled with zero.  This
>>      field contains the full executable path and any arguments passed
>>      to the executable.  If there's nothing sensible to write in this
>>      field then fill it with zero.
>>
>> Note NT_PRSTATUS is required, its data layout is:
>>
>>    struct prstatus32_t		/* For RV32.  */
>>    {
>>      uint8_t padding_1[12];
>>      uint16_t sig;
>>      uint8_t padding_2[10];
>>      uint32_t thread_id;
>>      uint8_t padding_3[44];
>>      uint32_t x_regs[32];
>>      uint8_t padding_4[4];
>>    }
>>
>>    struct prstatus64_t		/* For RV64.  */
>>    {
>>      uint8_t padding_1[12];
>>      uint16_t sig;
>>      uint8_t padding_2[18];
>>      uint32_t thread_id;
>>      uint8_t padding_3[76];
>>      uint64_t x_regs[32];
>>      uint8_t padding_4[4];
>>    }
>>
>> Field 'sig' - the signal that stopped this thread.  Its implementation
>
> Its -> It's
>
>>      defined what this field actually means.  Within GDB this will be
>>      the signal number that the remote target reports as the stop
>>      reason for this thread.
>>
>> Field 'thread_is' - the thread id for this thread, its implementation
>
> its -> it's

IIUC "it's" means "it is", and "its" is the possessive of "it".  So "its" is 
correct here.

>
>>      defined what this field actually means.  Within GDB this will be
>>      thread thread-id that is assigned to each remote thread.
>>
>> Field 'x_regs' - at index 0 we store the program counter, and at
>>      indices 1 to 31 we store x-registers 1 to 31.  x-register 0 is not
>>      stored, its value is always zero anyway.
>>
>> Note NT_FPREGSET is optional, its data layout is:
>>
>>    fpregset32_t			/* For targets with 'F' extension.  */
>>    {
>>      uint32_t f_regs[32];
>>      uint32_t fcsr;
>>    }
>>
>>    fpregset64_t			/* For targets with 'D' extension .  */
>>    {
>>      uint64_t f_regs[32];
>>      uint32_t fcsr;
>>    }
>>
>> Field 'f_regs' - stores f-registers 0 to 31.
>>
>> Field 'fcsr' - stores the fcsr CSR register, and is always 4-bytes.
>>
>> The rules for ordering the notes is the same as for Linux.  The
>> NT_PRSTATUS note must come before any other notes about additional
>> register sets.  And for multi-threaded targets all registers for a
>> single thread should be grouped together.  This is because only
>> NT_PRSTATUS includes a thread-id, all additional register notes after
>> a NT_PRSTATUS are assumed to belong to the same thread until a
>> different NT_PRSTATUS is seen.
>
> I think the above documentation is a good start, and we can expand it
> with more information.
>
> I'm wondering if we should document this in the GDB manual. Then again,
> it may be a bit too technical for the user manual.
>
> Another idea is to document arch-specific bits in arch-specific files
> and the generic parts in the none-tdep.c file?
>
>>
>> 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.
>> 	* none-tdep.c: New file.
>> 	* none-tdep.h: New file.
>> 	* riscv-none-tdep.c: New file.
>> ---
>>   gdb/ChangeLog         |  10 ++++
>>   gdb/Makefile.in       |   4 ++
>>   gdb/configure.tgt     |   2 +-
>>   gdb/none-tdep.c       | 119 ++++++++++++++++++++++++++++++++++++++++++
>>   gdb/none-tdep.h       |  30 +++++++++++
>>   gdb/riscv-none-tdep.c | 108 ++++++++++++++++++++++++++++++++++++++
>>   6 files changed, 272 insertions(+), 1 deletion(-)
>>   create mode 100644 gdb/none-tdep.c
>>   create mode 100644 gdb/none-tdep.h
>>   create mode 100644 gdb/riscv-none-tdep.c
>>
>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index 9267bea7beb..5f88b6a78cf 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 \
>> @@ -1100,6 +1101,7 @@ COMMON_SFILES = \
>>   	minsyms.c \
>>   	mipsread.c \
>>   	namespace.c \
>> +	none-tdep.c \
>>   	objc-lang.c \
>>   	objfiles.c \
>>   	observable.c \
>> @@ -1360,6 +1362,7 @@ HFILES_NO_SRCDIR = \
>>   	netbsd-tdep.h \
>>   	nds32-tdep.h \
>>   	nios2-tdep.h \
>> +	none-tdep.h \
>>   	nto-tdep.h \
>>   	objc-lang.h \
>>   	objfiles.h \
>> @@ -2271,6 +2274,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/none-tdep.c b/gdb/none-tdep.c
>> new file mode 100644
>> index 00000000000..8ec2407ad45
>> --- /dev/null
>> +++ b/gdb/none-tdep.c
>> @@ -0,0 +1,119 @@
>> +/* Target-dependent code for none, architecture independent.
>> +
>> +   Copyright (C) 2020 Free Software Foundation, Inc.
>
> 2020~2021 now I think.
>
>> +
>> +   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 "none-tdep.h"
>> +#include "regset.h"
>> +#include "elf-bfd.h"            /* for elfcore_write_* */
>> +#include "inferior.h"
>> +#include "regcache.h"
>> +#include "gdbarch.h"
>> +#include "gcore.h"
>> +
>> +/* Build the note section for a corefile, and return it in a malloc
>> +   buffer.  Currently this just  dumps all available registers for each
>
> Spurious whitespace in "just  dumps"
>
>> +   thread.  */
>> +
>> +static gdb::unique_xmalloc_ptr<char>
>> +none_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
>> +{
>> +  gdb::unique_xmalloc_ptr<char> note_data;
>> +
>> +  /* Add note information about the executable and its arguments.  */
>> +  std::string fname;
>> +  std::string psargs;
>> +  static const size_t fname_len = 16;
>> +  static const size_t psargs_len = 80;
>> +  if (get_exec_file (0))
>> +    {
>> +      const char *exe = get_exec_file (0);
>> +      fname = lbasename (exe);
>> +      psargs = std::string (exe);
>> +
>> +      const char *infargs = get_inferior_args ();
>> +      if (infargs != nullptr)
>> +	psargs += " " + std::string (infargs);
>> +
>> +      /* All existing targets that handling writing out prpsinfo expect the
>
> handling -> handle
>
>> +	 fname and psargs strings to be at least 16 and 80 characters long
>> +	 respectively, including a null terminator at the end.  Resize to
>> +	 the expected length minus one to ensure there is a null within the
>> +	 required length.  */
>> +      fname.resize (fname_len - 1);
>> +      psargs.resize (psargs_len - 1);
>> +    }
>> +
>> +  /* Resize the buffers up to their required lengths.  This will fill any
>> +     remaining space with the null character.  */
>> +  fname.resize (fname_len);
>> +  psargs.resize (psargs_len);
>> +
>> +  /* Now write out the prpsinfo structure.  */
>> +  note_data.reset (elfcore_write_prpsinfo (obfd, note_data.release (),
>> +					   note_size, fname.c_str (),
>> +					   psargs.c_str ()));
>> +  if (note_data == nullptr)
>> +    return nullptr;
>> +
>> +  /* Thread register information.  */
>> +  try
>> +    {
>> +      update_thread_list ();
>> +    }
>> +  catch (const gdb_exception_error &e)
>> +    {
>> +      exception_print (gdb_stderr, e);
>> +    }
>> +
>> +  /* Like the Linux kernel, prefer dumping the signalled thread first.
>> +     "First thread" is what tools use to infer the signalled thread.  */
>> +  thread_info *signalled_thr = gcore_find_signalled_thread ();
>> +
>> +  /* All threads are reported as having been stopped by the same signal
>> +     that stopped SIGNALLED_THR.  */
>> +  gdb_signal stop_signal;
>> +  if (signalled_thr != nullptr)
>> +    stop_signal = signalled_thr->suspend.stop_signal;
>> +  else
>> +    stop_signal = GDB_SIGNAL_0;
>> +
>> +  if (signalled_thr != nullptr)
>> +    gcore_build_thread_register_notes (gdbarch, signalled_thr,
>> +				       stop_signal, obfd, &note_data,
>> +				       note_size);
>> +  for (thread_info *thr : current_inferior ()->non_exited_threads ())
>> +    {
>> +      if (thr == signalled_thr)
>> +	continue;
>> +
>> +      gcore_build_thread_register_notes (gdbarch, thr, stop_signal, obfd,
>> +					 &note_data, note_size);
>> +    }
>> +
>> +  return note_data;
>> +}
>> +
>> +/* See none-tdep.h.  */
>> +
>> +void
>> +none_init_abi (struct gdbarch *gdbarch)
>> +{
>> +  /* Default core file support.  */
>> +  set_gdbarch_make_corefile_notes (gdbarch, none_make_corefile_notes);
>> +}
>> diff --git a/gdb/none-tdep.h b/gdb/none-tdep.h
>> new file mode 100644
>> index 00000000000..46dcfe219ef
>> --- /dev/null
>> +++ b/gdb/none-tdep.h
>> @@ -0,0 +1,30 @@
>> +/* Architecture independent code for ABI 'none' (bare-metal).
>> +
>> +   Copyright (C) 2021 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
>> +
>> +struct gdbarch;
>> +
>> +/* Initialize support for cross-architecture features applicable for the
>> +   GDB_OSABI_NONE ABI, that is bare-metal targets.  */
>> +
>> +void none_init_abi (struct gdbarch *gdbarch);
>> +
>> +#endif /* NONE_TDEP_H */
>> diff --git a/gdb/riscv-none-tdep.c b/gdb/riscv-none-tdep.c
>> new file mode 100644
>> index 00000000000..e06ee0981fe
>> --- /dev/null
>> +++ b/gdb/riscv-none-tdep.c
>> @@ -0,0 +1,108 @@
>> +/* Copyright (C) 2020 Free Software Foundation, Inc.
>
> 2020~2021 now.
>
>> +
>> +   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 "arch-utils.h"
>> +#include "regcache.h"
>> +#include "riscv-tdep.h"
>> +#include "elf-bfd.h"
>> +#include "regset.h"
>> +#include "none-tdep.h"
>> +
>> +/* 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, 4 },	/* Always stored as 4-bytes.  */
>> +  { 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
>> +};
>> +
>> +/* 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)
>> +{
>> +  none_init_abi (gdbarch);
>> +
>> +  /* 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);
>> +}
>>
>
> Otherwise this looks good to me.


More information about the Binutils mailing list