From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id a77UIc5E+2SUHxMAWB0awg (envelope-from ) for ; Fri, 08 Sep 2023 11:59:10 -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=vpSS7gVy; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 808171E0C3; Fri, 8 Sep 2023 11:59:10 -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 ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 72BAA1E092 for ; Fri, 8 Sep 2023 11:59:08 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 10BE63858404 for ; Fri, 8 Sep 2023 15:59:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 10BE63858404 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1694188748; bh=RPTrz3rY9NEpTZoFkM9CAbNxsomjDQ3/Ph4cpCfPVZk=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=vpSS7gVydrejqBQMlPQ7rTTFpYQoB2RFnLKB53VrY4dzNOAL7RlyZEh77sBYGj4jF VDZwCRypuH8jtw1PLtHu6Ex4sYmyRnjdAmNTTH4OSE3XAASV0GnvJuykLyOTtxUe83 tTLnSTDtMq7ZqXJsf77+DQa/hZ9+jHKl1t6nVInA= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id D827D385842C for ; Fri, 8 Sep 2023 15:58:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D827D385842C 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 388FwdUb002658 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 8 Sep 2023 11:58:44 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 388FwdUb002658 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 DB6771E092; Fri, 8 Sep 2023 11:58:38 -0400 (EDT) Message-ID: <680b67cc-4330-4425-9860-02666f7d0e0b@polymtl.ca> Date: Fri, 8 Sep 2023 11:58:38 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 12/16] [gdb/generic] corefile/bug: Use thread-specific gdbarch when dumping register state to core files 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-13-luis.machado@arm.com> <971a9e69-a802-7354-728e-573d4c85c3f9@arm.com> In-Reply-To: <971a9e69-a802-7354-728e-573d4c85c3f9@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 15:58:39 +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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 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 9/8/23 07:09, Luis Machado via Gdb-patches wrote: > Could a global maintainer please go through this change and let me know if it is OK? It touches a generic part of gdb. > > Though I don't think it should change the behavior of non-aarch64 targets. > > On 9/7/23 16:20, Luis Machado via Gdb-patches wrote: >> When we have a core file generated by gdb (via the gcore command), gdb dumps >> the target description to a note. During loading of that core file, gdb will >> first try to load that saved target description. >> >> This works fine for almost all architectures. But AArch64 has a few >> dynamically-generated target descriptions/gdbarch depending on the vector >> length that was in use at the time the core file was generated. >> >> The target description gdb dumps to the core file note is the one generated >> at the time of attachment/startup. If, for example, the SVE vector length >> changed during execution, this would not reflect on the core file, as gdb >> would still dump the initial target description. >> >> Another issue is that the gdbarch potentially doesn't match the thread's >> real gdbarch, and so things like the register cache may have different formats >> and sizes. >> >> To address this, fetch the thread's architecture before dumping its register >> state. That way we will always use the correct target description/gdbarch. >> --- >> gdb/linux-tdep.c | 18 +++++++++++++++++- >> 1 file changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c >> index b5eee5e108c..7d0976932c6 100644 >> --- a/gdb/linux-tdep.c >> +++ b/gdb/linux-tdep.c >> @@ -2099,12 +2099,28 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size) >> stop_signal); >> >> if (signalled_thr != nullptr) >> - linux_corefile_thread (signalled_thr, &thread_args); >> + { >> + /* On some architectures, like AArch64, each thread can have a distinct >> + gdbarch (due to scalable extensions), and using the inferior gdbarch >> + is incorrect. >> + >> + Fetch each thread's gdbarch and pass it down to the lower layers so >> + we can dump the right set of registers. */ >> + thread_args.gdbarch = target_thread_architecture (signalled_thr->ptid); >> + linux_corefile_thread (signalled_thr, &thread_args); >> + } >> for (thread_info *thr : current_inferior ()->non_exited_threads ()) >> { >> if (thr == signalled_thr) >> continue; >> >> + /* On some architectures, like AArch64, each thread can have a distinct >> + gdbarch (due to scalable extensions), and using the inferior gdbarch >> + is incorrect. >> + >> + Fetch each thread's gdbarch and pass it down to the lower layers so >> + we can dump the right set of registers. */ >> + thread_args.gdbarch = target_thread_architecture (thr->ptid); >> linux_corefile_thread (thr, &thread_args); >> } >> > Makes sense to me: Approved-By: Simon Marchi I think the linux_corefile_thread_data structure is not useful nowadays. It was probably used through some callback's void pointer before. But now linux_corefile_thread could be changed to accept individual arguments instead, it would make things simpler. Would you mind doing this change as a cleanup on top of this series? Or you can do it before if you prefer. Please remind me, does an AArch64 core file contain one target description per thread, to account for the fact that different threads could have different register layouts? Or right now we just hope that all threads use the same target description (which might be different from what the inferior started with)? Simon