From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id wUSVFbMYEmmxEDMAWB0awg (envelope-from ) for ; Mon, 10 Nov 2025 11:54:11 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1762793651; bh=h6CxjN+smHH98/9bjfcs41+VuGQNbA6C/EWKNd1HCko=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=NOKk7mGh2JEixf3LoWoV3mCaXUmHCwq3FBUKAdxeNr2ryAdi9WDGIgfzznyDWnr2b KxoeYMOKlYaVlfXrjTCEVxLLBhPAHI4u4Pgm5EpVzu8jyk/h4JN/iiYVWc9QSD1xKl o8FvnHiObXTHZ/RP7hXaXBSdj4V0Q7SwNxj4Kx5k= Received: by simark.ca (Postfix, from userid 112) id 4D72F1E04C; Mon, 10 Nov 2025 11:54:11 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=d3kiLgrt; dkim-atps=neutral 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 888A61E04C for ; Mon, 10 Nov 2025 11:54:10 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EEB1C3858417 for ; Mon, 10 Nov 2025 16:54:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EEB1C3858417 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=d3kiLgrt Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 8AE043858D1E; Mon, 10 Nov 2025 16:52:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8AE043858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8AE043858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1762793560; cv=none; b=vULdhG1GxCO301nDQVt60ZKoW3WfmHHPUGSuGYncBEyEif4hj3y9qrg+yAAwb5iKix10KIwT+GG3ULroGP3zaGJovbjAYdWQdNSdeHv/uNjwcTlscpK3PaBsV0Jsl0I/GAzDrDfg5s/zv3xwqdky5nL3VWd1lVzONRCd2thT46I= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1762793560; c=relaxed/simple; bh=h6CxjN+smHH98/9bjfcs41+VuGQNbA6C/EWKNd1HCko=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=A7AHUETbdYwiabCKoGWlmWCF/HyLDfhARPcMk3liZrRsszP+14I30pLTUGjNQ7cT3Fdzds9NktenDQzMKCLwKR8z7UEfSqP0XWDP9fAgTOTUyuy1fEfJCzJ3Du9w46FOltZQAkBnIANGYsaSEvDcOmHGhfuwDzayidx62yMezEQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8AE043858D1E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1762793560; bh=h6CxjN+smHH98/9bjfcs41+VuGQNbA6C/EWKNd1HCko=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=d3kiLgrtn2aRhnUirAnCw/VKhRgCO/ix6AMPSPs/x4CI6mTVuYkuawSf5YnkKr6Gl UvO9KVSpQzCEY0JM6msJqF1w0FQATu83czh3hwriJB6Kyt+9t9c/fDdtVrL3csBP9N zDEDvhIbR5OjwdL/JrksVcL2VXdbmkHhPsAzcEhQ= Received: by simark.ca (Postfix) id 08D491E04C; Mon, 10 Nov 2025 11:52:39 -0500 (EST) Message-ID: Date: Mon, 10 Nov 2025 11:52:39 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] gdb/hppa: guess g packet size To: Sven Schnelle , gdb-patches@sourceware.org Cc: Helge Deller , John David Anglin , binutils@sourceware.org References: <20251104063038.62645-1-svens@stackframe.org> Content-Language: fr From: Simon Marchi In-Reply-To: <20251104063038.62645-1-svens@stackframe.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 11/4/25 1:30 AM, Sven Schnelle wrote: > With qemu supporting 64 bit now, add some code to determine the > register size of a hppa remote target. > > Signed-off-by: Sven Schnelle > --- > > Changes in v2: > > - adjust coding style > - use target_desc_up, make it static > - use nullptr instead of NULL > > gdb/hppa-tdep.c | 51 +++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 45 insertions(+), 6 deletions(-) > > diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c > index 96cb797c023..bd250408951 100644 > --- a/gdb/hppa-tdep.c > +++ b/gdb/hppa-tdep.c > @@ -33,6 +33,8 @@ > #include "trad-frame.h" > #include "frame-unwind.h" > #include "frame-base.h" > +#include "remote.h" > +#include "target-descriptions.h" > > #include "gdbcore.h" > #include "cli/cli-cmds.h" > @@ -43,6 +45,14 @@ > > static bool hppa_debug = false; > > +/* Properties (for struct target_desc) describing the g/G packet > + layout. */ > +#define PROPERTY_GP32 "internal: transfers-32bit-registers" > +#define PROPERTY_GP64 "internal: transfers-64bit-registers" > + > +static target_desc_up hppa_tdesc32; > +static target_desc_up hppa_tdesc64; > + > /* Some local constants. */ > static const int hppa32_num_regs = 128; > static const int hppa64_num_regs = 96; > @@ -2978,6 +2988,17 @@ hppa_skip_trampoline_code (const frame_info_ptr &frame, CORE_ADDR pc) > > -- chastain 2003-12-18 */ > > +static void > +hppa_register_g_packet_guesses (struct gdbarch *gdbarch) > +{ > + /* If the size matches the set of 32-bit or 64-bit integer registers, > + assume that's what we've got. */ > + register_remote_g_packet_guess (gdbarch, hppa32_num_regs * 4, hppa_tdesc32.get()); > + register_remote_g_packet_guess (gdbarch, hppa64_num_regs * 8, hppa_tdesc64.get()); Missing space before parenthesis, and line too long. This would work: register_remote_g_packet_guess (gdbarch, hppa32_num_regs * 4, hppa_tdesc32.get ()); register_remote_g_packet_guess (gdbarch, hppa64_num_regs * 8, hppa_tdesc64.get ()); > + > + /* Otherwise we don't have a useful guess. */ > +} > + > static struct gdbarch * > hppa_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > { > @@ -2991,15 +3012,28 @@ hppa_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > = gdbarch_alloc (&info, gdbarch_tdep_up (new hppa_gdbarch_tdep)); > hppa_gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); > > - /* Determine from the bfd_arch_info structure if we are dealing with > - a 32 or 64 bits architecture. If the bfd_arch_info is not available, > - then default to a 32bit machine. */ > - if (info.bfd_arch_info != NULL) > - tdep->bytes_per_address = > - info.bfd_arch_info->bits_per_address / info.bfd_arch_info->bits_per_byte; > + /* Determine from the target description if we are dealing with > + a 32 or 64 bits architecture. If the target description is not > + available, then check whether bfd_arch_info could be used. > + Otherwise default to a 32bit machine. > + */ > + if (info.target_desc != nullptr) > + { > + if (tdesc_property (info.target_desc, PROPERTY_GP64) != nullptr) > + tdep->bytes_per_address = 8; > + else if (tdesc_property (info.target_desc, PROPERTY_GP32) != nullptr) > + tdep->bytes_per_address = 4; What happens in the (not shown) "else" branch here? It seems like bytes_per_address won't be set and we'll hit the internal error below. Should we error out? > + } > + else if (info.bfd_arch_info != nullptr) > + { > + tdep->bytes_per_address = > + info.bfd_arch_info->bits_per_address / info.bfd_arch_info->bits_per_byte; > + } Remove curly braces: else if (info.bfd_arch_info != nullptr) tdep->bytes_per_address = info.bfd_arch_info->bits_per_address / info.bfd_arch_info->bits_per_byte; else > else > tdep->bytes_per_address = 4; > > + hppa_register_g_packet_guesses (gdbarch); > + > tdep->find_global_pointer = hppa_find_global_pointer; > > /* Some parts of the gdbarch vector depend on whether we are running > @@ -3122,6 +3156,11 @@ hppa_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file) > INIT_GDB_FILE (hppa_tdep) > { > gdbarch_register (bfd_arch_hppa, hppa_gdbarch_init, hppa_dump_tdep); > + hppa_tdesc32 = allocate_target_description (); > + set_tdesc_property (hppa_tdesc32.get(), PROPERTY_GP32, ""); > + > + hppa_tdesc64 = allocate_target_description (); > + set_tdesc_property (hppa_tdesc64.get(), PROPERTY_GP64, ""); > > add_cmd ("unwind", class_maintenance, unwind_command, > _("Print unwind table entry at given address."), I'm not super familiar with remote target descriptions, here's my understanding of what is happening, please let me know if this is correct. The target descriptions you create are never actually used are target descriptions, but are just some "flags" to indicate whether the g packet size guess resulted in 32 or 64. It seems a bit silly / strange to use a target description this way, but perhaps there's no better way with what we currently have. Are there other arches in GDB that work this way, that I could reference as "prior art"? I guess I'm just slightly worried that something that uses this gdbarch_info with a non-nullptr target_desc will see "oh, there a target_desc, let me use it". Then the results will be bogus because it's not an actual target description. But it's perhaps fine in practice, and it's still a step forward. If the above is correct, can you add some comment to indicate that the target descriptions are not actually used to describe target registers, and what its real purpose is? Otherwise code-wise the patch seems ok. Simon