[PATCH v3] gdb/hppa: guess g packet size

[PATCH v3] gdb/hppa: guess g packet size

[Sven Schnelle]

With qemu supporting 64 bit now, add some code to determine the
register size of a hppa remote target.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
Changes in v3:
- adjust coding style
- default to 4 bytes and print a warning when target description
  misses size attribute (or uses an unknown size)

Changes in v2:
- adjust coding style
- use target_desc_up, make it static
- use nullptr instead of NULL

gdb/hppa-tdep.c | 57 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 96cb797c023..929d0758991 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,19 @@ 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 ());
+
+  /* Otherwise we don't have a useful guess.  */
+}
+
 static struct gdbarch *
 hppa_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -2991,15 +3014,32 @@ 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<hppa_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;
+      else
+	{
+	  warning (_("target returned a target description this gdb doesn't "
+		     "support for the HP/PA architecture"));
+	  tdep->bytes_per_address = 4;
+	}
+    }
+  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
     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 +3162,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."),
--
2.51.0

Re: [PATCH v3] gdb/hppa: guess g packet size

[Simon Marchi]

> @@ -2991,15 +3014,32 @@ 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<hppa_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;
> +      else
> +	{
> +	  warning (_("target returned a target description this gdb doesn't "
> +		     "support for the HP/PA architecture"));

Good idea to put out a warning.  I would suggesting some minor changes
to the message:

	  warning (_("The target returned a target description but this gdb "
		     "doesn't support target descriptions for the HP/PA
		     "architecture"));

Just thinking out loud: it would actually be nice to have this warning
for all architectures that don't support target descriptions, if there
was a way to put it at a single common place.

Other than that, the patch looks fine.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Do you have push access or would you like me to push the patch on your
behalf (with the modification proposed above)?

Simon

Re: [PATCH v3] gdb/hppa: guess g packet size

[Sven Schnelle]

Simon Marchi <simark@simark.ca> writes:

>> +	  warning (_("target returned a target description this gdb doesn't "
>> +		     "support for the HP/PA architecture"));
>
> Good idea to put out a warning.  I would suggesting some minor changes
> to the message:
>
> 	  warning (_("The target returned a target description but this gdb "
> 		     "doesn't support target descriptions for the HP/PA
> 		     "architecture"));
>
> Just thinking out loud: it would actually be nice to have this warning
> for all architectures that don't support target descriptions, if there
> was a way to put it at a single common place.
>
> Other than that, the patch looks fine.
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>
> Do you have push access or would you like me to push the patch on your
> behalf (with the modification proposed above)?

I don't have push acess - would appreciate if you could adjust the
message and push it.

Re: [PATCH v3] gdb/hppa: guess g packet size

[Simon Marchi]

On 11/10/25 4:34 PM, Sven Schnelle wrote:
> Simon Marchi <simark@simark.ca> writes:
> 
>>> +	  warning (_("target returned a target description this gdb doesn't "
>>> +		     "support for the HP/PA architecture"));
>>
>> Good idea to put out a warning.  I would suggesting some minor changes
>> to the message:
>>
>> 	  warning (_("The target returned a target description but this gdb "
>> 		     "doesn't support target descriptions for the HP/PA
>> 		     "architecture"));
>>
>> Just thinking out loud: it would actually be nice to have this warning
>> for all architectures that don't support target descriptions, if there
>> was a way to put it at a single common place.
>>
>> Other than that, the patch looks fine.
>>
>> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>>
>> Do you have push access or would you like me to push the patch on your
>> behalf (with the modification proposed above)?
> 
> I don't have push acess - would appreciate if you could adjust the
> message and push it.

Done, thanks.  I even extended the message to:

	  warning (_("The target returned a target description but this GDB "
		     "doesn't support target descriptions for the HP/PA"
		     "architecture.  Assuming standard 32-bit register"
		     "layout."));

Simon

Re: [PATCH v3] gdb/hppa: guess g packet size

[Sven Schnelle]

Simon Marchi <simark@simark.ca> writes:

> On 11/10/25 4:34 PM, Sven Schnelle wrote:
>> Simon Marchi <simark@simark.ca> writes:
>> 
>>>> +	  warning (_("target returned a target description this gdb doesn't "
>>>> +		     "support for the HP/PA architecture"));
>>>
>>> Good idea to put out a warning.  I would suggesting some minor changes
>>> to the message:
>>>
>>> 	  warning (_("The target returned a target description but this gdb "
>>> 		     "doesn't support target descriptions for the HP/PA
>>> 		     "architecture"));
>>>
>>> Just thinking out loud: it would actually be nice to have this warning
>>> for all architectures that don't support target descriptions, if there
>>> was a way to put it at a single common place.
>>>
>>> Other than that, the patch looks fine.
>>>
>>> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>>>
>>> Do you have push access or would you like me to push the patch on your
>>> behalf (with the modification proposed above)?
>> 
>> I don't have push acess - would appreciate if you could adjust the
>> message and push it.
>
> Done, thanks.  I even extended the message to:
>
> 	  warning (_("The target returned a target description but this GDB "
> 		     "doesn't support target descriptions for the HP/PA"
> 		     "architecture.  Assuming standard 32-bit register"
> 		     "layout."));

Thanks! One nit pick - I think there's a superfluous whitespace before
'Assuming standard 32-bit...'

Re: [PATCH v3] gdb/hppa: guess g packet size

[Simon Marchi]

On 2025-11-10 16:39, Sven Schnelle wrote:
> Simon Marchi <simark@simark.ca> writes:
> 
>> On 11/10/25 4:34 PM, Sven Schnelle wrote:
>>> Simon Marchi <simark@simark.ca> writes:
>>>
>>>>> +	  warning (_("target returned a target description this gdb doesn't "
>>>>> +		     "support for the HP/PA architecture"));
>>>>
>>>> Good idea to put out a warning.  I would suggesting some minor changes
>>>> to the message:
>>>>
>>>> 	  warning (_("The target returned a target description but this gdb "
>>>> 		     "doesn't support target descriptions for the HP/PA
>>>> 		     "architecture"));
>>>>
>>>> Just thinking out loud: it would actually be nice to have this warning
>>>> for all architectures that don't support target descriptions, if there
>>>> was a way to put it at a single common place.
>>>>
>>>> Other than that, the patch looks fine.
>>>>
>>>> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>>>>
>>>> Do you have push access or would you like me to push the patch on your
>>>> behalf (with the modification proposed above)?
>>>
>>> I don't have push acess - would appreciate if you could adjust the
>>> message and push it.
>>
>> Done, thanks.  I even extended the message to:
>>
>> 	  warning (_("The target returned a target description but this GDB "
>> 		     "doesn't support target descriptions for the HP/PA"
>> 		     "architecture.  Assuming standard 32-bit register"
>> 		     "layout."));
> 
> Thanks! One nit pick - I think there's a superfluous whitespace before
> 'Assuming standard 32-bit...'

The GNU standard calls for two spaces after a period (in comments as
well).

Simon