[v2] Update the 'g' packet documentation
Commit Message
On 2023-01-13 6:58 p.m., Tom Tromey wrote:
>>> -When reading registers from a trace frame (@pxref{Analyze Collected
>>> -Data,,Using the Collected Data}), the stub may also return a string of
>>> +When reading registers, the stub may also return a string of
>>> literal @samp{x}'s in place of the register data digits, to indicate
>>> that the corresponding register has not been collected, thus its value
>>> is unavailable. For example, for an architecture with 4 registers of
>>>
>
> Pedro> Here, the new text still uses "collected", but lost the reference to trace frames.
> Pedro> It seems to me that that will result in people not knowing what "collected"
> Pedro> means in this context.
>
> Yeah, I wanted to get rid of the trace frame note, because it's
> confusing -- 'x' can be sent any time, not just a trace frame.
Yeah, but then people won't know what "collected" here means. Also, in the normal
case you shouldn't really end up with unavailable registers -- if some register
really doesn't exist, then the target description should ideally not describe it.
How about this version of the patch? It combines your original patch with
my suggestions, and also extends it further to describe the normal live target
scenario and include a remark about just not including the register in the tdesc.
(Note: I did not add an xref for the target description section at the end because
there's already one in the preceding paragraph.)
From 0d6c9a0b451933042e2b0d28c6a13bac0d044433 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tom@tromey.com>
Date: Wed, 11 Jan 2023 11:37:25 -0700
Subject: [PATCH] Update the 'g' packet documentation
The 'g' packet documentation references a macro that no longer exists,
and it also claims that the 'x' response for an unavailable register
is limited to trace frames. This patch updates the documentation to
reflect what I think is currently correct.
Co-Authored-By: Pedro Alves <pedro@palves.net>
Change-Id: I863baa3b9293059cfd4aa3d534602cbcb693ba87
---
gdb/doc/gdb.texinfo | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
base-commit: 1c66b8a03989b963534689ec0a9cce57e419afd5
Comments
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 27 Jan 2023 15:25:33 +0000
>
> >From 0d6c9a0b451933042e2b0d28c6a13bac0d044433 Mon Sep 17 00:00:00 2001
> From: Tom Tromey <tom@tromey.com>
> Date: Wed, 11 Jan 2023 11:37:25 -0700
> Subject: [PATCH] Update the 'g' packet documentation
>
> The 'g' packet documentation references a macro that no longer exists,
> and it also claims that the 'x' response for an unavailable register
> is limited to trace frames. This patch updates the documentation to
> reflect what I think is currently correct.
>
> Co-Authored-By: Pedro Alves <pedro@palves.net>
> Change-Id: I863baa3b9293059cfd4aa3d534602cbcb693ba87
> ---
> gdb/doc/gdb.texinfo | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
This is OK, thanks.
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> Here, the new text still uses "collected", but lost the reference to trace frames.
Pedro> It seems to me that that will result in people not knowing what "collected"
Pedro> means in this context.
>> Yeah, I wanted to get rid of the trace frame note, because it's
>> confusing -- 'x' can be sent any time, not just a trace frame.
Pedro> Yeah, but then people won't know what "collected" here means. Also, in the normal
Pedro> case you shouldn't really end up with unavailable registers -- if some register
Pedro> really doesn't exist, then the target description should ideally not describe it.
While I agree with that, it's also the case that tdep code can reject
such a description, and we ran into a case like this in the wild --
where a gdbserver does not report a register, causing i386-tdep to
reject the description. However, at one point AdaCore had a port for
this target, and that port reported the register but sent back 'x' --
which worked.
Also, I wanted to say, I think I misread your first reply. If I'd read
it correctly I would have rewritten the text. I'm sorry about that.
Pedro> How about this version of the patch? It combines your original patch with
Pedro> my suggestions, and also extends it further to describe the normal live target
Pedro> scenario and include a remark about just not including the register in the tdesc.
Pedro> (Note: I did not add an xref for the target description section at the end because
Pedro> there's already one in the preceding paragraph.)
This sounds great. Thank you.
Tom
On 2023-01-30 9:15 p.m., Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>
> Pedro> Here, the new text still uses "collected", but lost the reference to trace frames.
> Pedro> It seems to me that that will result in people not knowing what "collected"
> Pedro> means in this context.
>
>>> Yeah, I wanted to get rid of the trace frame note, because it's
>>> confusing -- 'x' can be sent any time, not just a trace frame.
>
> Pedro> Yeah, but then people won't know what "collected" here means. Also, in the normal
> Pedro> case you shouldn't really end up with unavailable registers -- if some register
> Pedro> really doesn't exist, then the target description should ideally not describe it.
>
> While I agree with that, it's also the case that tdep code can reject
> such a description, and we ran into a case like this in the wild --
> where a gdbserver does not report a register, causing i386-tdep to
> reject the description. However, at one point AdaCore had a port for
> this target, and that port reported the register but sent back 'x' --
> which worked.
Curious. It sounds like we made some register in a tdesc feature mandatory,
even after the tdesc feature existed in previous releases? That shouldn't have
happened.
>
> Also, I wanted to say, I think I misread your first reply. If I'd read
> it correctly I would have rewritten the text. I'm sorry about that.
No worries at all.
> Pedro> How about this version of the patch? It combines your original patch with
> Pedro> my suggestions, and also extends it further to describe the normal live target
> Pedro> scenario and include a remark about just not including the register in the tdesc.
> Pedro> (Note: I did not add an xref for the target description section at the end because
> Pedro> there's already one in the preceding paragraph.)
>
> This sounds great. Thank you.
Great, I'm going to merge it.
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>> While I agree with that, it's also the case that tdep code can reject
>> such a description, and we ran into a case like this in the wild --
>> where a gdbserver does not report a register, causing i386-tdep to
>> reject the description. However, at one point AdaCore had a port for
>> this target, and that port reported the register but sent back 'x' --
>> which worked.
Pedro> Curious. It sounds like we made some register in a tdesc feature
Pedro> mandatory, even after the tdesc feature existed in previous
Pedro> releases? That shouldn't have happened.
No, we are in the clear, what happened is that a vendor out there hacked
gdbserver to drop a register (the x86 "fop" register) for reasons we do
not know. They also hacked their gdb to accept this.
AdaCore also had a patch for this target's gdbserver to not supply "fop"
(the kernel, mysteriously, does not make it available) -- but did not
change the XML or gdb, instead having the register report as
unavailable.
Tom
@@ -41352,17 +41352,27 @@ Reply:
Each byte of register data is described by two hex digits. The bytes
with the register are transmitted in target byte order. The size of
each register and their position within the @samp{g} packet are
-determined by the @value{GDBN} internal gdbarch functions
-@code{DEPRECATED_REGISTER_RAW_SIZE} and @code{gdbarch_register_name}.
-
-When reading registers from a trace frame (@pxref{Analyze Collected
-Data,,Using the Collected Data}), the stub may also return a string of
-literal @samp{x}'s in place of the register data digits, to indicate
-that the corresponding register has not been collected, thus its value
-is unavailable. For example, for an architecture with 4 registers of
+determined by the target description (@pxref{Target Descriptions}); in
+the absence of a target description, this is done using code internal
+to @value{GDBN}; typically this is some customary register layout for
+the architecture in question.
+
+When reading registers, the stub may also return a string of literal
+@samp{x}'s in place of the register data digits, to indicate that the
+corresponding register's value is unavailable. For example, when
+reading registers from a trace frame (@pxref{Analyze Collected
+Data,,Using the Collected Data}), this means that the register has not
+been collected in the trace frame. When reading registers from a live
+program, this indicates that the stub has no means to access the
+register contents, even though the corresponding register is known to
+exist. Note that if a register truly does not exist on the target,
+then it is better to not include it in the target description in the
+first place.
+
+For example, for an architecture with 4 registers of
4 bytes each, the following reply indicates to @value{GDBN} that
-registers 0 and 2 have not been collected, while registers 1 and 3
-have been collected, and both have zero value:
+registers 0 and 2 are unavailable, while registers 1 and 3
+are available, and both have zero value:
@smallexample
-> @code{g}