Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Yodel Eldar <yodel.eldar@gmail.com>
To: Simon Marchi <simark@simark.ca>
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>,
	Yodel Eldar <yodel.eldar@gmail.com>,
	gdb-patches@sourceware.org
Subject: Re: [PATCH 1/2] gdb/alpha: Add target description support
Date: Thu, 12 Jun 2025 14:29:35 -0500	[thread overview]
Message-ID: <40cede5f-1b8c-4eb9-b702-3805499efbf5@gmail.com> (raw)
In-Reply-To: <3ef4c598-28d8-4c66-844d-58e5629415ab@gmail.com>


On 6/4/25 12:29 PM, Yodel Eldar wrote:
>
> On 6/4/25 9:49 AM, Maciej W. Rozycki wrote:
>> On Wed, 4 Jun 2025, Yodel Eldar wrote:
>>
>>>> Do we need to call tdesc_numbered_register for the register whose name
>>>> is ""?  I suppose that we do, for backwards compatibility, when
>>>> debugging against a remote target that doesn't send a target
>>>> description?
>>> Hi Simon and thanks for your time and question! I think we do, because
>>> (IIUC) suppose an older remote target responds to, say, a 'p n' packet,
>>> but the anonymous register is unaccounted for by the client, wouldn't
>>> that break compatibility? Can't say for certain, so I defer to your and
>>> the community's wisdom and err on the side of caution as I investigate
>>> it.
>>   Are there any remote Alpha targets there in the first place?
>>
>>   I only have writing Alpha/Linux gdbserver on my todo list once I'm 
>> done
>> with the more urgent Alpha stuff (Linux kernel EV4 support restoration,
>> GCC LRA conversion) and I haven't heard of any other Alpha GDB RSP debug
>> stub either.  There *might* be something in QEMU; I guess that'd be the
>> only place to check.
>>
>>    Maciej
>>
>
> Maciej, you hit the nail on the head! Indeed, QEMU is the project I
> alluded to in the cover letter for the patch, and I have been using it
> for testing throughout the development of the patch. It also provided
> the impetus to write the target description for Alpha in the first
> place as it uses them for its execlog plugin.
> Thanks for taking the time to reply despite having more pressing
> Alpha tasks. I will keep an eye out for your forthcoming submissions,
> and provide my modest feedback if it's helpful.
>
> Yodel
>

Hi Simon et al!

Update regarding the feasibility of skipping the tdesc_numbered_register
(TNR) call on the anonymous register:

I modified the patch to skip the TNR call (see below for diff), and ran
the "maintenance print remote-registers" command while connected to a
QEMU Alpha remote with "set debug remote on" and observed the following
(see below for the abridged command output):
   1. GDB internally creates a 0-bit wide placeholder at the anonymous
   register's expected index (65), and that placeholder shares the GDB
   internal offset of the unique register (520).
   2. GDB infers the anonymous register as its 67th register with
   internal offset 528 and type int64_t.
   3. Despite having an internal number of 67, GDB does assign the
   anonymous register the correct remote number of 65 and the correct
   g/G offset of 520.
   4. 'g' and 'p n' packets (particularly, where n is unique's remote
   number 66) do not appear to corrupt the regcache as I had feared.

On the one hand, observations 3 and 4 suggest that skipping TNR on the
anonymous register could work, but the discrepancy between the internal
versus g/G offsets and the internal versus remote numbers (observations
1 and 2) makes me wary of some hard-to-find side-effect that I may have
yet overlooked. Furthermore, 67 is the number of total registers, so
assigning 67 for the anonymous register's internal number (observation
3) could be problematic.

Moreover, having GDB implicitly account for the anonymous register at 67
while also displaying a 0-bit register placeholder at 65 seems
gratuitous and probably offsets any performance benefit derived from
skipping TNR on the anonymous register.

With regard to backwards compatibility, I compiled a 2011 version of
QEMU, commit b758aca1f6c, and performed the same experiment as above,
and the output of the maintenance command was identical to the recent
version used earlier. Likewise for the 2008 commit 19bf517b7f7 that
introduced GDB stub support for QEMU's Alpha target, although this
version had not implemented PALcode functionality and lacked the unique
register. Fortunately, all three did not appear to have problems
communicating with the GDB client, notwithstanding my concerns, but the
scope of my testing was limited to the QEMU targets, and I don't think
it qualifies as a comprehensive answer to the compatibility question as
my inability to find other targets does not preclude their existence.

Thus, I recommend against the omission of tdesc_numbered_register on the
anonymous register.

Please let me know how to proceed; should v2 of the patch omit the
tdesc_numbered_register call on the anonymous register? I defer to your
judgment. Thanks!

Yodel

------------------------
Diff of changes to patch
------------------------

diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 60888a6f475..262a578b39c 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -1719,10 +1719,13 @@ alpha_gdbarch_init (struct gdbarch_info info, 
struct gdbarch_list *arches)

        tdesc_data = tdesc_data_alloc ();
        valid_p = true;
-      for (int i = 0; i < ALPHA_NUM_REGS; ++i)
+      for (int i = 0; i <= ALPHA_PC_REGNUM; ++i)
         valid_p &= tdesc_numbered_register (feature, tdesc_data.get (), i,
alpha_register_names[i]);

+      valid_p &= tdesc_numbered_register(feature, tdesc_data.get (),
+                               ALPHA_UNIQUE_REGNUM,
+  alpha_register_names[ALPHA_UNIQUE_REGNUM]);
        if (!valid_p)
         return nullptr;
      }

-------------------------------------------------------
Remote registers with omitted TNR on anonymous register
-------------------------------------------------------

(gdb) maintenance print remote-registers
Name       Nr   Rel  Offset   Size  Type            Rmt Nr  g/G Offset  
Expedited
v0         0    0    0        8     int64_t         0       0
t0         1    1    8        8     int64_t         1       8
   ...
pc         64   64   512      8     *1              64      512
''         65   65   520      0     int0_t
unique     66   66   520      8     int64_t         66      528
''         67   67   528      8     int64_t         65      520
*1: Register type's name NULL.

-----------------------------------------------
Remote registers with TNR on anonymous register
-----------------------------------------------

(gdb) maintenance print remote-registers
Name       Nr   Rel  Offset   Size  Type            Rmt Nr  g/G Offset  
Expedited
v0         0    0    0        8     int64_t         0       0
t0         1    1    8        8     int64_t         1       8
   ...
pc         64   64   512      8     *1              64      512
''         65   65   520      8     int64_t         65      520
unique     66   66   528      8     int64_t         66      528
*1: Register type's name NULL.


  reply	other threads:[~2025-06-12 19:30 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-26 15:12 [PATCH 0/2] gdb/alpha: Add target description support and fpcr flags Yodel Eldar
2025-05-26 15:12 ` [PATCH 1/2] gdb/alpha: Add target description support Yodel Eldar
2025-05-26 16:13   ` Eli Zaretskii
2025-05-26 17:14     ` Yodel
2025-06-03 18:49   ` Simon Marchi
2025-06-04 13:44     ` Yodel Eldar
2025-06-04 14:49       ` Maciej W. Rozycki
2025-06-04 17:29         ` Yodel Eldar
2025-06-12 19:29           ` Yodel Eldar [this message]
2025-06-12 20:30             ` Simon Marchi
2025-06-12 20:33               ` Simon Marchi
2025-06-12 22:07                 ` Yodel Eldar
2025-06-13 14:34                   ` Simon Marchi
2025-06-30  0:08                     ` Yodel Eldar
2025-07-02 17:14                       ` Simon Marchi
2025-07-02 18:51                         ` Yodel Eldar
2025-07-02 18:53               ` Maciej W. Rozycki
2025-07-02 19:02                 ` Simon Marchi
2025-07-02 19:21                   ` Maciej W. Rozycki
2025-07-02 20:52                     ` Yodel Eldar
2025-07-02 21:12                       ` Maciej W. Rozycki
2025-07-03 16:49                         ` Yodel Eldar
2025-07-03 16:02                     ` Simon Marchi
2025-07-02 18:48   ` Maciej W. Rozycki
2025-05-26 15:12 ` [PATCH 2/2] gdb/alpha: Redefine fpcr with fpcr_flags type Yodel Eldar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=40cede5f-1b8c-4eb9-b702-3805499efbf5@gmail.com \
    --to=yodel.eldar@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=macro@orcam.me.uk \
    --cc=simark@simark.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox