[v2,2/3] Detect SVE when reading aarch64 core files
Commit Message
Add a function which reads the vector length from the SVE section within an
aarch64 core file.
The SVE section in a core file contains a header followed by the registers.
All macros to parse access this structure.
2018-07-30 Alan Hayward <alan.hayward@arm.com>
* aarch64-linux-tdep.c (struct struct_contents): Add struct.
(SVE_HEADER_SIZE): Add Macro.
(SVE_HEADER_WRITE): Likewise.
(SVE_HEADER_READ): Likewise.
(aarch64_linux_core_read_vq): Add function.
(aarch64_linux_core_read_description): Check for SVE section.
---
gdb/aarch64-linux-tdep.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 71 insertions(+), 3 deletions(-)
Comments
On 2018-07-30 05:25 AM, Alan Hayward wrote:
> Add a function which reads the vector length from the SVE section within an
> aarch64 core file.
>
> The SVE section in a core file contains a header followed by the registers.
> All macros to parse access this structure.
>
> 2018-07-30 Alan Hayward <alan.hayward@arm.com>
>
> * aarch64-linux-tdep.c (struct struct_contents): Add struct.
> (SVE_HEADER_SIZE): Add Macro.
> (SVE_HEADER_WRITE): Likewise.
> (SVE_HEADER_READ): Likewise.
> (aarch64_linux_core_read_vq): Add function.
> (aarch64_linux_core_read_description): Check for SVE section.
> ---
> gdb/aarch64-linux-tdep.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 71 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 7b63cddbe6..f9a95950da 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -219,6 +219,75 @@ const struct regset aarch64_linux_fpregset =
> regcache_supply_regset, regcache_collect_regset
> };
>
> +/* Description of the fields in an SVE header at the start of a SVE regset. */
> +
> +struct struct_contents {
> + int len;
> + int offset;
> +} sve_header[] =
> + {
> + { 4, 0 }, /* __u32 size. */
> + { 4, 4 }, /* __u32 max_size. */
> + { 2, 8 }, /* __u16 vl. */
> + { 2, 10 }, /* __u16 max_vl. */
> + { 2, 12 }, /* __u16 flags. */
> + { 2, 14 }, /* __u16 reserved. */
> + { -1, 16 }
> + };
I don't think the resulting code is very readable, for example this bit from the next
patch:
SVE_HEADER_WRITE (buf, 0, byte_order, size);
SVE_HEADER_WRITE (buf, 1, byte_order, size);
SVE_HEADER_WRITE (buf, 2, byte_order, sve_vl_from_vq (vq));
SVE_HEADER_WRITE (buf, 3, byte_order, sve_vl_from_vq (vq));
SVE_HEADER_WRITE (buf, 4, byte_order, 1);
SVE_HEADER_WRITE (buf, 5, byte_order, 0);
Maybe have some macros to access the index of the fields in the structure, like this?
#define SVE_HEADER_SIZE_FIELD 0
#define SVE_HEADER_MAX_SIZE_FIELD 1
#define SVE_HEADER_VL_FIELD 2
...
> +
> +#define SVE_HEADER_SIZE (sve_header[6].offset)
Perhaps use
ARRAY_SIZE (sve_header) - 1
instead of 6?
> +
> +#define SVE_HEADER_WRITE(header, entry, byte_order, value) \
> + store_unsigned_integer ((gdb_byte *) header + sve_header[entry].offset, \
> + sve_header[entry].len, byte_order, value)
> +
> +#define SVE_HEADER_READ(header, entry, byte_order) \
> + extract_unsigned_integer ((gdb_byte *) header + sve_header[entry].offset, \
> + sve_header[entry].len, byte_order)
I'm not a big fan of macros when the same work can be done with a function, which
seems to be the case here.
> +
> +/* Get VQ value from SVE section in the core dump. */
> +
> +static uint64_t
> +aarch64_linux_core_read_vq (struct gdbarch *gdbarch, bfd *abfd)
> +{
> + gdb_byte header[SVE_HEADER_SIZE];
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> + asection *sve_section = bfd_get_section_by_name (abfd, ".reg-aarch-sve");
> +
> + if (!sve_section)
== nullptr
> + {
> + /* No SVE state. */
> + return 0;
> + }
> +
> + size_t size = bfd_section_size (abfd, sve_section);
> +
> + /* Check extended state size. */
> + if (size < SVE_HEADER_SIZE)
> + {
> + warning (_("'.reg-aarch-sve' section in core file too small."));
> + return 0;
> + }
> +
> + if (!bfd_get_section_contents (abfd, sve_section, header, 0, SVE_HEADER_SIZE))
> + {
> + warning (_("Couldn't read sve header from "
> + "'.reg-aarch-sve' section in core file."));
> + return 0;
> + }
> +
> + uint64_t vq = sve_vq_from_vl (SVE_HEADER_READ (header, 2, byte_order));
> +
> + if (vq > AARCH64_MAX_SVE_VQ)
> + {
> + warning (_("sve header invalid in "
> + "'.reg-aarch-sve' section in core file."));
> + return 0;
> + }
Is 0 a valid value for vq (in other words, should it show a warning)?
Simon
@@ -219,6 +219,75 @@ const struct regset aarch64_linux_fpregset =
regcache_supply_regset, regcache_collect_regset
};
+/* Description of the fields in an SVE header at the start of a SVE regset. */
+
+struct struct_contents {
+ int len;
+ int offset;
+} sve_header[] =
+ {
+ { 4, 0 }, /* __u32 size. */
+ { 4, 4 }, /* __u32 max_size. */
+ { 2, 8 }, /* __u16 vl. */
+ { 2, 10 }, /* __u16 max_vl. */
+ { 2, 12 }, /* __u16 flags. */
+ { 2, 14 }, /* __u16 reserved. */
+ { -1, 16 }
+ };
+
+#define SVE_HEADER_SIZE (sve_header[6].offset)
+
+#define SVE_HEADER_WRITE(header, entry, byte_order, value) \
+ store_unsigned_integer ((gdb_byte *) header + sve_header[entry].offset, \
+ sve_header[entry].len, byte_order, value)
+
+#define SVE_HEADER_READ(header, entry, byte_order) \
+ extract_unsigned_integer ((gdb_byte *) header + sve_header[entry].offset, \
+ sve_header[entry].len, byte_order)
+
+/* Get VQ value from SVE section in the core dump. */
+
+static uint64_t
+aarch64_linux_core_read_vq (struct gdbarch *gdbarch, bfd *abfd)
+{
+ gdb_byte header[SVE_HEADER_SIZE];
+ enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+ 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 < SVE_HEADER_SIZE)
+ {
+ warning (_("'.reg-aarch-sve' section in core file too small."));
+ return 0;
+ }
+
+ if (!bfd_get_section_contents (abfd, sve_section, header, 0, SVE_HEADER_SIZE))
+ {
+ warning (_("Couldn't read sve header from "
+ "'.reg-aarch-sve' section in core file."));
+ return 0;
+ }
+
+ uint64_t vq = sve_vq_from_vl (SVE_HEADER_READ (header, 2, byte_order));
+
+ if (vq > AARCH64_MAX_SVE_VQ)
+ {
+ warning (_("sve header invalid in "
+ "'.reg-aarch-sve' section in core file."));
+ return 0;
+ }
+
+ return vq;
+}
+
/* Implement the "regset_from_core_section" gdbarch method. */
static void
@@ -233,8 +302,7 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
&aarch64_linux_fpregset, NULL, cb_data);
}
-/* Implement the "core_read_description" gdbarch method. SVE not yet
- supported. */
+/* Implement the "core_read_description" gdbarch method. */
static const struct target_desc *
aarch64_linux_core_read_description (struct gdbarch *gdbarch,
@@ -245,7 +313,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 (gdbarch, abfd));
}
/* Implementation of `gdbarch_stap_is_single_operand', as defined in