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 7C1C33858D1E for ; Fri, 8 Sep 2023 17:10:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7C1C33858D1E 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 388HAfXE007820 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 8 Sep 2023 13:10:46 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 388HAfXE007820 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1694193046; bh=+GncqNqDCCK8PeCrrnwwgYTikBYe5EH9gFkqZ3uAJ6Q=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=SrrzzNK2MTFaVN8XC92bDlBAxmLpLMW8x0Mz7AjkF2egShzhbAxUGFIndjTvu/czl ldp6OrIj/vAECWnOdSfqSoHSXvMtvBZuMzl+H8boWclfKCKb3mdt1RX344hoAHi1us sZOng3W33zSQOJMCjr41PyRH0Sz3Rw7n3YOFs5Bc= Received: from [172.16.0.192] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 129011E092; Fri, 8 Sep 2023 13:10:40 -0400 (EDT) Message-ID: Date: Fri, 8 Sep 2023 13:10:40 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 13/16] [gdb/generic] corefile/bug: Fixup (gcore) core file target description reading order Content-Language: fr To: Luis Machado , gdb-patches@sourceware.org Cc: thiago.bauermann@linaro.org References: <20230907152018.1031257-1-luis.machado@arm.com> <20230907152018.1031257-14-luis.machado@arm.com> From: Simon Marchi In-Reply-To: <20230907152018.1031257-14-luis.machado@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 8 Sep 2023 17:10:41 +0000 X-Spam-Status: No, score=-3037.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP 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 9/7/23 11:20, Luis Machado via Gdb-patches wrote: > Due to the nature of the AArch64 SVE/SME extensions in GDB, each thread > can potentially have distinct target descriptions/gdbarches. > > When loading a gcore-generated core file, at the moment GDB gives priority > to the target description dumped to NT_GDB_TDESC. Though technically correct > for most target, it doesn't work correctly for AArch64 with SVE or SME > support. > > The correct approach for AArch64/Linux is to rely on the > gdbarch_core_read_description hook, so it can figure out the proper target > description for a given thread based on the various available register notes. > > I think this should work for other architectures as well. If not, we may > need to adjust things so all architectures get the information that they > need for discovering the target description of the core file. > > Regression-tested on aarch64-linux Ubuntu 22.04/20.04. > --- > gdb/corelow.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/gdb/corelow.c b/gdb/corelow.c > index 439270f5559..ae1641fe5d2 100644 > --- a/gdb/corelow.c > +++ b/gdb/corelow.c > @@ -1229,6 +1229,21 @@ core_target::thread_alive (ptid_t ptid) > const struct target_desc * > core_target::read_description () > { > + /* If the architecture provides a corefile target description hook, use > + it now. Even if the core file contains a target description in a note > + section, it is not useful for targets that can potentially have distinct > + descriptions for each thread. One example is AArch64's SVE/SME > + extensions that allow per-thread vector length changes, resulting in > + registers with different sizes. */ > + if (m_core_gdbarch && gdbarch_core_read_description_p (m_core_gdbarch)) > + { > + const struct target_desc *result; > + > + result = gdbarch_core_read_description (m_core_gdbarch, this, core_bfd); > + if (result != nullptr) > + return result; > + } > + > /* If the core file contains a target description note then we will use > that in preference to anything else. */ > bfd_size_type tdesc_note_size = 0; > @@ -1252,15 +1267,6 @@ core_target::read_description () > } > } > > - if (m_core_gdbarch && gdbarch_core_read_description_p (m_core_gdbarch)) > - { > - const struct target_desc *result; > - > - result = gdbarch_core_read_description (m_core_gdbarch, this, core_bfd); > - if (result != NULL) > - return result; > - } > - > return this->beneath ()->read_description (); > } I'm not convinced that this is right. The current role of gdbarch_core_read_description (AFAIU) is to provide a fallback to the note method. Usually, the note method is preferred, because it's precise, but if there's no note, maybe the gdbarch can derive a tdesc from what it sees in the core. Naturally, this use of gdbarch_core_read_description (as a fallback) has to go after trying the note method. Now, you want to to use gdbarch_core_read_description as an override to the note method, which is why you want to call it before trying the note method. I don't think the same gdbarch method can be used for both fallback and override. With your change, with an arch that defines a gdbarch_core_read_description hook, where we would have used the note before, we will now always use the hook. Not what we want. Some options I see: - Add another gdbarch hook, so one is called before trying the note, and one after. - Add another gdbarch hook that allows the arch to modify the target desc read from the note. So the flow would be: core_target::read_description creates a target from the note, then calls the gdbarch hook. The latter could return the same tdesc, or a new tdesc. The AArch64 hook could then create a new tdesc based on the one read from the note, but with the SVE/SME bits tweaked. Simon