Re: [PATCH v6 00/15] Handle variable XSAVE layouts

[John Baldwin <jhb@FreeBSD.org>]

On 7/25/23 1:42 PM, Keith Seitz wrote:
> On 7/25/23 11:59, John Baldwin wrote:
>>
>> Hopefully it triggers the assertion, and then what will be useful is to see what
>> features the tdesc contains ('info locals' would cover that if they aren't all
>> optimized out).
> 
> Yes, that assertion does trigger (on all -m32 targets).
> 
>   From the native-gdbserver/-m32 case at the assertion (I have a debug build):
> 
> (top-gdb) info locals
> tdesc = 0x197b410
> feature_core = 0x197d5a0
> feature_sse = 0x1987200
> feature_avx = 0x1989db0
> feature_mpx = 0x0
> feature_avx512 = 0x198a310
> feature_pkeys = 0x198aec0
> feature_segments = 0x0
> i = 0
> num_regs = 32767
> valid_p = 1
> __func__ = "i386_validate_tdesc_p"
> (top-gdb) p *tdep
> $2 = {<gdbarch_tdep_base> = {
>       _vptr.gdbarch_tdep_base = 0xf5df38 <vtable for i386_gdbarch_tdep+16>},
>     gregset_reg_offset = 0x159fb20 <i386_linux_gregset_reg_offset>,
>     gregset_num_regs = 73, sizeof_gregset = 68, sizeof_fpregset = 108,
>     st0_regnum = 16, num_mmx_regs = 8, mm0_regnum = 0, num_ymm_regs = 0,
>     ymm0_regnum = 0, num_k_regs = 0, k0_regnum = 55, num_zmm_regs = 8,
>     zmm0_regnum = 0, num_byte_regs = 8, al_regnum = 0, num_word_regs = 8,
>     ax_regnum = 0, num_dword_regs = 0, eax_regnum = 0, num_core_regs = 32,
>     num_xmm_regs = 8, num_xmm_avx512_regs = 0, xmm16_regnum = -1,
>     num_ymm_avx512_regs = 0, ymm16_regnum = 0, xcr0 = 231,
>     xsave_xcr0_offset = 464, xsave_layout = {sizeof_xsave = 0, avx_offset = 0,
>       bndregs_offset = 0, bndcfg_offset = 0, k_offset = 0, zmm_h_offset = 0,
>       zmm_offset = 0, pkru_offset = 0},
>     register_names = 0xf5a6a0 <i386_register_names>, ymm0h_regnum = -1,
>     ymmh_register_names = 0x0, ymm16h_regnum = -1, ymm16h_register_names = 0x0,
>     bnd0r_regnum = -1, bnd0_regnum = 0, bndcfgu_regnum = -1,
>     mpx_register_names = 0x0, zmm0h_regnum = 63,
>     k_register_names = 0xf5a900 <i386_k_names>,
>     zmmh_register_names = 0xf5a8a0 <i386_zmmh_names>,
>     xmm_avx512_register_names = 0x0, ymm_avx512_register_names = 0x0,
>     num_pkeys_regs = 0, pkru_regnum = -1, pkeys_register_names = 0x0,
>     fsbase_regnum = -1, tdesc = 0x197b410, register_reggroup_p = 0x7a9578
>        <i386_linux_register_reggroup_p(gdbarch*, int, reggroup const*)>,
>     jb_pc_offset = 20, struct_return = pcc_struct_return, sigtramp_start = 0x0,
>     sigtramp_end = 0x0,
>     sigtramp_p = 0x7a99ae <i386_linux_sigtramp_p(frame_info_ptr)>,
>     sigcontext_addr = 0x7a9c52 <i386_linux_sigcontext_addr(frame_info_ptr)>,
>     sc_reg_offset = 0x159fc60 <i386_linux_sc_reg_offset>, sc_num_regs = 16,
>     sc_pc_offset = -1, sc_sp_offset = -1, i386_mmx_type = 0x0,
>     i386_ymm_type = 0x0, i386_zmm_type = 0x0, i387_ext_type = 0x0,
>     i386_bnd_type = 0x0, record_regmap = 0xf5d5a0 <i386_record_regmap>,
>     i386_intx80_record = 0x7aa2fc <i386_linux_intx80_sysenter_syscall_record(regcache*)>,
>     i386_sysenter_record = 0x7aa2fc <i386_linux_intx80_sysenter_syscall_record(regcache*)>,
>     i386_syscall_record = 0x7aa2fc <i386_linux_intx80_sysenter_syscall_record(regcache*)>, fpregset = 0xf5b5c0 <i386_fpregset>}

Hmm, so what I have been assuming is that you only get a tdesc with those register
sets if target::read_description (either x86_linux_nat_target::read_description
in x86-linux-nat.c for a native process or the gdbarch_core_read_description
handler in (i386|amd64)-linux-tdep.c) returns a tdesc with it, and the ones I just
mentioned all set a xsave_layout at the same time.  However, I think I had overlooked
the remote target.  That is, if you are connected to a remote target but want to
use gcore to write out a local core dump, that I think is the case that is breaking.

This case is a bit weird.  I had assumed I could avoid having to send the
layout across the remote protocol because the individual registers are
sent across the protocol and gdbserver is required to deal with the layout
and reinterpret the XSAVE "block" as individual registers.  However, to write
out a core dump note, we have to use some sort of XSAVE layout.  I could either
go back to making it a new TARGET_OBJECT and it could be fetched across the
wire from gdbserver (but only used for gcore, not for anything else), but
you still have a compatiblity problem in that old gdbserver's won't know about
it so you need a fallback based on the XCR0 mask.  Alternatively, we could
just always use a fallback in this case of picking a layout based on the XCR0
mask.  Previously this was always using the hardcoded Intel layout which is
why this worked before.

It's a bit of a shame to have to fetch it over the wire, but probably that is
the cleaner long term solution, even though it will require a fallback for now
to pick an arbitrary layout based on the XCR0 mask.

-- 
John Baldwin