Re: [PATCHv3 0/9] Bare-metal core dumps for RISC-V

[Andrew Burgess <andrew.burgess@embecosm.com>]

Ping!

I'm planning to start merging this series later this week.  The code
was all reviewed in the v2 series.  The v3 mostly moves stuff around
to avoid adding dependencies on ELF support.

I still wonder if there's a better solution than the #ifdef block
added in patches 5 and 9, but these are pretty self-contained.  If
someone suggests a better approach later on then I'm happy to refactor
this code.

Thanks,
Andrew


* Andrew Burgess <andrew.burgess@embecosm.com> [2021-02-15 17:29:03 +0000]:

> After I pushed patch #1 from the v2 series it was pointed out that
> this broke any build of GDB for non-ELF targets.  I then reverted
> patch #1.  Now I'm back with an updated series.
> 
> For testing the build I have been using 'x86_64-apple-darwin20.3.0' as
> a non-ELF target.  I haven't done any actual functional testing on
> this target other than "it builds", but that's more than I did before
> (for non-ELF targets).
> 
> Between v2 and v3 then the changes are basically around the idea of
> not calling ELF specific functions from any generic code.  As far as
> possible I wanted to avoid just blocking code out with #ifdef, though
> I did use this trick in a limited way in patches #5 and #9, hopefully
> this is acceptable, though I'm open to suggestions for better ways to
> structure the code in these patches.
> 
> Here's a summary of which patches have changed:
> 
> (1) - Changed, common code from Linux and FreeBSD targets is now split
>       between gcore.c (for non-ELF code) and a new file gcore-elf.c
>       (for ELF specific code).  Changes to configure.ac ensure the ELF
>       specific file is only pulled in when needed.  This probably
>       needs a new review.
> 
> (2) - Minor updates to address Jim's review feedback.  Reviews are
>       welcome, but I'm considering this patch approved.
> 
> (3) - Code to add the target description note is moved to gcore-elf.c,
>       and each target must call this code (if the target is creating
>       an ELF based core file).  I don't like this as much as the
>       previous code where the target description note was added in one
>       place, but I can't see a good way to do this without adding a
>       #ifdef block to gcore.c.  This probably needs a new review.
> 
> (4) - Minor updates to address Jim's review feedback.  Reviews are
>       welcome, but I'm considering this patch approved.
> 
> (5) - The common bare metal core dump code is now in elf-none-tdep.c.
>       This is then called from the riscv-none-tdep.c code ONLY if we
>       have ELF support.  This is the use of '#ifdef HAVE_ELF' which is
>       a little yuck, but I can't see a better solution.  This probably
>       needs a new review.
> 
> (6) - Minor updates to address Jim's review feedback.  Reviews are
>       welcome, but I'm considering this patch approved.
> 
> (7) - No changes.  Reviews are welcome, but I'm considering this patch
>       approved.
> 
> (8) - No changes.  Reviews are welcome, but I'm considering this patch
>       approved.
> 
> (9) - No significant changes.  Like patch #5 it now includes a '#ifdef
>       HAVE_ELF' block in order to guard the call from arm-none-tdep.c
>       to elf-none-tdep.c.  I'd prefer reviews of this approach be
>       addressed to patch #5, I'll apply any feedback I get there to
>       this patch too.
> 
> ---
> 
> Andrew Burgess (9):
>   gdb: unify parts of the Linux and FreeBSD core dumping code
>   bfd/binutils: support for gdb target descriptions in the core file
>   gdb: write target description into core file
>   bfd/riscv: prepare to handle bare metal core dump creation
>   gdb/riscv: introduce bare metal core dump support
>   bfd/binutils: add support for RISC-V CSRs in core files
>   gdb/riscv: make riscv target description names global
>   gdb/riscv: write CSRs into baremetal core dumps
>   gdb/arm: add support for bare-metal core dumps
> 
>  bfd/ChangeLog         |  27 ++++++
>  bfd/elf-bfd.h         |   4 +
>  bfd/elf.c             |  70 ++++++++++++++
>  bfd/elfnn-riscv.c     |  84 ++++++++++++++++-
>  binutils/ChangeLog    |  10 ++
>  binutils/readelf.c    |   4 +
>  gdb/ChangeLog         |  88 +++++++++++++++++
>  gdb/Makefile.in       |   8 ++
>  gdb/arm-none-tdep.c   | 213 ++++++++++++++++++++++++++++++++++++++++++
>  gdb/configure         |   3 +-
>  gdb/configure.ac      |   3 +-
>  gdb/configure.tgt     |   5 +-
>  gdb/corelow.c         |  24 +++++
>  gdb/elf-none-tdep.c   | 126 +++++++++++++++++++++++++
>  gdb/elf-none-tdep.h   |  30 ++++++
>  gdb/fbsd-tdep.c       | 137 ++-------------------------
>  gdb/gcore-elf.c       | 166 ++++++++++++++++++++++++++++++++
>  gdb/gcore-elf.h       |  47 ++++++++++
>  gdb/gcore.c           |  21 +++++
>  gdb/gcore.h           |   9 ++
>  gdb/linux-tdep.c      | 146 +++--------------------------
>  gdb/riscv-none-tdep.c | 173 ++++++++++++++++++++++++++++++++++
>  gdb/riscv-tdep.c      |  14 ++-
>  gdb/riscv-tdep.h      |   3 +
>  include/ChangeLog     |  10 ++
>  include/elf/common.h  |   6 ++
>  26 files changed, 1159 insertions(+), 272 deletions(-)
>  create mode 100644 gdb/arm-none-tdep.c
>  create mode 100644 gdb/elf-none-tdep.c
>  create mode 100644 gdb/elf-none-tdep.h
>  create mode 100644 gdb/gcore-elf.c
>  create mode 100644 gdb/gcore-elf.h
>  create mode 100644 gdb/riscv-none-tdep.c
> 
> -- 
> 2.25.4
>