From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 4F1183858CDA for ; Fri, 28 Jul 2023 17:58:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4F1183858CDA Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 36SHwNEn018326 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 28 Jul 2023 13:58:28 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 36SHwNEn018326 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1690567108; bh=+ZEhte8F2B61Vt0B6Xrc9C4I8geAYS1Cz/BY6NXl0oo=; h=Date:Subject:To:References:From:In-Reply-To:From; b=c6z3JXOr3zh1+uGEoS69S+vM3vBrvLj27vcrAqjoon1ak6RWomkHd+J556jNlA4lE rvoIPTwfUc7WN7DNivTvUWWfYA01fwQWtIrELouY8B3olCYUBBi1cFPTZn6/EI/EQu 2++T2yGjzTas+qBzw5gvxtl44OFBCo4+Sblkm6Nc= Received: from [10.0.0.11] (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 2BC431E00F; Fri, 28 Jul 2023 13:58:23 -0400 (EDT) Message-ID: Date: Fri, 28 Jul 2023 13:58:22 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 08/15] gdb: Update x86 Linux architectures to support XSAVE layouts. Content-Language: en-US To: John Baldwin , gdb-patches@sourceware.org References: <20230714155151.21723-1-jhb@FreeBSD.org> <20230714155151.21723-9-jhb@FreeBSD.org> <45c8bfbf-42d2-c9d9-d246-0ffd6fc4668c@polymtl.ca> <70fd2a39-f5a4-7188-dd8c-4e0eda971733@FreeBSD.org> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 28 Jul 2023 17:58:23 +0000 X-Spam-Status: No, score=-3031.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,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 2023-07-28 12:30, John Baldwin wrote: > On 7/27/23 2:48 PM, Simon Marchi wrote: >> >>> Ooo, that's a good catch. This function is shared with amd64, so I think >>> it's best if it keeps returning an xcr0 value, but we could patch >>> i386_linux_core_read_description to instead do this: >>> >>> static const struct target_desc * >>> i386_linux_core_read_description (struct gdbarch *gdbarch, >>> struct target_ops *target, >>> bfd *abfd) >>> { >>> /* Linux/i386. */ >>> x86_xsave_layout layout; >>> uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout); >>> if (xcr0 == X86_XSTATE_X87_MASK >>> && bfd_get_section_by_name (abfd, ".reg-xfp") != NULL) >>> xcr0 = X86_XSTATE_SSE_MASK; >>> >>> return i386_linux_read_description (xcr0); >>> } >> >> And i386_linux_core_read_xsave_info would return X86_XSTATE_X87_MASK if >> it does not find a .reg-xstate section? > > Hmmm. It would need to do something like that yes. I realize I have a > bug for i386-fbsd as well where it would return SSE when it should be > returning X87. For amd64, both Linux and FreeBSD use FXSAVE (which > includes SSE) as .reg2 (generic FP regs). For i386, both use FSAVE (which > only includes X87) for .reg2, and Linux/i386 also has .reg-fxp that uses > FXSAVE. This means that the "default" xcr0 value really should be SSE > for amd64, and X87 for i386. > > I was considering returning a bool from the helper methods > (i386_*_core_read_xsave_info yesterday (it makes the new gdbarch method > implementations slightly easier to read IMO). Other options are to add a > parameter to the helper that is the "default" value of xcr0 to use, or to > return a value of 0 for xcr0 when no valid XSAVE info is found. Returning > 0 is pretty close to the bool approach without requiring a dummy xcr0 arg > in the gdbarch methods, so I think I'll go with that. > > In that case, i386_linux_core_read_description would go back to what it > was in this patch, but the amd64 version would change slightly: > > static const struct target_desc * > amd64_linux_core_read_description (struct gdbarch *gdbarch, > struct target_ops *target, > bfd *abfd) > { > /* Linux/x86-64. */ > 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); > } > > Namely to add the 'xcr == 0' case. If that approach seems sensible I > will post a new series since it touches both this patch and the FreeBSD > one. I previously considered suggesting making the *_core_read_xsave_info functions return gdb::optional, where they would have returned an empty optional if the core does not have xsave info. The calling code could then fall back to some other strategy. Returning 0 achieves the same thing in a different way, it's fine with me (as long as it's properly documented). I prefer that over the "passing a default value" approach. If the changes only touch this patch, you can send an update just for this one (instead of the whole series). Simon