From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.freebsd.org (mx2.freebsd.org [96.47.72.81]) by sourceware.org (Postfix) with ESMTPS id 8C7003858C36 for ; Wed, 26 Jul 2023 22:31:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8C7003858C36 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=FreeBSD.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits)) (Client CN "mx1.freebsd.org", Issuer "R3" (verified OK)) by mx2.freebsd.org (Postfix) with ESMTPS id 4RB7sM0xJ5z4FbN for ; Wed, 26 Jul 2023 22:31:03 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RB7sM0CG3z3hGy for ; Wed, 26 Jul 2023 22:31:03 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1690410663; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=imGzWG0ACncCxkxc4xu4RpN6jiL5kGT93qCjrNtqv00=; b=vQ1E9QUTtpThtAWwyUNtlQCCwTErpqNohbcwpRNn3cpGXqb9KVJ/EDjM+lLf9KZvn0oov9 Rx86lhrjsrZC/VB1UhNAQ5koRYX5Rl4jVol9gCpyPcNJEc1wWNEn5KhBygypLjkGaIWqLi V4pr+H2fzXe6/mBJmfhOnmugorthmN31k4LxMSVuc3PtktT5EBKIoFmi/mQ3C+NLahNTBT YpA9TZbWUdrWapJs/sbp5dS+5W9q/GLaB013gd68BVCOQtcH+kQZcDvoBNR/eqfbTTbOAS zWVeAX9ls6RqtomEAI3IFJQwjhtXIrxDJtmwCgP20hH/BFTMgI8r5TqOgCa91A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1690410663; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=imGzWG0ACncCxkxc4xu4RpN6jiL5kGT93qCjrNtqv00=; b=VEUBPUocf3TS2NJmr+D8xDxzAR1jotpVvTghNS52uCD+zBUvOaZ0tLjTnxcAx5La5Qox5t aDFgCfoSb821mxKPxuRNpzRjrjOZiLWkxOqyg/8VNr6S5F4lw/9lcTmRRRzO/IqMNhcFzP rRd9xHxXim4Wg69mMY7rKZOv2umVs4WCjMpJ6Ux9H6NJFVhxP8OdRyqyQIKqPFMphLLRZb o7hriatfFtrAiDpWgfwZ7dC6dTipOpmwW2GzSGQjqn0b0r/pFx9SDcXG5b2V6ejD4knwus Jp8D7a8rA82EehqURb+Uq5S2acXCmIxPnXDgRnePzPAtM3qnSn5TOTmkJu0sqg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1690410663; a=rsa-sha256; cv=none; b=UhIcaeEDyGCAl3As9fv+I/2hwc8IoUzH8KGXN+xWg84/6mdHrrQOWA7Ky8MpOAf89OC2ZS RZhJdKbX7+hCab/lm7zcd9f3TA9dGml9DsthPCwzisxLFV1jFhxpaEA1ZXFmlbUkBE3Mv0 iVOYmUWLG0l1XkuKqYEM9FM6UYkadXLlD1XxG1HBUHV9ZlzYGcDg+G7ZP7riZcfHQpGxwH IduAm4jPqGx9dCPHFOkFKgqO6QnjX0YirfLIXy9RMYOfMXTa5RNPz8ZhiiT0R5eMk+4BXh akQHQHs2s6hC55NYp9z2xk9jUdbe3mO7eGxB+oL5vJo3KoR1kiIIGeILHe6a2w== Received: from [IPV6:2601:648:8680:16b0:3912:750d:bdfa:28ab] (unknown [IPv6:2601:648:8680:16b0:3912:750d:bdfa:28ab]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4RB7sL51FXzJ8w for ; Wed, 26 Jul 2023 22:31:02 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <298952cf-d29e-0c58-a89c-3b29f5f9a49a@FreeBSD.org> Date: Wed, 26 Jul 2023 15:31:01 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Content-Language: en-US To: gdb-patches@sourceware.org References: <20230714155151.21723-1-jhb@FreeBSD.org> <6a412964-2a25-da39-7c3b-38050437795a@FreeBSD.org> <08cce201-d044-cfcd-4147-6ea1f0eb0fef@FreeBSD.org> From: John Baldwin Subject: Re: [PATCH v6 00/15] Handle variable XSAVE layouts In-Reply-To: <08cce201-d044-cfcd-4147-6ea1f0eb0fef@FreeBSD.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 7/25/23 3:05 PM, John Baldwin wrote: > 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 = { = { >> _vptr.gdbarch_tdep_base = 0xf5df38 }, >> gregset_reg_offset = 0x159fb20 , >> 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 , 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 , >> zmmh_register_names = 0xf5a8a0 , >> 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 >> , >> jb_pc_offset = 20, struct_return = pcc_struct_return, sigtramp_start = 0x0, >> sigtramp_end = 0x0, >> sigtramp_p = 0x7a99ae , >> sigcontext_addr = 0x7a9c52 , >> sc_reg_offset = 0x159fc60 , 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_intx80_record = 0x7aa2fc , >> i386_sysenter_record = 0x7aa2fc , >> i386_syscall_record = 0x7aa2fc , fpregset = 0xf5b5c0 } > > 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. So I have a couple of things to try here. First, a simple fix for the crash is this change (relative to the branch), but it does mean the core dumps generated by gcore for a remote target will not contain extended XSAVE state like AVX, etc.: diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index c8b467a0416..a7d61f33a08 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -768,7 +768,7 @@ i386_linux_iterate_over_regset_sections (struct gdbarch *gdbarch, cb (".reg", 68, 68, &i386_gregset, NULL, cb_data); - if (tdep->xcr0 & X86_XSTATE_AVX) + 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); Arguably that is more correct as it lines up with the way the rest of the x86 arches in this series. The other change I have locally is one to generate a "fallback" layout to use in this case that a target (such as remote) doesn't provide a layout. I'm still not quite happy with what I would want to do over the wire. In some ways I'd rather have a way to send across CPUID requests to mimic what we do for live processes to determine the layout. I really don't want to encode a binary form of the current structure as it will likely change in the future for new XSAVE regions. Perhaps an XML blob that described the total size and the size and offset of each region could work. Still, you would always need a fallback for old servers. The fallback change: diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index d5423681802..30968522824 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -8773,6 +8773,14 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) gdbarch_free (gdbarch); return NULL; } + + /* If the target didn't provide an XSAVE layout, generate a fallback + layout. This currently happens with remote targets and is used + to determine the layout of the XSAVE register notes when + generating a core dump. */ + if (tdep->xcr0 != X86_XSTATE_X87_MASK && tdep->xcr0 != X86_XSTATE_SSE_MASK + && xsave_layout.sizeof_xsave == 0) + xsave_layout = i387_fallback_xsave_layout (tdep->xcr0); tdep->xsave_layout = xsave_layout; num_bnd_cooked = (tdep->bnd0r_regnum > 0 ? I387_NUM_BND_REGS : 0); diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c index 47667da21c7..90bd7a5db9e 100644 --- a/gdb/i387-tdep.c +++ b/gdb/i387-tdep.c @@ -987,6 +987,48 @@ i387_guess_xsave_layout (uint64_t xcr0, size_t xsave_size, return true; } +/* See i387-tdep.h. */ + +x86_xsave_layout +i387_fallback_xsave_layout (uint64_t xcr0) +{ + x86_xsave_layout layout; + if (HAS_PKRU (xcr0)) + { + layout.sizeof_xsave = 2696; + layout.avx_offset = 576; + layout.bndregs_offset = 960; + layout.bndcfg_offset = 1024; + layout.k_offset = 1088; + layout.zmm_h_offset = 1152; + layout.zmm_offset = 1664; + layout.pkru_offset = 2688; + } + else if (HAS_AVX512 (xcr0)) + { + layout.sizeof_xsave = 2688; + layout.avx_offset = 576; + layout.bndregs_offset = 960; + layout.bndcfg_offset = 1024; + layout.k_offset = 1088; + layout.zmm_h_offset = 1152; + layout.zmm_offset = 1664; + } + else if (HAS_MPX (xcr0)) + { + layout.sizeof_xsave = 1088; + layout.avx_offset = 576; + layout.bndregs_offset = 960; + layout.bndcfg_offset = 1024; + } + else + { + layout.sizeof_xsave = 832; + layout.avx_offset = 576; + } + return layout; +} + /* Extract from XSAVE a bitset of the features that are available on the target, but which have not yet been enabled. */ diff --git a/gdb/i387-tdep.h b/gdb/i387-tdep.h index e149e30e52e..8de68d00f6e 100644 --- a/gdb/i387-tdep.h +++ b/gdb/i387-tdep.h @@ -147,6 +147,10 @@ extern void i387_supply_fxsave (struct regcache *regcache, int regnum, extern bool i387_guess_xsave_layout (uint64_t xcr0, size_t xsave_size, x86_xsave_layout &layout); +/* Generate a fallback XSAVE layout based on the XCR0 bitmask. */ + +extern x86_xsave_layout i387_fallback_xsave_layout (uint64_t xcr0); + /* Similar to i387_supply_fxsave, but use XSAVE extended state. */ extern void i387_supply_xsave (struct regcache *regcache, int regnum, -- John Baldwin