* [PATCH 1/8] gdb/riscv: use a single regset supply function for riscv fbsd & linux
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
@ 2020-12-02 17:39 ` Andrew Burgess
2021-01-18 14:15 ` Andrew Burgess
2020-12-02 17:39 ` [PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file Andrew Burgess
` (8 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-02 17:39 UTC (permalink / raw)
To: binutils, gdb-patches
Cc: Nelson Chu, Jim Wilson, John Baldwin, Andrew Burgess
The RISC-V x0 register is hard-coded to zero. As such neither Linux
or FreeBSD supply the value of the register x0 in their core dump
files.
For FreeBSD we take care of this by manually supplying the value of x0
in riscv_fbsd_supply_gregset, however we don't do this for Linux. As
a result after loading a core file on Linux we see this behaviour:
(gdb) p $x0
$1 = <unavailable>
In this commit I make riscv_fbsd_supply_gregset a common function that
can be shared between RISC-V for FreeBSD and Linux, this resolves the
above issue.
There is a similar problem for the two registers `fflags` and `frm`.
These two floating point related CSRs are a little weird. They are
separate CSRs in the RISC-V specification, but are actually sub-fields
of the `fcsr` CSR.
As a result neither Linux or FreeBSD supply the `fflags` or `frm`
registers as separate fields in their core dumps, and so, after
restoring a core dump these register are similarly unavailable.
In this commit I supply `fflags` and `frm` by first asking for the
value of `fcsr`, extracting the two fields, and using these to supply
the values for `fflags` and `frm`.
gdb/ChangeLog:
* riscv-fbsd-tdep.c (riscv_fbsd_supply_gregset): Delete.
(riscv_fbsd_gregset): Use riscv_supply_regset.
(riscv_fbsd_fpregset): Likewise.
* riscv-linux-tdep.c (riscv_linux_gregset): Likewise.
(riscv_linux_fregset): Likewise.
* riscv-tdep.c (riscv_supply_regset): Define new function.
* riscv-tdep.h (riscv_supply_regset): Declare new function.
---
gdb/ChangeLog | 10 +++++++++
gdb/riscv-fbsd-tdep.c | 20 ++---------------
gdb/riscv-linux-tdep.c | 4 ++--
gdb/riscv-tdep.c | 50 ++++++++++++++++++++++++++++++++++++++++++
gdb/riscv-tdep.h | 23 +++++++++++++++++++
5 files changed, 87 insertions(+), 20 deletions(-)
diff --git a/gdb/riscv-fbsd-tdep.c b/gdb/riscv-fbsd-tdep.c
index 6e0eb2bb788..f9eaaa16b25 100644
--- a/gdb/riscv-fbsd-tdep.c
+++ b/gdb/riscv-fbsd-tdep.c
@@ -53,32 +53,16 @@ static const struct regcache_map_entry riscv_fbsd_fpregmap[] =
{ 0 }
};
-/* Supply the general-purpose registers stored in GREGS to REGCACHE.
- This function only exists to supply the always-zero x0 in addition
- to the registers in GREGS. */
-
-static void
-riscv_fbsd_supply_gregset (const struct regset *regset,
- struct regcache *regcache, int regnum,
- const void *gregs, size_t len)
-{
- regcache->supply_regset (&riscv_fbsd_gregset, regnum, gregs, len);
- if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
- regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
-}
-
/* Register set definitions. */
const struct regset riscv_fbsd_gregset =
{
- riscv_fbsd_gregmap,
- riscv_fbsd_supply_gregset, regcache_collect_regset
+ riscv_fbsd_gregmap, riscv_supply_regset, regcache_collect_regset
};
const struct regset riscv_fbsd_fpregset =
{
- riscv_fbsd_fpregmap,
- regcache_supply_regset, regcache_collect_regset
+ riscv_fbsd_fpregmap, riscv_supply_regset, regcache_collect_regset
};
/* Implement the "iterate_over_regset_sections" gdbarch method. */
diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
index fce838097e2..86dd536edf4 100644
--- a/gdb/riscv-linux-tdep.c
+++ b/gdb/riscv-linux-tdep.c
@@ -52,14 +52,14 @@ static const struct regcache_map_entry riscv_linux_fregmap[] =
static const struct regset riscv_linux_gregset =
{
- riscv_linux_gregmap, regcache_supply_regset, regcache_collect_regset
+ riscv_linux_gregmap, riscv_supply_regset, regcache_collect_regset
};
/* Define the FP register regset. */
static const struct regset riscv_linux_fregset =
{
- riscv_linux_fregmap, regcache_supply_regset, regcache_collect_regset
+ riscv_linux_fregmap, riscv_supply_regset, regcache_collect_regset
};
/* Define hook for core file support. */
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 4e255056863..a428f79940d 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -3693,6 +3693,56 @@ riscv_init_reggroups ()
csr_reggroup = reggroup_new ("csr", USER_REGGROUP);
}
+/* See riscv-tdep.h. */
+
+void
+riscv_supply_regset (const struct regset *regset,
+ struct regcache *regcache, int regnum,
+ const void *regs, size_t len)
+{
+ regcache->supply_regset (regset, regnum, regs, len);
+
+ if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
+ regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
+
+ if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM
+ || regnum == RISCV_CSR_FRM_REGNUM)
+ {
+ int fcsr_regnum = RISCV_CSR_FCSR_REGNUM;
+
+ /* Ensure that FCSR has been read into REGCACHE. */
+ if (regnum != -1)
+ regcache->supply_regset (regset, fcsr_regnum, regs, len);
+
+ /* Grab the FCSR value if it is now in the regcache. We must check
+ the status first as, if the register was not supplied by REGSET,
+ this call will trigger a recursive attempt to fetch the
+ registers. */
+ if (regcache->get_register_status (fcsr_regnum) == REG_VALID)
+ {
+ ULONGEST fcsr_val;
+ regcache->raw_read (fcsr_regnum, &fcsr_val);
+
+ /* Extract the fflags and frm values. */
+ ULONGEST fflags_val = fcsr_val & 0x1f;
+ ULONGEST frm_val = (fcsr_val >> 5) & 0x7;
+
+ /* And supply these if needed. */
+ if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM)
+ regcache->raw_supply_integer (RISCV_CSR_FFLAGS_REGNUM,
+ (gdb_byte *) &fflags_val,
+ sizeof (fflags_val),
+ /* is_signed */ false);
+
+ if (regnum == -1 || regnum == RISCV_CSR_FRM_REGNUM)
+ regcache->raw_supply_integer (RISCV_CSR_FRM_REGNUM,
+ (gdb_byte *)&frm_val,
+ sizeof (fflags_val),
+ /* is_signed */ false);
+ }
+ }
+}
+
void _initialize_riscv_tdep ();
void
_initialize_riscv_tdep ()
diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
index 5bd3314d450..1064ced1192 100644
--- a/gdb/riscv-tdep.h
+++ b/gdb/riscv-tdep.h
@@ -132,4 +132,27 @@ extern int riscv_abi_flen (struct gdbarch *gdbarch);
extern std::vector<CORE_ADDR> riscv_software_single_step
(struct regcache *regcache);
+/* Supply register REGNUM from the buffer REGS (length LEN) into
+ REGCACHE. REGSET describes the layout of the buffer. If REGNUM is -1
+ then all registers described by REGSET are supplied.
+
+ The register RISCV_ZERO_REGNUM should not be described by REGSET,
+ however, this register (which always has the value 0) will be supplied
+ by this function if requested.
+
+ The registers RISCV_CSR_FFLAGS_REGNUM and RISCV_CSR_FRM_REGNUM should
+ not be described by REGSET, however, these register will be provided if
+ requested assuming either:
+ (a) REGCACHE already contains the value of RISCV_CSR_FCSR_REGNUM, or
+ (b) REGSET describes the location of RISCV_CSR_FCSR_REGNUM in the REGS
+ buffer.
+
+ This function can be used as the supply function for either x-regs or
+ f-regs when loading corefiles, and doesn't care which abi is currently
+ in use. */
+
+extern void riscv_supply_regset (const struct regset *regset,
+ struct regcache *regcache, int regnum,
+ const void *regs, size_t len);
+
#endif /* RISCV_TDEP_H */
--
2.25.4
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 1/8] gdb/riscv: use a single regset supply function for riscv fbsd & linux
2020-12-02 17:39 ` [PATCH 1/8] gdb/riscv: use a single regset supply function for riscv fbsd & linux Andrew Burgess
@ 2021-01-18 14:15 ` Andrew Burgess
0 siblings, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2021-01-18 14:15 UTC (permalink / raw)
To: binutils, gdb-patches; +Cc: Nelson Chu, Jim Wilson, John Baldwin
* Andrew Burgess <andrew.burgess@embecosm.com> [2020-12-02 17:39:25 +0000]:
> The RISC-V x0 register is hard-coded to zero. As such neither Linux
> or FreeBSD supply the value of the register x0 in their core dump
> files.
>
> For FreeBSD we take care of this by manually supplying the value of x0
> in riscv_fbsd_supply_gregset, however we don't do this for Linux. As
> a result after loading a core file on Linux we see this behaviour:
>
> (gdb) p $x0
> $1 = <unavailable>
>
> In this commit I make riscv_fbsd_supply_gregset a common function that
> can be shared between RISC-V for FreeBSD and Linux, this resolves the
> above issue.
>
> There is a similar problem for the two registers `fflags` and `frm`.
> These two floating point related CSRs are a little weird. They are
> separate CSRs in the RISC-V specification, but are actually sub-fields
> of the `fcsr` CSR.
>
> As a result neither Linux or FreeBSD supply the `fflags` or `frm`
> registers as separate fields in their core dumps, and so, after
> restoring a core dump these register are similarly unavailable.
>
> In this commit I supply `fflags` and `frm` by first asking for the
> value of `fcsr`, extracting the two fields, and using these to supply
> the values for `fflags` and `frm`.
>
> gdb/ChangeLog:
>
> * riscv-fbsd-tdep.c (riscv_fbsd_supply_gregset): Delete.
> (riscv_fbsd_gregset): Use riscv_supply_regset.
> (riscv_fbsd_fpregset): Likewise.
> * riscv-linux-tdep.c (riscv_linux_gregset): Likewise.
> (riscv_linux_fregset): Likewise.
> * riscv-tdep.c (riscv_supply_regset): Define new function.
> * riscv-tdep.h (riscv_supply_regset): Declare new function.
I have now pushed this patch only from this series.
I'm going to revise the rest of this series and post a new version.
Thanks,
Andrew
> ---
> gdb/ChangeLog | 10 +++++++++
> gdb/riscv-fbsd-tdep.c | 20 ++---------------
> gdb/riscv-linux-tdep.c | 4 ++--
> gdb/riscv-tdep.c | 50 ++++++++++++++++++++++++++++++++++++++++++
> gdb/riscv-tdep.h | 23 +++++++++++++++++++
> 5 files changed, 87 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/riscv-fbsd-tdep.c b/gdb/riscv-fbsd-tdep.c
> index 6e0eb2bb788..f9eaaa16b25 100644
> --- a/gdb/riscv-fbsd-tdep.c
> +++ b/gdb/riscv-fbsd-tdep.c
> @@ -53,32 +53,16 @@ static const struct regcache_map_entry riscv_fbsd_fpregmap[] =
> { 0 }
> };
>
> -/* Supply the general-purpose registers stored in GREGS to REGCACHE.
> - This function only exists to supply the always-zero x0 in addition
> - to the registers in GREGS. */
> -
> -static void
> -riscv_fbsd_supply_gregset (const struct regset *regset,
> - struct regcache *regcache, int regnum,
> - const void *gregs, size_t len)
> -{
> - regcache->supply_regset (&riscv_fbsd_gregset, regnum, gregs, len);
> - if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
> - regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
> -}
> -
> /* Register set definitions. */
>
> const struct regset riscv_fbsd_gregset =
> {
> - riscv_fbsd_gregmap,
> - riscv_fbsd_supply_gregset, regcache_collect_regset
> + riscv_fbsd_gregmap, riscv_supply_regset, regcache_collect_regset
> };
>
> const struct regset riscv_fbsd_fpregset =
> {
> - riscv_fbsd_fpregmap,
> - regcache_supply_regset, regcache_collect_regset
> + riscv_fbsd_fpregmap, riscv_supply_regset, regcache_collect_regset
> };
>
> /* Implement the "iterate_over_regset_sections" gdbarch method. */
> diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
> index fce838097e2..86dd536edf4 100644
> --- a/gdb/riscv-linux-tdep.c
> +++ b/gdb/riscv-linux-tdep.c
> @@ -52,14 +52,14 @@ static const struct regcache_map_entry riscv_linux_fregmap[] =
>
> static const struct regset riscv_linux_gregset =
> {
> - riscv_linux_gregmap, regcache_supply_regset, regcache_collect_regset
> + riscv_linux_gregmap, riscv_supply_regset, regcache_collect_regset
> };
>
> /* Define the FP register regset. */
>
> static const struct regset riscv_linux_fregset =
> {
> - riscv_linux_fregmap, regcache_supply_regset, regcache_collect_regset
> + riscv_linux_fregmap, riscv_supply_regset, regcache_collect_regset
> };
>
> /* Define hook for core file support. */
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 4e255056863..a428f79940d 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -3693,6 +3693,56 @@ riscv_init_reggroups ()
> csr_reggroup = reggroup_new ("csr", USER_REGGROUP);
> }
>
> +/* See riscv-tdep.h. */
> +
> +void
> +riscv_supply_regset (const struct regset *regset,
> + struct regcache *regcache, int regnum,
> + const void *regs, size_t len)
> +{
> + regcache->supply_regset (regset, regnum, regs, len);
> +
> + if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
> + regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
> +
> + if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM
> + || regnum == RISCV_CSR_FRM_REGNUM)
> + {
> + int fcsr_regnum = RISCV_CSR_FCSR_REGNUM;
> +
> + /* Ensure that FCSR has been read into REGCACHE. */
> + if (regnum != -1)
> + regcache->supply_regset (regset, fcsr_regnum, regs, len);
> +
> + /* Grab the FCSR value if it is now in the regcache. We must check
> + the status first as, if the register was not supplied by REGSET,
> + this call will trigger a recursive attempt to fetch the
> + registers. */
> + if (regcache->get_register_status (fcsr_regnum) == REG_VALID)
> + {
> + ULONGEST fcsr_val;
> + regcache->raw_read (fcsr_regnum, &fcsr_val);
> +
> + /* Extract the fflags and frm values. */
> + ULONGEST fflags_val = fcsr_val & 0x1f;
> + ULONGEST frm_val = (fcsr_val >> 5) & 0x7;
> +
> + /* And supply these if needed. */
> + if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM)
> + regcache->raw_supply_integer (RISCV_CSR_FFLAGS_REGNUM,
> + (gdb_byte *) &fflags_val,
> + sizeof (fflags_val),
> + /* is_signed */ false);
> +
> + if (regnum == -1 || regnum == RISCV_CSR_FRM_REGNUM)
> + regcache->raw_supply_integer (RISCV_CSR_FRM_REGNUM,
> + (gdb_byte *)&frm_val,
> + sizeof (fflags_val),
> + /* is_signed */ false);
> + }
> + }
> +}
> +
> void _initialize_riscv_tdep ();
> void
> _initialize_riscv_tdep ()
> diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
> index 5bd3314d450..1064ced1192 100644
> --- a/gdb/riscv-tdep.h
> +++ b/gdb/riscv-tdep.h
> @@ -132,4 +132,27 @@ extern int riscv_abi_flen (struct gdbarch *gdbarch);
> extern std::vector<CORE_ADDR> riscv_software_single_step
> (struct regcache *regcache);
>
> +/* Supply register REGNUM from the buffer REGS (length LEN) into
> + REGCACHE. REGSET describes the layout of the buffer. If REGNUM is -1
> + then all registers described by REGSET are supplied.
> +
> + The register RISCV_ZERO_REGNUM should not be described by REGSET,
> + however, this register (which always has the value 0) will be supplied
> + by this function if requested.
> +
> + The registers RISCV_CSR_FFLAGS_REGNUM and RISCV_CSR_FRM_REGNUM should
> + not be described by REGSET, however, these register will be provided if
> + requested assuming either:
> + (a) REGCACHE already contains the value of RISCV_CSR_FCSR_REGNUM, or
> + (b) REGSET describes the location of RISCV_CSR_FCSR_REGNUM in the REGS
> + buffer.
> +
> + This function can be used as the supply function for either x-regs or
> + f-regs when loading corefiles, and doesn't care which abi is currently
> + in use. */
> +
> +extern void riscv_supply_regset (const struct regset *regset,
> + struct regcache *regcache, int regnum,
> + const void *regs, size_t len);
> +
> #endif /* RISCV_TDEP_H */
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
2020-12-02 17:39 ` [PATCH 1/8] gdb/riscv: use a single regset supply function for riscv fbsd & linux Andrew Burgess
@ 2020-12-02 17:39 ` Andrew Burgess
2020-12-02 18:21 ` Luis Machado
2020-12-02 17:39 ` [PATCH 3/8] gdb: write target description into " Andrew Burgess
` (7 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-02 17:39 UTC (permalink / raw)
To: binutils, gdb-patches
Cc: Nelson Chu, Jim Wilson, John Baldwin, Andrew Burgess
This commit lays the ground work for allowing GDB to write its target
description into a generated core file.
The goal of this work is to allow a user to connect to a remote
target, capture a core file from within GDB, then pass the executable
and core file to another user and have the user be able to examine the
state of the machine without needing to connect to a running target.
Different remote targets can have different register sets and this
information is communicated from the target to GDB in the target
description.
It is possible for a user to extract the target description from GDB
and pass this along with the core file so that when the core file is
used the target description can be fed back into GDB, however this is
not a great user experience.
It would be nicer, I think, if GDB could write the target description
directly into the core file, and then make use of this description
when loading a core file.
This commit performs the binutils/bfd side of this task, adding the
boiler plate functions to access the target description from within a
core file note, and reserving a new number for a note containing the
target description.
Later commits will extend GDB to make use of this.
bfd/ChangeLog:
* elf-bfd.h (elfcore_write_gdb_tdesc): Declare new function.
* elf.c (elfcore_grok_gdb_tdesc): New function.
(elfcore_grok_note): Handle NT_GDB_TDESC.
(elfcore_write_gdb_tdesc): New function.
(elfcore_write_register_note): Handle NT_GDB_TDESC.
binutils/ChangeLog:
* readelf.c (get_note_type): Handle NT_GDB_TDESC.
include/ChangeLog:
* elf/common.h (NT_GDB_TDESC): Define.
---
bfd/ChangeLog | 9 +++++++++
bfd/elf-bfd.h | 2 ++
bfd/elf.c | 23 +++++++++++++++++++++++
binutils/ChangeLog | 5 +++++
binutils/readelf.c | 2 ++
include/ChangeLog | 5 +++++
include/elf/common.h | 2 ++
7 files changed, 48 insertions(+)
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index e9c890f6f16..5ef69ab5b13 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2796,6 +2796,8 @@ extern char *elfcore_write_aarch_pauth
(bfd *, char *, int *, const void *, int);
extern char *elfcore_write_arc_v2
(bfd *, char *, int *, const void *, int);
+extern char *elfcore_write_gdb_tdesc
+ (bfd *, char *, int *, const void *, int);
extern char *elfcore_write_lwpstatus
(bfd *, char *, int *, long, int, const void *);
extern char *elfcore_write_register_note
diff --git a/bfd/elf.c b/bfd/elf.c
index 419c5f4420c..bea5ab12773 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9909,6 +9909,12 @@ elfcore_grok_arc_v2 (bfd *abfd, Elf_Internal_Note *note)
return elfcore_make_note_pseudosection (abfd, ".reg-arc-v2", note);
}
+static bfd_boolean
+elfcore_grok_gdb_tdesc (bfd *abfd, Elf_Internal_Note *note)
+{
+ return elfcore_make_note_pseudosection (abfd, ".gdb-tdesc", note);
+}
+
#if defined (HAVE_PRPSINFO_T)
typedef prpsinfo_t elfcore_psinfo_t;
#if defined (HAVE_PRPSINFO32_T) /* Sparc64 cross Sparc32 */
@@ -10566,6 +10572,9 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
else
return TRUE;
+ case NT_GDB_TDESC:
+ return elfcore_grok_gdb_tdesc (abfd, note);
+
case NT_PRPSINFO:
case NT_PSINFO:
if (bed->elf_backend_grok_psinfo)
@@ -11947,6 +11956,18 @@ elfcore_write_arc_v2 (bfd *abfd,
note_name, NT_ARC_V2, arc_v2, size);
}
+char *
+elfcore_write_gdb_tdesc (bfd *abfd,
+ char *buf,
+ int *bufsiz,
+ const void *tdesc,
+ int size)
+{
+ const char *note_name = "CORE";
+ return elfcore_write_note (abfd, buf, bufsiz,
+ note_name, NT_GDB_TDESC, tdesc, size);
+}
+
char *
elfcore_write_register_note (bfd *abfd,
char *buf,
@@ -12031,6 +12052,8 @@ elfcore_write_register_note (bfd *abfd,
return elfcore_write_aarch_pauth (abfd, buf, bufsiz, data, size);
if (strcmp (section, ".reg-arc-v2") == 0)
return elfcore_write_arc_v2 (abfd, buf, bufsiz, data, size);
+ if (strcmp (section, ".gdb-tdesc") == 0)
+ return elfcore_write_gdb_tdesc (abfd, buf, bufsiz, data, size);
return NULL;
}
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 57e0f1de459..5b3871d3e5f 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -18246,6 +18246,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
return _("NT_PRPSINFO (prpsinfo structure)");
case NT_TASKSTRUCT:
return _("NT_TASKSTRUCT (task structure)");
+ case NT_GDB_TDESC:
+ return _("NT_GDB_TDESC (GDB XML target description)");
case NT_PRXFPREG:
return _("NT_PRXFPREG (user_xfpregs structure)");
case NT_PPC_VMX:
diff --git a/include/elf/common.h b/include/elf/common.h
index 95a852f0cf5..1dbf0b11983 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -666,6 +666,8 @@
#define NT_SIGINFO 0x53494749 /* Fields of siginfo_t. */
#define NT_FILE 0x46494c45 /* Description of mapped files. */
+#define NT_GDB_TDESC 0x54444553 /* Contains copy of GDB's target description XML. */
+
/* Note segments for core files on dir-style procfs systems. */
#define NT_PSTATUS 10 /* Has a struct pstatus */
--
2.25.4
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file
2020-12-02 17:39 ` [PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file Andrew Burgess
@ 2020-12-02 18:21 ` Luis Machado
2020-12-02 22:58 ` Jim Wilson
2020-12-07 12:48 ` Andrew Burgess
0 siblings, 2 replies; 38+ messages in thread
From: Luis Machado @ 2020-12-02 18:21 UTC (permalink / raw)
To: Andrew Burgess, binutils, gdb-patches
On 12/2/20 2:39 PM, Andrew Burgess wrote:
> This commit lays the ground work for allowing GDB to write its target
> description into a generated core file.
>
> The goal of this work is to allow a user to connect to a remote
> target, capture a core file from within GDB, then pass the executable
> and core file to another user and have the user be able to examine the
> state of the machine without needing to connect to a running target.
>
> Different remote targets can have different register sets and this
> information is communicated from the target to GDB in the target
> description.
>
> It is possible for a user to extract the target description from GDB
> and pass this along with the core file so that when the core file is
> used the target description can be fed back into GDB, however this is
> not a great user experience.
>
> It would be nicer, I think, if GDB could write the target description
> directly into the core file, and then make use of this description
> when loading a core file.
>
> This commit performs the binutils/bfd side of this task, adding the
> boiler plate functions to access the target description from within a
> core file note, and reserving a new number for a note containing the
> target description.
>
> Later commits will extend GDB to make use of this.
>
> bfd/ChangeLog:
>
> * elf-bfd.h (elfcore_write_gdb_tdesc): Declare new function.
> * elf.c (elfcore_grok_gdb_tdesc): New function.
> (elfcore_grok_note): Handle NT_GDB_TDESC.
> (elfcore_write_gdb_tdesc): New function.
> (elfcore_write_register_note): Handle NT_GDB_TDESC.
>
> binutils/ChangeLog:
>
> * readelf.c (get_note_type): Handle NT_GDB_TDESC.
>
> include/ChangeLog:
>
> * elf/common.h (NT_GDB_TDESC): Define.
> ---
> bfd/ChangeLog | 9 +++++++++
> bfd/elf-bfd.h | 2 ++
> bfd/elf.c | 23 +++++++++++++++++++++++
> binutils/ChangeLog | 5 +++++
> binutils/readelf.c | 2 ++
> include/ChangeLog | 5 +++++
> include/elf/common.h | 2 ++
> 7 files changed, 48 insertions(+)
>
> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index e9c890f6f16..5ef69ab5b13 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -2796,6 +2796,8 @@ extern char *elfcore_write_aarch_pauth
> (bfd *, char *, int *, const void *, int);
> extern char *elfcore_write_arc_v2
> (bfd *, char *, int *, const void *, int);
> +extern char *elfcore_write_gdb_tdesc
> + (bfd *, char *, int *, const void *, int);
> extern char *elfcore_write_lwpstatus
> (bfd *, char *, int *, long, int, const void *);
> extern char *elfcore_write_register_note
> diff --git a/bfd/elf.c b/bfd/elf.c
> index 419c5f4420c..bea5ab12773 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -9909,6 +9909,12 @@ elfcore_grok_arc_v2 (bfd *abfd, Elf_Internal_Note *note)
> return elfcore_make_note_pseudosection (abfd, ".reg-arc-v2", note);
> }
>
> +static bfd_boolean
> +elfcore_grok_gdb_tdesc (bfd *abfd, Elf_Internal_Note *note)
> +{
> + return elfcore_make_note_pseudosection (abfd, ".gdb-tdesc", note);
> +}
> +
> #if defined (HAVE_PRPSINFO_T)
> typedef prpsinfo_t elfcore_psinfo_t;
> #if defined (HAVE_PRPSINFO32_T) /* Sparc64 cross Sparc32 */
> @@ -10566,6 +10572,9 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
> else
> return TRUE;
>
> + case NT_GDB_TDESC:
> + return elfcore_grok_gdb_tdesc (abfd, note);
> +
> case NT_PRPSINFO:
> case NT_PSINFO:
> if (bed->elf_backend_grok_psinfo)
> @@ -11947,6 +11956,18 @@ elfcore_write_arc_v2 (bfd *abfd,
> note_name, NT_ARC_V2, arc_v2, size);
> }
>
> +char *
> +elfcore_write_gdb_tdesc (bfd *abfd,
> + char *buf,
> + int *bufsiz,
> + const void *tdesc,
> + int size)
> +{
> + const char *note_name = "CORE";
> + return elfcore_write_note (abfd, buf, bufsiz,
> + note_name, NT_GDB_TDESC, tdesc, size);
> +}
> +
> char *
> elfcore_write_register_note (bfd *abfd,
> char *buf,
> @@ -12031,6 +12052,8 @@ elfcore_write_register_note (bfd *abfd,
> return elfcore_write_aarch_pauth (abfd, buf, bufsiz, data, size);
> if (strcmp (section, ".reg-arc-v2") == 0)
> return elfcore_write_arc_v2 (abfd, buf, bufsiz, data, size);
> + if (strcmp (section, ".gdb-tdesc") == 0)
> + return elfcore_write_gdb_tdesc (abfd, buf, bufsiz, data, size);
> return NULL;
> }
>
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index 57e0f1de459..5b3871d3e5f 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -18246,6 +18246,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
> return _("NT_PRPSINFO (prpsinfo structure)");
> case NT_TASKSTRUCT:
> return _("NT_TASKSTRUCT (task structure)");
> + case NT_GDB_TDESC:
> + return _("NT_GDB_TDESC (GDB XML target description)");
> case NT_PRXFPREG:
> return _("NT_PRXFPREG (user_xfpregs structure)");
> case NT_PPC_VMX:
> diff --git a/include/elf/common.h b/include/elf/common.h
> index 95a852f0cf5..1dbf0b11983 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -666,6 +666,8 @@
> #define NT_SIGINFO 0x53494749 /* Fields of siginfo_t. */
> #define NT_FILE 0x46494c45 /* Description of mapped files. */
>
> +#define NT_GDB_TDESC 0x54444553 /* Contains copy of GDB's target description XML. */
> +
How about a generic name without the tool's name on it? Other tools may
want to use it as well, and that can be a little confusing.
NT_XML_TDESC, maybe?
For the constant, I find the magic number amusing, but I don't think it
does any good when you're trying to find out why that particular magic
number was picked, and what the next number should be.
Should we go with the basic increasing ID instead? I think this would be
7, right after NT_AUXV.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file
2020-12-02 18:21 ` Luis Machado
@ 2020-12-02 22:58 ` Jim Wilson
2020-12-03 12:16 ` Luis Machado
2020-12-07 12:48 ` Andrew Burgess
1 sibling, 1 reply; 38+ messages in thread
From: Jim Wilson @ 2020-12-02 22:58 UTC (permalink / raw)
To: Luis Machado; +Cc: Andrew Burgess, Binutils, gdb-patches
On Wed, Dec 2, 2020 at 10:21 AM Luis Machado via Gdb-patches <
gdb-patches@sourceware.org> wrote:
> For the constant, I find the magic number amusing, but I don't think it
> does any good when you're trying to find out why that particular magic
> number was picked, and what the next number should be.
>
> Should we go with the basic increasing ID instead? I think this would be
> 7, right after NT_AUXV.
>
7 is NT_FREEBSD_THRMISC which would prevent use of this on FreeBSD. I know
this is primarily for bare metal core files, but it might be useful for
linux and *bsd systems that have different register sets, because of
different extensions, with vector versus without vector, or because
different hardware has different sets of implemented CSRs. We don't appear
to have reserved ranges for NT_* values, though we sort of have a scheme
for processor specific values, e.g. 0x1XX for ppc, 0x2XX for x86, 0x3XX for
s390, etc. OS specific values tend to be low values below 0x100 but above
6, with only FreeBSD starting at 7 and most others starting at 10. So that
leaves high values like ascii encodings of the NT_* name as safe for new
uses. There are 3 of these already, this would be a fourth one.
Jim
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file
2020-12-02 22:58 ` Jim Wilson
@ 2020-12-03 12:16 ` Luis Machado
[not found] ` <20201214115512.GI2945@embecosm.com>
0 siblings, 1 reply; 38+ messages in thread
From: Luis Machado @ 2020-12-03 12:16 UTC (permalink / raw)
To: Jim Wilson; +Cc: Andrew Burgess, Binutils, gdb-patches
On 12/2/20 7:58 PM, Jim Wilson wrote:
> On Wed, Dec 2, 2020 at 10:21 AM Luis Machado via Gdb-patches
> <gdb-patches@sourceware.org <mailto:gdb-patches@sourceware.org>> wrote:
>
> For the constant, I find the magic number amusing, but I don't think it
> does any good when you're trying to find out why that particular magic
> number was picked, and what the next number should be.
>
> Should we go with the basic increasing ID instead? I think this
> would be
> 7, right after NT_AUXV.
>
>
> 7 is NT_FREEBSD_THRMISC which would prevent use of this on FreeBSD. I
> know this is primarily for bare metal core files, but it might be useful
> for linux and *bsd systems that have different register sets, because of
> different extensions, with vector versus without vector, or because
> different hardware has different sets of implemented CSRs. We don't
> appear to have reserved ranges for NT_* values, though we sort of have a
> scheme for processor specific values, e.g. 0x1XX for ppc, 0x2XX for x86,
> 0x3XX for s390, etc. OS specific values tend to be low values below
> 0x100 but above 6, with only FreeBSD starting at 7 and most others
> starting at 10. So that leaves high values like ascii encodings of the
> NT_* name as safe for new uses. There are 3 of these already, this
> would be a fourth one.
Sure, we should make this generic for all OS' to use.
The processor-specific values are a good example of organized ID
structure. They're all pretty obvious to look at and to identify.
If the OS-specific values are low ID's, then maybe we should start a new
range of high values for the generic NT_* notes. That should make things
more organized in the future, and folks will know what the next ID is.
I don't particularly think having 3 ascii encoded constants is a good
motivation for having more. NT_PRXFPREG is Linux-specific, which is not
good.
Otherwise, if we're sticking to the ascii encodings, we should document
that and group them together in their own section instead of just
grouping them with the Linux ones, which is a bit misleading.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file
2020-12-02 18:21 ` Luis Machado
2020-12-02 22:58 ` Jim Wilson
@ 2020-12-07 12:48 ` Andrew Burgess
1 sibling, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 12:48 UTC (permalink / raw)
To: Luis Machado; +Cc: binutils, gdb-patches
* Luis Machado <luis.machado@linaro.org> [2020-12-02 15:21:49 -0300]:
> On 12/2/20 2:39 PM, Andrew Burgess wrote:
> > This commit lays the ground work for allowing GDB to write its target
> > description into a generated core file.
> >
> > The goal of this work is to allow a user to connect to a remote
> > target, capture a core file from within GDB, then pass the executable
> > and core file to another user and have the user be able to examine the
> > state of the machine without needing to connect to a running target.
> >
> > Different remote targets can have different register sets and this
> > information is communicated from the target to GDB in the target
> > description.
> >
> > It is possible for a user to extract the target description from GDB
> > and pass this along with the core file so that when the core file is
> > used the target description can be fed back into GDB, however this is
> > not a great user experience.
> >
> > It would be nicer, I think, if GDB could write the target description
> > directly into the core file, and then make use of this description
> > when loading a core file.
> >
> > This commit performs the binutils/bfd side of this task, adding the
> > boiler plate functions to access the target description from within a
> > core file note, and reserving a new number for a note containing the
> > target description.
> >
> > Later commits will extend GDB to make use of this.
> >
> > bfd/ChangeLog:
> >
> > * elf-bfd.h (elfcore_write_gdb_tdesc): Declare new function.
> > * elf.c (elfcore_grok_gdb_tdesc): New function.
> > (elfcore_grok_note): Handle NT_GDB_TDESC.
> > (elfcore_write_gdb_tdesc): New function.
> > (elfcore_write_register_note): Handle NT_GDB_TDESC.
> >
> > binutils/ChangeLog:
> >
> > * readelf.c (get_note_type): Handle NT_GDB_TDESC.
> >
> > include/ChangeLog:
> >
> > * elf/common.h (NT_GDB_TDESC): Define.
> > ---
> > bfd/ChangeLog | 9 +++++++++
> > bfd/elf-bfd.h | 2 ++
> > bfd/elf.c | 23 +++++++++++++++++++++++
> > binutils/ChangeLog | 5 +++++
> > binutils/readelf.c | 2 ++
> > include/ChangeLog | 5 +++++
> > include/elf/common.h | 2 ++
> > 7 files changed, 48 insertions(+)
> >
> > diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> > index e9c890f6f16..5ef69ab5b13 100644
> > --- a/bfd/elf-bfd.h
> > +++ b/bfd/elf-bfd.h
> > @@ -2796,6 +2796,8 @@ extern char *elfcore_write_aarch_pauth
> > (bfd *, char *, int *, const void *, int);
> > extern char *elfcore_write_arc_v2
> > (bfd *, char *, int *, const void *, int);
> > +extern char *elfcore_write_gdb_tdesc
> > + (bfd *, char *, int *, const void *, int);
> > extern char *elfcore_write_lwpstatus
> > (bfd *, char *, int *, long, int, const void *);
> > extern char *elfcore_write_register_note
> > diff --git a/bfd/elf.c b/bfd/elf.c
> > index 419c5f4420c..bea5ab12773 100644
> > --- a/bfd/elf.c
> > +++ b/bfd/elf.c
> > @@ -9909,6 +9909,12 @@ elfcore_grok_arc_v2 (bfd *abfd, Elf_Internal_Note *note)
> > return elfcore_make_note_pseudosection (abfd, ".reg-arc-v2", note);
> > }
> > +static bfd_boolean
> > +elfcore_grok_gdb_tdesc (bfd *abfd, Elf_Internal_Note *note)
> > +{
> > + return elfcore_make_note_pseudosection (abfd, ".gdb-tdesc", note);
> > +}
> > +
> > #if defined (HAVE_PRPSINFO_T)
> > typedef prpsinfo_t elfcore_psinfo_t;
> > #if defined (HAVE_PRPSINFO32_T) /* Sparc64 cross Sparc32 */
> > @@ -10566,6 +10572,9 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
> > else
> > return TRUE;
> > + case NT_GDB_TDESC:
> > + return elfcore_grok_gdb_tdesc (abfd, note);
> > +
> > case NT_PRPSINFO:
> > case NT_PSINFO:
> > if (bed->elf_backend_grok_psinfo)
> > @@ -11947,6 +11956,18 @@ elfcore_write_arc_v2 (bfd *abfd,
> > note_name, NT_ARC_V2, arc_v2, size);
> > }
> > +char *
> > +elfcore_write_gdb_tdesc (bfd *abfd,
> > + char *buf,
> > + int *bufsiz,
> > + const void *tdesc,
> > + int size)
> > +{
> > + const char *note_name = "CORE";
> > + return elfcore_write_note (abfd, buf, bufsiz,
> > + note_name, NT_GDB_TDESC, tdesc, size);
> > +}
> > +
> > char *
> > elfcore_write_register_note (bfd *abfd,
> > char *buf,
> > @@ -12031,6 +12052,8 @@ elfcore_write_register_note (bfd *abfd,
> > return elfcore_write_aarch_pauth (abfd, buf, bufsiz, data, size);
> > if (strcmp (section, ".reg-arc-v2") == 0)
> > return elfcore_write_arc_v2 (abfd, buf, bufsiz, data, size);
> > + if (strcmp (section, ".gdb-tdesc") == 0)
> > + return elfcore_write_gdb_tdesc (abfd, buf, bufsiz, data, size);
> > return NULL;
> > }
> > diff --git a/binutils/readelf.c b/binutils/readelf.c
> > index 57e0f1de459..5b3871d3e5f 100644
> > --- a/binutils/readelf.c
> > +++ b/binutils/readelf.c
> > @@ -18246,6 +18246,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
> > return _("NT_PRPSINFO (prpsinfo structure)");
> > case NT_TASKSTRUCT:
> > return _("NT_TASKSTRUCT (task structure)");
> > + case NT_GDB_TDESC:
> > + return _("NT_GDB_TDESC (GDB XML target description)");
> > case NT_PRXFPREG:
> > return _("NT_PRXFPREG (user_xfpregs structure)");
> > case NT_PPC_VMX:
> > diff --git a/include/elf/common.h b/include/elf/common.h
> > index 95a852f0cf5..1dbf0b11983 100644
> > --- a/include/elf/common.h
> > +++ b/include/elf/common.h
> > @@ -666,6 +666,8 @@
> > #define NT_SIGINFO 0x53494749 /* Fields of siginfo_t. */
> > #define NT_FILE 0x46494c45 /* Description of mapped files. */
> > +#define NT_GDB_TDESC 0x54444553 /* Contains copy of GDB's target description XML. */
> > +
>
> How about a generic name without the tool's name on it? Other tools may want
> to use it as well, and that can be a little confusing. NT_XML_TDESC,
> maybe?
I originally did name the note NT_XML_TDESC, however, I moved away
from this because NT_GDB_TDESC describes what this actually is, and I
don't think switching back is a good choice.
If some other tool can process these descriptions that this is because
that tool has made the choice to accept GDB format target
descriptions, so I don't think having GDB in the name would be a
problem.
Further, if some other tool comes up with its own target description
specification then it's quite possible that it would also use XML then
we have to find a new name that (a) doesn't include the tool name, but
(b) doesn't clash with NT_XML_TDESC.
>
> For the constant, I find the magic number amusing, but I don't think it does
> any good when you're trying to find out why that particular magic number was
> picked, and what the next number should be.
>
> Should we go with the basic increasing ID instead? I think this would be 7,
> right after NT_AUXV.
The number certainly wasn't picked for amusement.
I didn't pick 7 for fear of clashing with a number that someone else
might already be using, if I understand Jim correctly then it seems
this was a good call.
The ASCII encoded string provided a very semi random large number
that was unlikely to clash, and was inline with how at least some
other notes were numbered.
I really have no strong feelings one way or the other on what the note
number should be, if someone wants to propose a scheme, then I'm more
than happy to adopt that.
I'll wait to see how the numbering conversation pans out and adapt
this patch to fit in.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 3/8] gdb: write target description into core file
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
2020-12-02 17:39 ` [PATCH 1/8] gdb/riscv: use a single regset supply function for riscv fbsd & linux Andrew Burgess
2020-12-02 17:39 ` [PATCH 2/8] bfd/binutils: support for gdb target descriptions in the core file Andrew Burgess
@ 2020-12-02 17:39 ` Andrew Burgess
2020-12-03 20:36 ` Tom Tromey
2020-12-02 17:39 ` [PATCH 4/8] bfd/riscv: prepare to handle bare metal core dump creation Andrew Burgess
` (6 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-02 17:39 UTC (permalink / raw)
To: binutils, gdb-patches
Cc: Nelson Chu, Jim Wilson, John Baldwin, Andrew Burgess
When a core file is created from within GDB add the target description
into a note within the core file.
When loading a core file, if the target description note is present
then load the target description from the core file.
The benefit of this is that we can be sure that, when analysing the
core file within GDB, that we are using the exact same target
description as was in use at the time the core file was created.
In future commits I intend to add support for bare metal core dumps
for some targets. These core dumps will include auxiliary registers,
the availability of which can only be established by looking at the
target description.
gdb/ChangeLog:
* corelow.c: Add 'xml-tdesc.h' include.
(core_target::read_description): Load the target description from
the core file when possible.
* gcore.c: Add 'gdbsupport/tdesc.h' include.
(write_gcore_file_1): Write out the target description.
---
gdb/ChangeLog | 9 +++++++++
gdb/corelow.c | 22 ++++++++++++++++++++++
gdb/gcore.c | 20 ++++++++++++++++++++
3 files changed, 51 insertions(+)
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 4c1f068053d..a35c829c55b 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -49,6 +49,7 @@
#include <unordered_map>
#include <unordered_set>
#include "gdbcmd.h"
+#include "xml-tdesc.h"
#ifndef O_LARGEFILE
#define O_LARGEFILE 0
@@ -1000,6 +1001,27 @@ core_target::thread_alive (ptid_t ptid)
const struct target_desc *
core_target::read_description ()
{
+ /* If the core file contains a target description note then we will use
+ that in preference to anything else. */
+ bfd_size_type tdesc_note_size = 0;
+ struct bfd_section *tdesc_note_section
+ = bfd_get_section_by_name (core_bfd, ".gdb-tdesc");
+ if (tdesc_note_section != nullptr)
+ tdesc_note_size = bfd_section_size (tdesc_note_section);
+ if (tdesc_note_size > 0)
+ {
+ gdb::char_vector contents (tdesc_note_size);
+ if (bfd_get_section_contents (core_bfd, tdesc_note_section,
+ contents.data(), (file_ptr) 0,
+ tdesc_note_size))
+ {
+ const struct target_desc *result
+ = string_read_description_xml (contents.data ());
+ if (result != NULL)
+ return result;
+ }
+ }
+
if (m_core_gdbarch && gdbarch_core_read_description_p (m_core_gdbarch))
{
const struct target_desc *result;
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 9a22b1005f0..bf6f44213bb 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -38,6 +38,7 @@
#include "gdbsupport/gdb_unlinker.h"
#include "gdbsupport/byte-vector.h"
#include "gdbsupport/scope-exit.h"
+#include "gdbsupport/tdesc.h"
/* The largest amount of memory to read from the target at once. We
must throttle it to limit the amount of memory used by GDB during
@@ -82,6 +83,25 @@ write_gcore_file_1 (bfd *obfd)
note_data = gdbarch_make_corefile_notes (target_gdbarch (), obfd,
¬e_size);
+ /* Append the target description to the core file. */
+ const struct target_desc *tdesc = gdbarch_target_desc (target_gdbarch ());
+ const char *tdesc_xml = tdesc_get_features_xml (tdesc);
+ if (tdesc_xml != nullptr && *tdesc_xml != '\0')
+ {
+ /* Skip the leading '@'. */
+ if (*tdesc_xml == '@')
+ ++tdesc_xml;
+
+ /* Include the null terminator in the length. */
+ size_t tdesc_len = strlen (tdesc_xml) + 1;
+
+ /* Now add the target description into the core file. */
+ note_data.reset (elfcore_write_register_note (obfd,
+ note_data.release (),
+ ¬e_size, ".gdb-tdesc",
+ tdesc_xml, tdesc_len));
+ }
+
if (note_data == NULL || note_size == 0)
error (_("Target does not support core file generation."));
--
2.25.4
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/8] gdb: write target description into core file
2020-12-02 17:39 ` [PATCH 3/8] gdb: write target description into " Andrew Burgess
@ 2020-12-03 20:36 ` Tom Tromey
2020-12-07 14:38 ` Andrew Burgess
0 siblings, 1 reply; 38+ messages in thread
From: Tom Tromey @ 2020-12-03 20:36 UTC (permalink / raw)
To: Andrew Burgess; +Cc: binutils, gdb-patches
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
Andrew> + if (bfd_get_section_contents (core_bfd, tdesc_note_section,
Andrew> + contents.data(), (file_ptr) 0,
Missing space after "data".
I suppose the trailing \0 is guaranteed to be in the section data.
Tom
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 3/8] gdb: write target description into core file
2020-12-03 20:36 ` Tom Tromey
@ 2020-12-07 14:38 ` Andrew Burgess
0 siblings, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 14:38 UTC (permalink / raw)
To: Tom Tromey; +Cc: binutils, gdb-patches
* Tom Tromey <tom@tromey.com> [2020-12-03 13:36:55 -0700]:
> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
>
> Andrew> + if (bfd_get_section_contents (core_bfd, tdesc_note_section,
> Andrew> + contents.data(), (file_ptr) 0,
>
> Missing space after "data".
>
> I suppose the trailing \0 is guaranteed to be in the section data.
Well, it should if the content is well formed, this can be seen in the
change to `write_gcore_file_1` where the length is extended to include
the null.
However, you make a good point. The content is coming from outside
GDB and could be corrupt, so we shouldn't assume that there is a null
present.
As such I've updated the patch so that we force a null at the end when
reading in the target description, this should ensure GDB doesn't read
outside the section contents.
Thanks
Andrew
---
commit 10e9c51c13a5f1da3ad53fc987d90cbffa8501a9
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Fri Nov 27 15:41:52 2020 +0000
gdb: write target description into core file
When a core file is created from within GDB add the target description
into a note within the core file.
When loading a core file, if the target description note is present
then load the target description from the core file.
The benefit of this is that we can be sure that, when analysing the
core file within GDB, that we are using the exact same target
description as was in use at the time the core file was created.
In future commits I intend to add support for bare metal core dumps
for some targets. These core dumps will include auxiliary registers,
the availability of which can only be established by looking at the
target description.
gdb/ChangeLog:
* corelow.c: Add 'xml-tdesc.h' include.
(core_target::read_description): Load the target description from
the core file when possible.
* gcore.c: Add 'gdbsupport/tdesc.h' include.
(write_gcore_file_1): Write out the target description.
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 4c1f068053d..f00770080d8 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -49,6 +49,7 @@
#include <unordered_map>
#include <unordered_set>
#include "gdbcmd.h"
+#include "xml-tdesc.h"
#ifndef O_LARGEFILE
#define O_LARGEFILE 0
@@ -1000,6 +1001,29 @@ core_target::thread_alive (ptid_t ptid)
const struct target_desc *
core_target::read_description ()
{
+ /* If the core file contains a target description note then we will use
+ that in preference to anything else. */
+ bfd_size_type tdesc_note_size = 0;
+ struct bfd_section *tdesc_note_section
+ = bfd_get_section_by_name (core_bfd, ".gdb-tdesc");
+ if (tdesc_note_section != nullptr)
+ tdesc_note_size = bfd_section_size (tdesc_note_section);
+ if (tdesc_note_size > 0)
+ {
+ gdb::char_vector contents (tdesc_note_size + 1);
+ if (bfd_get_section_contents (core_bfd, tdesc_note_section,
+ contents.data (), (file_ptr) 0,
+ tdesc_note_size))
+ {
+ /* Ensure we have a null terminator. */
+ contents [tdesc_note_size] = '\0';
+ const struct target_desc *result
+ = string_read_description_xml (contents.data ());
+ if (result != NULL)
+ return result;
+ }
+ }
+
if (m_core_gdbarch && gdbarch_core_read_description_p (m_core_gdbarch))
{
const struct target_desc *result;
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 9a22b1005f0..bf6f44213bb 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -38,6 +38,7 @@
#include "gdbsupport/gdb_unlinker.h"
#include "gdbsupport/byte-vector.h"
#include "gdbsupport/scope-exit.h"
+#include "gdbsupport/tdesc.h"
/* The largest amount of memory to read from the target at once. We
must throttle it to limit the amount of memory used by GDB during
@@ -82,6 +83,25 @@ write_gcore_file_1 (bfd *obfd)
note_data = gdbarch_make_corefile_notes (target_gdbarch (), obfd,
¬e_size);
+ /* Append the target description to the core file. */
+ const struct target_desc *tdesc = gdbarch_target_desc (target_gdbarch ());
+ const char *tdesc_xml = tdesc_get_features_xml (tdesc);
+ if (tdesc_xml != nullptr && *tdesc_xml != '\0')
+ {
+ /* Skip the leading '@'. */
+ if (*tdesc_xml == '@')
+ ++tdesc_xml;
+
+ /* Include the null terminator in the length. */
+ size_t tdesc_len = strlen (tdesc_xml) + 1;
+
+ /* Now add the target description into the core file. */
+ note_data.reset (elfcore_write_register_note (obfd,
+ note_data.release (),
+ ¬e_size, ".gdb-tdesc",
+ tdesc_xml, tdesc_len));
+ }
+
if (note_data == NULL || note_size == 0)
error (_("Target does not support core file generation."));
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 4/8] bfd/riscv: prepare to handle bare metal core dump creation
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
` (2 preceding siblings ...)
2020-12-02 17:39 ` [PATCH 3/8] gdb: write target description into " Andrew Burgess
@ 2020-12-02 17:39 ` Andrew Burgess
2020-12-02 23:24 ` Jim Wilson
2020-12-02 17:39 ` [PATCH 5/8] gdb/riscv: introduce bare metal core dump support Andrew Burgess
` (5 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-02 17:39 UTC (permalink / raw)
To: binutils, gdb-patches
Cc: Nelson Chu, Jim Wilson, John Baldwin, Andrew Burgess
When creating a core file GDB will call the function
elfcore_write_prstatus to write out the general purpose registers
along with the pid/tid for the thread.
However, for a bare metal RISC-V tool chain the prstatus*_t types are
not defined so the elfcore_write_prstatus function will return NULL,
preventing core file creation.
This commit provides the `elf_backend_write_core_note' hook and uses
the provided function to write out the ptstatus information.
In order to keep changes in the non bare metal tools to a minimum, the
provided backend function will itself return NULL when the prstatus*_t
types are available, the consequence of this is that the generic code
in elfcore_write_prstatus will be used just as before. But, when
prstatus*_t is not available, the new backend function will write out
the prstatus information using predefined offsets.
This new functionality will be used by a later GDB commit that will
add bare metal core dumps for RISC-V.
bfd/ChangeLog:
* elfnn-riscv.c (riscv_write_core_note): New function.
(elf_backend_write_core_note): Define.
---
bfd/ChangeLog | 6 ++++
bfd/elfnn-riscv.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 20944c8109f..cffdf0b21f2 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -4874,6 +4874,77 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
# define PRPSINFO_OFFSET_PR_PSARGS 56
#endif
+/* Write PRSTATUS note into core file. This will be called before the
+ generic code in elf.c. By checking the compiler defines we only perform
+ any action here if the generic code would otherwise not be able to help
+ us. The intention is that bare metal core dumps (where the prstatus32_t
+ might not be available) will use this code, while non bare metal tools
+ will use the generic elf code. */
+
+static char *
+riscv_write_core_note (bfd *abfd ATTRIBUTE_UNUSED,
+ char *buf ATTRIBUTE_UNUSED,
+ int *bufsiz ATTRIBUTE_UNUSED,
+ int note_type ATTRIBUTE_UNUSED, ...)
+{
+ switch (note_type)
+ {
+ default:
+ return NULL;
+
+#if !defined (HAVE_PRPSINFO_T)
+ case NT_PRPSINFO:
+ {
+ char data[PRPSINFO_SIZE] ATTRIBUTE_NONSTRING;
+ va_list ap;
+
+ va_start (ap, note_type);
+ memset (data, 0, sizeof (data));
+ strncpy (data + PRPSINFO_OFFSET_PR_FNAME, va_arg (ap, const char *), 16);
+#if GCC_VERSION == 8000 || GCC_VERSION == 8001
+ DIAGNOSTIC_PUSH;
+ /* GCC 8.0 and 8.1 warn about 80 equals destination size with
+ -Wstringop-truncation:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+ */
+ DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
+ strncpy (data + PRPSINFO_OFFSET_PR_PSARGS, va_arg (ap, const char *), 80);
+#if GCC_VERSION == 8000 || GCC_VERSION == 8001
+ DIAGNOSTIC_POP;
+#endif
+ va_end (ap);
+ return elfcore_write_note (abfd, buf, bufsiz,
+ "CORE", note_type, data, sizeof (data));
+ }
+#endif /* !HAVE_PRPSINFO_T */
+
+#if !defined (HAVE_PRSTATUS_T)
+ case NT_PRSTATUS:
+ {
+ char data[PRSTATUS_SIZE];
+ va_list ap;
+ long pid;
+ int cursig;
+ const void *greg;
+
+ va_start (ap, note_type);
+ memset (data, 0, sizeof(data));
+ pid = va_arg (ap, long);
+ bfd_put_32 (abfd, pid, data + PRSTATUS_OFFSET_PR_PID);
+ cursig = va_arg (ap, int);
+ bfd_put_16 (abfd, cursig, data + PRSTATUS_OFFSET_PR_CURSIG);
+ greg = va_arg (ap, const void *);
+ memcpy (data + PRSTATUS_OFFSET_PR_REG, greg,
+ PRSTATUS_SIZE - PRSTATUS_OFFSET_PR_REG - ARCH_SIZE / 8);
+ va_end (ap);
+ return elfcore_write_note (abfd, buf, bufsiz,
+ "CORE", note_type, data, sizeof (data));
+ }
+#endif /* !HAVE_PRSTATUS_T */
+ }
+}
+
/* Support for core dump NOTE sections. */
static bfd_boolean
@@ -4984,6 +5055,8 @@ riscv_elf_obj_attrs_arg_type (int tag)
#define elf_backend_grok_prstatus riscv_elf_grok_prstatus
#define elf_backend_grok_psinfo riscv_elf_grok_psinfo
#define elf_backend_object_p riscv_elf_object_p
+#undef elf_backend_write_core_note
+#define elf_backend_write_core_note riscv_write_core_note
#define elf_info_to_howto_rel NULL
#define elf_info_to_howto riscv_info_to_howto_rela
#define bfd_elfNN_bfd_relax_section _bfd_riscv_relax_section
--
2.25.4
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 4/8] bfd/riscv: prepare to handle bare metal core dump creation
2020-12-02 17:39 ` [PATCH 4/8] bfd/riscv: prepare to handle bare metal core dump creation Andrew Burgess
@ 2020-12-02 23:24 ` Jim Wilson
2020-12-07 14:39 ` Andrew Burgess
0 siblings, 1 reply; 38+ messages in thread
From: Jim Wilson @ 2020-12-02 23:24 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Binutils, gdb-patches, Nelson Chu, John Baldwin
On Wed, Dec 2, 2020 at 9:39 AM Andrew Burgess <andrew.burgess@embecosm.com>
wrote:
> +/* Write PRSTATUS note into core file. This will be called before the
> + generic code in elf.c. By checking the compiler defines we only
> perform
> + any action here if the generic code would otherwise not be able to help
> + us. The intention is that bare metal core dumps (where the
> prstatus32_t
> + might not be available) will use this code, while non bare metal tools
> + will use the generic elf code. */
>
The reference to prstatus32_t is a little confusing, as that appears to be
an x64_64 specific type. I think that should be prstatus_t.
The function handles both PRSTATUS and PRPSINFO but the comment only
mentions PRSTATUS. Maybe it should mention both?
Otherwise this looks OK to me, with a minor comment clarification.
Jim
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 4/8] bfd/riscv: prepare to handle bare metal core dump creation
2020-12-02 23:24 ` Jim Wilson
@ 2020-12-07 14:39 ` Andrew Burgess
0 siblings, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 14:39 UTC (permalink / raw)
To: Jim Wilson; +Cc: Binutils, gdb-patches, Nelson Chu, John Baldwin
* Jim Wilson <jimw@sifive.com> [2020-12-02 15:24:41 -0800]:
> On Wed, Dec 2, 2020 at 9:39 AM Andrew Burgess <andrew.burgess@embecosm.com>
> wrote:
>
> > +/* Write PRSTATUS note into core file. This will be called before the
> > + generic code in elf.c. By checking the compiler defines we only
> > perform
> > + any action here if the generic code would otherwise not be able to help
> > + us. The intention is that bare metal core dumps (where the
> > prstatus32_t
> > + might not be available) will use this code, while non bare metal tools
> > + will use the generic elf code. */
> >
>
> The reference to prstatus32_t is a little confusing, as that appears to be
> an x64_64 specific type. I think that should be prstatus_t.
>
> The function handles both PRSTATUS and PRPSINFO but the comment only
> mentions PRSTATUS. Maybe it should mention both?
>
> Otherwise this looks OK to me, with a minor comment clarification.
I updated the comment and posted the revised patch below.
Thanks,
Andrew
---
commit 00f409a72ba183ef0f80b5874f7d0c6464d85073
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Mon Nov 30 12:14:38 2020 +0000
bfd/riscv: prepare to handle bare metal core dump creation
When creating a core file GDB will call the function
elfcore_write_prstatus to write out the general purpose registers
along with the pid/tid for the thread.
However, for a bare metal RISC-V tool chain the prstatus*_t types are
not defined so the elfcore_write_prstatus function will return NULL,
preventing core file creation.
This commit provides the `elf_backend_write_core_note' hook and uses
the provided function to write out the ptstatus information.
In order to keep changes in the non bare metal tools to a minimum, the
provided backend function will itself return NULL when the prstatus*_t
types are available, the consequence of this is that the generic code
in elfcore_write_prstatus will be used just as before. But, when
prstatus*_t is not available, the new backend function will write out
the prstatus information using predefined offsets.
This new functionality will be used by a later GDB commit that will
add bare metal core dumps for RISC-V.
bfd/ChangeLog:
* elfnn-riscv.c (riscv_write_core_note): New function.
(elf_backend_write_core_note): Define.
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 20944c8109f..29cccf06e67 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -4874,6 +4874,77 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
# define PRPSINFO_OFFSET_PR_PSARGS 56
#endif
+/* Write PRSTATUS and PRPSINFO note into core file. This will be called
+ before the generic code in elf.c. By checking the compiler defines we
+ only perform any action here if the generic code would otherwise not be
+ able to help us. The intention is that bare metal core dumps (where the
+ prstatus_t and/or prpsinfo_t might not be available) will use this code,
+ while non bare metal tools will use the generic elf code. */
+
+static char *
+riscv_write_core_note (bfd *abfd ATTRIBUTE_UNUSED,
+ char *buf ATTRIBUTE_UNUSED,
+ int *bufsiz ATTRIBUTE_UNUSED,
+ int note_type ATTRIBUTE_UNUSED, ...)
+{
+ switch (note_type)
+ {
+ default:
+ return NULL;
+
+#if !defined (HAVE_PRPSINFO_T)
+ case NT_PRPSINFO:
+ {
+ char data[PRPSINFO_SIZE] ATTRIBUTE_NONSTRING;
+ va_list ap;
+
+ va_start (ap, note_type);
+ memset (data, 0, sizeof (data));
+ strncpy (data + PRPSINFO_OFFSET_PR_FNAME, va_arg (ap, const char *), 16);
+#if GCC_VERSION == 8000 || GCC_VERSION == 8001
+ DIAGNOSTIC_PUSH;
+ /* GCC 8.0 and 8.1 warn about 80 equals destination size with
+ -Wstringop-truncation:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+ */
+ DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
+ strncpy (data + PRPSINFO_OFFSET_PR_PSARGS, va_arg (ap, const char *), 80);
+#if GCC_VERSION == 8000 || GCC_VERSION == 8001
+ DIAGNOSTIC_POP;
+#endif
+ va_end (ap);
+ return elfcore_write_note (abfd, buf, bufsiz,
+ "CORE", note_type, data, sizeof (data));
+ }
+#endif /* !HAVE_PRPSINFO_T */
+
+#if !defined (HAVE_PRSTATUS_T)
+ case NT_PRSTATUS:
+ {
+ char data[PRSTATUS_SIZE];
+ va_list ap;
+ long pid;
+ int cursig;
+ const void *greg;
+
+ va_start (ap, note_type);
+ memset (data, 0, sizeof(data));
+ pid = va_arg (ap, long);
+ bfd_put_32 (abfd, pid, data + PRSTATUS_OFFSET_PR_PID);
+ cursig = va_arg (ap, int);
+ bfd_put_16 (abfd, cursig, data + PRSTATUS_OFFSET_PR_CURSIG);
+ greg = va_arg (ap, const void *);
+ memcpy (data + PRSTATUS_OFFSET_PR_REG, greg,
+ PRSTATUS_SIZE - PRSTATUS_OFFSET_PR_REG - ARCH_SIZE / 8);
+ va_end (ap);
+ return elfcore_write_note (abfd, buf, bufsiz,
+ "CORE", note_type, data, sizeof (data));
+ }
+#endif /* !HAVE_PRSTATUS_T */
+ }
+}
+
/* Support for core dump NOTE sections. */
static bfd_boolean
@@ -4984,6 +5055,8 @@ riscv_elf_obj_attrs_arg_type (int tag)
#define elf_backend_grok_prstatus riscv_elf_grok_prstatus
#define elf_backend_grok_psinfo riscv_elf_grok_psinfo
#define elf_backend_object_p riscv_elf_object_p
+#undef elf_backend_write_core_note
+#define elf_backend_write_core_note riscv_write_core_note
#define elf_info_to_howto_rel NULL
#define elf_info_to_howto riscv_info_to_howto_rela
#define bfd_elfNN_bfd_relax_section _bfd_riscv_relax_section
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
` (3 preceding siblings ...)
2020-12-02 17:39 ` [PATCH 4/8] bfd/riscv: prepare to handle bare metal core dump creation Andrew Burgess
@ 2020-12-02 17:39 ` Andrew Burgess
2020-12-02 18:12 ` Luis Machado
2020-12-02 17:39 ` [PATCH 6/8] bfd/binutils: add support for RISC-V CSRs in core files Andrew Burgess
` (4 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-02 17:39 UTC (permalink / raw)
To: binutils, gdb-patches
Cc: Nelson Chu, Jim Wilson, John Baldwin, Andrew Burgess
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, ¬e_data, note_size);
+ for (thread_info *thr : current_inferior ()->non_exited_threads ())
+ {
+ if (thr == signalled_thr)
+ continue;
+ riscv_corefile_thread (gdbarch, obfd, signalled_thr, ¬e_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);
+}
+
--
2.25.4
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-02 17:39 ` [PATCH 5/8] gdb/riscv: introduce bare metal core dump support Andrew Burgess
@ 2020-12-02 18:12 ` Luis Machado
2020-12-07 15:17 ` Andrew Burgess
0 siblings, 1 reply; 38+ messages in thread
From: Luis Machado @ 2020-12-02 18:12 UTC (permalink / raw)
To: Andrew Burgess, binutils, gdb-patches; +Cc: Paul Mathieu, Fredrik Hederstierna
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.
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, ¬e_data, note_size);
> + for (thread_info *thr : current_inferior ()->non_exited_threads ())
> + {
> + if (thr == signalled_thr)
> + continue;
> + riscv_corefile_thread (gdbarch, obfd, signalled_thr, ¬e_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);
> +}
> +
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-02 18:12 ` Luis Machado
@ 2020-12-07 15:17 ` Andrew Burgess
2020-12-07 15:58 ` Luis Machado
0 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 15:17 UTC (permalink / raw)
To: Luis Machado; +Cc: binutils, gdb-patches, Paul Mathieu, Fredrik Hederstierna
* 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, ¬e_data, note_size);
> > + for (thread_info *thr : current_inferior ()->non_exited_threads ())
> > + {
> > + if (thr == signalled_thr)
> > + continue;
> > + riscv_corefile_thread (gdbarch, obfd, signalled_thr, ¬e_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);
> > +}
> > +
> >
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 15:17 ` Andrew Burgess
@ 2020-12-07 15:58 ` Luis Machado
2020-12-07 16:58 ` Andrew Burgess
0 siblings, 1 reply; 38+ messages in thread
From: Luis Machado @ 2020-12-07 15:58 UTC (permalink / raw)
To: Andrew Burgess; +Cc: binutils, gdb-patches, Paul Mathieu, Fredrik Hederstierna
On 12/7/20 12:17 PM, Andrew Burgess wrote:
> * 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.
Sure. What I'm proposing here is the same I proposed in this thread
about ARM bare metal core files...
https://sourceware.org/pipermail/gdb-patches/2020-October/172617.html
... and also suggested by Simon here:
https://sourceware.org/pipermail/gdb-patches/2020-October/172270.html
In summary, we should document what a bare metal core file is, what
pieces it is expected to contain (registers, target descriptions,
hardware information, execution environment etc) and what format it will
be in.
>
> 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
Having a common format makes it easier to maintain the code and expand
the features when needed. This is not different from the Linux core file
we have today. The Linux core file is a common format.
But unlike Linux core files, which have less than ideal documentation
and specifications, we want to take this opportunity to start clean and
to document everything required to have a proper bare metal core file.
I think this initial definition and documentation will benefit
developers moving forward.
Does that make it more clear?
>
> 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, ¬e_data, note_size);
>>> + for (thread_info *thr : current_inferior ()->non_exited_threads ())
>>> + {
>>> + if (thr == signalled_thr)
>>> + continue;
>>> + riscv_corefile_thread (gdbarch, obfd, signalled_thr, ¬e_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);
>>> +}
>>> +
>>>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 15:58 ` Luis Machado
@ 2020-12-07 16:58 ` Andrew Burgess
2020-12-07 17:24 ` Luis Machado
0 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 16:58 UTC (permalink / raw)
To: Luis Machado; +Cc: binutils, gdb-patches, Paul Mathieu, Fredrik Hederstierna
* Luis Machado <luis.machado@linaro.org> [2020-12-07 12:58:19 -0300]:
> On 12/7/20 12:17 PM, Andrew Burgess wrote:
> > * 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.
>
> Sure. What I'm proposing here is the same I proposed in this thread about
> ARM bare metal core files...
>
> https://sourceware.org/pipermail/gdb-patches/2020-October/172617.html
>
> ... and also suggested by Simon here:
>
> https://sourceware.org/pipermail/gdb-patches/2020-October/172270.html
>
> In summary, we should document what a bare metal core file is, what pieces
> it is expected to contain (registers, target descriptions, hardware
> information, execution environment etc) and what format it will be in.
>
> >
> > 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
>
> Having a common format makes it easier to maintain the code and expand the
> features when needed. This is not different from the Linux core file we have
> today. The Linux core file is a common format.
>
> But unlike Linux core files, which have less than ideal documentation and
> specifications, we want to take this opportunity to start clean and to
> document everything required to have a proper bare metal core file.
>
> I think this initial definition and documentation will benefit developers
> moving forward.
>
> Does that make it more clear?
Not really. I'll go read the various threads that you referenced and
see come back once I have more specific questions.
Thanks for the pointers.
Andrew
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 16:58 ` Andrew Burgess
@ 2020-12-07 17:24 ` Luis Machado
2020-12-07 18:11 ` Andrew Burgess
0 siblings, 1 reply; 38+ messages in thread
From: Luis Machado @ 2020-12-07 17:24 UTC (permalink / raw)
To: Andrew Burgess; +Cc: binutils, gdb-patches, Paul Mathieu, Fredrik Hederstierna
On 12/7/20 1:58 PM, Andrew Burgess wrote:
> * Luis Machado <luis.machado@linaro.org> [2020-12-07 12:58:19 -0300]:
>
>> On 12/7/20 12:17 PM, Andrew Burgess wrote:
>>> * 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.
>>
>> Sure. What I'm proposing here is the same I proposed in this thread about
>> ARM bare metal core files...
>>
>> https://sourceware.org/pipermail/gdb-patches/2020-October/172617.html
>>
>> ... and also suggested by Simon here:
>>
>> https://sourceware.org/pipermail/gdb-patches/2020-October/172270.html
>>
>> In summary, we should document what a bare metal core file is, what pieces
>> it is expected to contain (registers, target descriptions, hardware
>> information, execution environment etc) and what format it will be in.
>>
>>>
>>> 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
>>
>> Having a common format makes it easier to maintain the code and expand the
>> features when needed. This is not different from the Linux core file we have
>> today. The Linux core file is a common format.
>>
>> But unlike Linux core files, which have less than ideal documentation and
>> specifications, we want to take this opportunity to start clean and to
>> document everything required to have a proper bare metal core file.
>>
>> I think this initial definition and documentation will benefit developers
>> moving forward.
>>
>> Does that make it more clear?
>
> Not really. I'll go read the various threads that you referenced and
> see come back once I have more specific questions.
>
> Thanks for the pointers.
>
> Andrew
>
I'm sorry that did not clarify things, but there isn't much more than
that really. All I'm suggesting (up to you to accept it) is that we
clarify/coordinate with the interested parties on what is needed for
bare metal core files and document it properly (I don't think code is
proper documentation).
Clearly there are more folks interested in this.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 17:24 ` Luis Machado
@ 2020-12-07 18:11 ` Andrew Burgess
2020-12-07 19:00 ` Luis Machado
0 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 18:11 UTC (permalink / raw)
To: Luis Machado; +Cc: binutils, gdb-patches, Paul Mathieu, Fredrik Hederstierna
* Luis Machado <luis.machado@linaro.org> [2020-12-07 14:24:33 -0300]:
> On 12/7/20 1:58 PM, Andrew Burgess wrote:
> > * Luis Machado <luis.machado@linaro.org> [2020-12-07 12:58:19 -0300]:
> >
> > > On 12/7/20 12:17 PM, Andrew Burgess wrote:
> > > > * 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.
> > >
> > > Sure. What I'm proposing here is the same I proposed in this thread about
> > > ARM bare metal core files...
> > >
> > > https://sourceware.org/pipermail/gdb-patches/2020-October/172617.html
> > >
> > > ... and also suggested by Simon here:
> > >
> > > https://sourceware.org/pipermail/gdb-patches/2020-October/172270.html
> > >
> > > In summary, we should document what a bare metal core file is, what pieces
> > > it is expected to contain (registers, target descriptions, hardware
> > > information, execution environment etc) and what format it will be in.
> > >
> > > >
> > > > 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
> > >
> > > Having a common format makes it easier to maintain the code and expand the
> > > features when needed. This is not different from the Linux core file we have
> > > today. The Linux core file is a common format.
> > >
> > > But unlike Linux core files, which have less than ideal documentation and
> > > specifications, we want to take this opportunity to start clean and to
> > > document everything required to have a proper bare metal core file.
> > >
> > > I think this initial definition and documentation will benefit developers
> > > moving forward.
> > >
> > > Does that make it more clear?
> >
> > Not really. I'll go read the various threads that you referenced and
> > see come back once I have more specific questions.
> >
> > Thanks for the pointers.
> >
> > Andrew
> >
>
> I'm sorry that did not clarify things, but there isn't much more than that
> really. All I'm suggesting (up to you to accept it) is that we
> clarify/coordinate with the interested parties on what is needed for bare
> metal core files and document it properly (I don't think code is proper
> documentation).
Sure.
What I don't understand is you talk about creating "a format that
works for multiple architectures".
Current Linux/FreeBSD core files are a container (often ELF) with
NOTES containing additional information.
By format are you suggesting we come up with something non-ELF? I
doubt this is the case, but if so, why would we want to do this?
Or are you suggesting some kind of coordination for note
names/numbers? But I don't see how this is different to what we have
right now.
I'm tight on time right now, but I skimmed the threads you linked, I
looked at Fredrik's v4 patch[1] from Oct-25. This patch had some of
the boiler-plate code lifted into a common file, which I could
integrate into my series, but otherwise the patches are pretty
similar.
You hadn't followed up to Fredrik's v4 patch[1], but based on your
comments to the v2 patch[2] I'm guessing you're still not happy with
v4 (please do correct me if this guess is wrong).
In your comments to Fredrik's v2 patch[2] you say:
We should really discuss what the generic core file format for
bare-metal targets will look like before having a possible patch to
implement it. At least that's my take on it.
To which there's no follow up. Is there a mail I've missed where you
sketch out your ideas for what this generic format might look like?
How about I propose something here to get the ball rolling, and you
(or others) can suggest improvements, or ask for clarifying questions:
- A container file format, lets say ELF,
- Loadable sections corresponding to the loaded memory contents,
- A NOTES section containing auxiliary information, e.g.
+ Threads (thread per core?)
+ Registers for each thread
Unsurprisingly this lines up with what I've proposed, and if I'm
reading the patch correctly, with what Fredrik is also proposing.
Thanks,
Andrew
[1] https://sourceware.org/pipermail/gdb-patches/2020-October/172845.html
[2] https://sourceware.org/pipermail/gdb-patches/2020-October/172721.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 18:11 ` Andrew Burgess
@ 2020-12-07 19:00 ` Luis Machado
2020-12-07 19:23 ` Andrew Burgess
0 siblings, 1 reply; 38+ messages in thread
From: Luis Machado @ 2020-12-07 19:00 UTC (permalink / raw)
To: Andrew Burgess; +Cc: binutils, gdb-patches, Paul Mathieu, Fredrik Hederstierna
On 12/7/20 3:11 PM, Andrew Burgess wrote:
> * Luis Machado <luis.machado@linaro.org> [2020-12-07 14:24:33 -0300]:
>
>> On 12/7/20 1:58 PM, Andrew Burgess wrote:
>>> * Luis Machado <luis.machado@linaro.org> [2020-12-07 12:58:19 -0300]:
>>>
>>>> On 12/7/20 12:17 PM, Andrew Burgess wrote:
>>>>> * 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.
>>>>
>>>> Sure. What I'm proposing here is the same I proposed in this thread about
>>>> ARM bare metal core files...
>>>>
>>>> https://sourceware.org/pipermail/gdb-patches/2020-October/172617.html
>>>>
>>>> ... and also suggested by Simon here:
>>>>
>>>> https://sourceware.org/pipermail/gdb-patches/2020-October/172270.html
>>>>
>>>> In summary, we should document what a bare metal core file is, what pieces
>>>> it is expected to contain (registers, target descriptions, hardware
>>>> information, execution environment etc) and what format it will be in.
>>>>
>>>>>
>>>>> 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
>>>>
>>>> Having a common format makes it easier to maintain the code and expand the
>>>> features when needed. This is not different from the Linux core file we have
>>>> today. The Linux core file is a common format.
>>>>
>>>> But unlike Linux core files, which have less than ideal documentation and
>>>> specifications, we want to take this opportunity to start clean and to
>>>> document everything required to have a proper bare metal core file.
>>>>
>>>> I think this initial definition and documentation will benefit developers
>>>> moving forward.
>>>>
>>>> Does that make it more clear?
>>>
>>> Not really. I'll go read the various threads that you referenced and
>>> see come back once I have more specific questions.
>>>
>>> Thanks for the pointers.
>>>
>>> Andrew
>>>
>>
>> I'm sorry that did not clarify things, but there isn't much more than that
>> really. All I'm suggesting (up to you to accept it) is that we
>> clarify/coordinate with the interested parties on what is needed for bare
>> metal core files and document it properly (I don't think code is proper
>> documentation).
>
> Sure.
>
> What I don't understand is you talk about creating "a format that
> works for multiple architectures".
>
> Current Linux/FreeBSD core files are a container (often ELF) with
> NOTES containing additional information.
>
> By format are you suggesting we come up with something non-ELF? I
> doubt this is the case, but if so, why would we want to do this?
>
> Or are you suggesting some kind of coordination for note
> names/numbers? But I don't see how this is different to what we have
> right now.
Sorry, I think you're reading too deeply into what I said.
All I've suggested is to let the interested parties know you're doing
this work, get their feedback on the strategy for dumping data for bare
metal targets (I've cc-ed two of them, so they can chime in), see if
that works for them with little to no extra work, and document it in a
formal way (say, in gdb.texinfo).
That's it. I'm not talking about code, patch or implementation. I'm
talking about a bare metal core file draft design that everyone is happy
with, and based on which folks can go and implement what they want
without having to dig into GDB's code for that.
You have the RISC-V implementation (it looks good to me FWIW), that's
fine. But what is it implementing? It is not documented anywhere, is it?
It is just following whatever Linux is doing, which is already badly
documented.
>
> I'm tight on time right now, but I skimmed the threads you linked, I
> looked at Fredrik's v4 patch[1] from Oct-25. This patch had some of
> the boiler-plate code lifted into a common file, which I could
> integrate into my series, but otherwise the patches are pretty
> similar.
>
> You hadn't followed up to Fredrik's v4 patch[1], but based on your
> comments to the v2 patch[2] I'm guessing you're still not happy with
> v4 (please do correct me if this guess is wrong).
>
> In your comments to Fredrik's v2 patch[2] you say:
>
> We should really discuss what the generic core file format for
> bare-metal targets will look like before having a possible patch to
> implement it. At least that's my take on it.
>
> To which there's no follow up. Is there a mail I've missed where you
> sketch out your ideas for what this generic format might look like?
>
> How about I propose something here to get the ball rolling, and you
> (or others) can suggest improvements, or ask for clarifying questions: >
> - A container file format, lets say ELF,
> - Loadable sections corresponding to the loaded memory contents,
> - A NOTES section containing auxiliary information, e.g.
> + Threads (thread per core?)
> + Registers for each thread >
> Unsurprisingly this lines up with what I've proposed, and if I'm
> reading the patch correctly, with what Fredrik is also proposing.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 19:00 ` Luis Machado
@ 2020-12-07 19:23 ` Andrew Burgess
2020-12-07 19:39 ` Luis Machado
0 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 19:23 UTC (permalink / raw)
To: Luis Machado; +Cc: binutils, gdb-patches, Paul Mathieu, Fredrik Hederstierna
* Luis Machado <luis.machado@linaro.org> [2020-12-07 16:00:06 -0300]:
> On 12/7/20 3:11 PM, Andrew Burgess wrote:
> > * Luis Machado <luis.machado@linaro.org> [2020-12-07 14:24:33 -0300]:
> >
> > > On 12/7/20 1:58 PM, Andrew Burgess wrote:
> > > > * Luis Machado <luis.machado@linaro.org> [2020-12-07 12:58:19 -0300]:
> > > >
> > > > > On 12/7/20 12:17 PM, Andrew Burgess wrote:
> > > > > > * 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.
> > > > >
> > > > > Sure. What I'm proposing here is the same I proposed in this thread about
> > > > > ARM bare metal core files...
> > > > >
> > > > > https://sourceware.org/pipermail/gdb-patches/2020-October/172617.html
> > > > >
> > > > > ... and also suggested by Simon here:
> > > > >
> > > > > https://sourceware.org/pipermail/gdb-patches/2020-October/172270.html
> > > > >
> > > > > In summary, we should document what a bare metal core file is, what pieces
> > > > > it is expected to contain (registers, target descriptions, hardware
> > > > > information, execution environment etc) and what format it will be in.
> > > > >
> > > > > >
> > > > > > 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
> > > > >
> > > > > Having a common format makes it easier to maintain the code and expand the
> > > > > features when needed. This is not different from the Linux core file we have
> > > > > today. The Linux core file is a common format.
> > > > >
> > > > > But unlike Linux core files, which have less than ideal documentation and
> > > > > specifications, we want to take this opportunity to start clean and to
> > > > > document everything required to have a proper bare metal core file.
> > > > >
> > > > > I think this initial definition and documentation will benefit developers
> > > > > moving forward.
> > > > >
> > > > > Does that make it more clear?
> > > >
> > > > Not really. I'll go read the various threads that you referenced and
> > > > see come back once I have more specific questions.
> > > >
> > > > Thanks for the pointers.
> > > >
> > > > Andrew
> > > >
> > >
> > > I'm sorry that did not clarify things, but there isn't much more than that
> > > really. All I'm suggesting (up to you to accept it) is that we
> > > clarify/coordinate with the interested parties on what is needed for bare
> > > metal core files and document it properly (I don't think code is proper
> > > documentation).
> >
> > Sure.
> >
> > What I don't understand is you talk about creating "a format that
> > works for multiple architectures".
> >
> > Current Linux/FreeBSD core files are a container (often ELF) with
> > NOTES containing additional information.
> >
> > By format are you suggesting we come up with something non-ELF? I
> > doubt this is the case, but if so, why would we want to do this?
> >
> > Or are you suggesting some kind of coordination for note
> > names/numbers? But I don't see how this is different to what we have
> > right now.
>
> Sorry, I think you're reading too deeply into what I said.
>
> All I've suggested is to let the interested parties know you're doing this
> work, get their feedback on the strategy for dumping data for bare metal
> targets (I've cc-ed two of them, so they can chime in), see if that works
> for them with little to no extra work, and document it in a formal way (say,
> in gdb.texinfo).
>
> That's it. I'm not talking about code, patch or implementation. I'm talking
> about a bare metal core file draft design that everyone is happy with, and
> based on which folks can go and implement what they want without having to
> dig into GDB's code for that.
>
> You have the RISC-V implementation (it looks good to me FWIW), that's fine.
> But what is it implementing? It is not documented anywhere, is it? It is
> just following whatever Linux is doing, which is already badly
> documented.
Oh, OK! That's much smaller in scope then.
Jim already asked about getting this documented in a different reply,
my plan is to document the RISC-V parts of this into the RISC-V elf
abi document (assuming the maintainers of that document accept such a
patch).
I'll re-review Fredrik's v4 patch and see if there's anything we can
share there, though from my first look through we're basically doing
the same thing already as that's the only obvious approach to take
(assuming the goal is ELF + NOTES).
I'll also reread your comments on numbering of the tdesc note and see
if there's a better number I can/should propose.
Thanks,
Andrew
>
> >
> > I'm tight on time right now, but I skimmed the threads you linked, I
> > looked at Fredrik's v4 patch[1] from Oct-25. This patch had some of
> > the boiler-plate code lifted into a common file, which I could
> > integrate into my series, but otherwise the patches are pretty
> > similar.
> >
> > You hadn't followed up to Fredrik's v4 patch[1], but based on your
> > comments to the v2 patch[2] I'm guessing you're still not happy with
> > v4 (please do correct me if this guess is wrong).
> >
> > In your comments to Fredrik's v2 patch[2] you say:
> >
> > We should really discuss what the generic core file format for
> > bare-metal targets will look like before having a possible patch to
> > implement it. At least that's my take on it.
> >
> > To which there's no follow up. Is there a mail I've missed where you
> > sketch out your ideas for what this generic format might look like?
> >
> > How about I propose something here to get the ball rolling, and you
> > (or others) can suggest improvements, or ask for clarifying questions: >
> > - A container file format, lets say ELF,
> > - Loadable sections corresponding to the loaded memory contents,
> > - A NOTES section containing auxiliary information, e.g.
> > + Threads (thread per core?)
> > + Registers for each thread >
> > Unsurprisingly this lines up with what I've proposed, and if I'm
> > reading the patch correctly, with what Fredrik is also proposing.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 19:23 ` Andrew Burgess
@ 2020-12-07 19:39 ` Luis Machado
2020-12-07 19:51 ` Paul Mathieu
0 siblings, 1 reply; 38+ messages in thread
From: Luis Machado @ 2020-12-07 19:39 UTC (permalink / raw)
To: Andrew Burgess; +Cc: binutils, gdb-patches, Paul Mathieu, Fredrik Hederstierna
On 12/7/20 4:23 PM, Andrew Burgess wrote:
> * Luis Machado <luis.machado@linaro.org> [2020-12-07 16:00:06 -0300]:
>
>> On 12/7/20 3:11 PM, Andrew Burgess wrote:
>>> * Luis Machado <luis.machado@linaro.org> [2020-12-07 14:24:33 -0300]:
>>>
>>>> On 12/7/20 1:58 PM, Andrew Burgess wrote:
>>>>> * Luis Machado <luis.machado@linaro.org> [2020-12-07 12:58:19 -0300]:
>>>>>
>>>>>> On 12/7/20 12:17 PM, Andrew Burgess wrote:
>>>>>>> * 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.
>>>>>>
>>>>>> Sure. What I'm proposing here is the same I proposed in this thread about
>>>>>> ARM bare metal core files...
>>>>>>
>>>>>> https://sourceware.org/pipermail/gdb-patches/2020-October/172617.html
>>>>>>
>>>>>> ... and also suggested by Simon here:
>>>>>>
>>>>>> https://sourceware.org/pipermail/gdb-patches/2020-October/172270.html
>>>>>>
>>>>>> In summary, we should document what a bare metal core file is, what pieces
>>>>>> it is expected to contain (registers, target descriptions, hardware
>>>>>> information, execution environment etc) and what format it will be in.
>>>>>>
>>>>>>>
>>>>>>> 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
>>>>>>
>>>>>> Having a common format makes it easier to maintain the code and expand the
>>>>>> features when needed. This is not different from the Linux core file we have
>>>>>> today. The Linux core file is a common format.
>>>>>>
>>>>>> But unlike Linux core files, which have less than ideal documentation and
>>>>>> specifications, we want to take this opportunity to start clean and to
>>>>>> document everything required to have a proper bare metal core file.
>>>>>>
>>>>>> I think this initial definition and documentation will benefit developers
>>>>>> moving forward.
>>>>>>
>>>>>> Does that make it more clear?
>>>>>
>>>>> Not really. I'll go read the various threads that you referenced and
>>>>> see come back once I have more specific questions.
>>>>>
>>>>> Thanks for the pointers.
>>>>>
>>>>> Andrew
>>>>>
>>>>
>>>> I'm sorry that did not clarify things, but there isn't much more than that
>>>> really. All I'm suggesting (up to you to accept it) is that we
>>>> clarify/coordinate with the interested parties on what is needed for bare
>>>> metal core files and document it properly (I don't think code is proper
>>>> documentation).
>>>
>>> Sure.
>>>
>>> What I don't understand is you talk about creating "a format that
>>> works for multiple architectures".
>>>
>>> Current Linux/FreeBSD core files are a container (often ELF) with
>>> NOTES containing additional information.
>>>
>>> By format are you suggesting we come up with something non-ELF? I
>>> doubt this is the case, but if so, why would we want to do this?
>>>
>>> Or are you suggesting some kind of coordination for note
>>> names/numbers? But I don't see how this is different to what we have
>>> right now.
>>
>> Sorry, I think you're reading too deeply into what I said.
>>
>> All I've suggested is to let the interested parties know you're doing this
>> work, get their feedback on the strategy for dumping data for bare metal
>> targets (I've cc-ed two of them, so they can chime in), see if that works
>> for them with little to no extra work, and document it in a formal way (say,
>> in gdb.texinfo).
>>
>> That's it. I'm not talking about code, patch or implementation. I'm talking
>> about a bare metal core file draft design that everyone is happy with, and
>> based on which folks can go and implement what they want without having to
>> dig into GDB's code for that.
>>
>> You have the RISC-V implementation (it looks good to me FWIW), that's fine.
>> But what is it implementing? It is not documented anywhere, is it? It is
>> just following whatever Linux is doing, which is already badly
>> documented.
>
> Oh, OK! That's much smaller in scope then.
>
Indeed! It was never my goal to create more work for you.
> Jim already asked about getting this documented in a different reply,
> my plan is to document the RISC-V parts of this into the RISC-V elf
> abi document (assuming the maintainers of that document accept such a
> patch).
Great. Apologies, I haven't been following the other bits too closely. I
just replied to the ones I had immediate feedback on.
>
> I'll re-review Fredrik's v4 patch and see if there's anything we can
> share there, though from my first look through we're basically doing
> the same thing already as that's the only obvious approach to take
> (assuming the goal is ELF + NOTES).
I think it is pretty safe to assume ELF + NOTES at this point.
>
> I'll also reread your comments on numbering of the tdesc note and see
> if there's a better number I can/should propose.
That's probably a matter of personal taste. I've been frustrated before,
trying to find the most logical new ID for a new NT_* constant and
failing to parse the data.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 19:39 ` Luis Machado
@ 2020-12-07 19:51 ` Paul Mathieu
2020-12-13 10:13 ` Fredrik Hederstierna
0 siblings, 1 reply; 38+ messages in thread
From: Paul Mathieu @ 2020-12-07 19:51 UTC (permalink / raw)
To: Luis Machado; +Cc: Andrew Burgess, binutils, gdb-patches, Fredrik Hederstierna
Thanks Luis for getting the ball rolling again!
As far as I'm concerned, I haven't deployed much infrastructure to
generate or otherwise deal with bare metal cores at the moment, so I'm
happy to work with anything. The one thing that is a big deal to me is
support for RTOS threads.
ELF + NOTES seems like the obvious choice to me, as it matches what
I'm used to. Most online resources about core dumps seem to be
specifically about Linux core dumps, so it would be less surprising
and more helpful to share as much as possible with it, IMO.
What I'm expecting from a bare metal core dump:
- memory dump sections
- CPU registers
- when available (RTOS support from the dumping side): inactive
threads' CPU registers
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
2020-12-07 19:51 ` Paul Mathieu
@ 2020-12-13 10:13 ` Fredrik Hederstierna
0 siblings, 0 replies; 38+ messages in thread
From: Fredrik Hederstierna @ 2020-12-13 10:13 UTC (permalink / raw)
To: Paul Mathieu, Luis Machado; +Cc: Andrew Burgess, binutils, gdb-patches
> From: Paul Mathieu <paulmathieu@google.com>
> Sent: Monday, December 7, 2020 8:51 PM
> To: Luis Machado <luis.machado@linaro.org>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>; binutils@sourceware.org <binutils@sourceware.org>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>; Fredrik Hederstierna <fredrik.hederstierna@verisure.com>
> Subject: Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support
>
> Thanks Luis for getting the ball rolling again!
>
> As far as I'm concerned, I haven't deployed much infrastructure to
> generate or otherwise deal with bare metal cores at the moment, so I'm
> happy to work with anything. The one thing that is a big deal to me is
> support for RTOS threads.
>
> ELF + NOTES seems like the obvious choice to me, as it matches what
> I'm used to. Most online resources about core dumps seem to be
> specifically about Linux core dumps, so it would be less surprising
> and more helpful to share as much as possible with it, IMO.
>
> What I'm expecting from a bare metal core dump:
> - memory dump sections
> - CPU registers
> - when available (RTOS support from the dumping side): inactive
> threads' CPU registers
Thanks all, good to see that bare metal corefile support involves even more people, and increased interest to get gdb support this feature.
I'm sorry I haven't had much time this fall to put more time into it. The v4-patch as discussed here is my latest proposal, and as I understood it was some 'leftovers' yet regarding specification/documentation, but I had not good idea how to start this work, so I think I would need to some help where to start off.
Hopefully the more generic parts of the v4-patch like the 'none-tdep.c' parts could be reused by riscv, the it can open paths to make the future ARM-support less work and easier to get it done. Then hopefully the riscv documentation parts could be very similar and also could be possibly reused for the ARM corefile format.
Anything that could help the arm-none-corefile support is great, nice if these patches for arm and riscv do not diverge, better if the patches could 'help each other' to get a good solution as generic as possible, also opening paths for future corefiles for even other CPU architectures.
Can I do anything right now to help proceeding this process, I'm sorry I do not really know what I can do currently to 'make it happen' :-)
Thanks all, Best Regards,
Fredrik
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 6/8] bfd/binutils: add support for RISC-V CSRs in core files
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
` (4 preceding siblings ...)
2020-12-02 17:39 ` [PATCH 5/8] gdb/riscv: introduce bare metal core dump support Andrew Burgess
@ 2020-12-02 17:39 ` Andrew Burgess
2020-12-02 23:50 ` Jim Wilson
2020-12-02 17:39 ` [PATCH 7/8] gdb/riscv: make riscv target description names global Andrew Burgess
` (3 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-02 17:39 UTC (permalink / raw)
To: binutils, gdb-patches
Cc: Nelson Chu, Jim Wilson, John Baldwin, Andrew Burgess
Adds support for including RISC-V control and status registers into
core files.
bfd/ChangeLog:
* elf-bfd.h (elfcore_write_riscv_csr): Declare.
* elf.c (elfcore_grok_riscv_csr): New function.
(elfcore_grok_note): Handle NT_RISCV_CSR.
(elfcore_write_riscv_csr): New function.
(elfcore_write_register_note): Handle '.reg-riscv-csr'.
binutils/ChangeLog:
* readelf.c (get_note_type): Handle NT_RISCV_CSR.
include/ChangeLog:
* elf/common.h (NT_RISCV_CSR): Define.
---
bfd/ChangeLog | 9 +++++++++
bfd/elf-bfd.h | 2 ++
bfd/elf.c | 31 +++++++++++++++++++++++++++----
binutils/ChangeLog | 5 +++++
binutils/readelf.c | 2 ++
include/ChangeLog | 5 +++++
include/elf/common.h | 2 ++
7 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 5ef69ab5b13..7de09c269f9 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2796,6 +2796,8 @@ extern char *elfcore_write_aarch_pauth
(bfd *, char *, int *, const void *, int);
extern char *elfcore_write_arc_v2
(bfd *, char *, int *, const void *, int);
+extern char *elfcore_write_riscv_csr
+ (bfd *, char *, int *, const void *, int);
extern char *elfcore_write_gdb_tdesc
(bfd *, char *, int *, const void *, int);
extern char *elfcore_write_lwpstatus
diff --git a/bfd/elf.c b/bfd/elf.c
index bea5ab12773..62036db4c69 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9909,6 +9909,12 @@ elfcore_grok_arc_v2 (bfd *abfd, Elf_Internal_Note *note)
return elfcore_make_note_pseudosection (abfd, ".reg-arc-v2", note);
}
+static bfd_boolean
+elfcore_grok_riscv_csr (bfd *abfd, Elf_Internal_Note *note)
+{
+ return elfcore_make_note_pseudosection (abfd, ".reg-riscv-csr", note);
+}
+
static bfd_boolean
elfcore_grok_gdb_tdesc (bfd *abfd, Elf_Internal_Note *note)
{
@@ -10575,6 +10581,9 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
case NT_GDB_TDESC:
return elfcore_grok_gdb_tdesc (abfd, note);
+ case NT_RISCV_CSR:
+ return elfcore_grok_riscv_csr (abfd, note);
+
case NT_PRPSINFO:
case NT_PSINFO:
if (bed->elf_backend_grok_psinfo)
@@ -11956,12 +11965,24 @@ elfcore_write_arc_v2 (bfd *abfd,
note_name, NT_ARC_V2, arc_v2, size);
}
+char *
+elfcore_write_riscv_csr (bfd *abfd,
+ char *buf,
+ int *bufsiz,
+ const void *csrs,
+ int size)
+{
+ const char *note_name = "CORE";
+ return elfcore_write_note (abfd, buf, bufsiz,
+ note_name, NT_RISCV_CSR, csrs, size);
+}
+
char *
elfcore_write_gdb_tdesc (bfd *abfd,
- char *buf,
- int *bufsiz,
- const void *tdesc,
- int size)
+ char *buf,
+ int *bufsiz,
+ const void *tdesc,
+ int size)
{
const char *note_name = "CORE";
return elfcore_write_note (abfd, buf, bufsiz,
@@ -12054,6 +12075,8 @@ elfcore_write_register_note (bfd *abfd,
return elfcore_write_arc_v2 (abfd, buf, bufsiz, data, size);
if (strcmp (section, ".gdb-tdesc") == 0)
return elfcore_write_gdb_tdesc (abfd, buf, bufsiz, data, size);
+ if (strcmp (section, ".reg-riscv-csr") == 0)
+ return elfcore_write_riscv_csr (abfd, buf, bufsiz, data, size);
return NULL;
}
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 5b3871d3e5f..4706d40f7ce 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -18324,6 +18324,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
return _("NT_ARM_HW_WATCH (AArch hardware watchpoint registers)");
case NT_ARC_V2:
return _("NT_ARC_V2 (ARC HS accumulator/extra registers)");
+ case NT_RISCV_CSR:
+ return _("NT_RISCV_CSR (RISC-V control and status registers)");
case NT_PSTATUS:
return _("NT_PSTATUS (pstatus structure)");
case NT_FPREGS:
diff --git a/include/elf/common.h b/include/elf/common.h
index 1dbf0b11983..54d5d989a39 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -663,6 +663,8 @@
/* note name must be "LINUX". */
#define NT_ARC_V2 0x600 /* ARC HS accumulator/extra registers. */
/* note name must be "LINUX". */
+#define NT_RISCV_CSR 0x501 /* RISC-V Control and Status Registers */
+ /* note name must be "CORE". */
#define NT_SIGINFO 0x53494749 /* Fields of siginfo_t. */
#define NT_FILE 0x46494c45 /* Description of mapped files. */
--
2.25.4
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 6/8] bfd/binutils: add support for RISC-V CSRs in core files
2020-12-02 17:39 ` [PATCH 6/8] bfd/binutils: add support for RISC-V CSRs in core files Andrew Burgess
@ 2020-12-02 23:50 ` Jim Wilson
2020-12-07 15:19 ` Andrew Burgess
2020-12-14 13:37 ` Andrew Burgess
0 siblings, 2 replies; 38+ messages in thread
From: Jim Wilson @ 2020-12-02 23:50 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Binutils, gdb-patches, Nelson Chu, John Baldwin
On Wed, Dec 2, 2020 at 9:39 AM Andrew Burgess <andrew.burgess@embecosm.com>
wrote:
> diff --git a/include/elf/common.h b/include/elf/common.h
> index 1dbf0b11983..54d5d989a39 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -663,6 +663,8 @@
> /* note name must be "LINUX". */
> #define NT_ARC_V2 0x600 /* ARC HS accumulator/extra
> registers. */
> /* note name must be "LINUX". */
> +#define NT_RISCV_CSR 0x501 /* RISC-V Control and Status
> Registers */
> + /* note name must be "CORE". */
> #define NT_SIGINFO 0x53494749 /* Fields of siginfo_t. */
> #define NT_FILE 0x46494c45 /* Description of mapped
> files. */
>
Odd that the 0x5XX numbering was skipped for ARC, though I don't see any
reason for it, so it appears OK to use for RISC-V. I would suggest keeping
them in numerical order to make the file easier to read, so if you want to
use 0x501 put it before the ARC entry.
Otherwise, this looks OK to me with a trivial change here.
Jim
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 6/8] bfd/binutils: add support for RISC-V CSRs in core files
2020-12-02 23:50 ` Jim Wilson
@ 2020-12-07 15:19 ` Andrew Burgess
2020-12-14 13:37 ` Andrew Burgess
1 sibling, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 15:19 UTC (permalink / raw)
To: Jim Wilson; +Cc: Binutils, gdb-patches, Nelson Chu, John Baldwin
* Jim Wilson <jimw@sifive.com> [2020-12-02 15:50:58 -0800]:
> On Wed, Dec 2, 2020 at 9:39 AM Andrew Burgess <andrew.burgess@embecosm.com>
> wrote:
>
> > diff --git a/include/elf/common.h b/include/elf/common.h
> > index 1dbf0b11983..54d5d989a39 100644
> > --- a/include/elf/common.h
> > +++ b/include/elf/common.h
> > @@ -663,6 +663,8 @@
> > /* note name must be "LINUX". */
> > #define NT_ARC_V2 0x600 /* ARC HS accumulator/extra
> > registers. */
> > /* note name must be "LINUX". */
> > +#define NT_RISCV_CSR 0x501 /* RISC-V Control and Status
> > Registers */
> > + /* note name must be "CORE". */
> > #define NT_SIGINFO 0x53494749 /* Fields of siginfo_t. */
> > #define NT_FILE 0x46494c45 /* Description of mapped
> > files. */
> >
>
> Odd that the 0x5XX numbering was skipped for ARC, though I don't see any
> reason for it, so it appears OK to use for RISC-V. I would suggest keeping
> them in numerical order to make the file easier to read, so if you want to
> use 0x501 put it before the ARC entry.
I've reordered the lines in my current patch tree as you suggest.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 6/8] bfd/binutils: add support for RISC-V CSRs in core files
2020-12-02 23:50 ` Jim Wilson
2020-12-07 15:19 ` Andrew Burgess
@ 2020-12-14 13:37 ` Andrew Burgess
1 sibling, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2020-12-14 13:37 UTC (permalink / raw)
To: Jim Wilson; +Cc: Binutils, gdb-patches, Nelson Chu, John Baldwin
* Jim Wilson <jimw@sifive.com> [2020-12-02 15:50:58 -0800]:
> On Wed, Dec 2, 2020 at 9:39 AM Andrew Burgess <andrew.burgess@embecosm.com>
> wrote:
>
> > diff --git a/include/elf/common.h b/include/elf/common.h
> > index 1dbf0b11983..54d5d989a39 100644
> > --- a/include/elf/common.h
> > +++ b/include/elf/common.h
> > @@ -663,6 +663,8 @@
> > /* note name must be "LINUX". */
> > #define NT_ARC_V2 0x600 /* ARC HS accumulator/extra
> > registers. */
> > /* note name must be "LINUX". */
> > +#define NT_RISCV_CSR 0x501 /* RISC-V Control and Status
> > Registers */
> > + /* note name must be "CORE". */
> > #define NT_SIGINFO 0x53494749 /* Fields of siginfo_t. */
> > #define NT_FILE 0x46494c45 /* Description of mapped
> > files. */
> >
>
> Odd that the 0x5XX numbering was skipped for ARC, though I don't see any
> reason for it, so it appears OK to use for RISC-V. I would suggest keeping
> them in numerical order to make the file easier to read, so if you want to
> use 0x501 put it before the ARC entry.
>
> Otherwise, this looks OK to me with a trivial change here.
On further reflection I wonder if I should submit a patch to the Linux
kernel to try and reserve this (or some other suitable) number,
otherwise there's a risk that whatever we pick could be used by the
kernel for some other architecture?
Thanks,
Andrew
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 7/8] gdb/riscv: make riscv target description names global
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
` (5 preceding siblings ...)
2020-12-02 17:39 ` [PATCH 6/8] bfd/binutils: add support for RISC-V CSRs in core files Andrew Burgess
@ 2020-12-02 17:39 ` Andrew Burgess
2020-12-02 17:39 ` [PATCH 8/8] gdb/riscv: write CSRs into baremetal core dumps Andrew Burgess
` (2 subsequent siblings)
9 siblings, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2020-12-02 17:39 UTC (permalink / raw)
To: binutils, gdb-patches
Cc: Nelson Chu, Jim Wilson, John Baldwin, Andrew Burgess
A later commit will need the names of the RISC-V target description
features in files other than riscv-tdep.c. This commit just makes the
names global strings that can be accessed from other riscv-*.c files.
There should be no user visible changes after this commit.
gdb/ChangeLog:
* riscv-tdep.c (riscv_feature_name_csr): Define.
(riscv_feature_name_cpu): Define.
(riscv_feature_name_fpu): Define.
(riscv_feature_name_virtual): Define.
(riscv_xreg_feature): Use riscv_feature_name_cpu.
(riscv_freg_feature): Use riscv_feature_name_fpu.
(riscv_virtual_feature): Use riscv_feature_name_virtual.
(riscv_csr_feature): Use riscv_feature_name_csr.
* riscv-tdep.h (riscv_feature_name_csr): Declare.
---
gdb/ChangeLog | 12 ++++++++++++
gdb/riscv-tdep.c | 14 ++++++++++----
gdb/riscv-tdep.h | 3 +++
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index a428f79940d..eb2ccb42d5e 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -74,6 +74,12 @@ static inline bool is_ ## INSN_NAME ## _insn (long insn) \
#include "opcode/riscv-opc.h"
#undef DECLARE_INSN
+/* The names of the RISC-V target description features. */
+const char *riscv_feature_name_csr = "org.gnu.gdb.riscv.csr";
+static const char *riscv_feature_name_cpu = "org.gnu.gdb.riscv.cpu";
+static const char *riscv_feature_name_fpu = "org.gnu.gdb.riscv.fpu";
+static const char *riscv_feature_name_virtual = "org.gnu.gdb.riscv.virtual";
+
/* Cached information about a frame. */
struct riscv_unwind_cache
@@ -252,7 +258,7 @@ riscv_register_feature::register_info::check
static const struct riscv_register_feature riscv_xreg_feature =
{
- "org.gnu.gdb.riscv.cpu", true,
+ riscv_feature_name_cpu, true,
{
{ RISCV_ZERO_REGNUM + 0, { "zero", "x0" }, RISCV_REG_REQUIRED },
{ RISCV_ZERO_REGNUM + 1, { "ra", "x1" }, RISCV_REG_REQUIRED },
@@ -294,7 +300,7 @@ static const struct riscv_register_feature riscv_xreg_feature =
static const struct riscv_register_feature riscv_freg_feature =
{
- "org.gnu.gdb.riscv.fpu", true,
+ riscv_feature_name_fpu, true,
{
{ RISCV_FIRST_FP_REGNUM + 0, { "ft0", "f0" }, RISCV_REG_REQUIRED },
{ RISCV_FIRST_FP_REGNUM + 1, { "ft1", "f1" }, RISCV_REG_REQUIRED },
@@ -344,7 +350,7 @@ static const struct riscv_register_feature riscv_freg_feature =
static const struct riscv_register_feature riscv_virtual_feature =
{
- "org.gnu.gdb.riscv.virtual", false,
+ riscv_feature_name_virtual, false,
{
{ RISCV_PRIV_REGNUM, { "priv" }, RISCV_REG_OPTIONAL }
}
@@ -356,7 +362,7 @@ static const struct riscv_register_feature riscv_virtual_feature =
static struct riscv_register_feature riscv_csr_feature =
{
- "org.gnu.gdb.riscv.csr", false,
+ riscv_feature_name_csr, false,
{
#define DECLARE_CSR(NAME,VALUE,CLASS,DEFINE_VER,ABORT_VER) \
{ RISCV_ ## VALUE ## _REGNUM, { # NAME }, RISCV_REG_OPTIONAL },
diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
index 1064ced1192..fc8013cf417 100644
--- a/gdb/riscv-tdep.h
+++ b/gdb/riscv-tdep.h
@@ -155,4 +155,7 @@ extern void riscv_supply_regset (const struct regset *regset,
struct regcache *regcache, int regnum,
const void *regs, size_t len);
+/* The names of the RISC-V target description features. */
+extern const char *riscv_feature_name_csr;
+
#endif /* RISCV_TDEP_H */
--
2.25.4
^ permalink raw reply [flat|nested] 38+ messages in thread* [PATCH 8/8] gdb/riscv: write CSRs into baremetal core dumps
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
` (6 preceding siblings ...)
2020-12-02 17:39 ` [PATCH 7/8] gdb/riscv: make riscv target description names global Andrew Burgess
@ 2020-12-02 17:39 ` Andrew Burgess
2020-12-02 23:59 ` [PATCH 0/8] Bare-metal core dumps for RISC-V Jim Wilson
2020-12-03 20:40 ` Tom Tromey
9 siblings, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2020-12-02 17:39 UTC (permalink / raw)
To: binutils, gdb-patches
Cc: Nelson Chu, Jim Wilson, John Baldwin, Andrew Burgess
Use the current target description to include CSRs into the RISC-V
baremetal core dumps.
Every CSR declared in the current target description will be included
in the core dump. If a CSR fails to read then the value 0 will be
placed into the core dump instead.
It will be critical for users that they have the same target
description in use when loading the core file as was in use when
writing the core file. This should be fine if the user allows the
target description to be written into the core file.
gdb/ChangeLog:
* riscv-non-tdep.c (riscv_csrset): New static global.
(riscv_update_csrmap): New function.
(riscv_iterate_over_regset_sections): Process CSRs.
---
gdb/ChangeLog | 7 +++++
gdb/riscv-none-tdep.c | 60 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/gdb/riscv-none-tdep.c b/gdb/riscv-none-tdep.c
index 0a4215a60e9..b20e2d8f475 100644
--- a/gdb/riscv-none-tdep.c
+++ b/gdb/riscv-none-tdep.c
@@ -27,6 +27,8 @@
#include "riscv-tdep.h"
#include "elf-bfd.h"
#include "regset.h"
+#include "user-regs.h"
+#include "target-descriptions.h"
/* Function declarations. */
@@ -250,6 +252,42 @@ static const struct regset riscv_fregset =
riscv_fregmap, riscv_supply_regset, regcache_collect_regset
};
+/* Define the CSR regset, this is not constant as the regmap field is
+ updated dynamically based on the current target description. */
+
+static struct regset riscv_csrset =
+{
+ nullptr, regcache_supply_regset, regcache_collect_regset
+};
+
+/* Update the regmap field of RISCV_CSRSET based on the CSRs available in
+ the current target description. */
+
+static void
+riscv_update_csrmap (struct gdbarch *gdbarch,
+ const struct tdesc_feature *feature_csr)
+{
+ int i = 0;
+
+ /* Release any previously defined map. */
+ delete[] ((struct regcache_map_entry *) riscv_csrset.regmap);
+
+ /* Now create a register map for every csr found in the target
+ description. */
+ struct regcache_map_entry *riscv_csrmap
+ = new struct regcache_map_entry[feature_csr->registers.size() + 1];
+ for (auto &csr : feature_csr->registers)
+ {
+ int regnum = user_reg_map_name_to_regnum (gdbarch, csr->name.c_str(),
+ csr->name.length());
+ riscv_csrmap[i++] = {1, regnum, 0};
+ }
+
+ /* Mark the end of the array. */
+ riscv_csrmap[i] = {0};
+ riscv_csrset.regmap = riscv_csrmap;
+}
+
/* Callback for iterate_over_regset_sections that records a single regset
in the corefile note section. */
@@ -313,6 +351,28 @@ riscv_iterate_over_regset_sections (struct gdbarch *gdbarch,
+ register_size (gdbarch, RISCV_CSR_FCSR_REGNUM));
cb (".reg2", sz, sz, &riscv_fregset, NULL, cb_data);
}
+
+ /* Read or write the CSRs. The set of CSRs is defined by the current
+ target description. The user is responsible for ensuring that the
+ same target description is in use when reading the core file as was
+ in use when writing the core file. */
+ const struct target_desc *tdesc = gdbarch_target_desc (gdbarch);
+
+ /* Do not dump/load any CSRs if there is no target description or the target
+ description does not contain any CSRs. */
+ if (tdesc != nullptr)
+ {
+ const struct tdesc_feature *feature_csr
+ = tdesc_find_feature (tdesc, riscv_feature_name_csr);
+ if (feature_csr != nullptr && feature_csr->registers.size () > 0)
+ {
+ riscv_update_csrmap (gdbarch, feature_csr);
+ cb (".reg-riscv-csr",
+ (feature_csr->registers.size() * riscv_isa_xlen (gdbarch)),
+ (feature_csr->registers.size() * riscv_isa_xlen (gdbarch)),
+ &riscv_csrset, NULL, cb_data);
+ }
+ }
}
/* Initialize RISC-V bare-metal ABI info. */
--
2.25.4
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 0/8] Bare-metal core dumps for RISC-V
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
` (7 preceding siblings ...)
2020-12-02 17:39 ` [PATCH 8/8] gdb/riscv: write CSRs into baremetal core dumps Andrew Burgess
@ 2020-12-02 23:59 ` Jim Wilson
2020-12-07 12:10 ` Andrew Burgess
2020-12-03 20:40 ` Tom Tromey
9 siblings, 1 reply; 38+ messages in thread
From: Jim Wilson @ 2020-12-02 23:59 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Binutils, gdb-patches, Nelson Chu, John Baldwin
On Wed, Dec 2, 2020 at 9:39 AM Andrew Burgess <andrew.burgess@embecosm.com>
wrote:
> This series touches both binutils and gdb. Patches #2, #4, and #6 are
> binutils patches, all the rest are gdb patches.
>
> The goal of this series is to add support to GDB for generating a core
> file for a bare metal RISC-V target.
>
> As part of this series patches #2 and #3 add a generic new feature to
> GDB, the ability to include the current target description in a
> generated core file.
>
Just as a general comment, the RISC-V psabi currently only specifies the
dwarf2 register numbers, which the compiler obviously needs. Otherwise, it
doesn't try to document anything gdb related, as it has been mainly
assembler/linker/compiler folk contributing to it. At some point we might
need some gdb related documentation. Some of the changes here potentially
affect llvm/lldb and the FreeBSD folks, and they may not be reading GNU
mailing lists.
Jim
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 0/8] Bare-metal core dumps for RISC-V
2020-12-02 23:59 ` [PATCH 0/8] Bare-metal core dumps for RISC-V Jim Wilson
@ 2020-12-07 12:10 ` Andrew Burgess
2020-12-07 19:57 ` Jim Wilson
0 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2020-12-07 12:10 UTC (permalink / raw)
To: Jim Wilson; +Cc: Binutils, gdb-patches, Nelson Chu, John Baldwin
* Jim Wilson <jimw@sifive.com> [2020-12-02 15:59:36 -0800]:
> On Wed, Dec 2, 2020 at 9:39 AM Andrew Burgess <andrew.burgess@embecosm.com>
> wrote:
>
> > This series touches both binutils and gdb. Patches #2, #4, and #6 are
> > binutils patches, all the rest are gdb patches.
> >
> > The goal of this series is to add support to GDB for generating a core
> > file for a bare metal RISC-V target.
> >
> > As part of this series patches #2 and #3 add a generic new feature to
> > GDB, the ability to include the current target description in a
> > generated core file.
> >
>
> Just as a general comment, the RISC-V psabi currently only specifies the
> dwarf2 register numbers, which the compiler obviously needs. Otherwise, it
> doesn't try to document anything gdb related, as it has been mainly
> assembler/linker/compiler folk contributing to it. At some point we might
> need some gdb related documentation. Some of the changes here potentially
> affect llvm/lldb and the FreeBSD folks, and they may not be reading GNU
> mailing lists.
Thanks for your feedback.
I agree that it would be a good idea to document this layout
somewhere. Am I correct in thinking that here:
https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md
Is where such things should be documented?
I guess it's probably worth getting the documentation approved before
merging to GDB in case changes are suggested.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 0/8] Bare-metal core dumps for RISC-V
2020-12-07 12:10 ` Andrew Burgess
@ 2020-12-07 19:57 ` Jim Wilson
0 siblings, 0 replies; 38+ messages in thread
From: Jim Wilson @ 2020-12-07 19:57 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Binutils, gdb-patches, Nelson Chu, John Baldwin
On Mon, Dec 7, 2020 at 4:10 AM Andrew Burgess <andrew.burgess@embecosm.com>
wrote:
> I agree that it would be a good idea to document this layout
> somewhere. Am I correct in thinking that here:
> https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md
> Is where such things should be documented?
>
Yes, this would be the correct place.
I guess it's probably worth getting the documentation approved before
> merging to GDB in case changes are suggested.
>
This would be nice, but I don't consider this required as a prerequisite,
considering that there has been no demand for gdb related docs as yet,
other than DWARF register numbers. I'm still waiting for the RVI T&R HSC
to create a committee to own the ABI, so meanwhile the ABI committee is
basically me, though I do usually try to get an opinion from at least one
LLVM developer before I accept patches. And I Iry to include FreeBSD folks
too when necessary.
FYI The binutils patches are OK now. I think the only unresolved issue is
the NT_ number to use, and I'm OK with the ascii number, though you may
need to get agreement from others on that.
Jim
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 0/8] Bare-metal core dumps for RISC-V
2020-12-02 17:39 [PATCH 0/8] Bare-metal core dumps for RISC-V Andrew Burgess
` (8 preceding siblings ...)
2020-12-02 23:59 ` [PATCH 0/8] Bare-metal core dumps for RISC-V Jim Wilson
@ 2020-12-03 20:40 ` Tom Tromey
9 siblings, 0 replies; 38+ messages in thread
From: Tom Tromey @ 2020-12-03 20:40 UTC (permalink / raw)
To: Andrew Burgess; +Cc: binutils, gdb-patches, John Baldwin
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
I read through this series. I think I had one comment. Most of it
seemed fine; though I don't really know much about the architecture
itself; and I don't have an opinion on the new names.
Andrew> As part of this series patches #2 and #3 add a generic new feature to
Andrew> GDB, the ability to include the current target description in a
Andrew> generated core file.
This seems pretty reasonable to me.
Tom
^ permalink raw reply [flat|nested] 38+ messages in thread