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

Message ID cover.1613410057.git.andrew.burgess@embecosm.com
Headers show
Series
  • Bare-metal core dumps for RISC-V
Related show

Message

Andrew Burgess Feb. 15, 2021, 5:29 p.m.
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

Comments

Andrew Burgess March 1, 2021, 10:32 a.m. | #1
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

>
Alan Modra via Binutils March 1, 2021, 2:45 p.m. | #2
Hi Andrew,

> Ping!


Pong!

Approved - please apply.

Cheers
   Nick
Andrew Burgess March 5, 2021, 5:35 p.m. | #3
I have now pushed patches 1 to 8 of this series.  I have not merged
the ARM bare metal core dumping patch.

Fredrik,

Are you able to run any tests on the ARM patch and confirm that it
still does everything your original patch did?  Are you happy with my
variant?

Thanks,
Andrew



* Andrew Burgess <andrew.burgess@embecosm.com> [2021-03-01 10:32:41 +0000]:

> 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

> >