Re: [PATCH v6a 08/15] gdb: Update x86 Linux architectures to support XSAVE layouts.

John Baldwin
2023-08-14T17:53:04Z

On 7/28/23 2:29 PM, John Baldwin wrote:
> Refactor i386_linux_core_read_xcr0 to fetch and return a corresponding
> x86_xsave_layout as well as xcr0 using the size of an existing
> NT_X86_XSTATE core dump to determine the offsets via
> i387_guess_xsave_layout. Use this to add an implementation of
> gdbarch_core_xfer_x86_xsave_layout.
>
> Use tdep->xsave_layout.sizeof_xsave as the size of the XSTATE register
> set.
> ---
> gdb/amd64-linux-tdep.c | 13 +++++++--
> gdb/i386-linux-tdep.c | 66 +++++++++++++++++++++++-------------------
> gdb/i386-linux-tdep.h | 25 +++++++++-------
> 3 files changed, 61 insertions(+), 43 deletions(-)
>
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index cbbac1a0c64..e2fdf5fb6c8 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -1616,7 +1616,10 @@ amd64_linux_core_read_description (struct gdbarch *gdbarch,
> bfd *abfd)
> {
> /* Linux/x86-64. */
> - uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
> + x86_xsave_layout layout;
> + uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
> + if (xcr0 == 0)
> + xcr0 = X86_XSTATE_SSE_MASK;
>
> return amd64_linux_read_description (xcr0 & X86_XSTATE_ALL_MASK,
> gdbarch_ptr_bit (gdbarch) == 32);
> @@ -1661,8 +1664,10 @@ amd64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
>
> cb (".reg", 27 * 8, 27 * 8, &i386_gregset, NULL, cb_data);
> cb (".reg2", 512, 512, &amd64_fpregset, NULL, cb_data);
> - cb (".reg-xstate", X86_XSTATE_SIZE (tdep->xcr0), X86_XSTATE_SIZE (tdep->xcr0),
> - &amd64_linux_xstateregset, "XSAVE extended state", cb_data);
> + if (tdep->xsave_layout.sizeof_xsave != 0)
> + cb (".reg-xstate", tdep->xsave_layout.sizeof_xsave,
> + tdep->xsave_layout.sizeof_xsave, &amd64_linux_xstateregset,
> + "XSAVE extended state", cb_data);
> }
>
> /* The instruction sequences used in x86_64 machines for a
> @@ -1804,6 +1809,8 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch,
> tdep->sc_num_regs = ARRAY_SIZE (amd64_linux_sc_reg_offset);
>
> tdep->xsave_xcr0_offset = I386_LINUX_XSAVE_XCR0_OFFSET;
> + set_gdbarch_core_read_x86_xsave_layout
> + (gdbarch, i386_linux_core_read_x86_xsave_layout);
>
> /* Add the %orig_rax register used for syscall restarting. */
> set_gdbarch_write_pc (gdbarch, amd64_linux_write_pc);
> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index 554c1455a5f..7ff7c155e2c 100644
> --- a/gdb/i386-linux-tdep.c
> +++ b/gdb/i386-linux-tdep.c
> @@ -638,45 +638,48 @@ static int i386_linux_sc_reg_offset[] =
> 0 * 4 /* %gs */
> };
>
> -/* Get XSAVE extended state xcr0 from core dump. */
> +/* See i386-linux-tdep.h. */
>
> uint64_t
> -i386_linux_core_read_xcr0 (bfd *abfd)
> +i386_linux_core_read_xsave_info (bfd *abfd, x86_xsave_layout &layout)
> {
> asection *xstate = bfd_get_section_by_name (abfd, ".reg-xstate");
> - uint64_t xcr0;
> + if (xstate == nullptr)
> + return 0;
>
> - if (xstate)
> + /* Check extended state size. */
> + size_t size = bfd_section_size (xstate);
> + if (size < X86_XSTATE_AVX_SIZE)
> + return 0;
> +
> + char contents[8];
> + if (! bfd_get_section_contents (abfd, xstate, contents,
> + I386_LINUX_XSAVE_XCR0_OFFSET, 8))
> {
> - size_t size = bfd_section_size (xstate);
> -
> - /* Check extended state size. */
> - if (size < X86_XSTATE_AVX_SIZE)
> - xcr0 = X86_XSTATE_SSE_MASK;
> - else
> - {
> - char contents[8];
> -
> - if (! bfd_get_section_contents (abfd, xstate, contents,
> - I386_LINUX_XSAVE_XCR0_OFFSET,
> - 8))
> - {
> - warning (_("Couldn't read `xcr0' bytes from "
> - "`.reg-xstate' section in core file."));
> - return 0;
> - }
> -
> - xcr0 = bfd_get_64 (abfd, contents);
> - }
> + warning (_("Couldn't read `xcr0' bytes from "
> + "`.reg-xstate' section in core file."));
> + return 0;
> }
> - else
> - xcr0 = 0;
> +
> + uint64_t xcr0 = bfd_get_64 (abfd, contents);
> +
> + if (!i387_guess_xsave_layout (xcr0, size, layout))
> + return 0;
>
> return xcr0;
> }
>
> /* See i386-linux-tdep.h. */
>
> +bool
> +i386_linux_core_read_x86_xsave_layout (struct gdbarch *gdbarch,
> + x86_xsave_layout &layout)
> +{
> + return i386_linux_core_read_xsave_info (core_bfd, layout) != 0;
> +}
> +
> +/* See i386-linux-tdep.h. */
> +
> const struct target_desc *
> i386_linux_read_description (uint64_t xcr0)
> {
> @@ -708,7 +711,8 @@ i386_linux_core_read_description (struct gdbarch *gdbarch,
> bfd *abfd)
> {
> /* Linux/i386. */
> - uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
> + x86_xsave_layout layout;
> + uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
> const struct target_desc *tdesc = i386_linux_read_description (xcr0);
>
> if (tdesc != NULL)
> @@ -767,9 +771,9 @@ i386_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
>
> cb (".reg", 68, 68, &i386_gregset, NULL, cb_data);
>
> - if (tdep->xcr0 & X86_XSTATE_AVX)
> - cb (".reg-xstate", X86_XSTATE_SIZE (tdep->xcr0),
> - X86_XSTATE_SIZE (tdep->xcr0), &i386_linux_xstateregset,
> + if (tdep->xsave_layout.sizeof_xsave != 0)
> + cb (".reg-xstate", tdep->xsave_layout.sizeof_xsave,
> + tdep->xsave_layout.sizeof_xsave, &i386_linux_xstateregset,
> "XSAVE extended state", cb_data);
> else if (tdep->xcr0 & X86_XSTATE_SSE)
> cb (".reg-xfp", 512, 512, &i386_fpregset, "extended floating-point",
> @@ -872,6 +876,8 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> tdep->sc_num_regs = ARRAY_SIZE (i386_linux_sc_reg_offset);
>
> tdep->xsave_xcr0_offset = I386_LINUX_XSAVE_XCR0_OFFSET;
> + set_gdbarch_core_read_x86_xsave_layout
> + (gdbarch, i386_linux_core_read_x86_xsave_layout);
>
> set_gdbarch_process_record (gdbarch, i386_process_record);
> set_gdbarch_process_record_signal (gdbarch, i386_linux_record_signal);
> diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
> index c0fd1f7f300..bd8e328b7fe 100644
> --- a/gdb/i386-linux-tdep.h
> +++ b/gdb/i386-linux-tdep.h
> @@ -20,6 +20,8 @@
> #ifndef I386_LINUX_TDEP_H
> #define I386_LINUX_TDEP_H
>
> +#include "gdbsupport/x86-xstate.h"
> +
> /* The Linux kernel pretends there is an additional "orig_eax"
> register. Since GDB needs access to that register to be able to
> properly restart system calls when necessary (see
> @@ -34,8 +36,18 @@
> /* Total number of registers for GNU/Linux. */
> #define I386_LINUX_NUM_REGS (I386_LINUX_ORIG_EAX_REGNUM + 1)
>
> -/* Get XSAVE extended state xcr0 from core dump. */
> -extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
> +/* Read the XSAVE extended state xcr0 value from the ABFD core file.
> + If it appears to be valid, return it and fill LAYOUT with values
> + inferred from that value.
> +
> + Otherwise, return 0 to indicate no state was found and leave LAYOUT
> + untouched. */
> +extern uint64_t i386_linux_core_read_xsave_info (bfd *abfd,
> + x86_xsave_layout &layout);
> +
> +/* Implement the core_read_x86_xsave_layout gdbarch method. */
> +extern bool i386_linux_core_read_x86_xsave_layout (struct gdbarch *gdbarch,
> + x86_xsave_layout &layout);
>
> /* Handle and display information related to the MPX bound violation
> to the user. */
> @@ -52,14 +64,7 @@ extern const struct target_desc *i386_linux_read_description (uint64_t xcr0);
> fxsave_bytes[0..463]
> sw_usable_bytes[464..511]
> xstate_hdr_bytes[512..575]
> - avx_bytes[576..831]
> - mpx_bytes [960..1032]
> - avx512_k_regs[1088..1152]
> - avx512_zmmh_regs0-7[1153..1407]
> - avx512_zmmh_regs8-15[1408..1663]
> - avx512_zmm_regs16-31[1664..2687]
> - pkru[2688..2752]
> - future_state etc
> + extended state regions (AVX, MPX, AVX512, PKRU, etc.)
> };
>
> Same memory layout will be used for the coredump NT_X86_XSTATE

Ping?

--
John Baldwin