[RFC] Core file support for Aaarch64 SVE
Commit Message
The following patch fails to compile for an all targets build.
This is due to aarch64-linux-tdep.c requiring functions from
nat/aarch64-sve-linux-ptrace.c. This is used because the SVE
section of a aarch64 core file starts with a header structure
(the same as in a ptrace read), which is defined in the kernel.
I considered putting the nat/ file into an all targets build, but
this seemed wrong (in addition it would require additional .deps
dir adding for nat/).
What is the correct solution here? Should I move the required nat/
functions into elsewhere? The arch/ still doesn't feel right.
Other than the above issue, the patch is ready and working. I thought
it best to ask here rather than guess the correct solution.
Thanks,
Alan
Support both the reading and writing of core files on aarch64 SVE.
SVE core files are doumented here:
https://github.com/torvalds/linux/blob/master/Documentation/arm64/sve.txt
* A NT_ARM_SVE note will be added to each coredump for each thread of the
dumped process. The contents will be equivalent to the data that would have
been read if a PTRACE_GETREGSET of NT_ARM_SVE were executed for each thread
when the coredump was generated.
When creating a core files target description, read the SVE section to find
the vector length.
When reading/writing core files, use the existing nat/ functions to parse
the structure. Given that the function is only called through the one call
chain, there is no need to support copying a single register or invalidating
the set.
Dependant on binutils patch "[PATCH] Add BFD core support for Aarch64 SVE"
https://sourceware.org/ml/binutils/2018-06/msg00314.html
Checked with make check on x86 and aarch64.
Generated core files on SVE emulator both from gdb "generate-core-file"
command and a segfault. Loaded these back into gdb.
2018-06-25 Alan Hayward <alan.hayward@arm.com>
gdb/
* aarch64-linux-tdep.c (aarch64_linux_core_read_vq): New function.
(aarch64_linux_supply_sve_regset): Likewise.
(aarch64_linux_collect_sve_regset): Likewise.
(aarch64_linux_iterate_over_regset_sections): Check for SVE.
(aarch64_linux_core_read_description): Likewise.
* nat/aarch64-sve-linux-sigcontext.h (struct _aarch64_ctx): Add for
build compatibility.
(struct user_fpsimd_state): Likewise.
---
gdb/aarch64-linux-tdep.c | 114 ++++++++++++++++++++++++++++++++-
gdb/nat/aarch64-sve-linux-sigcontext.h | 14 ++++
2 files changed, 125 insertions(+), 3 deletions(-)
Comments
Ping.
Hoping patch should mostly be fine, just need some advice on where
to move functions to get the all-targets build working.
Alan.
> On 25 Jun 2018, at 16:10, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> The following patch fails to compile for an all targets build.
> This is due to aarch64-linux-tdep.c requiring functions from
> nat/aarch64-sve-linux-ptrace.c. This is used because the SVE
> section of a aarch64 core file starts with a header structure
> (the same as in a ptrace read), which is defined in the kernel.
>
> I considered putting the nat/ file into an all targets build, but
> this seemed wrong (in addition it would require additional .deps
> dir adding for nat/).
>
> What is the correct solution here? Should I move the required nat/
> functions into elsewhere? The arch/ still doesn't feel right.
>
> Other than the above issue, the patch is ready and working. I thought
> it best to ask here rather than guess the correct solution.
>
> Thanks,
> Alan
>
> Support both the reading and writing of core files on aarch64 SVE.
>
> SVE core files are doumented here:
> https://github.com/torvalds/linux/blob/master/Documentation/arm64/sve.txt
> * A NT_ARM_SVE note will be added to each coredump for each thread of the
> dumped process. The contents will be equivalent to the data that would have
> been read if a PTRACE_GETREGSET of NT_ARM_SVE were executed for each thread
> when the coredump was generated.
>
> When creating a core files target description, read the SVE section to find
> the vector length.
>
> When reading/writing core files, use the existing nat/ functions to parse
> the structure. Given that the function is only called through the one call
> chain, there is no need to support copying a single register or invalidating
> the set.
>
> Dependant on binutils patch "[PATCH] Add BFD core support for Aarch64 SVE"
> https://sourceware.org/ml/binutils/2018-06/msg00314.html
>
> Checked with make check on x86 and aarch64.
> Generated core files on SVE emulator both from gdb "generate-core-file"
> command and a segfault. Loaded these back into gdb.
>
> 2018-06-25 Alan Hayward <alan.hayward@arm.com>
>
> gdb/
> * aarch64-linux-tdep.c (aarch64_linux_core_read_vq): New function.
> (aarch64_linux_supply_sve_regset): Likewise.
> (aarch64_linux_collect_sve_regset): Likewise.
> (aarch64_linux_iterate_over_regset_sections): Check for SVE.
> (aarch64_linux_core_read_description): Likewise.
> * nat/aarch64-sve-linux-sigcontext.h (struct _aarch64_ctx): Add for
> build compatibility.
> (struct user_fpsimd_state): Likewise.
> ---
> gdb/aarch64-linux-tdep.c | 114 ++++++++++++++++++++++++++++++++-
> gdb/nat/aarch64-sve-linux-sigcontext.h | 14 ++++
> 2 files changed, 125 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 93b6d416a3..919873b816 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -47,6 +47,7 @@
> #include "linux-record.h"
> #include "auxv.h"
> #include "elf/common.h"
> +#include "nat/aarch64-sve-linux-ptrace.h"
>
> /* Signal frame handling.
>
> @@ -169,6 +170,100 @@ aarch64_linux_sigframe_init (const struct tramp_frame *self,
> trad_frame_set_id (this_cache, frame_id_build (sp, func));
> }
>
> +
> +/* Get VG value from SVE section in the core dump. */
> +
> +static uint64_t
> +aarch64_linux_core_read_vq (bfd *abfd)
> +{
> + struct user_sve_header header;
> + asection *sve_section = bfd_get_section_by_name (abfd, ".reg-aarch-sve");
> +
> + if (!sve_section)
> + {
> + /* No SVE state. */
> + return 0;
> + }
> +
> + size_t size = bfd_section_size (abfd, sve_section);
> +
> + /* Check extended state size. */
> + if (size < sizeof (struct user_sve_header))
> + {
> + warning (_("'.reg-aarch-sve' section in core file too small."));
> + return 0;
> + }
> +
> + if (! bfd_get_section_contents (abfd, sve_section, &header, 0,
> + sizeof (struct user_sve_header)))
> + {
> + warning (_("Couldn't read sve header from "
> + "'.reg-aarch-sve' section in core file."));
> + return 0;
> + }
> +
> + if (!sve_vl_valid (header.vl))
> + {
> + warning (_("sve header invalid in"
> + "'.reg-aarch-sve' section in core file."));
> + return 0;
> + }
> +
> + return sve_vq_from_vl (header.vl);
> +}
> +
> +
> +/* Supply register REGNUM from BUF to REGCACHE, ignoring the register map
> + in REGSET. */
> +
> +static void
> +aarch64_linux_supply_sve_regset (const struct regset *regset,
> + struct regcache *regcache,
> + int regnum, const void *buf, size_t size)
> +{
> + /* Only called via aarch64_linux_iterate_over_regset_sections loop, so no
> + need to support anything other than a full regset, and no need to support
> + null buffers. */
> + gdb_assert (buf != NULL);
> + gdb_assert (regnum == -1);
> +
> + aarch64_sve_regs_copy_to_reg_buf (regcache, buf);
> +}
> +
> +/* Collect register REGNUM from REGCACHE to BUF, ignoring the register
> + map in REGSET. If REGNUM is -1, do this for all registers in
> + REGSET. */
> +
> +static void
> +aarch64_linux_collect_sve_regset (const struct regset *regset,
> + const struct regcache *regcache,
> + int regnum, void *buf, size_t size)
> +{
> + /* Only called via aarch64_linux_iterate_over_regset_sections loop, so no
> + need to support anything other than a full regset, and no need to support
> + null buffers. */
> + gdb_assert (buf != NULL);
> + gdb_assert (regnum == -1);
> +
> + /* Read vector size from regcache. */
> + uint64_t vg;
> + regcache->raw_collect_integer (AARCH64_SVE_VG_REGNUM, (gdb_byte *)&vg,
> + sizeof (uint64_t), false);
> + uint64_t vq = sve_vq_from_vg (vg);
> +
> + /* Initialize a valid SVE header. */
> + struct user_sve_header *header = (struct user_sve_header *)buf;
> + header->flags = SVE_PT_REGS_SVE;
> + header->size = SVE_PT_SIZE (vq, header->flags);
> + header->max_size = size;
> + header->vl = sve_vl_from_vg (vg);
> + header->max_vl = SVE_VL_MAX;
> +
> + gdb_assert (size >= header->size);
> +
> + aarch64_sve_regs_copy_from_reg_buf (regcache, buf);
> +}
> +
> static const struct tramp_frame aarch64_linux_rt_sigframe =
> {
> SIGTRAMP_FRAME,
> @@ -219,6 +314,12 @@ const struct regset aarch64_linux_fpregset =
> regcache_supply_regset, regcache_collect_regset
> };
>
> +const struct regset aarch64_linux_sve_regset =
> + {
> + NULL, aarch64_linux_supply_sve_regset, aarch64_linux_collect_sve_regset,
> + REGSET_VARIABLE_SIZE
> + };
> +
> /* Implement the "regset_from_core_section" gdbarch method. */
>
> static void
> @@ -227,10 +328,17 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
> void *cb_data,
> const struct regcache *regcache)
> {
> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> cb (".reg", AARCH64_LINUX_SIZEOF_GREGSET, &aarch64_linux_gregset,
> NULL, cb_data);
> - cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset,
> - NULL, cb_data);
> +
> + if (tdep->has_sve ())
> + cb (".reg-aarch-sve", SVE_PT_SIZE (tdep->vq, 0),
> + &aarch64_linux_sve_regset, "SVE registers", cb_data);
> + else
> + cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset, NULL,
> + cb_data);
> }
>
> /* Implement the "core_read_description" gdbarch method. SVE not yet
> @@ -245,7 +353,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
> if (target_auxv_search (target, AT_HWCAP, &aarch64_hwcap) != 1)
> return NULL;
>
> - return aarch64_read_description (0);
> + return aarch64_read_description (aarch64_linux_core_read_vq (abfd));
> }
>
> /* Implementation of `gdbarch_stap_is_single_operand', as defined in
> diff --git a/gdb/nat/aarch64-sve-linux-sigcontext.h b/gdb/nat/aarch64-sve-linux-sigcontext.h
> index bdece8e17d..ad6e111e71 100644
> --- a/gdb/nat/aarch64-sve-linux-sigcontext.h
> +++ b/gdb/nat/aarch64-sve-linux-sigcontext.h
> @@ -19,6 +19,20 @@
> #ifndef AARCH64_SVE_LINUX_SIGCONTEXT_H
> #define AARCH64_SVE_LINUX_SIGCONTEXT_H
>
> +#ifndef _aarch64_ctx
> +struct _aarch64_ctx {
> + __u32 magic;
> + __u32 size;
> +};
> +
> +struct user_fpsimd_state {
> + __uint128_t vregs[32];
> + __u32 fpsr;
> + __u32 fpcr;
> + __u32 __reserved[2];
> +};
> +#endif
> +
> #define SVE_MAGIC 0x53564501
>
> struct sve_context {
> --
> 2.15.2 (Apple Git-101.1)
>
On 2018-06-25 11:10 AM, Alan Hayward wrote:
> The following patch fails to compile for an all targets build.
> This is due to aarch64-linux-tdep.c requiring functions from
> nat/aarch64-sve-linux-ptrace.c. This is used because the SVE
> section of a aarch64 core file starts with a header structure
> (the same as in a ptrace read), which is defined in the kernel.
>
> I considered putting the nat/ file into an all targets build, but
> this seemed wrong (in addition it would require additional .deps
> dir adding for nat/).
Indeed, this can't be done. Suppose you are building a GDB with
target=aarch64 and host=x86_64, then the structure definition
wouldn't be found and it wouldn't compile.
> What is the correct solution here? Should I move the required nat/
> functions into elsewhere? The arch/ still doesn't feel right.
arch/ is for arch-specific things, ideally shareable between GDB and
GDBserver, but that are host-agnostic.
Since the required structure definition is only available when building
AArch64 programs but you want to be able to parse cores with a GDB that
is itself a non-AArch64 program, then that structure can't be used. You
also can't just copy the structure in GDB's source tree and use it, because
you can't rely on the type sizes, padding and byte order being the same
on other architectures. So I think the only safe thing to do is to read
the fields "by hand", using extract_unsigned_integer and friends.
> Other than the above issue, the patch is ready and working. I thought
> it best to ask here rather than guess the correct solution.
>
> Thanks,
> Alan
>
> Support both the reading and writing of core files on aarch64 SVE.
>
> SVE core files are doumented here:
> https://github.com/torvalds/linux/blob/master/Documentation/arm64/sve.txt
> * A NT_ARM_SVE note will be added to each coredump for each thread of the
> dumped process. The contents will be equivalent to the data that would have
> been read if a PTRACE_GETREGSET of NT_ARM_SVE were executed for each thread
> when the coredump was generated.
>
> When creating a core files target description, read the SVE section to find
> the vector length.
>
> When reading/writing core files, use the existing nat/ functions to parse
> the structure. Given that the function is only called through the one call
> chain, there is no need to support copying a single register or invalidating
> the set.
>
> Dependant on binutils patch "[PATCH] Add BFD core support for Aarch64 SVE"
> https://sourceware.org/ml/binutils/2018-06/msg00314.html
>
> Checked with make check on x86 and aarch64.
> Generated core files on SVE emulator both from gdb "generate-core-file"
> command and a segfault. Loaded these back into gdb.
>
> 2018-06-25 Alan Hayward <alan.hayward@arm.com>
>
> gdb/
> * aarch64-linux-tdep.c (aarch64_linux_core_read_vq): New function.
> (aarch64_linux_supply_sve_regset): Likewise.
> (aarch64_linux_collect_sve_regset): Likewise.
> (aarch64_linux_iterate_over_regset_sections): Check for SVE.
> (aarch64_linux_core_read_description): Likewise.
> * nat/aarch64-sve-linux-sigcontext.h (struct _aarch64_ctx): Add for
> build compatibility.
> (struct user_fpsimd_state): Likewise.
> ---
> gdb/aarch64-linux-tdep.c | 114 ++++++++++++++++++++++++++++++++-
> gdb/nat/aarch64-sve-linux-sigcontext.h | 14 ++++
> 2 files changed, 125 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 93b6d416a3..919873b816 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -47,6 +47,7 @@
> #include "linux-record.h"
> #include "auxv.h"
> #include "elf/common.h"
> +#include "nat/aarch64-sve-linux-ptrace.h"
>
>
> /* Signal frame handling.
>
> @@ -169,6 +170,100 @@ aarch64_linux_sigframe_init (const struct tramp_frame *self,
> trad_frame_set_id (this_cache, frame_id_build (sp, func));
> }
>
> +
> +/* Get VG value from SVE section in the core dump. */
VG or VQ?
> +
> +static uint64_t
> +aarch64_linux_core_read_vq (bfd *abfd)
> +{
> + struct user_sve_header header;
> + asection *sve_section = bfd_get_section_by_name (abfd, ".reg-aarch-sve");
> +
> + if (!sve_section)
> + {
> + /* No SVE state. */
> + return 0;
> + }
> +
> + size_t size = bfd_section_size (abfd, sve_section);
> +
> + /* Check extended state size. */
> + if (size < sizeof (struct user_sve_header))
> + {
> + warning (_("'.reg-aarch-sve' section in core file too small."));
> + return 0;
> + }
> +
> + if (! bfd_get_section_contents (abfd, sve_section, &header, 0,
> + sizeof (struct user_sve_header)))
As mentioned earlier, even if this could compile on non-AArch64
systems, it might give bad results. For example, if the host
is big-endian, the values will all be read with the wrong endianness.
> + {
> + warning (_("Couldn't read sve header from "
> + "'.reg-aarch-sve' section in core file."));
> + return 0;
> + }
> +
> + if (!sve_vl_valid (header.vl))
> + {
> + warning (_("sve header invalid in"
> + "'.reg-aarch-sve' section in core file."));
> + return 0;
> + }
> +
> + return sve_vq_from_vl (header.vl);
> +}
> +
> +
> +/* Supply register REGNUM from BUF to REGCACHE, ignoring the register map
> + in REGSET. */
> +
> +static void
> +aarch64_linux_supply_sve_regset (const struct regset *regset,
> + struct regcache *regcache,
> + int regnum, const void *buf, size_t size)
> +{
> + /* Only called via aarch64_linux_iterate_over_regset_sections loop, so no
> + need to support anything other than a full regset, and no need to support
> + null buffers. */
> + gdb_assert (buf != NULL);
> + gdb_assert (regnum == -1);
> +
> + aarch64_sve_regs_copy_to_reg_buf (regcache, buf);
> +}
> +
> +/* Collect register REGNUM from REGCACHE to BUF, ignoring the register
> + map in REGSET. If REGNUM is -1, do this for all registers in
> + REGSET. */
> +
> +static void
> +aarch64_linux_collect_sve_regset (const struct regset *regset,
> + const struct regcache *regcache,
> + int regnum, void *buf, size_t size)
> +{
> + /* Only called via aarch64_linux_iterate_over_regset_sections loop, so no
> + need to support anything other than a full regset, and no need to support
> + null buffers. */
> + gdb_assert (buf != NULL);
> + gdb_assert (regnum == -1);
> +
> + /* Read vector size from regcache. */
> + uint64_t vg;
> + regcache->raw_collect_integer (AARCH64_SVE_VG_REGNUM, (gdb_byte *)&vg,
> + sizeof (uint64_t), false);
> + uint64_t vq = sve_vq_from_vg (vg);
> +
> + /* Initialize a valid SVE header. */
> + struct user_sve_header *header = (struct user_sve_header *)buf;
> + header->flags = SVE_PT_REGS_SVE;
> + header->size = SVE_PT_SIZE (vq, header->flags);
> + header->max_size = size;
> + header->vl = sve_vl_from_vg (vg);
> + header->max_vl = SVE_VL_MAX;
> +
> + gdb_assert (size >= header->size);
> +
> + aarch64_sve_regs_copy_from_reg_buf (regcache, buf);
> +}
> +
> static const struct tramp_frame aarch64_linux_rt_sigframe =
> {
> SIGTRAMP_FRAME,
> @@ -219,6 +314,12 @@ const struct regset aarch64_linux_fpregset =
> regcache_supply_regset, regcache_collect_regset
> };
>
> +const struct regset aarch64_linux_sve_regset =
> + {
> + NULL, aarch64_linux_supply_sve_regset, aarch64_linux_collect_sve_regset,
> + REGSET_VARIABLE_SIZE
> + };
> +
> /* Implement the "regset_from_core_section" gdbarch method. */
>
> static void
> @@ -227,10 +328,17 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
> void *cb_data,
> const struct regcache *regcache)
> {
> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> cb (".reg", AARCH64_LINUX_SIZEOF_GREGSET, &aarch64_linux_gregset,
> NULL, cb_data);
> - cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset,
> - NULL, cb_data);
> +
> + if (tdep->has_sve ())
> + cb (".reg-aarch-sve", SVE_PT_SIZE (tdep->vq, 0),
> + &aarch64_linux_sve_regset, "SVE registers", cb_data);
> + else
> + cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset, NULL,
> + cb_data);
> }
>
> /* Implement the "core_read_description" gdbarch method. SVE not yet
> @@ -245,7 +353,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
> if (target_auxv_search (target, AT_HWCAP, &aarch64_hwcap) != 1)
> return NULL;
>
> - return aarch64_read_description (0);
> + return aarch64_read_description (aarch64_linux_core_read_vq (abfd));
> }
>
> /* Implementation of `gdbarch_stap_is_single_operand', as defined in
> diff --git a/gdb/nat/aarch64-sve-linux-sigcontext.h b/gdb/nat/aarch64-sve-linux-sigcontext.h
> index bdece8e17d..ad6e111e71 100644
> --- a/gdb/nat/aarch64-sve-linux-sigcontext.h
> +++ b/gdb/nat/aarch64-sve-linux-sigcontext.h
> @@ -19,6 +19,20 @@
> #ifndef AARCH64_SVE_LINUX_SIGCONTEXT_H
> #define AARCH64_SVE_LINUX_SIGCONTEXT_H
>
> +#ifndef _aarch64_ctx
> +struct _aarch64_ctx {
> + __u32 magic;
> + __u32 size;
> +};
> +
> +struct user_fpsimd_state {
> + __uint128_t vregs[32];
> + __u32 fpsr;
> + __u32 fpcr;
> + __u32 __reserved[2];
> +};
> +#endif
Does this ifndef really work? AFAIK, you can't use ifdefs to
check whether a struct type has been defined. The ifdefs are
processed by the preprocessor and the struct types are processed
by the compiler.
An alternative would be an autoconf test.
Simon
> On 2 Jul 2018, at 22:22, Simon Marchi <simark@simark.ca> wrote:
>
> On 2018-06-25 11:10 AM, Alan Hayward wrote:
>> The following patch fails to compile for an all targets build.
>> This is due to aarch64-linux-tdep.c requiring functions from
>> nat/aarch64-sve-linux-ptrace.c. This is used because the SVE
>> section of a aarch64 core file starts with a header structure
>> (the same as in a ptrace read), which is defined in the kernel.
>>
>> I considered putting the nat/ file into an all targets build, but
>> this seemed wrong (in addition it would require additional .deps
>> dir adding for nat/).
>
> Indeed, this can't be done. Suppose you are building a GDB with
> target=aarch64 and host=x86_64, then the structure definition
> wouldn't be found and it wouldn't compile.
>
>> What is the correct solution here? Should I move the required nat/
>> functions into elsewhere? The arch/ still doesn't feel right.
>
> arch/ is for arch-specific things, ideally shareable between GDB and
> GDBserver, but that are host-agnostic.
>
> Since the required structure definition is only available when building
> AArch64 programs but you want to be able to parse cores with a GDB that
> is itself a non-AArch64 program, then that structure can't be used. You
> also can't just copy the structure in GDB's source tree and use it, because
> you can't rely on the type sizes, padding and byte order being the same
> on other architectures. So I think the only safe thing to do is to read
> the fields "by hand", using extract_unsigned_integer and friends.
>
That makes sense, and explains why existing code for the other targets is
as it is. I’ll rework the patch, should be able to build a
regcache_map_entry based on the vector size and call into the regcache
functions.
Thanks.
>
>> Other than the above issue, the patch is ready and working. I thought
>> it best to ask here rather than guess the correct solution.
>>
>> Thanks,
>> Alan
>>
>> Support both the reading and writing of core files on aarch64 SVE.
>>
>> SVE core files are doumented here:
>> https://github.com/torvalds/linux/blob/master/Documentation/arm64/sve.txt
>> * A NT_ARM_SVE note will be added to each coredump for each thread of the
>> dumped process. The contents will be equivalent to the data that would have
>> been read if a PTRACE_GETREGSET of NT_ARM_SVE were executed for each thread
>> when the coredump was generated.
>>
>> When creating a core files target description, read the SVE section to find
>> the vector length.
>>
>> When reading/writing core files, use the existing nat/ functions to parse
>> the structure. Given that the function is only called through the one call
>> chain, there is no need to support copying a single register or invalidating
>> the set.
>>
>> Dependant on binutils patch "[PATCH] Add BFD core support for Aarch64 SVE"
>> https://sourceware.org/ml/binutils/2018-06/msg00314.html
>>
>> Checked with make check on x86 and aarch64.
>> Generated core files on SVE emulator both from gdb "generate-core-file"
>> command and a segfault. Loaded these back into gdb.
>>
>> 2018-06-25 Alan Hayward <alan.hayward@arm.com>
>>
>> gdb/
>> * aarch64-linux-tdep.c (aarch64_linux_core_read_vq): New function.
>> (aarch64_linux_supply_sve_regset): Likewise.
>> (aarch64_linux_collect_sve_regset): Likewise.
>> (aarch64_linux_iterate_over_regset_sections): Check for SVE.
>> (aarch64_linux_core_read_description): Likewise.
>> * nat/aarch64-sve-linux-sigcontext.h (struct _aarch64_ctx): Add for
>> build compatibility.
>> (struct user_fpsimd_state): Likewise.
>> ---
>> gdb/aarch64-linux-tdep.c | 114 ++++++++++++++++++++++++++++++++-
>> gdb/nat/aarch64-sve-linux-sigcontext.h | 14 ++++
>> 2 files changed, 125 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>> index 93b6d416a3..919873b816 100644
>> --- a/gdb/aarch64-linux-tdep.c
>> +++ b/gdb/aarch64-linux-tdep.c
>> @@ -47,6 +47,7 @@
>> #include "linux-record.h"
>> #include "auxv.h"
>> #include "elf/common.h"
>> +#include "nat/aarch64-sve-linux-ptrace.h"
>>
>>
>> /* Signal frame handling.
>>
>> @@ -169,6 +170,100 @@ aarch64_linux_sigframe_init (const struct tramp_frame *self,
>> trad_frame_set_id (this_cache, frame_id_build (sp, func));
>> }
>>
>> +
>> +/* Get VG value from SVE section in the core dump. */
>
> VG or VQ?
>
>> +
>> +static uint64_t
>> +aarch64_linux_core_read_vq (bfd *abfd)
>> +{
>> + struct user_sve_header header;
>> + asection *sve_section = bfd_get_section_by_name (abfd, ".reg-aarch-sve");
>> +
>> + if (!sve_section)
>> + {
>> + /* No SVE state. */
>> + return 0;
>> + }
>> +
>> + size_t size = bfd_section_size (abfd, sve_section);
>> +
>> + /* Check extended state size. */
>> + if (size < sizeof (struct user_sve_header))
>> + {
>> + warning (_("'.reg-aarch-sve' section in core file too small."));
>> + return 0;
>> + }
>> +
>> + if (! bfd_get_section_contents (abfd, sve_section, &header, 0,
>> + sizeof (struct user_sve_header)))
>
> As mentioned earlier, even if this could compile on non-AArch64
> systems, it might give bad results. For example, if the host
> is big-endian, the values will all be read with the wrong endianness.
>
>> + {
>> + warning (_("Couldn't read sve header from "
>> + "'.reg-aarch-sve' section in core file."));
>> + return 0;
>> + }
>> +
>> + if (!sve_vl_valid (header.vl))
>> + {
>> + warning (_("sve header invalid in"
>> + "'.reg-aarch-sve' section in core file."));
>> + return 0;
>> + }
>> +
>> + return sve_vq_from_vl (header.vl);
>> +}
>> +
>> +
>> +/* Supply register REGNUM from BUF to REGCACHE, ignoring the register map
>> + in REGSET. */
>> +
>> +static void
>> +aarch64_linux_supply_sve_regset (const struct regset *regset,
>> + struct regcache *regcache,
>> + int regnum, const void *buf, size_t size)
>> +{
>> + /* Only called via aarch64_linux_iterate_over_regset_sections loop, so no
>> + need to support anything other than a full regset, and no need to support
>> + null buffers. */
>> + gdb_assert (buf != NULL);
>> + gdb_assert (regnum == -1);
>> +
>> + aarch64_sve_regs_copy_to_reg_buf (regcache, buf);
>> +}
>> +
>> +/* Collect register REGNUM from REGCACHE to BUF, ignoring the register
>> + map in REGSET. If REGNUM is -1, do this for all registers in
>> + REGSET. */
>> +
>> +static void
>> +aarch64_linux_collect_sve_regset (const struct regset *regset,
>> + const struct regcache *regcache,
>> + int regnum, void *buf, size_t size)
>> +{
>> + /* Only called via aarch64_linux_iterate_over_regset_sections loop, so no
>> + need to support anything other than a full regset, and no need to support
>> + null buffers. */
>> + gdb_assert (buf != NULL);
>> + gdb_assert (regnum == -1);
>> +
>> + /* Read vector size from regcache. */
>> + uint64_t vg;
>> + regcache->raw_collect_integer (AARCH64_SVE_VG_REGNUM, (gdb_byte *)&vg,
>> + sizeof (uint64_t), false);
>> + uint64_t vq = sve_vq_from_vg (vg);
>> +
>> + /* Initialize a valid SVE header. */
>> + struct user_sve_header *header = (struct user_sve_header *)buf;
>> + header->flags = SVE_PT_REGS_SVE;
>> + header->size = SVE_PT_SIZE (vq, header->flags);
>> + header->max_size = size;
>> + header->vl = sve_vl_from_vg (vg);
>> + header->max_vl = SVE_VL_MAX;
>> +
>> + gdb_assert (size >= header->size);
>> +
>> + aarch64_sve_regs_copy_from_reg_buf (regcache, buf);
>> +}
>> +
>> static const struct tramp_frame aarch64_linux_rt_sigframe =
>> {
>> SIGTRAMP_FRAME,
>> @@ -219,6 +314,12 @@ const struct regset aarch64_linux_fpregset =
>> regcache_supply_regset, regcache_collect_regset
>> };
>>
>> +const struct regset aarch64_linux_sve_regset =
>> + {
>> + NULL, aarch64_linux_supply_sve_regset, aarch64_linux_collect_sve_regset,
>> + REGSET_VARIABLE_SIZE
>> + };
>> +
>> /* Implement the "regset_from_core_section" gdbarch method. */
>>
>> static void
>> @@ -227,10 +328,17 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
>> void *cb_data,
>> const struct regcache *regcache)
>> {
>> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>> +
>> cb (".reg", AARCH64_LINUX_SIZEOF_GREGSET, &aarch64_linux_gregset,
>> NULL, cb_data);
>> - cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset,
>> - NULL, cb_data);
>> +
>> + if (tdep->has_sve ())
>> + cb (".reg-aarch-sve", SVE_PT_SIZE (tdep->vq, 0),
>> + &aarch64_linux_sve_regset, "SVE registers", cb_data);
>> + else
>> + cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset, NULL,
>> + cb_data);
>> }
>>
>> /* Implement the "core_read_description" gdbarch method. SVE not yet
>> @@ -245,7 +353,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
>> if (target_auxv_search (target, AT_HWCAP, &aarch64_hwcap) != 1)
>> return NULL;
>>
>> - return aarch64_read_description (0);
>> + return aarch64_read_description (aarch64_linux_core_read_vq (abfd));
>> }
>>
>> /* Implementation of `gdbarch_stap_is_single_operand', as defined in
>> diff --git a/gdb/nat/aarch64-sve-linux-sigcontext.h b/gdb/nat/aarch64-sve-linux-sigcontext.h
>> index bdece8e17d..ad6e111e71 100644
>> --- a/gdb/nat/aarch64-sve-linux-sigcontext.h
>> +++ b/gdb/nat/aarch64-sve-linux-sigcontext.h
>> @@ -19,6 +19,20 @@
>> #ifndef AARCH64_SVE_LINUX_SIGCONTEXT_H
>> #define AARCH64_SVE_LINUX_SIGCONTEXT_H
>>
>> +#ifndef _aarch64_ctx
>> +struct _aarch64_ctx {
>> + __u32 magic;
>> + __u32 size;
>> +};
>> +
>> +struct user_fpsimd_state {
>> + __uint128_t vregs[32];
>> + __u32 fpsr;
>> + __u32 fpcr;
>> + __u32 __reserved[2];
>> +};
>> +#endif
>
> Does this ifndef really work? AFAIK, you can't use ifdefs to
> check whether a struct type has been defined. The ifdefs are
> processed by the preprocessor and the struct types are processed
> by the compiler.
>
> An alternative would be an autoconf test.
>
> Simon
@@ -47,6 +47,7 @@
#include "linux-record.h"
#include "auxv.h"
#include "elf/common.h"
+#include "nat/aarch64-sve-linux-ptrace.h"
/* Signal frame handling.
@@ -169,6 +170,100 @@ aarch64_linux_sigframe_init (const struct tramp_frame *self,
trad_frame_set_id (this_cache, frame_id_build (sp, func));
}
+
+/* Get VG value from SVE section in the core dump. */
+
+static uint64_t
+aarch64_linux_core_read_vq (bfd *abfd)
+{
+ struct user_sve_header header;
+ asection *sve_section = bfd_get_section_by_name (abfd, ".reg-aarch-sve");
+
+ if (!sve_section)
+ {
+ /* No SVE state. */
+ return 0;
+ }
+
+ size_t size = bfd_section_size (abfd, sve_section);
+
+ /* Check extended state size. */
+ if (size < sizeof (struct user_sve_header))
+ {
+ warning (_("'.reg-aarch-sve' section in core file too small."));
+ return 0;
+ }
+
+ if (! bfd_get_section_contents (abfd, sve_section, &header, 0,
+ sizeof (struct user_sve_header)))
+ {
+ warning (_("Couldn't read sve header from "
+ "'.reg-aarch-sve' section in core file."));
+ return 0;
+ }
+
+ if (!sve_vl_valid (header.vl))
+ {
+ warning (_("sve header invalid in"
+ "'.reg-aarch-sve' section in core file."));
+ return 0;
+ }
+
+ return sve_vq_from_vl (header.vl);
+}
+
+
+/* Supply register REGNUM from BUF to REGCACHE, ignoring the register map
+ in REGSET. */
+
+static void
+aarch64_linux_supply_sve_regset (const struct regset *regset,
+ struct regcache *regcache,
+ int regnum, const void *buf, size_t size)
+{
+ /* Only called via aarch64_linux_iterate_over_regset_sections loop, so no
+ need to support anything other than a full regset, and no need to support
+ null buffers. */
+ gdb_assert (buf != NULL);
+ gdb_assert (regnum == -1);
+
+ aarch64_sve_regs_copy_to_reg_buf (regcache, buf);
+}
+
+/* Collect register REGNUM from REGCACHE to BUF, ignoring the register
+ map in REGSET. If REGNUM is -1, do this for all registers in
+ REGSET. */
+
+static void
+aarch64_linux_collect_sve_regset (const struct regset *regset,
+ const struct regcache *regcache,
+ int regnum, void *buf, size_t size)
+{
+ /* Only called via aarch64_linux_iterate_over_regset_sections loop, so no
+ need to support anything other than a full regset, and no need to support
+ null buffers. */
+ gdb_assert (buf != NULL);
+ gdb_assert (regnum == -1);
+
+ /* Read vector size from regcache. */
+ uint64_t vg;
+ regcache->raw_collect_integer (AARCH64_SVE_VG_REGNUM, (gdb_byte *)&vg,
+ sizeof (uint64_t), false);
+ uint64_t vq = sve_vq_from_vg (vg);
+
+ /* Initialize a valid SVE header. */
+ struct user_sve_header *header = (struct user_sve_header *)buf;
+ header->flags = SVE_PT_REGS_SVE;
+ header->size = SVE_PT_SIZE (vq, header->flags);
+ header->max_size = size;
+ header->vl = sve_vl_from_vg (vg);
+ header->max_vl = SVE_VL_MAX;
+
+ gdb_assert (size >= header->size);
+
+ aarch64_sve_regs_copy_from_reg_buf (regcache, buf);
+}
+
static const struct tramp_frame aarch64_linux_rt_sigframe =
{
SIGTRAMP_FRAME,
@@ -219,6 +314,12 @@ const struct regset aarch64_linux_fpregset =
regcache_supply_regset, regcache_collect_regset
};
+const struct regset aarch64_linux_sve_regset =
+ {
+ NULL, aarch64_linux_supply_sve_regset, aarch64_linux_collect_sve_regset,
+ REGSET_VARIABLE_SIZE
+ };
+
/* Implement the "regset_from_core_section" gdbarch method. */
static void
@@ -227,10 +328,17 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
void *cb_data,
const struct regcache *regcache)
{
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
cb (".reg", AARCH64_LINUX_SIZEOF_GREGSET, &aarch64_linux_gregset,
NULL, cb_data);
- cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset,
- NULL, cb_data);
+
+ if (tdep->has_sve ())
+ cb (".reg-aarch-sve", SVE_PT_SIZE (tdep->vq, 0),
+ &aarch64_linux_sve_regset, "SVE registers", cb_data);
+ else
+ cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset, NULL,
+ cb_data);
}
/* Implement the "core_read_description" gdbarch method. SVE not yet
@@ -245,7 +353,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
if (target_auxv_search (target, AT_HWCAP, &aarch64_hwcap) != 1)
return NULL;
- return aarch64_read_description (0);
+ return aarch64_read_description (aarch64_linux_core_read_vq (abfd));
}
/* Implementation of `gdbarch_stap_is_single_operand', as defined in
@@ -19,6 +19,20 @@
#ifndef AARCH64_SVE_LINUX_SIGCONTEXT_H
#define AARCH64_SVE_LINUX_SIGCONTEXT_H
+#ifndef _aarch64_ctx
+struct _aarch64_ctx {
+ __u32 magic;
+ __u32 size;
+};
+
+struct user_fpsimd_state {
+ __uint128_t vregs[32];
+ __u32 fpsr;
+ __u32 fpcr;
+ __u32 __reserved[2];
+};
+#endif
+
#define SVE_MAGIC 0x53564501
struct sve_context {