From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id KREgFN0BxGRAvzEAWB0awg (envelope-from ) for ; Fri, 28 Jul 2023 13:58:53 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=YdirTry7; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 491E21E0C0; Fri, 28 Jul 2023 13:58:53 -0400 (EDT) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 38B0F1E00F for ; Fri, 28 Jul 2023 13:58:51 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AE9BD3858032 for ; Fri, 28 Jul 2023 17:58:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AE9BD3858032 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1690567130; bh=+ZEhte8F2B61Vt0B6Xrc9C4I8geAYS1Cz/BY6NXl0oo=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=YdirTry7+hO0S3JEGi/VHHyEZ/VzcVpEUsLO6Y4rOW+Bn56W+I0cNzqt4m93N6Y5T cnu50XelpfRHrNJMLmnB1B5MdRggr9F3+Xiu/d2LMPbbukC+brTFzjunm4pAA9JSqV YgsSnVntfeusHosrr8j1IE0rYDCcPvslFRXfXYWE= 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 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 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> 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" 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