* [PATCH 01/12] Document conventions for describing packet syntax
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 15:25 ` Eli Zaretskii
2024-04-19 15:13 ` [PATCH 02/12] Centralize documentation of error and empty RSP responses Pedro Alves
` (11 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches; +Cc: Jim Blandy, Mike Wrighton, Nathan Sidwell, Hafiz Abid Qadeer
This comment documents conventions for describing packet syntax in the
Overview section.
Change-Id: I96198592601b24c983da563d143666137e4d0a4e
Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
---
gdb/doc/gdb.texinfo | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 31a531ee992..e9d54527c90 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42517,6 +42517,22 @@ For any @var{command} not supported by the stub, an empty response
protocol. A newer @value{GDBN} can tell if a packet is supported based
on that response.
+In describing packets (and responses), each description has a template
+showing the overall syntax, followed by an explanation of the packet's
+meaning. We include spaces in some of the templates for clarity;
+these are not part of the packet's syntax. No @value{GDBN} packet
+uses spaces to separate its components. For example, a template like
+@samp{foo @var{bar} @var{baz}} describes a packet beginning with the
+three ASCII bytes @samp{foo}, followed by a @var{bar}, followed
+directly by a @var{baz}. @value{GDBN} does not transmit a space
+character between the @samp{foo} and the @var{bar}, or between the
+@var{bar} and the @var{baz}.
+
+We place optional portions of a packet in @r{[}square brackets@r{]};
+for example, a template like @samp{c @r{[}@var{addr}@r{]}} describes a
+packet beginning with the single ASCII character @samp{c}, possibly
+followed by an @var{addr}.
+
At a minimum, a stub is required to support the @samp{?} command to
tell @value{GDBN} the reason for halting, @samp{g} and @samp{G}
commands for register access, and the @samp{m} and @samp{M} commands
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 01/12] Document conventions for describing packet syntax
2024-04-19 15:13 ` [PATCH 01/12] Document conventions for describing packet syntax Pedro Alves
@ 2024-04-19 15:25 ` Eli Zaretskii
2024-04-19 15:42 ` Eli Zaretskii
2024-04-22 19:01 ` Pedro Alves
0 siblings, 2 replies; 55+ messages in thread
From: Eli Zaretskii @ 2024-04-19 15:25 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, jimb, mike_wrighton, nathan, abidh
> From: Pedro Alves <pedro@palves.net>
> Cc: Jim Blandy <jimb@codesourcery.com>,
> Mike Wrighton <mike_wrighton@mentor.com>,
> Nathan Sidwell <nathan@codesourcery.com>,
> Hafiz Abid Qadeer <abidh@codesourcery.com>
> Date: Fri, 19 Apr 2024 16:13:31 +0100
>
> This comment documents conventions for describing packet syntax in the
> Overview section.
Thanks.
> +We place optional portions of a packet in @r{[}square brackets@r{]};
This should be in @samp, otherwise the @r parts are redundant and
won't have any effect.
> +for example, a template like @samp{c @r{[}@var{addr}@r{]}} describes a
> +packet beginning with the single ASCII character @samp{c}, possibly
> +followed by an @var{addr}.
OK with this nit fixed.
Approved-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 01/12] Document conventions for describing packet syntax
2024-04-19 15:25 ` Eli Zaretskii
@ 2024-04-19 15:42 ` Eli Zaretskii
2024-04-22 19:10 ` Pedro Alves
2024-04-22 19:01 ` Pedro Alves
1 sibling, 1 reply; 55+ messages in thread
From: Eli Zaretskii @ 2024-04-19 15:42 UTC (permalink / raw)
To: pedro; +Cc: gdb-patches
> Date: Fri, 19 Apr 2024 18:25:26 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb-patches@sourceware.org, jimb@codesourcery.com,
> mike_wrighton@mentor.com, nathan@codesourcery.com, abidh@codesourcery.com
>
> > From: Pedro Alves <pedro@palves.net>
> > Cc: Jim Blandy <jimb@codesourcery.com>,
> > Mike Wrighton <mike_wrighton@mentor.com>,
> > Nathan Sidwell <nathan@codesourcery.com>,
> > Hafiz Abid Qadeer <abidh@codesourcery.com>
> > Date: Fri, 19 Apr 2024 16:13:31 +0100
> >
> > This comment documents conventions for describing packet syntax in the
> > Overview section.
>
> Thanks.
Btw, this response bounced from all the addresses except yours and
gdb-patches.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/12] Document conventions for describing packet syntax
2024-04-19 15:42 ` Eli Zaretskii
@ 2024-04-22 19:10 ` Pedro Alves
0 siblings, 0 replies; 55+ messages in thread
From: Pedro Alves @ 2024-04-22 19:10 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2024-04-19 16:42, Eli Zaretskii wrote:
>> Date: Fri, 19 Apr 2024 18:25:26 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: gdb-patches@sourceware.org, jimb@codesourcery.com,
>> mike_wrighton@mentor.com, nathan@codesourcery.com, abidh@codesourcery.com
>>
>>> From: Pedro Alves <pedro@palves.net>
>>> Cc: Jim Blandy <jimb@codesourcery.com>,
>>> Mike Wrighton <mike_wrighton@mentor.com>,
>>> Nathan Sidwell <nathan@codesourcery.com>,
>>> Hafiz Abid Qadeer <abidh@codesourcery.com>
>>> Date: Fri, 19 Apr 2024 16:13:31 +0100
>>>
>>> This comment documents conventions for describing packet syntax in the
>>> Overview section.
>>
>> Thanks.
>
> Btw, this response bounced from all the addresses except yours and
> gdb-patches.
>
Right, sorry. The original patch was from 2007-06-13, and all those inboxes
don't exist any more. I have:
[sendemail]
suppresscc = author
in my .gitconfig, and I thought that that would be sufficient for git to not
auto-add the co-authors to CC automatically. Looks like I was wrong, or I did
something else wrong.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/12] Document conventions for describing packet syntax
2024-04-19 15:25 ` Eli Zaretskii
2024-04-19 15:42 ` Eli Zaretskii
@ 2024-04-22 19:01 ` Pedro Alves
2024-04-22 19:44 ` Eli Zaretskii
1 sibling, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-22 19:01 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches, jimb, mike_wrighton, nathan, abidh
Hi!
On 2024-04-19 16:25, Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Cc: Jim Blandy <jimb@codesourcery.com>,
>> Mike Wrighton <mike_wrighton@mentor.com>,
>> Nathan Sidwell <nathan@codesourcery.com>,
>> Hafiz Abid Qadeer <abidh@codesourcery.com>
>> Date: Fri, 19 Apr 2024 16:13:31 +0100
>>
>> This comment documents conventions for describing packet syntax in the
>> Overview section.
>
> Thanks.
>
>> +We place optional portions of a packet in @r{[}square brackets@r{]};
>
> This should be in @samp, otherwise the @r parts are redundant and
> won't have any effect.
>
Hmm, if I wrap that in @samp, then it renders as:
We place optional portions of a packet in '[square brackets]'; for example, a template like ‘c [addr]’ describes a packet beginning with the single ASCII character ‘c’, possibly followed by an addr.
Those '' around [square brackets] seem off to me there. The original author must have wanted the
rendered text without the single quotes too, since they didn't use @samp and they certainly
looked at the rendered output back then. So given @r are nops, I think we should just remove them.
That results in:
We place optional portions of a packet in [square brackets]; for example, a template like ‘c [addr]’ describes a packet beginning with the single ASCII character ‘c’, possibly followed by an addr.
which IMHO looks better.
If you dislike that, I noticed that:
https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Command-List.html
says:
Square brackets, [ ], indicate optional arguments
We could switch to a similar wording. I kind of like the original better, but
this is a very tiny detail, of course.
Let me know what you think.
Pedro Alves
>> +for example, a template like @samp{c @r{[}@var{addr}@r{]}} describes a
>> +packet beginning with the single ASCII character @samp{c}, possibly
>> +followed by an @var{addr}.
>
> OK with this nit fixed.
>
> Approved-By: Eli Zaretskii <eliz@gnu.org>
>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 01/12] Document conventions for describing packet syntax
2024-04-22 19:01 ` Pedro Alves
@ 2024-04-22 19:44 ` Eli Zaretskii
0 siblings, 0 replies; 55+ messages in thread
From: Eli Zaretskii @ 2024-04-22 19:44 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, jimb, mike_wrighton, nathan, abidh
> Date: Mon, 22 Apr 2024 20:01:17 +0100
> Cc: gdb-patches@sourceware.org, jimb@codesourcery.com,
> mike_wrighton@mentor.com, nathan@codesourcery.com, abidh@codesourcery.com
> From: Pedro Alves <pedro@palves.net>
>
> >> +We place optional portions of a packet in @r{[}square brackets@r{]};
> >
> > This should be in @samp, otherwise the @r parts are redundant and
> > won't have any effect.
> >
>
> Hmm, if I wrap that in @samp, then it renders as:
>
> We place optional portions of a packet in '[square brackets]'; for example, a template like ‘c [addr]’ describes a packet beginning with the single ASCII character ‘c’, possibly followed by an addr.
>
> Those '' around [square brackets] seem off to me there. The original author must have wanted the
> rendered text without the single quotes too, since they didn't use @samp and they certainly
> looked at the rendered output back then. So given @r are nops, I think we should just remove them.
Removing @r is fine by me, thanks.
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 02/12] Centralize documentation of error and empty RSP responses
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
2024-04-19 15:13 ` [PATCH 01/12] Document conventions for describing packet syntax Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 15:36 ` Eli Zaretskii
2024-04-19 15:13 ` [PATCH 03/12] Document "E.MESSAGE" RSP errors Pedro Alves
` (10 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches; +Cc: Jim Blandy, Mike Wrighton, Nathan Sidwell, Hafiz Abid Qadeer
Currently, for each packet, we document the "E NN" response (error),
and the empty response (unsupported). This patch centralizes that in
a new "Standard Replies" section.
In the "Packets", "General Query Packets", "Tracepoint Packets"
sections, Remove explicit mention of empty and error replies, except
when they provide detail not covered in Standard Replies.
Note this hunk:
-@item E @var{NN}
-@var{NN} is errno
and this one:
-@item E00
-The request was malformed, or @var{annex} was invalid.
-
-@item E @var{nn}
-The offset was invalid, or there was an error encountered reading the data.
-The @var{nn} part is a hex-encoded @code{errno} value.
were really documenting things that don't really work that way.
The first is the documentation of the "m" packet. GDB does _not_
interpret the NN as an errno. It can't, in fact, because the
remote/target errno numbers have nothing to do with GDB/host errno
numbers in a cross debugging scenario.
The second hunk above is from the documentation of qXfer. Again, GDB
does not give any interpretation to the NN error code at all. Nor
does GDBserver. And again, an errno number can't be interpreted in a
cross debugging scenario.
Change-Id: I973695c80809cdb5a5e8d5be8b78ba4d1ecdb513
Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
---
gdb/doc/gdb.texinfo | 244 +++++++-------------------------------------
1 file changed, 36 insertions(+), 208 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e9d54527c90..57260a5b2fa 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -40242,8 +40242,6 @@ The @var{gdb_jump_pad_head} is updated head of jumppad. Both of
@var{target_address} and @var{gdb_jump_pad_head} are 8-byte long.
The @var{fjump} contains a sequence of instructions jump to jumppad entry.
The @var{fjump_size}, 4-byte long, is the size of @var{fjump}.
-@item E @var{NN}
-for an error
@end table
@@ -40262,8 +40260,6 @@ Asks in-process agent to probe the marker at @var{address}.
Replies:
@table @samp
-@item E @var{NN}
-for an error
@end table
@item unprobe_marker_at:@var{address}
Asks in-process agent to unprobe the marker at @var{address}.
@@ -42368,6 +42364,7 @@ Show the current setting of the target wait timeout.
@menu
* Overview::
+* Standard Replies::
* Packets::
* Stop Reply Packets::
* General Query Packets::
@@ -42508,14 +42505,8 @@ seven repeats (@samp{$}) can be expanded using a repeat count of only
five (@samp{"}). For example, @samp{00000000} can be encoded as
@samp{0*"00}.
-The error response returned for some packets includes a two character
-error number. That number is not well defined.
-
-@cindex empty response, for unsupported packets
-For any @var{command} not supported by the stub, an empty response
-(@samp{$#00}) should be returned. That way it is possible to extend the
-protocol. A newer @value{GDBN} can tell if a packet is supported based
-on that response.
+See @ref{Standard Replies} for standard error responses, and how to
+respond indicating a packet is not supported.
In describing packets (and responses), each description has a template
showing the overall syntax, followed by an explanation of the packet's
@@ -42543,6 +42534,31 @@ the @samp{s} (step) command. Stubs that support multi-threading
targets should support the @samp{vCont} command. All other commands
are optional.
+@node Standard Replies
+@section Standard Replies
+
+The remote protocol specifies a few standard replies. All commands
+support these, except as noted in the individual command descriptions.
+
+@table @asis
+
+@item empty response
+
+@cindex empty response, for unsupported packets
+@cindex unsupported packets, empty response for
+For any @var{command} not supported by the stub, an empty response
+(@samp{$#00}) should be returned. That way it is possible to extend the
+protocol. A newer @value{GDBN} can tell if a packet is supported based
+on that response (but see also @ref{qSupported}).
+
+@item @samp{E @var{xx}}
+An error has occurred; @var{xx} is a two-digit hexadecimal error
+number. In almost all cases, the protocol does not specify the
+meaning of the error numbers; @value{GDBN} usually ignores the
+numbers, or displays them to the user without further interpretation.
+
+@end table
+
@node Packets
@section Packets
@@ -42630,8 +42646,6 @@ Reply:
@table @samp
@item OK
The arguments were set.
-@item E @var{NN}
-An error occurred.
@end table
@item b @var{baud}
@@ -42720,8 +42734,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item F @var{RC},@var{EE},@var{CF};@var{XX}
@@ -42768,8 +42780,6 @@ are available, and both have zero value:
<- @code{xxxxxxxx00000000xxxxxxxx00000000}
@end smallexample
-@item E @var{NN}
-for an error.
@end table
@item G @var{XX@dots{}}
@@ -42781,8 +42791,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item H @var{op} @var{thread-id}
@@ -42799,8 +42807,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@c FIXME: JTC:
@@ -42875,8 +42881,6 @@ Reply:
Memory contents; each byte is transmitted as a two-digit hexadecimal number.
The reply may contain fewer addressable memory units than requested if the
server was able to read only part of the region of memory.
-@item E @var{NN}
-@var{NN} is errno
@end table
@item M @var{addr},@var{length}:@var{XX@dots{}}
@@ -42888,10 +42892,8 @@ byte is transmitted as a two-digit hexadecimal number.
Reply:
@table @samp
@item OK
-for success
-@item E @var{NN}
-for an error (this includes the case where only part of the data was
-written).
+All the data was written successfully. (If only part of the data was
+written, this command returns an error.)
@end table
@item p @var{n}
@@ -42904,10 +42906,6 @@ Reply:
@table @samp
@item @var{XX@dots{}}
the register's value
-@item E @var{NN}
-for an error
-@item @w{}
-Indicating an unrecognized @var{query}.
@end table
@item P @var{n@dots{}}=@var{r@dots{}}
@@ -42921,8 +42919,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item q @var{name} @var{params}@dots{}
@@ -42982,8 +42978,6 @@ Reply:
@table @samp
@item OK
thread is still alive
-@item E @var{NN}
-thread is dead
@end table
@item v
@@ -43011,8 +43005,6 @@ This packet is only available in extended mode (@pxref{extended mode}).
Reply:
@table @samp
-@item E @var{nn}
-for an error
@item @r{Any stop packet}
for success in all-stop mode (@pxref{Stop Reply Packets})
@item OK
@@ -43101,8 +43093,6 @@ Reply:
@item vCont@r{[};@var{action}@dots{}@r{]}
The @samp{vCont} packet is supported. Each @var{action} is a supported
command in the @samp{vCont} packet.
-@item @w{}
-The @samp{vCont} packet is not supported.
@end table
@anchor{vCtrlC packet}
@@ -43117,8 +43107,6 @@ variant.
Reply:
@table @samp
-@item E @var{nn}
-for an error
@item OK
for success
@end table
@@ -43143,8 +43131,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item vFlashWrite:@var{addr}:@var{XX@dots{}}
@@ -43167,8 +43153,6 @@ Reply:
for success
@item E.memtype
for vFlashWrite addressing non-flash memory
-@item E @var{NN}
-for an error
@end table
@item vFlashDone
@@ -43190,8 +43174,6 @@ supported; see @ref{multiprocess extensions}.
Reply:
@table @samp
-@item E @var{nn}
-for an error
@item OK
for success
@end table
@@ -43223,8 +43205,6 @@ This packet is only available in extended mode (@pxref{extended mode}).
Reply:
@table @samp
-@item E @var{nn}
-for an error
@item @r{Any stop packet}
for success (@pxref{Stop Reply Packets})
@end table
@@ -43245,8 +43225,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item z @var{type},@var{addr},@var{kind}
@@ -43327,10 +43305,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@item z1,@var{addr},@var{kind}
@@ -43352,10 +43326,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@item z2,@var{addr},@var{kind}
@@ -43369,10 +43339,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@item z3,@var{addr},@var{kind}
@@ -43386,10 +43352,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@item z4,@var{addr},@var{kind}
@@ -43403,10 +43365,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@end table
@@ -43776,8 +43734,6 @@ detect trailing zeros.
Reply:
@table @samp
-@item E @var{NN}
-An error (such as memory fault)
@item C @var{crc32}
The specified memory region's checksum is @var{crc32}.
@end table
@@ -43800,13 +43756,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QDisableRandomization} is not supported
-by the stub.
@end table
This packet is not probed by default; the remote stub must request it,
@@ -43835,9 +43784,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
@end table
This packet is not probed by default; the remote stub must request it,
@@ -44027,12 +43973,6 @@ Reply:
@item @var{XX}@dots{}
Hex encoded (big endian) bytes representing the address of the thread
local storage requested.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{qGetTLSAddr} is not supported by the stub.
@end table
@item qGetTIBAddr:@var{thread-id}
@@ -44047,13 +43987,6 @@ Reply:
@item @var{XX}@dots{}
Hex encoded (big endian) bytes representing the linear address of the
thread information block.
-
-@item E @var{nn}
-An error occurred. This means that either the thread was not found, or the
-address could not be retrieved.
-
-@item @w{}
-An empty reply indicates that @samp{qGetTIBAddr} is not supported by the stub.
@end table
@item qL @var{startflag} @var{threadcount} @var{nextthread}
@@ -44093,20 +44026,15 @@ is architecture-specific.
@var{type} is the type of tag the request wants to fetch. The type is a signed
integer.
+@value{GDBN} will only send this packet if the stub has advertised
+support for memory tagging via @samp{qSupported}.
+
Reply:
@table @samp
@item @var{mxx}@dots{}
Hex encoded sequence of uninterpreted bytes, @var{xx}@dots{}, representing the
tags found in the requested memory range.
-@item E @var{nn}
-An error occurred. This means that fetching of memory tags failed for some
-reason.
-
-@item @w{}
-An empty reply indicates that @samp{qMemTags} is not supported by the stub,
-although this should not happen given @value{GDBN} will only send this packet
-if the stub has advertised support for memory tagging via @samp{qSupported}.
@end table
@cindex check if a given address is in a memory tagged region
@@ -44171,20 +44099,14 @@ integer.
interpreted by the target. Each pair of hex digits is interpreted as a
single byte.
+@value{GDBN} will only send this packet if the stub has advertised
+support for memory tagging via @samp{qSupported}.
+
Reply:
@table @samp
@item OK
The request was successful and the memory tag granules were modified
accordingly.
-
-@item E @var{nn}
-An error occurred. This means that modifying the memory tag granules failed
-for some reason.
-
-@item @w{}
-An empty reply indicates that @samp{QMemTags} is not supported by the stub,
-although this should not happen given @value{GDBN} will only send this packet
-if the stub has advertised support for memory tagging via @samp{qSupported}.
@end table
@item qOffsets
@@ -44241,13 +44163,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QNonStop} is not supported by
-the stub.
@end table
This packet is not probed by default; the remote stub must request it,
@@ -44284,13 +44199,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. @var{nn} are hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QCatchSyscalls} is not supported by
-the stub.
@end table
Use of this packet is controlled by the @code{set remote catch-syscalls}
@@ -44316,13 +44224,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QPassSignals} is not supported by
-the stub.
@end table
Use of this packet is controlled by the @code{set remote pass-signals}
@@ -44358,13 +44259,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QProgramSignals} is not supported
-by the stub.
@end table
Use of this packet is controlled by the @code{set remote program-signals}
@@ -44398,13 +44292,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QThreadEvents} is not supported by
-the stub.
@end table
Use of this packet is controlled by the @code{set remote thread-events}
@@ -44487,13 +44374,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QThreadOptions} is not supported by
-the stub.
@end table
Use of this packet is controlled by the @code{set remote thread-options}
@@ -44515,11 +44395,6 @@ Reply:
A command response with no output.
@item @var{OUTPUT}
A command response with the hex encoded output string @var{OUTPUT}.
-@item E @var{NN}
-Indicate a badly formed request. The error number @var{NN} is given as
-hex digits.
-@item @w{}
-An empty reply indicates that @samp{qRcmd} is not recognized.
@end table
(Note that the @code{qRcmd} packet's name is separated from the
@@ -44544,10 +44419,6 @@ Reply:
The pattern was not found.
@item 1,address
The pattern was found at @var{address}.
-@item E @var{NN}
-A badly formed request or an error was encountered while searching memory.
-@item @w{}
-An empty reply indicates that @samp{qSearch:memory} is not recognized.
@end table
@item QStartNoAckMode
@@ -44563,8 +44434,6 @@ The stub has switched to no-acknowledgment mode.
@value{GDBN} acknowledges this response,
but neither the stub nor @value{GDBN} shall send or expect further
@samp{+}/@samp{-} acknowledgments in the current connection.
-@item @w{}
-An empty reply indicates that the stub does not support no-acknowledgment mode.
@end table
@item qSupported @r{[}:@var{gdbfeature} @r{[};@var{gdbfeature}@r{]}@dots{} @r{]}
@@ -44593,9 +44462,6 @@ Reply:
The stub supports or does not support each returned @var{stubfeature},
depending on the form of each @var{stubfeature} (see below for the
possible forms).
-@item @w{}
-An empty reply indicates that @samp{qSupported} is not recognized,
-or that no features needed to be reported to @value{GDBN}.
@end table
The allowed forms for each feature (either a @var{gdbfeature} in the
@@ -45316,17 +45182,6 @@ have fewer bytes than the @var{length} in the request.
@item l
The @var{offset} in the request is at the end of the data.
There is no more data to be read.
-
-@item E00
-The request was malformed, or @var{annex} was invalid.
-
-@item E @var{nn}
-The offset was invalid, or there was an error encountered reading the data.
-The @var{nn} part is a hex-encoded @code{errno} value.
-
-@item @w{}
-An empty reply indicates the @var{object} string was not recognized by
-the stub, or that the object does not support reading.
@end table
Here are the specific requests of this form defined so far. All the
@@ -45545,17 +45400,6 @@ Reply:
@item @var{nn}
@var{nn} (hex encoded) is the number of bytes written.
This may be fewer bytes than supplied in the request.
-
-@item E00
-The request was malformed, or @var{annex} was invalid.
-
-@item E @var{nn}
-The offset was invalid, or there was an error encountered writing the data.
-The @var{nn} part is a hex-encoded @code{errno} value.
-
-@item @w{}
-An empty reply indicates the @var{object} string was not
-recognized by the stub, or that the object does not support writing.
@end table
Here are the specific requests of this form defined so far. All the
@@ -45600,8 +45444,6 @@ Reply:
The remote server attached to an existing process.
@item 0
The remote server created a new process.
-@item E @var{NN}
-A badly formed request or an error was encountered.
@end table
@item Qbtrace:bts
@@ -45805,8 +45647,6 @@ Replies:
The packet was understood and carried out.
@item qRelocInsn
@xref{Tracepoint Packets,,Relocate instruction reply packet}.
-@item @w{}
-The packet was not recognized.
@end table
@item QTDP:-@var{n}:@var{addr}:@r{[}S@r{]}@var{action}@dots{}@r{[}-@r{]}
@@ -45871,8 +45711,6 @@ Replies:
The packet was understood and carried out.
@item qRelocInsn
@xref{Tracepoint Packets,,Relocate instruction reply packet}.
-@item @w{}
-The packet was not recognized.
@end table
@item QTDPsrc:@var{n}:@var{addr}:@var{type}:@var{start}:@var{slen}:@var{bytes}
@@ -45989,8 +45827,6 @@ of 1 means that a fast tracepoint may be placed on any instruction
regardless of size.
@item E
An error has occurred.
-@item @w{}
-An empty reply indicates that the request is not supported by the stub.
@end table
@item QTStart
@@ -46199,11 +46035,6 @@ A single marker
a comma-separated list of markers
@item l
(lower case letter @samp{L}) denotes end of list.
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-@item @w{}
-An empty reply indicates that the request is not supported by the
-stub.
@end table
The @var{address} is encoded in hex;
@@ -46292,9 +46123,6 @@ Replies:
@item qRelocInsn:@var{adjusted_size}
Informs the stub the relocation is complete. The @var{adjusted_size} is
the length in bytes of resulting relocated instruction sequence.
-@item E @var{NN}
-A badly formed request was detected, or an error was encountered while
-relocating the instruction.
@end table
@node Host I/O Packets
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 02/12] Centralize documentation of error and empty RSP responses
2024-04-19 15:13 ` [PATCH 02/12] Centralize documentation of error and empty RSP responses Pedro Alves
@ 2024-04-19 15:36 ` Eli Zaretskii
2024-04-19 15:42 ` Eli Zaretskii
2024-04-22 19:00 ` Pedro Alves
0 siblings, 2 replies; 55+ messages in thread
From: Eli Zaretskii @ 2024-04-19 15:36 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, jimb, mike_wrighton, nathan, abidh
> From: Pedro Alves <pedro@palves.net>
> Cc: Jim Blandy <jimb@codesourcery.com>,
> Mike Wrighton <mike_wrighton@mentor.com>,
> Nathan Sidwell <nathan@codesourcery.com>,
> Hafiz Abid Qadeer <abidh@codesourcery.com>
> Date: Fri, 19 Apr 2024 16:13:32 +0100
>
> gdb/doc/gdb.texinfo | 244 +++++++-------------------------------------
> 1 file changed, 36 insertions(+), 208 deletions(-)
Thanks.
> +See @ref{Standard Replies} for standard error responses, and how to
Please use @xref here instead of "See @ref", as that's what @xref is
for. Also, some old versions of makeinfo insist on a period or comma
after the closing brace (with the single exception of @pxref), so
please add it.
> +@node Standard Replies
> +@section Standard Replies
This section should have an index entry. I suggest
@cindex standard responses for remote packets
@cindex remote packets, standard replies
> +@cindex empty response, for unsupported packets
> +@cindex unsupported packets, empty response for
> +For any @var{command} not supported by the stub, an empty response
^^^^^^^
"command", not "packet"?
> +(@samp{$#00}) should be returned.
I don't understand what you mean by $#00, and why that is considered
and "empty response". I'm probably missing something.
Also, "should be returned" doesn't really fit the purpose of this
section, which AFAIU is to describe the standard responses. So
something like below would be IMO more appropriate:
An empty response means this packet is not supported by the stub.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 02/12] Centralize documentation of error and empty RSP responses
2024-04-19 15:36 ` Eli Zaretskii
@ 2024-04-19 15:42 ` Eli Zaretskii
2024-04-22 19:00 ` Pedro Alves
1 sibling, 0 replies; 55+ messages in thread
From: Eli Zaretskii @ 2024-04-19 15:42 UTC (permalink / raw)
To: pedro; +Cc: gdb-patches
> Date: Fri, 19 Apr 2024 18:36:04 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb-patches@sourceware.org, jimb@codesourcery.com,
> mike_wrighton@mentor.com, nathan@codesourcery.com, abidh@codesourcery.com
>
> > From: Pedro Alves <pedro@palves.net>
> > Cc: Jim Blandy <jimb@codesourcery.com>,
> > Mike Wrighton <mike_wrighton@mentor.com>,
> > Nathan Sidwell <nathan@codesourcery.com>,
> > Hafiz Abid Qadeer <abidh@codesourcery.com>
> > Date: Fri, 19 Apr 2024 16:13:32 +0100
> >
> > gdb/doc/gdb.texinfo | 244 +++++++-------------------------------------
> > 1 file changed, 36 insertions(+), 208 deletions(-)
>
> Thanks.
This one also bounced.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/12] Centralize documentation of error and empty RSP responses
2024-04-19 15:36 ` Eli Zaretskii
2024-04-19 15:42 ` Eli Zaretskii
@ 2024-04-22 19:00 ` Pedro Alves
2024-04-22 19:42 ` Eli Zaretskii
1 sibling, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-22 19:00 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2024-04-19 16:36, Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Cc: Jim Blandy <jimb@codesourcery.com>,
>> Mike Wrighton <mike_wrighton@mentor.com>,
>> Nathan Sidwell <nathan@codesourcery.com>,
>> Hafiz Abid Qadeer <abidh@codesourcery.com>
>> Date: Fri, 19 Apr 2024 16:13:32 +0100
>>
>> gdb/doc/gdb.texinfo | 244 +++++++-------------------------------------
>> 1 file changed, 36 insertions(+), 208 deletions(-)
>
> Thanks.
>
>> +See @ref{Standard Replies} for standard error responses, and how to
>
> Please use @xref here instead of "See @ref", as that's what @xref is
> for. Also, some old versions of makeinfo insist on a period or comma
> after the closing brace (with the single exception of @pxref), so
> please add it.
Fixed.
>
>> +@node Standard Replies
>> +@section Standard Replies
>
> This section should have an index entry. I suggest
>
> @cindex standard responses for remote packets
> @cindex remote packets, standard replies
>
Thanks, added.
>> +@cindex empty response, for unsupported packets
>> +@cindex unsupported packets, empty response for
>> +For any @var{command} not supported by the stub, an empty response
> ^^^^^^^
> "command", not "packet"?
Yes. The protocol is described like this:
The host (@value{GDBN}) sends @var{command}s, and the target (the debugging stub incorporated in your program) sends a @var{response}.
... and ...
All @value{GDBN} commands and responses (other than acknowledgments
and notifications, see @ref{Notification Packets}) are sent as a
@var{packet}.
So both command and response are packets. But not all packets are commands.
>
>> +(@samp{$#00}) should be returned.
>
> I don't understand what you mean by $#00, and why that is considered
> and "empty response". I'm probably missing something.
"$#00" is the raw sequence of characters that is sent over the wire for a
packet with an empty payload. It comes from this:
A @var{packet} is introduced with the character
@samp{$}, the actual @var{packet-data}, and the terminating character
@samp{#} followed by a two-digit @var{checksum}:
@smallexample
@code{$}@var{packet-data}@code{#}@var{checksum}
@end smallexample
@noindent
When packet-data, which is the payload, is empty, you end up with "$#00".
Note I just moved that sentence, it was already described that way. But we
can certainly improve it. I propose adding "raw character sequence" (see below).
>
> Also, "should be returned" doesn't really fit the purpose of this
> section, which AFAIU is to describe the standard responses. So
> something like below would be IMO more appropriate:
>
> An empty response means this packet is not supported by the stub.
>
Makes sense. Like so?
+@cindex empty response, for unsupported packets
+@cindex unsupported packets, empty response for
+An empty response (raw character sequence @samp{$#00}) means the
+@var{command} is not supported by the stub. This way it is possible
+to extend the protocol. A newer @value{GDBN} can tell if a command is
+supported based on that response (but see also @ref{qSupported}).
Here is the full updated patch.
From c3e60049bd08c97f8b665af4d5861154aac3975a Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Thu, 18 Apr 2024 20:22:36 +0100
Subject: [PATCH] Centralize documentation of error and empty RSP responses
Currently, for each packet, we document the "E NN" response (error),
and the empty response (unsupported). This patch centralizes that in
a new "Standard Replies" section.
In the "Packets", "General Query Packets", "Tracepoint Packets"
sections, Remove explicit mention of empty and error replies, except
when they provide detail not covered in Standard Replies.
Note this hunk:
-@item E @var{NN}
-@var{NN} is errno
and this one:
-@item E00
-The request was malformed, or @var{annex} was invalid.
-
-@item E @var{nn}
-The offset was invalid, or there was an error encountered reading the data.
-The @var{nn} part is a hex-encoded @code{errno} value.
were really documenting things that don't really work that way.
The first is the documentation of the "m" packet. GDB does _not_
interpret the NN as an errno. It can't, in fact, because the
remote/target errno numbers have nothing to do with GDB/host errno
numbers in a cross debugging scenario.
The second hunk above is from the documentation of qXfer. Again, GDB
does not give any interpretation to the NN error code at all. Nor
does GDBserver. And again, an errno number can't be interpreted in a
cross debugging scenario.
Change-Id: I973695c80809cdb5a5e8d5be8b78ba4d1ecdb513
Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
---
gdb/doc/gdb.texinfo | 246 +++++++-------------------------------------
1 file changed, 38 insertions(+), 208 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6fa32d1bd01..9781ce76226 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -40242,8 +40242,6 @@ The @var{gdb_jump_pad_head} is updated head of jumppad. Both of
@var{target_address} and @var{gdb_jump_pad_head} are 8-byte long.
The @var{fjump} contains a sequence of instructions jump to jumppad entry.
The @var{fjump_size}, 4-byte long, is the size of @var{fjump}.
-@item E @var{NN}
-for an error
@end table
@@ -40262,8 +40260,6 @@ Asks in-process agent to probe the marker at @var{address}.
Replies:
@table @samp
-@item E @var{NN}
-for an error
@end table
@item unprobe_marker_at:@var{address}
Asks in-process agent to unprobe the marker at @var{address}.
@@ -42368,6 +42364,7 @@ Show the current setting of the target wait timeout.
@menu
* Overview::
+* Standard Replies::
* Packets::
* Stop Reply Packets::
* General Query Packets::
@@ -42508,14 +42505,8 @@ seven repeats (@samp{$}) can be expanded using a repeat count of only
five (@samp{"}). For example, @samp{00000000} can be encoded as
@samp{0*"00}.
-The error response returned for some packets includes a two character
-error number. That number is not well defined.
-
-@cindex empty response, for unsupported packets
-For any @var{command} not supported by the stub, an empty response
-(@samp{$#00}) should be returned. That way it is possible to extend the
-protocol. A newer @value{GDBN} can tell if a packet is supported based
-on that response.
+@xref{Standard Replies} for standard error responses, and how to
+respond indicating a command is not supported.
In describing packets (commands and responses), each description has a
template showing the overall syntax, followed by an explanation of the
@@ -42543,6 +42534,33 @@ the @samp{s} (step) command. Stubs that support multi-threading
targets should support the @samp{vCont} command. All other commands
are optional.
+@node Standard Replies
+@section Standard Replies
+@cindex standard responses for remote packets
+@cindex remote packets, standard replies
+
+The remote protocol specifies a few standard replies. All commands
+support these, except as noted in the individual command descriptions.
+
+@table @asis
+
+@item empty response
+
+@cindex empty response, for unsupported packets
+@cindex unsupported packets, empty response for
+An empty response (raw character sequence @samp{$#00}) means the
+@var{command} is not supported by the stub. This way it is possible
+to extend the protocol. A newer @value{GDBN} can tell if a command is
+supported based on that response (but see also @ref{qSupported}).
+
+@item @samp{E @var{xx}}
+An error has occurred; @var{xx} is a two-digit hexadecimal error
+number. In almost all cases, the protocol does not specify the
+meaning of the error numbers; @value{GDBN} usually ignores the
+numbers, or displays them to the user without further interpretation.
+
+@end table
+
@node Packets
@section Packets
@@ -42630,8 +42648,6 @@ Reply:
@table @samp
@item OK
The arguments were set.
-@item E @var{NN}
-An error occurred.
@end table
@item b @var{baud}
@@ -42720,8 +42736,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item F @var{RC},@var{EE},@var{CF};@var{XX}
@@ -42768,8 +42782,6 @@ are available, and both have zero value:
<- @code{xxxxxxxx00000000xxxxxxxx00000000}
@end smallexample
-@item E @var{NN}
-for an error.
@end table
@item G @var{XX@dots{}}
@@ -42781,8 +42793,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item H @var{op} @var{thread-id}
@@ -42799,8 +42809,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@c FIXME: JTC:
@@ -42875,8 +42883,6 @@ Reply:
Memory contents; each byte is transmitted as a two-digit hexadecimal number.
The reply may contain fewer addressable memory units than requested if the
server was able to read only part of the region of memory.
-@item E @var{NN}
-@var{NN} is errno
@end table
@item M @var{addr},@var{length}:@var{XX@dots{}}
@@ -42888,10 +42894,8 @@ byte is transmitted as a two-digit hexadecimal number.
Reply:
@table @samp
@item OK
-for success
-@item E @var{NN}
-for an error (this includes the case where only part of the data was
-written).
+All the data was written successfully. (If only part of the data was
+written, this command returns an error.)
@end table
@item p @var{n}
@@ -42904,10 +42908,6 @@ Reply:
@table @samp
@item @var{XX@dots{}}
the register's value
-@item E @var{NN}
-for an error
-@item @w{}
-Indicating an unrecognized @var{query}.
@end table
@item P @var{n@dots{}}=@var{r@dots{}}
@@ -42921,8 +42921,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item q @var{name} @var{params}@dots{}
@@ -42982,8 +42980,6 @@ Reply:
@table @samp
@item OK
thread is still alive
-@item E @var{NN}
-thread is dead
@end table
@item v
@@ -43011,8 +43007,6 @@ This packet is only available in extended mode (@pxref{extended mode}).
Reply:
@table @samp
-@item E @var{nn}
-for an error
@item @r{Any stop packet}
for success in all-stop mode (@pxref{Stop Reply Packets})
@item OK
@@ -43101,8 +43095,6 @@ Reply:
@item vCont@r{[};@var{action}@dots{}@r{]}
The @samp{vCont} packet is supported. Each @var{action} is a supported
command in the @samp{vCont} packet.
-@item @w{}
-The @samp{vCont} packet is not supported.
@end table
@anchor{vCtrlC packet}
@@ -43117,8 +43109,6 @@ variant.
Reply:
@table @samp
-@item E @var{nn}
-for an error
@item OK
for success
@end table
@@ -43143,8 +43133,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item vFlashWrite:@var{addr}:@var{XX@dots{}}
@@ -43167,8 +43155,6 @@ Reply:
for success
@item E.memtype
for vFlashWrite addressing non-flash memory
-@item E @var{NN}
-for an error
@end table
@item vFlashDone
@@ -43190,8 +43176,6 @@ supported; see @ref{multiprocess extensions}.
Reply:
@table @samp
-@item E @var{nn}
-for an error
@item OK
for success
@end table
@@ -43223,8 +43207,6 @@ This packet is only available in extended mode (@pxref{extended mode}).
Reply:
@table @samp
-@item E @var{nn}
-for an error
@item @r{Any stop packet}
for success (@pxref{Stop Reply Packets})
@end table
@@ -43245,8 +43227,6 @@ Reply:
@table @samp
@item OK
for success
-@item E @var{NN}
-for an error
@end table
@item z @var{type},@var{addr},@var{kind}
@@ -43327,10 +43307,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@item z1,@var{addr},@var{kind}
@@ -43352,10 +43328,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@item z2,@var{addr},@var{kind}
@@ -43369,10 +43341,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@item z3,@var{addr},@var{kind}
@@ -43386,10 +43354,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@item z4,@var{addr},@var{kind}
@@ -43403,10 +43367,6 @@ Reply:
@table @samp
@item OK
success
-@item @w{}
-not supported
-@item E @var{NN}
-for an error
@end table
@end table
@@ -43776,8 +43736,6 @@ detect trailing zeros.
Reply:
@table @samp
-@item E @var{NN}
-An error (such as memory fault)
@item C @var{crc32}
The specified memory region's checksum is @var{crc32}.
@end table
@@ -43800,13 +43758,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QDisableRandomization} is not supported
-by the stub.
@end table
This packet is not probed by default; the remote stub must request it,
@@ -43835,9 +43786,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
@end table
This packet is not probed by default; the remote stub must request it,
@@ -44027,12 +43975,6 @@ Reply:
@item @var{XX}@dots{}
Hex encoded (big endian) bytes representing the address of the thread
local storage requested.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{qGetTLSAddr} is not supported by the stub.
@end table
@item qGetTIBAddr:@var{thread-id}
@@ -44047,13 +43989,6 @@ Reply:
@item @var{XX}@dots{}
Hex encoded (big endian) bytes representing the linear address of the
thread information block.
-
-@item E @var{nn}
-An error occurred. This means that either the thread was not found, or the
-address could not be retrieved.
-
-@item @w{}
-An empty reply indicates that @samp{qGetTIBAddr} is not supported by the stub.
@end table
@item qL @var{startflag} @var{threadcount} @var{nextthread}
@@ -44093,20 +44028,15 @@ is architecture-specific.
@var{type} is the type of tag the request wants to fetch. The type is a signed
integer.
+@value{GDBN} will only send this packet if the stub has advertised
+support for memory tagging via @samp{qSupported}.
+
Reply:
@table @samp
@item @var{mxx}@dots{}
Hex encoded sequence of uninterpreted bytes, @var{xx}@dots{}, representing the
tags found in the requested memory range.
-@item E @var{nn}
-An error occurred. This means that fetching of memory tags failed for some
-reason.
-
-@item @w{}
-An empty reply indicates that @samp{qMemTags} is not supported by the stub,
-although this should not happen given @value{GDBN} will only send this packet
-if the stub has advertised support for memory tagging via @samp{qSupported}.
@end table
@cindex check if a given address is in a memory tagged region
@@ -44171,20 +44101,14 @@ integer.
interpreted by the target. Each pair of hex digits is interpreted as a
single byte.
+@value{GDBN} will only send this packet if the stub has advertised
+support for memory tagging via @samp{qSupported}.
+
Reply:
@table @samp
@item OK
The request was successful and the memory tag granules were modified
accordingly.
-
-@item E @var{nn}
-An error occurred. This means that modifying the memory tag granules failed
-for some reason.
-
-@item @w{}
-An empty reply indicates that @samp{QMemTags} is not supported by the stub,
-although this should not happen given @value{GDBN} will only send this packet
-if the stub has advertised support for memory tagging via @samp{qSupported}.
@end table
@item qOffsets
@@ -44241,13 +44165,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QNonStop} is not supported by
-the stub.
@end table
This packet is not probed by default; the remote stub must request it,
@@ -44284,13 +44201,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. @var{nn} are hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QCatchSyscalls} is not supported by
-the stub.
@end table
Use of this packet is controlled by the @code{set remote catch-syscalls}
@@ -44316,13 +44226,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QPassSignals} is not supported by
-the stub.
@end table
Use of this packet is controlled by the @code{set remote pass-signals}
@@ -44358,13 +44261,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QProgramSignals} is not supported
-by the stub.
@end table
Use of this packet is controlled by the @code{set remote program-signals}
@@ -44398,13 +44294,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QThreadEvents} is not supported by
-the stub.
@end table
Use of this packet is controlled by the @code{set remote thread-events}
@@ -44487,13 +44376,6 @@ Reply:
@table @samp
@item OK
The request succeeded.
-
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-
-@item @w{}
-An empty reply indicates that @samp{QThreadOptions} is not supported by
-the stub.
@end table
Use of this packet is controlled by the @code{set remote thread-options}
@@ -44515,11 +44397,6 @@ Reply:
A command response with no output.
@item @var{OUTPUT}
A command response with the hex encoded output string @var{OUTPUT}.
-@item E @var{NN}
-Indicate a badly formed request. The error number @var{NN} is given as
-hex digits.
-@item @w{}
-An empty reply indicates that @samp{qRcmd} is not recognized.
@end table
(Note that the @code{qRcmd} packet's name is separated from the
@@ -44544,10 +44421,6 @@ Reply:
The pattern was not found.
@item 1,address
The pattern was found at @var{address}.
-@item E @var{NN}
-A badly formed request or an error was encountered while searching memory.
-@item @w{}
-An empty reply indicates that @samp{qSearch:memory} is not recognized.
@end table
@item QStartNoAckMode
@@ -44563,8 +44436,6 @@ The stub has switched to no-acknowledgment mode.
@value{GDBN} acknowledges this response,
but neither the stub nor @value{GDBN} shall send or expect further
@samp{+}/@samp{-} acknowledgments in the current connection.
-@item @w{}
-An empty reply indicates that the stub does not support no-acknowledgment mode.
@end table
@item qSupported @r{[}:@var{gdbfeature} @r{[};@var{gdbfeature}@r{]}@dots{} @r{]}
@@ -44593,9 +44464,6 @@ Reply:
The stub supports or does not support each returned @var{stubfeature},
depending on the form of each @var{stubfeature} (see below for the
possible forms).
-@item @w{}
-An empty reply indicates that @samp{qSupported} is not recognized,
-or that no features needed to be reported to @value{GDBN}.
@end table
The allowed forms for each feature (either a @var{gdbfeature} in the
@@ -45316,17 +45184,6 @@ have fewer bytes than the @var{length} in the request.
@item l
The @var{offset} in the request is at the end of the data.
There is no more data to be read.
-
-@item E00
-The request was malformed, or @var{annex} was invalid.
-
-@item E @var{nn}
-The offset was invalid, or there was an error encountered reading the data.
-The @var{nn} part is a hex-encoded @code{errno} value.
-
-@item @w{}
-An empty reply indicates the @var{object} string was not recognized by
-the stub, or that the object does not support reading.
@end table
Here are the specific requests of this form defined so far. All the
@@ -45545,17 +45402,6 @@ Reply:
@item @var{nn}
@var{nn} (hex encoded) is the number of bytes written.
This may be fewer bytes than supplied in the request.
-
-@item E00
-The request was malformed, or @var{annex} was invalid.
-
-@item E @var{nn}
-The offset was invalid, or there was an error encountered writing the data.
-The @var{nn} part is a hex-encoded @code{errno} value.
-
-@item @w{}
-An empty reply indicates the @var{object} string was not
-recognized by the stub, or that the object does not support writing.
@end table
Here are the specific requests of this form defined so far. All the
@@ -45600,8 +45446,6 @@ Reply:
The remote server attached to an existing process.
@item 0
The remote server created a new process.
-@item E @var{NN}
-A badly formed request or an error was encountered.
@end table
@item Qbtrace:bts
@@ -45805,8 +45649,6 @@ Replies:
The packet was understood and carried out.
@item qRelocInsn
@xref{Tracepoint Packets,,Relocate instruction reply packet}.
-@item @w{}
-The packet was not recognized.
@end table
@item QTDP:-@var{n}:@var{addr}:@r{[}S@r{]}@var{action}@dots{}@r{[}-@r{]}
@@ -45871,8 +45713,6 @@ Replies:
The packet was understood and carried out.
@item qRelocInsn
@xref{Tracepoint Packets,,Relocate instruction reply packet}.
-@item @w{}
-The packet was not recognized.
@end table
@item QTDPsrc:@var{n}:@var{addr}:@var{type}:@var{start}:@var{slen}:@var{bytes}
@@ -45989,8 +45829,6 @@ of 1 means that a fast tracepoint may be placed on any instruction
regardless of size.
@item E
An error has occurred.
-@item @w{}
-An empty reply indicates that the request is not supported by the stub.
@end table
@item QTStart
@@ -46199,11 +46037,6 @@ A single marker
a comma-separated list of markers
@item l
(lower case letter @samp{L}) denotes end of list.
-@item E @var{nn}
-An error occurred. The error number @var{nn} is given as hex digits.
-@item @w{}
-An empty reply indicates that the request is not supported by the
-stub.
@end table
The @var{address} is encoded in hex;
@@ -46292,9 +46125,6 @@ Replies:
@item qRelocInsn:@var{adjusted_size}
Informs the stub the relocation is complete. The @var{adjusted_size} is
the length in bytes of resulting relocated instruction sequence.
-@item E @var{NN}
-A badly formed request was detected, or an error was encountered while
-relocating the instruction.
@end table
@node Host I/O Packets
base-commit: 0e6747d2a638693ad2f20e7929c8364913c87279
prerequisite-patch-id: 69a7241ff790954e07942cd212971bcfaa8f3a08
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 02/12] Centralize documentation of error and empty RSP responses
2024-04-22 19:00 ` Pedro Alves
@ 2024-04-22 19:42 ` Eli Zaretskii
0 siblings, 0 replies; 55+ messages in thread
From: Eli Zaretskii @ 2024-04-22 19:42 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Mon, 22 Apr 2024 20:00:23 +0100
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> >> +(@samp{$#00}) should be returned.
> >
> > I don't understand what you mean by $#00, and why that is considered
> > and "empty response". I'm probably missing something.
>
> "$#00" is the raw sequence of characters that is sent over the wire for a
> packet with an empty payload. It comes from this:
>
> A @var{packet} is introduced with the character
> @samp{$}, the actual @var{packet-data}, and the terminating character
> @samp{#} followed by a two-digit @var{checksum}:
>
> @smallexample
> @code{$}@var{packet-data}@code{#}@var{checksum}
> @end smallexample
> @noindent
>
> When packet-data, which is the payload, is empty, you end up with "$#00".
>
> Note I just moved that sentence, it was already described that way. But we
> can certainly improve it. I propose adding "raw character sequence" (see below).
>
> >
> > Also, "should be returned" doesn't really fit the purpose of this
> > section, which AFAIU is to describe the standard responses. So
> > something like below would be IMO more appropriate:
> >
> > An empty response means this packet is not supported by the stub.
> >
>
> Makes sense. Like so?
>
> +@cindex empty response, for unsupported packets
> +@cindex unsupported packets, empty response for
> +An empty response (raw character sequence @samp{$#00}) means the
> +@var{command} is not supported by the stub. This way it is possible
> +to extend the protocol. A newer @value{GDBN} can tell if a command is
> +supported based on that response (but see also @ref{qSupported}).
Yes, this is OK.
Thanks.
Approved-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 03/12] Document "E.MESSAGE" RSP errors
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
2024-04-19 15:13 ` [PATCH 01/12] Document conventions for describing packet syntax Pedro Alves
2024-04-19 15:13 ` [PATCH 02/12] Centralize documentation of error and empty RSP responses Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 15:37 ` Eli Zaretskii
2024-04-22 8:50 ` Andrew Burgess
2024-04-19 15:13 ` [PATCH 04/12] Windows: Fix run/attach hang after bad run/attach Pedro Alves
` (9 subsequent siblings)
12 siblings, 2 replies; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches; +Cc: Jim Blandy, Mike Wrighton, Nathan Sidwell, Hafiz Abid Qadeer
For many years, GDB has accepted a "E.MESSAGE" error reponse, in
addition to "E NN". For many packets, GDB strips the "E." before
giving the error message to the user. For others, GDB does not strip
the "E.", but still understands that it is an error, as it starts with
"E", and either prints the whole string, or ignores it and just
mentions an error occured (same as for "E NN").
This has been the case for as long as I remember. Now that I check, I
see that it's been there since 2006 (commit a76d924dffcb, also here:
https://sourceware.org/pipermail/gdb-patches/2006-September/047286.html).
All along, I actually thought it was documented. Turns out it wasn't.
This commit documents it, in the new "Standard Replies" section, near
where we document "E NN".
The original version of this 3-patch documentation series was a single
CodeSourcery patch that documented the textual error as
"E.NAME.MESSAGE", with MESSAGE being 8-bit binary encoded. But I
think the ship has sailed for that. GDBserver has been sending error
messages with more than one "." for a long while, and with no binary
encoding. Still, I've preserved the "Co-Authored-By" list of the
original larger patch.
Change-Id: Ie4fee3d00d82ede39e439bf162e8cb7485532fd8
Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
---
gdb/doc/gdb.texinfo | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 57260a5b2fa..d6184d52841 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42557,6 +42557,10 @@ number. In almost all cases, the protocol does not specify the
meaning of the error numbers; @value{GDBN} usually ignores the
numbers, or displays them to the user without further interpretation.
+@item @samp{E.@var{message}}
+An error has occurred; @var{message} is the textual error message,
+encoded in @sc{ascii}.
+
@end table
@node Packets
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors
2024-04-19 15:13 ` [PATCH 03/12] Document "E.MESSAGE" RSP errors Pedro Alves
@ 2024-04-19 15:37 ` Eli Zaretskii
2024-04-22 8:50 ` Andrew Burgess
1 sibling, 0 replies; 55+ messages in thread
From: Eli Zaretskii @ 2024-04-19 15:37 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, jimb, mike_wrighton, nathan, abidh
> From: Pedro Alves <pedro@palves.net>
> Cc: Jim Blandy <jimb@codesourcery.com>,
> Mike Wrighton <mike_wrighton@mentor.com>,
> Nathan Sidwell <nathan@codesourcery.com>,
> Hafiz Abid Qadeer <abidh@codesourcery.com>
> Date: Fri, 19 Apr 2024 16:13:33 +0100
>
> For many years, GDB has accepted a "E.MESSAGE" error reponse, in
> addition to "E NN". For many packets, GDB strips the "E." before
> giving the error message to the user. For others, GDB does not strip
> the "E.", but still understands that it is an error, as it starts with
> "E", and either prints the whole string, or ignores it and just
> mentions an error occured (same as for "E NN").
>
> This has been the case for as long as I remember. Now that I check, I
> see that it's been there since 2006 (commit a76d924dffcb, also here:
> https://sourceware.org/pipermail/gdb-patches/2006-September/047286.html).
> All along, I actually thought it was documented. Turns out it wasn't.
>
> This commit documents it, in the new "Standard Replies" section, near
> where we document "E NN".
>
> The original version of this 3-patch documentation series was a single
> CodeSourcery patch that documented the textual error as
> "E.NAME.MESSAGE", with MESSAGE being 8-bit binary encoded. But I
> think the ship has sailed for that. GDBserver has been sending error
> messages with more than one "." for a long while, and with no binary
> encoding. Still, I've preserved the "Co-Authored-By" list of the
> original larger patch.
>
> Change-Id: Ie4fee3d00d82ede39e439bf162e8cb7485532fd8
> Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
> Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
> Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
> Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
> ---
> gdb/doc/gdb.texinfo | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 57260a5b2fa..d6184d52841 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -42557,6 +42557,10 @@ number. In almost all cases, the protocol does not specify the
> meaning of the error numbers; @value{GDBN} usually ignores the
> numbers, or displays them to the user without further interpretation.
>
> +@item @samp{E.@var{message}}
> +An error has occurred; @var{message} is the textual error message,
> +encoded in @sc{ascii}.
> +
> @end table
>
> @node Packets
This is OK, thanks.
Approved-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors
2024-04-19 15:13 ` [PATCH 03/12] Document "E.MESSAGE" RSP errors Pedro Alves
2024-04-19 15:37 ` Eli Zaretskii
@ 2024-04-22 8:50 ` Andrew Burgess
2024-04-22 19:04 ` Pedro Alves
1 sibling, 1 reply; 55+ messages in thread
From: Andrew Burgess @ 2024-04-22 8:50 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
Cc: Jim Blandy, Mike Wrighton, Nathan Sidwell, Hafiz Abid Qadeer
Pedro Alves <pedro@palves.net> writes:
> For many years, GDB has accepted a "E.MESSAGE" error reponse, in
> addition to "E NN". For many packets, GDB strips the "E." before
> giving the error message to the user. For others, GDB does not strip
> the "E.", but still understands that it is an error, as it starts with
> "E", and either prints the whole string, or ignores it and just
> mentions an error occured (same as for "E NN").
>
> This has been the case for as long as I remember. Now that I check, I
> see that it's been there since 2006 (commit a76d924dffcb, also here:
> https://sourceware.org/pipermail/gdb-patches/2006-September/047286.html).
> All along, I actually thought it was documented. Turns out it wasn't.
>
> This commit documents it, in the new "Standard Replies" section, near
> where we document "E NN".
>
> The original version of this 3-patch documentation series was a single
> CodeSourcery patch that documented the textual error as
> "E.NAME.MESSAGE", with MESSAGE being 8-bit binary encoded. But I
> think the ship has sailed for that. GDBserver has been sending error
> messages with more than one "." for a long while, and with no binary
> encoding. Still, I've preserved the "Co-Authored-By" list of the
> original larger patch.
>
> Change-Id: Ie4fee3d00d82ede39e439bf162e8cb7485532fd8
> Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
> Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
> Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
> Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
> ---
> gdb/doc/gdb.texinfo | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 57260a5b2fa..d6184d52841 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -42557,6 +42557,10 @@ number. In almost all cases, the protocol does not specify the
> meaning of the error numbers; @value{GDBN} usually ignores the
> numbers, or displays them to the user without further interpretation.
>
> +@item @samp{E.@var{message}}
> +An error has occurred; @var{message} is the textual error message,
> +encoded in @sc{ascii}.
I think we should document here that the 'qRcmd' and 'm' packets don't
accept this reply format.
Alexandra Hájková has a patch series which has not been posted yet which
will extend these packets in a backward compatible way so that E.message
can be used, but until that arrives we should probably document this
limitation.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors
2024-04-22 8:50 ` Andrew Burgess
@ 2024-04-22 19:04 ` Pedro Alves
2024-04-26 19:02 ` Pedro Alves
0 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-22 19:04 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 2024-04-22 09:50, Andrew Burgess wrote:
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 57260a5b2fa..d6184d52841 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -42557,6 +42557,10 @@ number. In almost all cases, the protocol does not specify the
>> meaning of the error numbers; @value{GDBN} usually ignores the
>> numbers, or displays them to the user without further interpretation.
>>
>> +@item @samp{E.@var{message}}
>> +An error has occurred; @var{message} is the textual error message,
>> +encoded in @sc{ascii}.
>
> I think we should document here that the 'qRcmd' and 'm' packets don't
> accept this reply format.
>
> Alexandra Hájková has a patch series which has not been posted yet which
> will extend these packets in a backward compatible way so that E.message
> can be used, but until that arrives we should probably document this
> limitation.
Done, see updated patch below. (See the bottom of the new commit log.)
Not sure when we'd want to make GDB print the error for the 'm' packet,
though. Currently it just returns TARGET_XFER_E_IO up to the caller.
I noticed a few packets were documenting an "E.errtext" response,
which I now removed in this version. Then "E.errtext" seemed more
descriptive than "E.message", so I switched to that too.
From 36ef5ca67f515039bfa68d2fb2515f181629a24f Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Thu, 18 Apr 2024 20:22:36 +0100
Subject: [PATCH] Document "E.MESSAGE" RSP errors
For many years, GDB has accepted a "E.MESSAGE" error response, in
addition to "E NN". For many packets, GDB strips the "E." before
giving the error message to the user. For others, GDB does not strip
the "E.", but still understands that it is an error, as it starts with
"E", and either prints the whole string, or ignores it and just
mentions an error occured (same as for "E NN").
This has been the case for as long as I remember. Now that I check, I
see that it's been there since 2006 (commit a76d924dffcb, also here:
https://sourceware.org/pipermail/gdb-patches/2006-September/047286.html).
All along, I actually thought it was documented. Turns out it wasn't.
This commit documents it, in the new "Standard Replies" section, near
where we document "E NN".
The original version of this 3-patch documentation series was a single
CodeSourcery patch that documented the textual error as
"E.NAME.MESSAGE", with MESSAGE being 8-bit binary encoded. But I
think the ship has sailed for that. GDBserver has been sending error
messages with more than one "." for a long while, and with no binary
encoding. Still, I've preserved the "Co-Authored-By" list of the
original larger patch.
The 'qRcmd' and 'm' commands are exceptions and do not accept this
reply format. The top of the "Standard Replies" section already says:
"All commands support these, except as noted in the individual
command descriptions."
So this adds a note to the description of 'qRcmd' and 'm', explicitly
stating that they do not support this error reply format.
Change-Id: Ie4fee3d00d82ede39e439bf162e8cb7485532fd8
Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
---
gdb/doc/gdb.texinfo | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9781ce76226..f55ada29e9c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42559,6 +42559,11 @@ number. In almost all cases, the protocol does not specify the
meaning of the error numbers; @value{GDBN} usually ignores the
numbers, or displays them to the user without further interpretation.
+@anchor{textual error reply}
+@item @samp{E.@var{errtext}}
+An error has occurred; @var{errtext} is the textual error message,
+encoded in @sc{ascii}.
+
@end table
@node Packets
@@ -42885,6 +42890,10 @@ The reply may contain fewer addressable memory units than requested if the
server was able to read only part of the region of memory.
@end table
+Unlike most packets, this packet does not support
+@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
+error reply}).
+
@item M @var{addr},@var{length}:@var{XX@dots{}}
@cindex @samp{M} packet
Write @var{length} addressable memory units starting at address @var{addr}
@@ -44399,6 +44408,10 @@ A command response with no output.
A command response with the hex encoded output string @var{OUTPUT}.
@end table
+Unlike most packets, this packet does not support
+@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
+error reply}).
+
(Note that the @code{qRcmd} packet's name is separated from the
command by a @samp{,}, not a @samp{:}, contrary to the naming
conventions above. Please don't use this packet as a model for new
@@ -45455,8 +45468,6 @@ Reply:
@table @samp
@item OK
Branch tracing has been enabled.
-@item E.errtext
-A badly formed request or an error was encountered.
@end table
@item Qbtrace:pt
@@ -45466,8 +45477,6 @@ Reply:
@table @samp
@item OK
Branch tracing has been enabled.
-@item E.errtext
-A badly formed request or an error was encountered.
@end table
@item Qbtrace:off
@@ -45477,8 +45486,6 @@ Reply:
@table @samp
@item OK
Branch tracing has been disabled.
-@item E.errtext
-A badly formed request or an error was encountered.
@end table
@item Qbtrace-conf:bts:size=@var{value}
@@ -45489,8 +45496,6 @@ Reply:
@table @samp
@item OK
The ring buffer size has been set.
-@item E.errtext
-A badly formed request or an error was encountered.
@end table
@item Qbtrace-conf:pt:size=@var{value}
@@ -45501,8 +45506,6 @@ Reply:
@table @samp
@item OK
The ring buffer size has been set.
-@item E.errtext
-A badly formed request or an error was encountered.
@end table
@end table
base-commit: 0e6747d2a638693ad2f20e7929c8364913c87279
prerequisite-patch-id: 69a7241ff790954e07942cd212971bcfaa8f3a08
prerequisite-patch-id: b398b6e111b68e06adccda7d9e016ebe39b5fd35
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors
2024-04-22 19:04 ` Pedro Alves
@ 2024-04-26 19:02 ` Pedro Alves
2024-04-26 19:18 ` Eli Zaretskii
2024-04-29 13:42 ` Andrew Burgess
0 siblings, 2 replies; 55+ messages in thread
From: Pedro Alves @ 2024-04-26 19:02 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches, Eli Zaretskii
[Adding Eli.]
AFAICT, this is the only patch left in the series that needs an OK.
Any comments on the updated documentation change below?
Pedro Alves
On 2024-04-22 20:04, Pedro Alves wrote:
> On 2024-04-22 09:50, Andrew Burgess wrote:
>
>>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>>> index 57260a5b2fa..d6184d52841 100644
>>> --- a/gdb/doc/gdb.texinfo
>>> +++ b/gdb/doc/gdb.texinfo
>>> @@ -42557,6 +42557,10 @@ number. In almost all cases, the protocol does not specify the
>>> meaning of the error numbers; @value{GDBN} usually ignores the
>>> numbers, or displays them to the user without further interpretation.
>>>
>>> +@item @samp{E.@var{message}}
>>> +An error has occurred; @var{message} is the textual error message,
>>> +encoded in @sc{ascii}.
>>
>> I think we should document here that the 'qRcmd' and 'm' packets don't
>> accept this reply format.
>>
>> Alexandra Hájková has a patch series which has not been posted yet which
>> will extend these packets in a backward compatible way so that E.message
>> can be used, but until that arrives we should probably document this
>> limitation.
>
> Done, see updated patch below. (See the bottom of the new commit log.)
>
> Not sure when we'd want to make GDB print the error for the 'm' packet,
> though. Currently it just returns TARGET_XFER_E_IO up to the caller.
>
> I noticed a few packets were documenting an "E.errtext" response,
> which I now removed in this version. Then "E.errtext" seemed more
> descriptive than "E.message", so I switched to that too.
>
>
> From 36ef5ca67f515039bfa68d2fb2515f181629a24f Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Thu, 18 Apr 2024 20:22:36 +0100
> Subject: [PATCH] Document "E.MESSAGE" RSP errors
>
> For many years, GDB has accepted a "E.MESSAGE" error response, in
> addition to "E NN". For many packets, GDB strips the "E." before
> giving the error message to the user. For others, GDB does not strip
> the "E.", but still understands that it is an error, as it starts with
> "E", and either prints the whole string, or ignores it and just
> mentions an error occured (same as for "E NN").
>
> This has been the case for as long as I remember. Now that I check, I
> see that it's been there since 2006 (commit a76d924dffcb, also here:
> https://sourceware.org/pipermail/gdb-patches/2006-September/047286.html).
> All along, I actually thought it was documented. Turns out it wasn't.
>
> This commit documents it, in the new "Standard Replies" section, near
> where we document "E NN".
>
> The original version of this 3-patch documentation series was a single
> CodeSourcery patch that documented the textual error as
> "E.NAME.MESSAGE", with MESSAGE being 8-bit binary encoded. But I
> think the ship has sailed for that. GDBserver has been sending error
> messages with more than one "." for a long while, and with no binary
> encoding. Still, I've preserved the "Co-Authored-By" list of the
> original larger patch.
>
> The 'qRcmd' and 'm' commands are exceptions and do not accept this
> reply format. The top of the "Standard Replies" section already says:
>
> "All commands support these, except as noted in the individual
> command descriptions."
>
> So this adds a note to the description of 'qRcmd' and 'm', explicitly
> stating that they do not support this error reply format.
>
> Change-Id: Ie4fee3d00d82ede39e439bf162e8cb7485532fd8
> Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
> Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
> Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
> Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
> ---
> gdb/doc/gdb.texinfo | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 9781ce76226..f55ada29e9c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -42559,6 +42559,11 @@ number. In almost all cases, the protocol does not specify the
> meaning of the error numbers; @value{GDBN} usually ignores the
> numbers, or displays them to the user without further interpretation.
>
> +@anchor{textual error reply}
> +@item @samp{E.@var{errtext}}
> +An error has occurred; @var{errtext} is the textual error message,
> +encoded in @sc{ascii}.
> +
> @end table
>
> @node Packets
> @@ -42885,6 +42890,10 @@ The reply may contain fewer addressable memory units than requested if the
> server was able to read only part of the region of memory.
> @end table
>
> +Unlike most packets, this packet does not support
> +@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
> +error reply}).
> +
> @item M @var{addr},@var{length}:@var{XX@dots{}}
> @cindex @samp{M} packet
> Write @var{length} addressable memory units starting at address @var{addr}
> @@ -44399,6 +44408,10 @@ A command response with no output.
> A command response with the hex encoded output string @var{OUTPUT}.
> @end table
>
> +Unlike most packets, this packet does not support
> +@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
> +error reply}).
> +
> (Note that the @code{qRcmd} packet's name is separated from the
> command by a @samp{,}, not a @samp{:}, contrary to the naming
> conventions above. Please don't use this packet as a model for new
> @@ -45455,8 +45468,6 @@ Reply:
> @table @samp
> @item OK
> Branch tracing has been enabled.
> -@item E.errtext
> -A badly formed request or an error was encountered.
> @end table
>
> @item Qbtrace:pt
> @@ -45466,8 +45477,6 @@ Reply:
> @table @samp
> @item OK
> Branch tracing has been enabled.
> -@item E.errtext
> -A badly formed request or an error was encountered.
> @end table
>
> @item Qbtrace:off
> @@ -45477,8 +45486,6 @@ Reply:
> @table @samp
> @item OK
> Branch tracing has been disabled.
> -@item E.errtext
> -A badly formed request or an error was encountered.
> @end table
>
> @item Qbtrace-conf:bts:size=@var{value}
> @@ -45489,8 +45496,6 @@ Reply:
> @table @samp
> @item OK
> The ring buffer size has been set.
> -@item E.errtext
> -A badly formed request or an error was encountered.
> @end table
>
> @item Qbtrace-conf:pt:size=@var{value}
> @@ -45501,8 +45506,6 @@ Reply:
> @table @samp
> @item OK
> The ring buffer size has been set.
> -@item E.errtext
> -A badly formed request or an error was encountered.
> @end table
>
> @end table
>
> base-commit: 0e6747d2a638693ad2f20e7929c8364913c87279
> prerequisite-patch-id: 69a7241ff790954e07942cd212971bcfaa8f3a08
> prerequisite-patch-id: b398b6e111b68e06adccda7d9e016ebe39b5fd35
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors
2024-04-26 19:02 ` Pedro Alves
@ 2024-04-26 19:18 ` Eli Zaretskii
2024-04-29 13:42 ` Andrew Burgess
1 sibling, 0 replies; 55+ messages in thread
From: Eli Zaretskii @ 2024-04-26 19:18 UTC (permalink / raw)
To: Pedro Alves; +Cc: aburgess, gdb-patches
> Date: Fri, 26 Apr 2024 20:02:22 +0100
> From: Pedro Alves <pedro@palves.net>
>
> [Adding Eli.]
>
> AFAICT, this is the only patch left in the series that needs an OK.
>
> Any comments on the updated documentation change below?
They are mostly mechanical changes, and where they aren't, you know
better than I do what GDB and gdbserver do with these packets. So, no
comments from me; feel free to install.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors
2024-04-26 19:02 ` Pedro Alves
2024-04-26 19:18 ` Eli Zaretskii
@ 2024-04-29 13:42 ` Andrew Burgess
1 sibling, 0 replies; 55+ messages in thread
From: Andrew Burgess @ 2024-04-29 13:42 UTC (permalink / raw)
To: Pedro Alves, gdb-patches, Eli Zaretskii
Pedro Alves <pedro@palves.net> writes:
> [Adding Eli.]
>
> AFAICT, this is the only patch left in the series that needs an OK.
Thanks for updating the docs. I'm happy with this.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Any comments on the updated documentation change below?
>
> Pedro Alves
>
> On 2024-04-22 20:04, Pedro Alves wrote:
>> On 2024-04-22 09:50, Andrew Burgess wrote:
>>
>>>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>>>> index 57260a5b2fa..d6184d52841 100644
>>>> --- a/gdb/doc/gdb.texinfo
>>>> +++ b/gdb/doc/gdb.texinfo
>>>> @@ -42557,6 +42557,10 @@ number. In almost all cases, the protocol does not specify the
>>>> meaning of the error numbers; @value{GDBN} usually ignores the
>>>> numbers, or displays them to the user without further interpretation.
>>>>
>>>> +@item @samp{E.@var{message}}
>>>> +An error has occurred; @var{message} is the textual error message,
>>>> +encoded in @sc{ascii}.
>>>
>>> I think we should document here that the 'qRcmd' and 'm' packets don't
>>> accept this reply format.
>>>
>>> Alexandra Hájková has a patch series which has not been posted yet which
>>> will extend these packets in a backward compatible way so that E.message
>>> can be used, but until that arrives we should probably document this
>>> limitation.
>>
>> Done, see updated patch below. (See the bottom of the new commit log.)
>>
>> Not sure when we'd want to make GDB print the error for the 'm' packet,
>> though. Currently it just returns TARGET_XFER_E_IO up to the caller.
>>
>> I noticed a few packets were documenting an "E.errtext" response,
>> which I now removed in this version. Then "E.errtext" seemed more
>> descriptive than "E.message", so I switched to that too.
>>
>>
>> From 36ef5ca67f515039bfa68d2fb2515f181629a24f Mon Sep 17 00:00:00 2001
>> From: Pedro Alves <pedro@palves.net>
>> Date: Thu, 18 Apr 2024 20:22:36 +0100
>> Subject: [PATCH] Document "E.MESSAGE" RSP errors
>>
>> For many years, GDB has accepted a "E.MESSAGE" error response, in
>> addition to "E NN". For many packets, GDB strips the "E." before
>> giving the error message to the user. For others, GDB does not strip
>> the "E.", but still understands that it is an error, as it starts with
>> "E", and either prints the whole string, or ignores it and just
>> mentions an error occured (same as for "E NN").
>>
>> This has been the case for as long as I remember. Now that I check, I
>> see that it's been there since 2006 (commit a76d924dffcb, also here:
>> https://sourceware.org/pipermail/gdb-patches/2006-September/047286.html).
>> All along, I actually thought it was documented. Turns out it wasn't.
>>
>> This commit documents it, in the new "Standard Replies" section, near
>> where we document "E NN".
>>
>> The original version of this 3-patch documentation series was a single
>> CodeSourcery patch that documented the textual error as
>> "E.NAME.MESSAGE", with MESSAGE being 8-bit binary encoded. But I
>> think the ship has sailed for that. GDBserver has been sending error
>> messages with more than one "." for a long while, and with no binary
>> encoding. Still, I've preserved the "Co-Authored-By" list of the
>> original larger patch.
>>
>> The 'qRcmd' and 'm' commands are exceptions and do not accept this
>> reply format. The top of the "Standard Replies" section already says:
>>
>> "All commands support these, except as noted in the individual
>> command descriptions."
>>
>> So this adds a note to the description of 'qRcmd' and 'm', explicitly
>> stating that they do not support this error reply format.
>>
>> Change-Id: Ie4fee3d00d82ede39e439bf162e8cb7485532fd8
>> Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
>> Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
>> Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
>> Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
>> ---
>> gdb/doc/gdb.texinfo | 23 +++++++++++++----------
>> 1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 9781ce76226..f55ada29e9c 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -42559,6 +42559,11 @@ number. In almost all cases, the protocol does not specify the
>> meaning of the error numbers; @value{GDBN} usually ignores the
>> numbers, or displays them to the user without further interpretation.
>>
>> +@anchor{textual error reply}
>> +@item @samp{E.@var{errtext}}
>> +An error has occurred; @var{errtext} is the textual error message,
>> +encoded in @sc{ascii}.
>> +
>> @end table
>>
>> @node Packets
>> @@ -42885,6 +42890,10 @@ The reply may contain fewer addressable memory units than requested if the
>> server was able to read only part of the region of memory.
>> @end table
>>
>> +Unlike most packets, this packet does not support
>> +@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
>> +error reply}).
>> +
>> @item M @var{addr},@var{length}:@var{XX@dots{}}
>> @cindex @samp{M} packet
>> Write @var{length} addressable memory units starting at address @var{addr}
>> @@ -44399,6 +44408,10 @@ A command response with no output.
>> A command response with the hex encoded output string @var{OUTPUT}.
>> @end table
>>
>> +Unlike most packets, this packet does not support
>> +@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
>> +error reply}).
>> +
>> (Note that the @code{qRcmd} packet's name is separated from the
>> command by a @samp{,}, not a @samp{:}, contrary to the naming
>> conventions above. Please don't use this packet as a model for new
>> @@ -45455,8 +45468,6 @@ Reply:
>> @table @samp
>> @item OK
>> Branch tracing has been enabled.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>> @end table
>>
>> @item Qbtrace:pt
>> @@ -45466,8 +45477,6 @@ Reply:
>> @table @samp
>> @item OK
>> Branch tracing has been enabled.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>> @end table
>>
>> @item Qbtrace:off
>> @@ -45477,8 +45486,6 @@ Reply:
>> @table @samp
>> @item OK
>> Branch tracing has been disabled.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>> @end table
>>
>> @item Qbtrace-conf:bts:size=@var{value}
>> @@ -45489,8 +45496,6 @@ Reply:
>> @table @samp
>> @item OK
>> The ring buffer size has been set.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>> @end table
>>
>> @item Qbtrace-conf:pt:size=@var{value}
>> @@ -45501,8 +45506,6 @@ Reply:
>> @table @samp
>> @item OK
>> The ring buffer size has been set.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>> @end table
>>
>> @end table
>>
>> base-commit: 0e6747d2a638693ad2f20e7929c8364913c87279
>> prerequisite-patch-id: 69a7241ff790954e07942cd212971bcfaa8f3a08
>> prerequisite-patch-id: b398b6e111b68e06adccda7d9e016ebe39b5fd35
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 04/12] Windows: Fix run/attach hang after bad run/attach
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (2 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 03/12] Document "E.MESSAGE" RSP errors Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:35 ` Tom Tromey
2024-04-19 15:13 ` [PATCH 05/12] Fix "run" failure handling with GDBserver Pedro Alves
` (8 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
On Cygwin, gdb.base/attach.exp exposes that an "attach" after a
previously failed "attach" hangs:
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: attach to digits-starting nonsense is prohibited
attach 0
Can't attach to process 0 (error 2: The system cannot find the file specified.)
(gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: attach to nonexistent process is prohibited
attach 10644
FAIL: gdb.base/attach.exp: do_attach_failure_tests: first attach (timeout)
The problem is that windows_nat_target::attach always returns success
even if the attach fails. When we return success, the helper thread
begins waiting for events (which will never come), and thus the next
attach deadlocks on the do_synchronously call within
windows_nat_target::attach.
"run" has the same problem, which is exposed by the new
gdb.base/run-fail-twice.exp testcase added in a following patch:
(gdb) run
Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox
Error creating process .../gdb.base/run-fail-twice/run-fail-twice.nox, (error 6: The handle is invalid.)
(gdb) PASS: gdb.base/run-fail-twice.exp: test: bad run 1
run
Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox
FAIL: gdb.base/run-fail-twice.exp: test: bad run 2 (timeout)
The problem here is the same, except that this time it is
windows_nat_target::create_inferior that returns the incorrect result.
This commit fixes both the "attach" and "run" paths, and the latter
both the Cygwin and MinGW paths. The tests mentioned above now pass
on Cygwin. Confirmed the fixes manually for MinGW GDB.
Change-Id: I15ec9fa279aff269d4982b00f4ea7c25ae917239
---
gdb/windows-nat.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a53b6a6e053..d61ed95a7fb 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2059,7 +2059,7 @@ windows_nat_target::attach (const char *args, int from_tty)
if (!ok)
err = (unsigned) GetLastError ();
- return true;
+ return ok;
});
if (err.has_value ())
@@ -2784,12 +2784,15 @@ windows_nat_target::create_inferior (const char *exec_file,
windows_init_thread_list ();
do_synchronously ([&] ()
{
- if (!create_process (nullptr, args, flags, w32_env,
- inferior_cwd != nullptr ? infcwd : nullptr,
- disable_randomization,
- &si, &pi))
+ BOOL ok = create_process (nullptr, args, flags, w32_env,
+ inferior_cwd != nullptr ? infcwd : nullptr,
+ disable_randomization,
+ &si, &pi);
+
+ if (!ok)
ret = (unsigned) GetLastError ();
- return true;
+
+ return ok;
});
if (w32_env)
@@ -2910,16 +2913,18 @@ windows_nat_target::create_inferior (const char *exec_file,
windows_init_thread_list ();
do_synchronously ([&] ()
{
- if (!create_process (nullptr, /* image */
- args, /* command line */
- flags, /* start flags */
- w32env, /* environment */
- inferior_cwd, /* current directory */
- disable_randomization,
- &si,
- &pi))
+ BOOL ok = create_process (nullptr, /* image */
+ args, /* command line */
+ flags, /* start flags */
+ w32env, /* environment */
+ inferior_cwd, /* current directory */
+ disable_randomization,
+ &si,
+ &pi);
+ if (!ok)
ret = (unsigned) GetLastError ();
- return true;
+
+ return ok;
});
if (tty != INVALID_HANDLE_VALUE)
CloseHandle (tty);
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH 05/12] Fix "run" failure handling with GDBserver
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (3 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 04/12] Windows: Fix run/attach hang after bad run/attach Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:41 ` Tom Tromey
2024-04-19 15:13 ` [PATCH 06/12] Improve vRun error reporting Pedro Alves
` (7 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
If starting the inferior process with "run" (vRun packet) fails,
GDBserver throws an error that escapes all the way to the top level.
When an error escapes all the way like that, GDBserver interprets it
as a disconnection, and either goes back to waiting for a new GDB
connection, or exits, if --once was specified.
E.g., with the testcase program added by this commit, we see:
On GDB side:
...
(gdb) tar extended-remote :999
...
Remote debugging using :9999
(gdb) r
Starting program:
Running ".../gdb.base/run-fail-twice/run-fail-twice.nox" on the remote target failed
(gdb)
On GDBserver side:
$ gdbserver --once --multi :9999
Remote debugging from host 127.0.0.1, port 34344
bash: line 1: .../gdb.base/run-fail-twice/run-fail-twice.nox: Permission denied
bash: line 1: exec: .../gdb.base/run-fail-twice/run-fail-twice.nox: cannot execute: Permission denied
gdbserver: During startup program exited with code 126.
$ # gdbserver exited
This is wrong, as we've connected with extended-remote/--multi.
GDBserver should just report an error to vCont, and continue connected
to GDB, waiting for other commands.
This commit fixes GDBserver by catching the error locally in
handle_v_run.
Change-Id: Ib386f267522603f554b52a885b15229c9639e870
---
gdb/testsuite/gdb.base/run-fail-twice.c | 20 +++++++
gdb/testsuite/gdb.base/run-fail-twice.exp | 63 +++++++++++++++++++++++
gdbserver/server.cc | 10 +++-
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.base/run-fail-twice.c
create mode 100644 gdb/testsuite/gdb.base/run-fail-twice.exp
diff --git a/gdb/testsuite/gdb.base/run-fail-twice.c b/gdb/testsuite/gdb.base/run-fail-twice.c
new file mode 100644
index 00000000000..fddf841eb3e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/run-fail-twice.c
@@ -0,0 +1,20 @@
+/* Copyright 2024 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int
+main (int argc, char **argv)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/run-fail-twice.exp b/gdb/testsuite/gdb.base/run-fail-twice.exp
new file mode 100644
index 00000000000..676fc486fbf
--- /dev/null
+++ b/gdb/testsuite/gdb.base/run-fail-twice.exp
@@ -0,0 +1,63 @@
+# Copyright 2024 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test doing a "run" that fails, and then another "run".
+
+require target_can_use_run_cmd
+
+standard_testfile
+
+if {[build_executable "failed to build" $testfile $srcfile {debug}]} {
+ return -1
+}
+
+proc test_run {testname} {
+ gdb_test_multiple "run" $testname {
+ -re -wrap "During startup program exited with code 126\\." {
+ # What we get on GNU/Linux.
+ pass $gdb_test_name
+ }
+ -re -wrap "Error creating process.*" {
+ # What we get on Windows.
+ pass $gdb_test_name
+ }
+ -re -wrap "Running .* on the remote target failed" {
+ # What we get with remote targets.
+ pass $gdb_test_name
+ }
+ }
+}
+
+proc_with_prefix test {} {
+ global gdb_prompt binfile
+
+ clean_restart $binfile
+
+ gdb_test_no_output "set confirm off"
+
+ gdb_remote_download host $binfile $binfile.nox
+ remote_exec target "chmod \"a-x\" $binfile.nox"
+ gdb_test_no_output \
+ "exec-file $binfile.nox" \
+ "exec-file \$binfile.nox"
+ gdb_test_no_output \
+ "set remote exec-file $binfile.nox" \
+ "set remote exec-file \$binfile.nox"
+
+ test_run "bad run 1"
+ test_run "bad run 2"
+}
+
+test
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index c7d5cc1c1b0..b170da44f6a 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -3427,7 +3427,15 @@ handle_v_run (char *own_buf)
free_vector_argv (program_args);
program_args = new_argv;
- target_create_inferior (program_path.get (), program_args);
+ try
+ {
+ target_create_inferior (program_path.get (), program_args);
+ }
+ catch (const gdb_exception_error &exception)
+ {
+ sprintf (own_buf, "E.%s", exception.what ());
+ return;
+ }
if (cs.last_status.kind () == TARGET_WAITKIND_STOPPED)
{
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH 06/12] Improve vRun error reporting
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (4 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 05/12] Fix "run" failure handling with GDBserver Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:43 ` Tom Tromey
2024-04-19 15:13 ` [PATCH 07/12] Fix "attach" failure handling with GDBserver Pedro Alves
` (6 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
After the previous commit, if starting the inferior process with "run"
(vRun packet) fails, GDBserver reports an error using the "E." textual
error packet. On the GDB side, however, GDB doesn't yet do anything
with the textual error string. This commit improves that.
This makes remote debugging output the same as native output, when
possible, another small step in the "local/remote parity" project.
E.g., before, against GNU/Linux GDBserver:
(gdb) run
Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox
Running ".../gdb.base/run-fail-twice/run-fail-twice.nox" on the remote target failed
After, against GNU/Linux GDBserver (same as native):
(gdb) run
Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox
During startup program exited with code 126.
To know whether we have a textual error message, extend packet_result
to carry that information. While at it, convert packet_result to use
factory methods, and change its std::string parameter to a plain const
char *, as that it always what we have handy to pass to it.
Change-Id: Ib386f267522603f554b52a885b15229c9639e870
---
gdb/remote.c | 67 ++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 55 insertions(+), 12 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index cfb54de157d..8ef808c5ad6 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -158,20 +158,27 @@ enum packet_status
/* Keeps packet's return value. If packet's return value is PACKET_ERROR,
err_msg contains an error message string from E.string or the number
stored as a string from E.num. */
-struct packet_result
+class packet_result
{
- packet_result (enum packet_status status, std::string err_msg)
- : m_status (status), m_err_msg (std::move (err_msg))
- {
- gdb_assert (status == PACKET_ERROR);
- }
+private:
+ /* Private ctors for internal use. Clients should use the public
+ factory static methods instead. */
+
+ /* Construct a PACKET_ERROR packet_result. */
+ packet_result (const char *err_msg, bool textual_err_msg)
+ : m_status (PACKET_ERROR),
+ m_err_msg (err_msg),
+ m_textual_err_msg (textual_err_msg)
+ {}
+ /* Construct an PACKET_OK/PACKET_UNKNOWN packet_result. */
explicit packet_result (enum packet_status status)
: m_status (status)
{
gdb_assert (status != PACKET_ERROR);
}
+public:
enum packet_status status () const
{
return this->m_status;
@@ -183,9 +190,39 @@ struct packet_result
return this->m_err_msg.c_str ();
}
+ bool textual_err_msg () const
+ {
+ gdb_assert (this->m_status == PACKET_ERROR);
+ return this->m_textual_err_msg;
+ }
+
+ static packet_result make_numeric_error (const char *err_msg)
+ {
+ return packet_result (err_msg, false);
+ }
+
+ static packet_result make_textual_error (const char *err_msg)
+ {
+ return packet_result (err_msg, true);
+ }
+
+ static packet_result make_ok ()
+ {
+ return packet_result (PACKET_OK);
+ }
+
+ static packet_result make_unknown ()
+ {
+ return packet_result (PACKET_UNKNOWN);
+ }
+
private:
enum packet_status m_status;
std::string m_err_msg;
+
+ /* True if we have a textual error message, from an "E.MESSAGE"
+ response. */
+ bool m_textual_err_msg;
};
/* Enumeration of packets for a remote target. */
@@ -2473,7 +2510,7 @@ packet_check_result (const char *buf, bool accept_msg)
&& isxdigit (buf[1]) && isxdigit (buf[2])
&& buf[3] == '\0')
/* "Enn" - definitely an error. */
- return { PACKET_ERROR, buf + 1 };
+ return packet_result::make_numeric_error (buf + 1);
/* Not every request accepts an error in a E.msg form.
Some packets accepts only Enn, in this case E. is not
@@ -2485,19 +2522,19 @@ packet_check_result (const char *buf, bool accept_msg)
if (buf[0] == 'E' && buf[1] == '.')
{
if (buf[2] != '\0')
- return { PACKET_ERROR, buf + 2 };
+ return packet_result::make_textual_error (buf + 2);
else
- return { PACKET_ERROR, "no error provided" };
+ return packet_result::make_textual_error ("no error provided");
}
}
/* The packet may or may not be OK. Just assume it is. */
- return packet_result (PACKET_OK);
+ return packet_result::make_ok ();
}
else
{
/* The stub does not support the packet. */
- return packet_result (PACKET_UNKNOWN);
+ return packet_result::make_unknown ();
}
}
@@ -10704,7 +10741,8 @@ remote_target::extended_remote_run (const std::string &args)
putpkt (rs->buf);
getpkt (&rs->buf);
- switch ((m_features.packet_ok (rs->buf, PACKET_vRun)).status ())
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_vRun);
+ switch (result.status ())
{
case PACKET_OK:
/* We have a wait response. All is well. */
@@ -10712,6 +10750,11 @@ remote_target::extended_remote_run (const std::string &args)
case PACKET_UNKNOWN:
return -1;
case PACKET_ERROR:
+ /* If we have a textual error message, print just that. This
+ makes remote debugging output the same as native output, when
+ possible. */
+ if (result.textual_err_msg ())
+ error (("%s"), result.err_msg ());
if (remote_exec_file[0] == '\0')
error (_("Running the default executable on the remote target failed; "
"try \"set remote exec-file\"?"));
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 06/12] Improve vRun error reporting
2024-04-19 15:13 ` [PATCH 06/12] Improve vRun error reporting Pedro Alves
@ 2024-04-19 18:43 ` Tom Tromey
2024-04-22 11:32 ` Alexandra Petlanova Hajkova
0 siblings, 1 reply; 55+ messages in thread
From: Tom Tromey @ 2024-04-19 18:43 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> To know whether we have a textual error message, extend packet_result
Pedro> to carry that information. While at it, convert packet_result to use
Pedro> factory methods, and change its std::string parameter to a plain const
Pedro> char *, as that it always what we have handy to pass to it.
Didn't Alexandra have a similar series?
Tom
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/12] Improve vRun error reporting
2024-04-19 18:43 ` Tom Tromey
@ 2024-04-22 11:32 ` Alexandra Petlanova Hajkova
0 siblings, 0 replies; 55+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-04-22 11:32 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 625 bytes --]
On Fri, Apr 19, 2024 at 8:44 PM Tom Tromey <tom@tromey.com> wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>
> Pedro> To know whether we have a textual error message, extend
> packet_result
> Pedro> to carry that information. While at it, convert packet_result to
> use
> Pedro> factory methods, and change its std::string parameter to a plain
> const
> Pedro> char *, as that it always what we have handy to pass to it.
>
> Didn't Alexandra have a similar series?
>
> My series touches the same area but this patch does not conflict with
them. I would say this enhances what I've done.
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 07/12] Fix "attach" failure handling with GDBserver
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (5 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 06/12] Improve vRun error reporting Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:47 ` Tom Tromey
2024-04-19 15:13 ` [PATCH 08/12] gdbserver: Fix vAttach response when attaching is not supported Pedro Alves
` (5 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
This fixes the same issue as the previous patch, but for "attach"
instead of "run".
If attaching to a process with "attach" (vAttach packet) fails,
GDBserver throws an error that escapes all the way to the top level.
When an error escapes all the way like that, GDBserver interprets it
as a disconnection, and either goes back to waiting for a new GDB
connection, or exits, if --once was specified.
Here's an example:
On the GDB side:
...
(gdb) tar extended-remote :9999
...
Remote debugging using :9999
(gdb) attach 1
Attaching to process 1
Attaching to process 1 failed
(gdb)
On the GDBserver side:
$ gdbserver --once --multi :9999
Listening on port 9999
Remote debugging from host 127.0.0.1, port 37464
gdbserver: Cannot attach to process 1: Operation not permitted (1)
$ # gdbserver exited
This is wrong, as we've connected with extended-remote/--multi.
GDBserver should just report an error to vAttach, and continue
connected to GDB, waiting for other commands.
This commit fixes GDBserver by catching the error locally in
handle_v_attach.
Note we now let pid == 0 pass down to attach_inferior. That is so we
get a useful textual error message to report to GDB.
This fixes a couple KFAILs in gdb.base/attach.exp. Still, I thought
it would be useful to add a new testcase specifically for this
scenario, in case gdb.base/attach.exp is ever split and stops trying
to attach again after a failed attach, with the same GDB session.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19558
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31554
Change-Id: I25314c7e5f1435eff69cb84d57ecac13d8de3393
---
gdb/testsuite/gdb.base/attach-fail-twice.c | 27 ++++++
gdb/testsuite/gdb.base/attach-fail-twice.exp | 94 ++++++++++++++++++++
gdb/testsuite/gdb.base/attach.exp | 6 --
gdbserver/server.cc | 41 +++++----
4 files changed, 145 insertions(+), 23 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/attach-fail-twice.c
create mode 100644 gdb/testsuite/gdb.base/attach-fail-twice.exp
diff --git a/gdb/testsuite/gdb.base/attach-fail-twice.c b/gdb/testsuite/gdb.base/attach-fail-twice.c
new file mode 100644
index 00000000000..d045e35e4b4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/attach-fail-twice.c
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2024 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <unistd.h>
+
+int
+main ()
+{
+ for (int i = 0; i < 60; i++)
+ sleep (1);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/attach-fail-twice.exp b/gdb/testsuite/gdb.base/attach-fail-twice.exp
new file mode 100644
index 00000000000..59020463c67
--- /dev/null
+++ b/gdb/testsuite/gdb.base/attach-fail-twice.exp
@@ -0,0 +1,94 @@
+# Copyright 2024 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test doing a "attach" that fails, and then another "attach".
+
+require can_spawn_for_attach
+
+standard_testfile
+
+if {[build_executable "failed to build" $testfile $srcfile {debug}]} {
+ return -1
+}
+
+set test_spawn_id [spawn_wait_for_attach $binfile]
+set testpid [spawn_id_get_pid $test_spawn_id]
+
+# Test an attach that fails.
+
+proc test_bad_attach {test} {
+ global testpid gdb_prompt
+
+ set boguspid 0
+ if { [istarget "*-*-*bsd*"] } {
+ # In FreeBSD 5.0, PID 0 is used for "swapper". Use -1 instead
+ # (which should have the desired effect on any version of
+ # FreeBSD, and probably other *BSD's too).
+ set boguspid -1
+ }
+ gdb_test_multiple "attach $boguspid" $test {
+ -re "Attaching to.*, process $boguspid.*No such process.*$gdb_prompt $" {
+ # Response expected on ptrace-based systems (i.e. GNU/Linux).
+ pass "$test"
+ }
+ -re "Attaching to.*, process $boguspid.*denied.*$gdb_prompt $" {
+ pass "$gdb_test_name"
+ }
+ -re "Attaching to.*, process $boguspid.*not permitted.*$gdb_prompt $" {
+ pass "$gdb_test_name"
+ }
+ -re "Attaching to.*, process .*couldn't open /proc file.*$gdb_prompt $" {
+ # Response expected from /proc-based systems.
+ pass "$gdb_test_name"
+ }
+ -re "Can't attach to process..*$gdb_prompt $" {
+ # Response expected on Windows.
+ pass "$gdb_test_name"
+ }
+ -re "Attaching to.*, process $boguspid.*failed.*$gdb_prompt $" {
+ # Response expected on the extended-remote target.
+ pass "$gdb_test_name"
+ }
+ }
+}
+
+# Test an attach that succeeds.
+
+proc test_good_attach {test} {
+ gdb_test "attach $::testpid" \
+ "Attaching to program.*, process $::testpid.*" \
+ "$test"
+
+ set thread_count [get_valueof "" "\$_inferior_thread_count" -1]
+ gdb_assert {$thread_count > 0} \
+ "attached"
+}
+
+proc_with_prefix test {} {
+ clean_restart $::binfile
+
+ # GDB used to have a bug on Windows where failing to attach once
+ # made a subsequent "attach" or "run" hang. So it's important for
+ # this regression test that we try to attach more than once.
+
+ test_bad_attach "bad attach 1"
+ test_bad_attach "bad attach 2"
+
+ # For good measure, test that we can attach to something after
+ # failing to attach previously.
+ test_good_attach "good attach"
+}
+
+test
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 7907441054b..84b2d27f3eb 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -173,10 +173,6 @@ proc_with_prefix do_attach_failure_tests {} {
gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"
gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2"
- # Probe this before the failing attach: the failed attach against GDBserver
- # currently leaves the extended-remote target in a bad state.
- set do_kfail [target_is_gdbserver]
-
set test "fail to attach again"
gdb_test_multiple "attach $testpid" "$test" {
-re "Attaching to process $testpid.*warning: process .* is already traced by process .*$gdb_prompt $" {
@@ -193,14 +189,12 @@ proc_with_prefix do_attach_failure_tests {} {
gdb_test_no_output "set confirm off"
gdb_test "inferior 1" "Switching to inferior 1.*" "switch to inferior 1"
- if { $do_kfail } { setup_kfail "gdb/19558" "*-*-*" }
gdb_test "kill" "killed.*" "exit after attach failures"
# This can probably be replaced with a call to runto or runto_main once
# the kfail is removed.
gdb_breakpoint "main"
gdb_run_cmd
- if { $do_kfail } { setup_kfail "gdb/19558" "*-*-*" }
gdb_test_multiple "" "stop at main" {
-wrap -re "Breakpoint $::decimal, main .*" {
pass $gdb_test_name
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index b170da44f6a..2633df08ddb 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -3294,29 +3294,36 @@ static void
handle_v_attach (char *own_buf)
{
client_state &cs = get_client_state ();
- int pid;
- pid = strtol (own_buf + 8, NULL, 16);
- if (pid != 0 && attach_inferior (pid) == 0)
- {
- /* Don't report shared library events after attaching, even if
- some libraries are preloaded. GDB will always poll the
- library list. Avoids the "stopped by shared library event"
- notice on the GDB side. */
- current_process ()->dlls_changed = false;
+ int pid = strtol (own_buf + 8, NULL, 16);
- if (non_stop)
+ try
+ {
+ if (attach_inferior (pid) == 0)
{
- /* In non-stop, we don't send a resume reply. Stop events
- will follow up using the normal notification
- mechanism. */
- write_ok (own_buf);
+ /* Don't report shared library events after attaching, even if
+ some libraries are preloaded. GDB will always poll the
+ library list. Avoids the "stopped by shared library event"
+ notice on the GDB side. */
+ current_process ()->dlls_changed = false;
+
+ if (non_stop)
+ {
+ /* In non-stop, we don't send a resume reply. Stop events
+ will follow up using the normal notification
+ mechanism. */
+ write_ok (own_buf);
+ }
+ else
+ prepare_resume_reply (own_buf, cs.last_ptid, cs.last_status);
}
else
- prepare_resume_reply (own_buf, cs.last_ptid, cs.last_status);
+ write_enn (own_buf);
+ }
+ catch (const gdb_exception_error &exception)
+ {
+ sprintf (own_buf, "E.%s", exception.what ());
}
- else
- write_enn (own_buf);
}
/* Decode an argument from the vRun packet buffer. PTR points to the
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH 08/12] gdbserver: Fix vAttach response when attaching is not supported
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (6 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 07/12] Fix "attach" failure handling with GDBserver Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:48 ` Tom Tromey
2024-04-19 15:13 ` [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native Pedro Alves
` (4 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
handle_v_attach calls attach_inferior, which says:
"return -1 if attaching is unsupported, 0 if it succeeded, and call
error() otherwise."
So if attach_inferior return != 0, we have the unsupported case,
meaning we should return the empty packet instead of an error.
In practice, this shouldn't trigger, as vAttach support is supposed to
be reported via qSupported. But it doesn't hurt to be pedantic here.
Change-Id: I99cce6fa678f2370571e6bca0657451300956127
---
gdbserver/server.cc | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 2633df08ddb..789af36d9a4 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -3318,7 +3318,10 @@ handle_v_attach (char *own_buf)
prepare_resume_reply (own_buf, cs.last_ptid, cs.last_status);
}
else
- write_enn (own_buf);
+ {
+ /* Not supported. */
+ own_buf[0] = 0;
+ }
}
catch (const gdb_exception_error &exception)
{
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (7 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 08/12] gdbserver: Fix vAttach response when attaching is not supported Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:50 ` Tom Tromey
2024-04-22 8:25 ` [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native Aktemur, Tankut Baris
2024-04-19 15:13 ` [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote Pedro Alves
` (3 subsequent siblings)
12 siblings, 2 replies; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
gdb_is_target_native uses "maint print target-stack", which is
unnecessary when checking whether gdb_protocol is empty would do.
Checking gdb_protocol is more efficient, and can be done before
starting GDB and running to main, unlike gdb_is_target_native.
This adds a new gdb_protocol_is_native procedure, and uses it in place
of gdb_is_target_native.
At first, I thought that we'd end up with a few testcases needing to
use gdb_is_target_native still, especially multi-target tests that
connect to targets different from the default board target, but no,
actually all uses of gdb_is_target_native could be converted.
gdb_is_target_native will be eliminated in a following patch.
In some spots, we no longer need to defer the check until after
starting GDB, so the patch adjusts accordingly.
Change-Id: Ia706232dbffac70f9d9740bcb89c609dbee5cee3
---
gdb/testsuite/gdb.base/attach.exp | 13 +++----------
gdb/testsuite/gdb.base/foll-exec-mode.exp | 4 +---
gdb/testsuite/gdb.base/load-command.exp | 11 +++++------
gdb/testsuite/gdb.multi/stop-all-on-exit.exp | 16 ++++++++--------
gdb/testsuite/gdb.python/py-inferior.exp | 2 +-
gdb/testsuite/gdb.threads/threads-after-exec.exp | 2 +-
gdb/testsuite/lib/gdb.exp | 15 +++++++++++++++
7 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 84b2d27f3eb..637f287f59e 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -470,16 +470,9 @@ proc_with_prefix test_command_line_attach_run {} {
global gdb_prompt
global binfile
- # The --pid option is used to attach to a process using the native target.
- # Start GDB and run to main just to see what the execution target is, skip
- # if it's not the native target.
- clean_restart $binfile
-
- if { ![runto_main] } {
- return
- }
-
- if { ![gdb_is_target_native] } {
+ # The --pid option is used to attach to a process using the native
+ # target.
+ if { ![gdb_protocol_is_native] } {
unsupported "commandline attach run test"
return
}
diff --git a/gdb/testsuite/gdb.base/foll-exec-mode.exp b/gdb/testsuite/gdb.base/foll-exec-mode.exp
index ff92c551bba..65054b530b3 100644
--- a/gdb/testsuite/gdb.base/foll-exec-mode.exp
+++ b/gdb/testsuite/gdb.base/foll-exec-mode.exp
@@ -109,8 +109,6 @@ proc do_follow_exec_mode_tests { mode cmd infswitch } {
return
}
- set target_is_native [gdb_is_target_native]
-
# Set the follow-exec mode.
#
gdb_test_no_output "set follow-exec-mode $mode"
@@ -150,7 +148,7 @@ proc do_follow_exec_mode_tests { mode cmd infswitch } {
# process target, which was automatically pushed when running, was
# automatically unpushed from inferior 1 on exec. Use a
# different regexp that verifies the Connection field is empty.
- if { $target_is_native } {
+ if { [gdb_protocol_is_native] } {
set expected_re " 1.*<null> +[string_to_regexp $binfile].*\r\n\\* 2.*process.*$testfile2 .*"
} else {
set expected_re " 1.*null.*$testfile.*\r\n\\* 2.*process.*$testfile2 .*"
diff --git a/gdb/testsuite/gdb.base/load-command.exp b/gdb/testsuite/gdb.base/load-command.exp
index ce6f9bcb730..2d3656e711a 100644
--- a/gdb/testsuite/gdb.base/load-command.exp
+++ b/gdb/testsuite/gdb.base/load-command.exp
@@ -17,6 +17,11 @@
standard_testfile
+if [gdb_protocol_is_native] {
+ unsupported "the native target does not support the load command"
+ return
+}
+
# Disable generation of position independent executable (PIE). Otherwise, we
# would have to manually specify an offset to load.
@@ -30,12 +35,6 @@ if ![runto_main] {
return -1
}
-# The native target does not support the load command.
-if [gdb_is_target_native] {
- unsupported "the native target does not support the load command"
- return
-}
-
# Manually change the value of the_variable.
gdb_test "print/x the_variable" " = 0x1234" "check initial value of the_variable"
gdb_test_no_output "set the_variable = 0x5555" "manually change the_variable"
diff --git a/gdb/testsuite/gdb.multi/stop-all-on-exit.exp b/gdb/testsuite/gdb.multi/stop-all-on-exit.exp
index f014037106d..1ac5388c0a4 100644
--- a/gdb/testsuite/gdb.multi/stop-all-on-exit.exp
+++ b/gdb/testsuite/gdb.multi/stop-all-on-exit.exp
@@ -18,6 +18,14 @@
# Test that in all-stop mode with multiple inferiors, GDB stops all
# threads upon receiving an exit event from one of the inferiors.
+# This is a test specific for a native target, where we use the
+# "-exec" argument to "add-inferior" and we explicitly don't do
+# "maint set target-non-stop on".
+if {![gdb_protocol_is_native]} {
+ untested "the test is aimed at a native target"
+ return 0
+}
+
standard_testfile
if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
@@ -28,14 +36,6 @@ if {![runto_main]} {
return -1
}
-# This is a test specific for a native target, where we use the
-# "-exec" argument to "add-inferior" and we explicitly don't do
-# "maint set target-non-stop on".
-if {![gdb_is_target_native]} {
- untested "the test is aimed at a native target"
- return 0
-}
-
# Add a second inferior that will sleep longer.
gdb_test "add-inferior -exec $binfile" "Added inferior 2.*" \
"add the second inferior"
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index e74fbfd050d..ee30390e29f 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -406,7 +406,7 @@ with_test_prefix "selected_inferior" {
gdb_test "py print (gdb.selected_inferior().connection.num)" "1" \
"first inferior's connection number, though connection object"
# Figure out if inf 1 has a native target.
- set inf_1_is_native [gdb_is_target_native]
+ set inf_1_is_native [gdb_protocol_is_native]
set num [add_inferior "-no-connection"]
gdb_test "inferior $num" ".*" "switch to inferior $num"
diff --git a/gdb/testsuite/gdb.threads/threads-after-exec.exp b/gdb/testsuite/gdb.threads/threads-after-exec.exp
index 4dc71dd76fd..32aec6b39db 100644
--- a/gdb/testsuite/gdb.threads/threads-after-exec.exp
+++ b/gdb/testsuite/gdb.threads/threads-after-exec.exp
@@ -38,7 +38,7 @@ proc do_test { } {
# leader detection racy") this isn't always thread 1.1.
set cur_thr [get_integer_valueof "\$_thread" 0]
- if {[istarget *-*-linux*] && [gdb_is_target_native]} {
+ if {[istarget *-*-linux*] && [gdb_protocol_is_native]} {
# Confirm there's only one LWP in the list as well, and that
# it is bound to the existing GDB thread.
set inf_pid [get_inferior_pid]
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index ddee928d510..c072a4502b4 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4698,6 +4698,10 @@ proc gdb_is_target_remote_prompt { prompt_regexp } {
# Check whether we're testing with the remote or extended-remote
# targets.
#
+# This is meant to be used on testcases that connect to targets
+# different from the default board protocol. For most tests, you can
+# check whether gdb_protocol is "remote" or "extended-remote" instead.
+#
# NOTE: GDB must be running BEFORE this procedure is called!
proc gdb_is_target_remote { } {
@@ -4708,6 +4712,10 @@ proc gdb_is_target_remote { } {
# Check whether we're testing with the native target.
#
+# This is meant to be used on testcases that connect to targets
+# different from the default board protocol. For most tests, you can
+# check whether gdb_protocol is the empty string instead.
+#
# NOTE: GDB must be running BEFORE this procedure is called!
proc gdb_is_target_native { } {
@@ -4716,6 +4724,13 @@ proc gdb_is_target_native { } {
return [gdb_is_target_1 "native" ".*native \\(Native process\\).*" "$gdb_prompt $"]
}
+# Returns true if gdb_protocol is empty, indicating use of the native
+# target.
+
+proc gdb_protocol_is_native { } {
+ return [expr {[target_info gdb_protocol] == ""}]
+}
+
# Like istarget, but checks a list of targets.
proc is_any_target {args} {
foreach targ $args {
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-04-19 15:13 ` [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native Pedro Alves
@ 2024-04-19 18:50 ` Tom Tromey
2024-05-09 8:47 ` Bernd Edlinger
2024-04-22 8:25 ` [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native Aktemur, Tankut Baris
1 sibling, 1 reply; 55+ messages in thread
From: Tom Tromey @ 2024-04-19 18:50 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> gdb_is_target_native uses "maint print target-stack", which is
Pedro> unnecessary when checking whether gdb_protocol is empty would do.
Pedro> Checking gdb_protocol is more efficient, and can be done before
Pedro> starting GDB and running to main, unlike gdb_is_target_native.
Pedro> This adds a new gdb_protocol_is_native procedure, and uses it in place
Pedro> of gdb_is_target_native.
Pedro> At first, I thought that we'd end up with a few testcases needing to
Pedro> use gdb_is_target_native still, especially multi-target tests that
Pedro> connect to targets different from the default board target, but no,
Pedro> actually all uses of gdb_is_target_native could be converted.
Pedro> gdb_is_target_native will be eliminated in a following patch.
Pedro> In some spots, we no longer need to defer the check until after
Pedro> starting GDB, so the patch adjusts accordingly.
Seems reasonable to me.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-04-19 18:50 ` Tom Tromey
@ 2024-05-09 8:47 ` Bernd Edlinger
2024-05-09 9:47 ` Pedro Alves
0 siblings, 1 reply; 55+ messages in thread
From: Bernd Edlinger @ 2024-05-09 8:47 UTC (permalink / raw)
To: Tom Tromey, Pedro Alves; +Cc: gdb-patches
On 4/19/24 20:50, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>
> Pedro> gdb_is_target_native uses "maint print target-stack", which is
> Pedro> unnecessary when checking whether gdb_protocol is empty would do.
> Pedro> Checking gdb_protocol is more efficient, and can be done before
> Pedro> starting GDB and running to main, unlike gdb_is_target_native.
>
> Pedro> This adds a new gdb_protocol_is_native procedure, and uses it in place
> Pedro> of gdb_is_target_native.
>
> Pedro> At first, I thought that we'd end up with a few testcases needing to
> Pedro> use gdb_is_target_native still, especially multi-target tests that
> Pedro> connect to targets different from the default board target, but no,
> Pedro> actually all uses of gdb_is_target_native could be converted.
> Pedro> gdb_is_target_native will be eliminated in a following patch.
>
> Pedro> In some spots, we no longer need to defer the check until after
> Pedro> starting GDB, so the patch adjusts accordingly.
>
> Seems reasonable to me.
> Approved-By: Tom Tromey <tom@tromey.com>
>
> Tom
>
Hi Pedro,
Sorry, for the late reply.
I am testing with riscv-unknown-elf target using simulator.
Before this patch I had:
PASS: gdb.base/load-command.exp: probe for target native
PASS: gdb.base/load-command.exp: check initial value of the_variable
PASS: gdb.base/load-command.exp: manually change the_variable
PASS: gdb.base/load-command.exp: check manually changed value of the_variable
PASS: gdb.base/load-command.exp: reload: re-load binary
PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
and now
UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
note: I use RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-sim"
but the test was working fine, because with this target
we use target sim, load, and can runto main, inspect variables, etc.
Regards
Bernd.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-05-09 8:47 ` Bernd Edlinger
@ 2024-05-09 9:47 ` Pedro Alves
2024-05-09 11:54 ` Bernd Edlinger
0 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-05-09 9:47 UTC (permalink / raw)
To: Bernd Edlinger, Tom Tromey; +Cc: gdb-patches
Hi Bernd,
On 2024-05-09 09:47, Bernd Edlinger wrote:
> Hi Pedro,
>
> Sorry, for the late reply.
>
> I am testing with riscv-unknown-elf target using simulator.
> Before this patch I had:
>
> PASS: gdb.base/load-command.exp: probe for target native
> PASS: gdb.base/load-command.exp: check initial value of the_variable
> PASS: gdb.base/load-command.exp: manually change the_variable
> PASS: gdb.base/load-command.exp: check manually changed value of the_variable
> PASS: gdb.base/load-command.exp: reload: re-load binary
> PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
>
> and now
>
> UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
>
> note: I use RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-sim"
> but the test was working fine, because with this target
> we use target sim, load, and can runto main, inspect variables, etc.
>
I'm trying to understand how the sim board works, to figure out what is the best course
of action.
I see that /usr/share/dejagnu/config/sim.exp has:
set_board_info protocol "sim"
but evidently (from your output) nothing sets gdb_protocol.
Where is the code that does "target sim" to connect to the sim inside GDB? I can't seem to find that.
Pedro Alves
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-05-09 9:47 ` Pedro Alves
@ 2024-05-09 11:54 ` Bernd Edlinger
2024-05-09 12:05 ` Pedro Alves
0 siblings, 1 reply; 55+ messages in thread
From: Bernd Edlinger @ 2024-05-09 11:54 UTC (permalink / raw)
To: Pedro Alves, Tom Tromey; +Cc: gdb-patches
Hi Pedro,
On 5/9/24 11:47, Pedro Alves wrote:
> Hi Bernd,
>
> On 2024-05-09 09:47, Bernd Edlinger wrote:
>
>> Hi Pedro,
>>
>> Sorry, for the late reply.
>>
>> I am testing with riscv-unknown-elf target using simulator.
>> Before this patch I had:
>>
>> PASS: gdb.base/load-command.exp: probe for target native
>> PASS: gdb.base/load-command.exp: check initial value of the_variable
>> PASS: gdb.base/load-command.exp: manually change the_variable
>> PASS: gdb.base/load-command.exp: check manually changed value of the_variable
>> PASS: gdb.base/load-command.exp: reload: re-load binary
>> PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
>>
>> and now
>>
>> UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
>>
>> note: I use RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-sim"
>> but the test was working fine, because with this target
>> we use target sim, load, and can runto main, inspect variables, etc.
>>
>
> I'm trying to understand how the sim board works, to figure out what is the best course
> of action.
>
> I see that /usr/share/dejagnu/config/sim.exp has:
>
> set_board_info protocol "sim"
>
> but evidently (from your output) nothing sets gdb_protocol.
>
> Where is the code that does "target sim" to connect to the sim inside GDB? I can't seem to find that.
>
I think that file would be used when I run the gcc testsuite,
there I set the path to my riscv-toolchain,
and invoke this in the gcc-build directory:
make check -i RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-run"
or SIM=qemu-riscv32 for comparison.
But if I run the gdb testsuite I can do the same, but the SIM=parameter I could even
use SIM=true, is only a dummy, it must be executable, but it is not used in the end.
And I believe for gdb the following file is picked up instead:
binutils-gdb/gdb/testsuite/config/sim.exp
where I see:
proc gdb_target_sim { } {
global gdb_prompt
set target_sim_options "[board_info target gdb,target_sim_options]"
send_gdb "target sim $target_sim_options\n"
gdb_expect 60 {
-re "Connected to the simulator.*$gdb_prompt $" {
verbose "Set target to sim"
}
and in the log file of this test case from my reference build which was
from commit 354f8d0a1f98a1ade9b669f9200a9c23d22349fe
this test case looked quite normal:
(gdb) target sim ^M
Connected to the simulator.^M
(gdb) load^M
Loading section .text, size 0x163c lma 10120^M
Loading section .eh_frame, size 0x4 lma 11760^M
Loading section .init_array, size 0x8 lma 12000^M
Loading section .fini_array, size 0x8 lma 12008^M
Loading section .data, size 0x9a0 lma 12010^M
Loading section .sdata, size 0x28 lma 129b0^M
Start address 10120^M
Transfer rate: 65728 bits in <1 sec.^M
(gdb) delete breakpoints^M
(gdb) info breakpoints^M
No breakpoints, watchpoints, tracepoints, or catchpoints.^M
(gdb) break -qualified main^M
warning: Can not parse XML memory map; XML support was disabled at compile time^M
Breakpoint 1 at 0x10200: file ..../binutils-gdb/gdb/testsuite/gdb.base/load-command.c, line 22.^M
(gdb) target sim ^M
Connected to the simulator.^M
(gdb) load^M
Loading section .text, size 0x163c lma 10120^M
Loading section .eh_frame, size 0x4 lma 11760^M
Loading section .init_array, size 0x8 lma 12000^M
Loading section .fini_array, size 0x8 lma 12008^M
Loading section .data, size 0x9a0 lma 12010^M
Loading section .sdata, size 0x28 lma 129b0^M
Start address 10120^M
Transfer rate: 65728 bits in <1 sec.^M
(gdb) run ^M
Starting program: ..../gdb/testsuite/outputs/gdb.base/load-command/load-command ^M
^M
Breakpoint 1, main () at ..../binutils-gdb/gdb/testsuite/gdb.base/load-command.c:22^M
22 return 0;^M
(gdb) maint print target-stack^M
The current target stack is:^M
- sim (simulator)^M
- exec (Local exec file)^M
- None (None)^M
(gdb) PASS: gdb.base/load-command.exp: probe for target native
print/x the_variable^M
$1 = 0x1234^M
(gdb) PASS: gdb.base/load-command.exp: check initial value of the_variable
set the_variable = 0x5555^M
(gdb) PASS: gdb.base/load-command.exp: manually change the_variable
print/x the_variable^M
$2 = 0x5555^M
(gdb) PASS: gdb.base/load-command.exp: check manually changed value of the_variable
...
If you want I can help with step by step instructions
how to setting up a working riscv-cross-toolchain.
Thanks
Bernd.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-05-09 11:54 ` Bernd Edlinger
@ 2024-05-09 12:05 ` Pedro Alves
2024-05-09 13:19 ` Bernd Edlinger
0 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-05-09 12:05 UTC (permalink / raw)
To: Bernd Edlinger, Tom Tromey; +Cc: gdb-patches
On 2024-05-09 12:54, Bernd Edlinger wrote:
> On 5/9/24 11:47, Pedro Alves wrote:
> I think that file would be used when I run the gcc testsuite,
> there I set the path to my riscv-toolchain,
> and invoke this in the gcc-build directory:
> make check -i RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-run"
> or SIM=qemu-riscv32 for comparison.
>
> But if I run the gdb testsuite I can do the same, but the SIM=parameter I could even
> use SIM=true, is only a dummy, it must be executable, but it is not used in the end.
> And I believe for gdb the following file is picked up instead:
>
> binutils-gdb/gdb/testsuite/config/sim.exp
>
Ah, OK, that makes a lot more sense. I got confused with the "multi-sim" thing.
Since it's our file, it makes it makes easier.
I guess we should try sticking:
set_board_info gdb_protocol "sim"
in that file. Like the patchlet below. Could you give it a try, please?
From 843a53be479e22926a3dbecc76c5de6160796af4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Thu, 9 May 2024 13:01:53 +0100
Subject: [PATCH] gdb_protocol sim
Change-Id: I48a7afed004a3517b90220674fe5bc856fe7d09a
---
gdb/testsuite/config/sim.exp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gdb/testsuite/config/sim.exp b/gdb/testsuite/config/sim.exp
index 81736d25814..da2780e1d47 100644
--- a/gdb/testsuite/config/sim.exp
+++ b/gdb/testsuite/config/sim.exp
@@ -14,6 +14,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# If we left this empty, the core of the testsuite would think we were
+# connecting to the native target.
+set_board_info gdb_protocol "sim"
+
#
# gdb_target_sim
# Set gdb to target the simulator
base-commit: 5021daf303393722f58f4422d7ad53d526aa2d50
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-05-09 12:05 ` Pedro Alves
@ 2024-05-09 13:19 ` Bernd Edlinger
2024-05-09 13:31 ` Pedro Alves
0 siblings, 1 reply; 55+ messages in thread
From: Bernd Edlinger @ 2024-05-09 13:19 UTC (permalink / raw)
To: Pedro Alves, Tom Tromey; +Cc: gdb-patches
On 5/9/24 14:05, Pedro Alves wrote:
> On 2024-05-09 12:54, Bernd Edlinger wrote:
>> On 5/9/24 11:47, Pedro Alves wrote:
>> I think that file would be used when I run the gcc testsuite,
>> there I set the path to my riscv-toolchain,
>> and invoke this in the gcc-build directory:
>> make check -i RUNTESTFLAGS="--target_board=multi-sim SIM=riscv-unknown-elf-run"
>> or SIM=qemu-riscv32 for comparison.
>>
>> But if I run the gdb testsuite I can do the same, but the SIM=parameter I could even
>> use SIM=true, is only a dummy, it must be executable, but it is not used in the end.
>> And I believe for gdb the following file is picked up instead:
>>
>> binutils-gdb/gdb/testsuite/config/sim.exp
>>
>
> Ah, OK, that makes a lot more sense. I got confused with the "multi-sim" thing.
>
> Since it's our file, it makes it makes easier.
>
> I guess we should try sticking:
>
> set_board_info gdb_protocol "sim"
>
> in that file. Like the patchlet below. Could you give it a try, please?
>
Okay sure.
Here are the test results for rv32 and rv64 simulator,
- is from before your patch + with your patch.
rv32 test result differences:
-Test run by ed on Thu May 9 09:07:20 2024
+Test run by ed on Thu May 9 14:46:45 2024
Target is riscv-unknown-elf
Host is x86_64-pc-linux-gnu
@@ -20210,7 +20210,11 @@
PASS: gdb.base/list.exp: list around current line again
PASS: gdb.base/list.exp: testing repeated invocations with GDB's auto-repeat
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/load-command.exp ...
-UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
+PASS: gdb.base/load-command.exp: check initial value of the_variable
+PASS: gdb.base/load-command.exp: manually change the_variable
+PASS: gdb.base/load-command.exp: check manually changed value of the_variable
+PASS: gdb.base/load-command.exp: reload: re-load binary
+PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/logical.exp ...
PASS: gdb.base/logical.exp: evaluate x; variables x = 0; expecting 0
PASS: gdb.base/logical.exp: evaluate !x; variables x = 0; expecting 1
@@ -20950,11 +20954,7 @@
PASS: gdb.base/many-completions.exp: set max-completions unlimited
PASS: gdb.base/many-completions.exp: duplicate check
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/many-headers.exp ...
-gdb compile failed, ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/many-headers.c:20:10: fatal error: sys/mman.h
: No such file or directory
- 20 | #include <sys/mman.h>
- | ^~~~~~~~~~~~
-compilation terminated.
-UNTESTED: gdb.base/many-headers.exp: failed to prepare
+UNSUPPORTED: gdb.base/many-headers.exp: not native
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/max-depth-c++.exp ...
PASS: gdb.base/max-depth-c++.exp: exp='s1': depth=0: set print max-depth 0
PASS: gdb.base/max-depth-c++.exp: exp='s1': depth=0: p s1
@@ -75717,10 +75717,7 @@
collect2: error: ld returned 1 exit status
UNTESTED: gdb.multi/start-inferior-specific.exp: start-inferior-specific.exp
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/stop-all-on-exit.exp ...
-gdb compile failed, ..../riscv-unknown-elf/lib/gcc/riscv-unknown-elf/14.1.0/../../../../riscv-unknown-elf/bin/ld: /home/ed/gnu/binutils-build-riscv/gdb/testsuite/outputs/gdb.multi/stop-all-on-exit/stop-all-on-exit0.o: in function `main':
-..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/stop-all-on-exit.c:25:(.text+0xe): undefined reference to `sleep'
-collect2: error: ld returned 1 exit status
-UNTESTED: gdb.multi/stop-all-on-exit.exp: failed to prepare
+UNTESTED: gdb.multi/stop-all-on-exit.exp: the test is aimed at a native target
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/tids-gid-reset.exp ...
PASS: gdb.multi/tids-gid-reset.exp: successfully compiled posix threads test case
PASS: gdb.multi/tids-gid-reset.exp: single-inferior: before restart: info threads -gid
@@ -76379,23 +76376,23 @@
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/watch-reverse.exp ...
UNSUPPORTED: gdb.reverse/watch-reverse.exp: require failed: supports_reverse
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/fork-exec-gpu-to-non-gpu.exp ...
-UNSUPPORTED: gdb.rocm/fork-exec-gpu-to-non-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/fork-exec-gpu-to-non-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/fork-exec-non-gpu-to-gpu.exp ...
-UNSUPPORTED: gdb.rocm/fork-exec-non-gpu-to-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/fork-exec-non-gpu-to-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp ...
-UNSUPPORTED: gdb.rocm/multi-inferior-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/multi-inferior-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-exec.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-exec.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-exec.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-fork.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-fork.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-fork.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-multi-inferiors.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-multi-inferiors.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-multi-inferiors.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-warning-sigsegv.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-warning-sigsegv.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-warning-sigsegv.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/simple.exp ...
-UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rust/completion.exp ...
UNSUPPORTED: gdb.rust/completion.exp: require failed: can_compile rust
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rust/dwindex.exp ...
@@ -77180,7 +77177,6 @@
| pthread_atfork
UNTESTED: gdb.threads/break-while-running.exp: failed to prepare
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/check-libthread-db.exp ...
-UNSUPPORTED: gdb.threads/check-libthread-db.exp: require failed: istarget *-linux*
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-attach-detach.exp ...
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-new-thread-event.exp ...
Running ..../binutils-build-riscv/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-thread_db.exp ...
@@ -78630,14 +78626,14 @@
=== gdb Summary ===
-# of expected passes 71715
+# of expected passes 71720
# of unexpected failures 921
# of expected failures 39
# of unknown successes 4
# of known failures 66
# of unresolved testcases 257
-# of untested testcases 297
-# of unsupported tests 1231
+# of untested testcases 296
+# of unsupported tests 1230
# of paths in test names 4
# of duplicate test names 4
(END)
and here the rv64 test results:
@@ -1,4 +1,4 @@
-Test run by ed on Thu May 9 09:12:00 2024
+Test run by ed on Thu May 9 14:48:12 2024
Target is riscv64-unknown-elf
Host is x86_64-pc-linux-gnu
@@ -20437,7 +20437,11 @@
PASS: gdb.base/list.exp: list around current line again
PASS: gdb.base/list.exp: testing repeated invocations with GDB's auto-repeat
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/load-command.exp ...
-UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
+PASS: gdb.base/load-command.exp: check initial value of the_variable
+PASS: gdb.base/load-command.exp: manually change the_variable
+PASS: gdb.base/load-command.exp: check manually changed value of the_variable
+PASS: gdb.base/load-command.exp: reload: re-load binary
+PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/logical.exp ...
PASS: gdb.base/logical.exp: evaluate x; variables x = 0; expecting 0
PASS: gdb.base/logical.exp: evaluate !x; variables x = 0; expecting 1
@@ -21177,11 +21181,7 @@
PASS: gdb.base/many-completions.exp: set max-completions unlimited
PASS: gdb.base/many-completions.exp: duplicate check
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/many-headers.exp ...
-gdb compile failed, ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/many-headers.c:20:10: fatal error: sys/mman.h: No such file or directory
- 20 | #include <sys/mman.h>
- | ^~~~~~~~~~~~
-compilation terminated.
-UNTESTED: gdb.base/many-headers.exp: failed to prepare
+UNSUPPORTED: gdb.base/many-headers.exp: not native
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/max-depth-c++.exp ...
PASS: gdb.base/max-depth-c++.exp: exp='s1': depth=0: set print max-depth 0
PASS: gdb.base/max-depth-c++.exp: exp='s1': depth=0: p s1
@@ -76019,10 +76019,7 @@
collect2: error: ld returned 1 exit status
UNTESTED: gdb.multi/start-inferior-specific.exp: start-inferior-specific.exp
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/stop-all-on-exit.exp ...
-gdb compile failed, ..../riscv64-unknown-elf/lib/gcc/riscv64-unknown-elf/14.1.0/../../../../riscv64-unknown-elf/bin/ld: ..../binutils-build-riscv64/gdb/testsuite/outputs/gdb.multi/stop-all-on-exit/stop-all-on-exit0.o: in function `main':
-..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/stop-all-on-exit.c:25:(.text+0xe): undefined reference to `sleep'
-collect2: error: ld returned 1 exit status
-UNTESTED: gdb.multi/stop-all-on-exit.exp: failed to prepare
+UNTESTED: gdb.multi/stop-all-on-exit.exp: the test is aimed at a native target
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.multi/tids-gid-reset.exp ...
PASS: gdb.multi/tids-gid-reset.exp: successfully compiled posix threads test case
PASS: gdb.multi/tids-gid-reset.exp: single-inferior: before restart: info threads -gid
@@ -76681,23 +76678,23 @@
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/watch-reverse.exp ...
UNSUPPORTED: gdb.reverse/watch-reverse.exp: require failed: supports_reverse
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/fork-exec-gpu-to-non-gpu.exp ...
-UNSUPPORTED: gdb.rocm/fork-exec-gpu-to-non-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/fork-exec-gpu-to-non-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/fork-exec-non-gpu-to-gpu.exp ...
-UNSUPPORTED: gdb.rocm/fork-exec-non-gpu-to-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/fork-exec-non-gpu-to-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp ...
-UNSUPPORTED: gdb.rocm/multi-inferior-gpu.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/multi-inferior-gpu.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-exec.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-exec.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-exec.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-fork.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-fork.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-fork.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-multi-inferiors.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-multi-inferiors.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-multi-inferiors.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory-warning-sigsegv.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory-warning-sigsegv.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory-warning-sigsegv.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/precise-memory.exp ...
-UNSUPPORTED: gdb.rocm/precise-memory.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/precise-memory.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rocm/simple.exp ...
-UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (target platform is not Linux)
+UNSUPPORTED: gdb.rocm/simple.exp: require failed: allow_hipcc_tests (remote debugging)
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rust/completion.exp ...
UNSUPPORTED: gdb.rust/completion.exp: require failed: can_compile rust
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.rust/dwindex.exp ...
@@ -77482,7 +77479,6 @@
| pthread_atfork
UNTESTED: gdb.threads/break-while-running.exp: failed to prepare
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/check-libthread-db.exp ...
-UNSUPPORTED: gdb.threads/check-libthread-db.exp: require failed: istarget *-linux*
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-attach-detach.exp ...
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-new-thread-event.exp ...
Running ..../binutils-build-riscv64/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.threads/clone-thread_db.exp ...
@@ -78932,14 +78928,14 @@
=== gdb Summary ===
-# of expected passes 71993
+# of expected passes 71998
# of unexpected failures 916
# of expected failures 41
# of unknown successes 4
# of known failures 66
# of unresolved testcases 257
-# of untested testcases 304
-# of unsupported tests 1226
+# of untested testcases 303
+# of unsupported tests 1225
# of paths in test names 4
# of duplicate test names 4
(END)
Hmm, okay, it is better than now, but the test casese that are affected, would probably
be broken by this change, if my target toolchain would either have the -linux in the name,
or the newlib would have a sleep and/or support the #include <sys/mman.h>.
Thanks
Bernd.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-05-09 13:19 ` Bernd Edlinger
@ 2024-05-09 13:31 ` Pedro Alves
2024-05-09 15:01 ` Bernd Edlinger
0 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-05-09 13:31 UTC (permalink / raw)
To: Bernd Edlinger, Tom Tromey; +Cc: gdb-patches
On 2024-05-09 14:19, Bernd Edlinger wrote:
> Hmm, okay, it is better than now,
Indeed, seems strictly better. Some tests that are meant for the native target are now properly
skipped.
> but the test casese that are affected, would probably
> be broken by this change, if my target toolchain would either have the -linux in the name,
> or the newlib would have a sleep and/or support the #include <sys/mman.h>.
I am afraid I don't know what you mean by this. Why would they be broken? Can you clarify?
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-05-09 13:31 ` Pedro Alves
@ 2024-05-09 15:01 ` Bernd Edlinger
2024-05-09 15:49 ` Pedro Alves
0 siblings, 1 reply; 55+ messages in thread
From: Bernd Edlinger @ 2024-05-09 15:01 UTC (permalink / raw)
To: Pedro Alves, Tom Tromey; +Cc: gdb-patches
On 5/9/24 15:31, Pedro Alves wrote:
>
> On 2024-05-09 14:19, Bernd Edlinger wrote:
>> Hmm, okay, it is better than now,
>
> Indeed, seems strictly better. Some tests that are meant for the native target are now properly
> skipped.
>
>> but the test casese that are affected, would probably
>> be broken by this change, if my target toolchain would either have the -linux in the name,
>> or the newlib would have a sleep and/or support the #include <sys/mman.h>.
>
> I am afraid I don't know what you mean by this. Why would they be broken? Can you clarify?
What I mean is this:
there are in total 44 test cases failed to compile because of undefined reference to sleep, usleep or nanosleep
with your patch it is now one less.
But it would be piece of cake to implement some functions like
sleep(), usleep(), and nanosleep in the newlib, and then make the simulator simulate it.
Likewise there are currently 5 test cases failed to compile because of missing sys/mman.h header,
and with your patch it will be one less, but what if we want to add that to the
simulator, why cant we simulate a linux O/S?
What is so special in the one test case that changed the behavior, that
explains why the other 4 are good enough to try to include, and see if <sys/mman.h> works?
Bernd.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-05-09 15:01 ` Bernd Edlinger
@ 2024-05-09 15:49 ` Pedro Alves
2024-05-09 18:44 ` Bernd Edlinger
0 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-05-09 15:49 UTC (permalink / raw)
To: Bernd Edlinger, Tom Tromey; +Cc: gdb-patches
On 2024-05-09 16:01, Bernd Edlinger wrote:
> On 5/9/24 15:31, Pedro Alves wrote:
>>
>> On 2024-05-09 14:19, Bernd Edlinger wrote:
>>> Hmm, okay, it is better than now,
>>
>> Indeed, seems strictly better. Some tests that are meant for the native target are now properly
>> skipped.
>>
>>> but the test casese that are affected, would probably
>>> be broken by this change, if my target toolchain would either have the -linux in the name,
>>> or the newlib would have a sleep and/or support the #include <sys/mman.h>.
>>
>> I am afraid I don't know what you mean by this. Why would they be broken? Can you clarify?
>
> What I mean is this:
>
> there are in total 44 test cases failed to compile because of undefined reference to sleep, usleep or nanosleep
> with your patch it is now one less.
>
> But it would be piece of cake to implement some functions like
> sleep(), usleep(), and nanosleep in the newlib, and then make the simulator simulate it.
>
> Likewise there are currently 5 test cases failed to compile because of missing sys/mman.h header,
> and with your patch it will be one less, but what if we want to add that to the
> simulator, why cant we simulate a linux O/S?
>
> What is so special in the one test case that changed the behavior, that
> explains why the other 4 are good enough to try to include, and see if <sys/mman.h> works?
>
The gdb.base/load-command.exp testcase just doesn't make sense to test with the native
target, since with the native target, you don't use the "load" command to load programs.
So the testcase checks for that:
if [gdb_protocol_is_native] {
unsupported "the native target does not support the load command"
return
}
Normally, a testcase that checks whether it is testing with the native target does so because it is
testing some feature of the native target (the ptrace support, etc.), and so such an early check
isn't that much about whether sleep or sys/mman.h exists (they don't exist on native Windows,
for example, and there are native-only tests that you'd still want to run there).
If a testcase should run against the sim target and would no longer run because we set
gdb_protocol, then that would be a problem with the testcase itself, for using the wrong
predicate to skip itself. We've had plenty of cases of testcases using the wrong
predicate over the years, that is something that we're fixing pretty frequently, so
please don't be surprised if we need to do that after this change too.
Looking at gdb.base/many-headers.exp for example (the first in your previous message
that shows compile errors), it has:
if { [target_info gdb_protocol] != "" } {
# Even though the feature under features being tested are supported by
# gdbserver, the way this test is written doesn't make it easy with a
# remote target.
unsupported "not native"
return
}
Note the comment -- it is saying to skip testing on remote, but then it skips
on any board that sets gdb_protocol to anything (meaning, not native).
If this truly should only be skipped with remote targets, then it should instead
do this:
require gdb_protocol_is_remote
which is equivalent to:
if { [target_info gdb_protocol] == "remote" || [target_info gdb_protocol] == "extended-remote" } {
return
}
But, actually looking at the testcase, we see that it launches a program outside gdb, so that
the program crashes and generates a core (core_find), and then debugs the core under gdb,
with GDB stack limited to 4MB. From the commit's log (31aceee86308), it's the equivalent
of doing:
$ ulimit -s 4096
$ gdb -batch ./a.out core.saved
[New LWP 19379]
Segmentation fault (core dumped)
...
So, I don't really understand what the "features being tested are supported by gdbserver"
remark is alluding to, since gdbserver does not support loading cores. But then again,
the test doesn't even connect to the native target nor the remote target anyhow. So
we could let the testcase run when testing against a gdbserver board anyhow.
Maybe what the comment is trying to say is, not easy to test with a "remote target"
in dejagnu sense, i.e., when you have to connect to a remote host board running on a
separate system, as in that scenario it's not easy to set ulimit. Is so, a better
predicate to check would be "require !is_remote host" or "require isnative".
A similar analysis would have to be done to the other testcases.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-05-09 15:49 ` Pedro Alves
@ 2024-05-09 18:44 ` Bernd Edlinger
2024-05-10 10:52 ` [pushed] gdb sim testing, set gdb_protocol to "sim" Pedro Alves
0 siblings, 1 reply; 55+ messages in thread
From: Bernd Edlinger @ 2024-05-09 18:44 UTC (permalink / raw)
To: Pedro Alves, Tom Tromey; +Cc: gdb-patches
On 5/9/24 17:49, Pedro Alves wrote:
> On 2024-05-09 16:01, Bernd Edlinger wrote:
>> On 5/9/24 15:31, Pedro Alves wrote:
>>>
>>> On 2024-05-09 14:19, Bernd Edlinger wrote:
>>>> Hmm, okay, it is better than now,
>>>
>>> Indeed, seems strictly better. Some tests that are meant for the native target are now properly
>>> skipped.
>>>
>>>> but the test casese that are affected, would probably
>>>> be broken by this change, if my target toolchain would either have the -linux in the name,
>>>> or the newlib would have a sleep and/or support the #include <sys/mman.h>.
>>>
>>> I am afraid I don't know what you mean by this. Why would they be broken? Can you clarify?
>>
>> What I mean is this:
>>
>> there are in total 44 test cases failed to compile because of undefined reference to sleep, usleep or nanosleep
>> with your patch it is now one less.
>>
>> But it would be piece of cake to implement some functions like
>> sleep(), usleep(), and nanosleep in the newlib, and then make the simulator simulate it.
>>
>> Likewise there are currently 5 test cases failed to compile because of missing sys/mman.h header,
>> and with your patch it will be one less, but what if we want to add that to the
>> simulator, why cant we simulate a linux O/S?
>>
>> What is so special in the one test case that changed the behavior, that
>> explains why the other 4 are good enough to try to include, and see if <sys/mman.h> works?
>>
>
> The gdb.base/load-command.exp testcase just doesn't make sense to test with the native
> target, since with the native target, you don't use the "load" command to load programs.
> So the testcase checks for that:
>
> if [gdb_protocol_is_native] {
> unsupported "the native target does not support the load command"
> return
> }
>
> Normally, a testcase that checks whether it is testing with the native target does so because it is
> testing some feature of the native target (the ptrace support, etc.), and so such an early check
> isn't that much about whether sleep or sys/mman.h exists (they don't exist on native Windows,
> for example, and there are native-only tests that you'd still want to run there).
>
> If a testcase should run against the sim target and would no longer run because we set
> gdb_protocol, then that would be a problem with the testcase itself, for using the wrong
> predicate to skip itself. We've had plenty of cases of testcases using the wrong
> predicate over the years, that is something that we're fixing pretty frequently, so
> please don't be surprised if we need to do that after this change too.
>
> Looking at gdb.base/many-headers.exp for example (the first in your previous message
> that shows compile errors), it has:
>
> if { [target_info gdb_protocol] != "" } {
> # Even though the feature under features being tested are supported by
> # gdbserver, the way this test is written doesn't make it easy with a
> # remote target.
> unsupported "not native"
> return
> }
>
> Note the comment -- it is saying to skip testing on remote, but then it skips
> on any board that sets gdb_protocol to anything (meaning, not native).
> If this truly should only be skipped with remote targets, then it should instead
> do this:
>
> require gdb_protocol_is_remote
>
> which is equivalent to:
>
> if { [target_info gdb_protocol] == "remote" || [target_info gdb_protocol] == "extended-remote" } {
> return
> }
>
> But, actually looking at the testcase, we see that it launches a program outside gdb, so that
> the program crashes and generates a core (core_find), and then debugs the core under gdb,
> with GDB stack limited to 4MB. From the commit's log (31aceee86308), it's the equivalent
> of doing:
>
> $ ulimit -s 4096
> $ gdb -batch ./a.out core.saved
> [New LWP 19379]
> Segmentation fault (core dumped)
> ...
>
> So, I don't really understand what the "features being tested are supported by gdbserver"
> remark is alluding to, since gdbserver does not support loading cores. But then again,
> the test doesn't even connect to the native target nor the remote target anyhow. So
> we could let the testcase run when testing against a gdbserver board anyhow.
>
> Maybe what the comment is trying to say is, not easy to test with a "remote target"
> in dejagnu sense, i.e., when you have to connect to a remote host board running on a
> separate system, as in that scenario it's not easy to set ulimit. Is so, a better
> predicate to check would be "require !is_remote host" or "require isnative".
>
> A similar analysis would have to be done to the other testcases.
Ah, okay, thanks for the explanation.
I was probably confused that sim looks an feels like a kind of native target,
but it supports the load command, and the test cases can read and write
local files, etc. Therefore it is of course easy to fix things in the wrong way.
So I agree with your patch, and that any fallout can be fixed later
in the test cases when necessary.
Thanks
Bernd.
^ permalink raw reply [flat|nested] 55+ messages in thread* [pushed] gdb sim testing, set gdb_protocol to "sim"
2024-05-09 18:44 ` Bernd Edlinger
@ 2024-05-10 10:52 ` Pedro Alves
0 siblings, 0 replies; 55+ messages in thread
From: Pedro Alves @ 2024-05-10 10:52 UTC (permalink / raw)
To: Bernd Edlinger, Tom Tromey; +Cc: gdb-patches
On 2024-05-09 19:44, Bernd Edlinger wrote:
> Ah, okay, thanks for the explanation.
>
No problem.
> I was probably confused that sim looks an feels like a kind of native target,
> but it supports the load command, and the test cases can read and write
> local files, etc. Therefore it is of course easy to fix things in the wrong way.
> So I agree with your patch, and that any fallout can be fixed later
> in the test cases when necessary.
Great, cool. I've merged the patch, as below.
From 78b2db9e7f5a04fdc030a5c65f8873a2c3d3c187 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Thu, 9 May 2024 13:01:53 +0100
Subject: [PATCH] gdb sim testing, set gdb_protocol to "sim"
Bernd reported that when testing with riscv-unknown-elf target using
the simulator, before commit c7a2ee649115 ("gdb_is_target_native ->
gdb_protocol_is_native"), he had:
PASS: gdb.base/load-command.exp: probe for target native
PASS: gdb.base/load-command.exp: check initial value of the_variable
PASS: gdb.base/load-command.exp: manually change the_variable
PASS: gdb.base/load-command.exp: check manually changed value of the_variable
PASS: gdb.base/load-command.exp: reload: re-load binary
PASS: gdb.base/load-command.exp: reload: check initial value of the_variable
and now:
UNSUPPORTED: gdb.base/load-command.exp: the native target does not support the load command
The problem is that the sim board/config isn't setting gdb_protocol
anywhere, so gdb_protocol_is_native returns true.
This commit fixes it by making gdb/testsuite/config/sim.exp set
gdb_protocol to "sim".
Reported-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Tested-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Change-Id: I48a7afed004a3517b90220674fe5bc856fe7d09a
---
gdb/testsuite/config/sim.exp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gdb/testsuite/config/sim.exp b/gdb/testsuite/config/sim.exp
index 81736d25814..da2780e1d47 100644
--- a/gdb/testsuite/config/sim.exp
+++ b/gdb/testsuite/config/sim.exp
@@ -14,6 +14,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# If we left this empty, the core of the testsuite would think we were
+# connecting to the native target.
+set_board_info gdb_protocol "sim"
+
#
# gdb_target_sim
# Set gdb to target the simulator
base-commit: 408bc9c5fc757bd4b8adb1c3d54ecd672beaedb7
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread
* RE: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-04-19 15:13 ` [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native Pedro Alves
2024-04-19 18:50 ` Tom Tromey
@ 2024-04-22 8:25 ` Aktemur, Tankut Baris
2024-04-23 12:33 ` Pedro Alves
1 sibling, 1 reply; 55+ messages in thread
From: Aktemur, Tankut Baris @ 2024-04-22 8:25 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On Friday, April 19, 2024 5:14 PM, Pedro Alves wrote:
> Subject: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
In the title, "gdb_target_is_native" should be spelled "gdb_is_target_native".
Thanks
-Baris
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
2024-04-22 8:25 ` [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native Aktemur, Tankut Baris
@ 2024-04-23 12:33 ` Pedro Alves
0 siblings, 0 replies; 55+ messages in thread
From: Pedro Alves @ 2024-04-23 12:33 UTC (permalink / raw)
To: Aktemur, Tankut Baris, gdb-patches
On 2024-04-22 09:25, Aktemur, Tankut Baris wrote:
> On Friday, April 19, 2024 5:14 PM, Pedro Alves wrote:
>> Subject: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native
>
> In the title, "gdb_target_is_native" should be spelled "gdb_is_target_native".
Thanks Baris. I've fixed that locally.
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (8 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:56 ` Tom Tromey
2024-04-22 8:30 ` Aktemur, Tankut Baris
2024-04-19 15:13 ` [PATCH 11/12] Eliminate gdb_is_target_remote / gdb_is_target_native & friends Pedro Alves
` (2 subsequent siblings)
12 siblings, 2 replies; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
This is similar to the previous patch, but for gdb_protocol_is_remote.
gdb_is_target_remote and its MI cousin mi_is_target_remote, use "maint
print target-stack", which is unnecessary when checking whether
gdb_protocol is "remote" or "extended-remote" would do. Checking
gdb_protocol is more efficient, and can be done before starting GDB
and running to main, unlike gdb_is_target_remote/mi_is_target_remote.
This adds a new gdb_protocol_is_remote procedure, and uses it in place
of gdb_is_target_remote/mi_is_target_remote throughout.
There are no uses of gdb_is_target_remote/mi_is_target_remote left
after this. Those will be eliminated in a following patch.
In some spots, we no longer need to defer the check until after
starting GDB, so the patch adjusts accordingly.
Change-Id: I90267c132f942f63426f46dbca0b77dbfdf9d2ef
---
.../gdb.arch/aarch64-sme-core.exp.tcl | 12 ++++-----
.../aarch64-sme-regs-available.exp.tcl | 25 ++++++++++---------
.../aarch64-sme-regs-sigframe.exp.tcl | 25 ++++++++++---------
.../aarch64-sme-regs-unavailable.exp.tcl | 12 ++++-----
gdb/testsuite/gdb.arch/aarch64-sme-sanity.exp | 16 ++++++------
gdb/testsuite/gdb.base/cond-eval-mode.exp | 2 +-
gdb/testsuite/gdb.base/dprintf.exp | 2 +-
.../gdb.base/hbreak-in-shr-unsupported.exp | 6 ++---
gdb/testsuite/gdb.mi/mi-nonstop.exp | 2 +-
gdb/testsuite/gdb.python/py-evsignal.exp | 3 +--
.../gdb.reverse/finish-reverse-next.exp | 1 -
.../gdb.threads/break-while-running.exp | 2 +-
.../main-thread-exit-during-detach.exp | 2 +-
.../process-dies-while-handling-bp.exp | 2 +-
gdb/testsuite/gdb.trace/change-loc.exp | 10 ++++----
gdb/testsuite/gdb.trace/ftrace.exp | 2 +-
gdb/testsuite/gdb.trace/qtro.exp | 11 ++++----
gdb/testsuite/lib/gdb.exp | 11 +++++++-
18 files changed, 76 insertions(+), 70 deletions(-)
diff --git a/gdb/testsuite/gdb.arch/aarch64-sme-core.exp.tcl b/gdb/testsuite/gdb.arch/aarch64-sme-core.exp.tcl
index b4868d389b1..b9b83b93772 100644
--- a/gdb/testsuite/gdb.arch/aarch64-sme-core.exp.tcl
+++ b/gdb/testsuite/gdb.arch/aarch64-sme-core.exp.tcl
@@ -158,20 +158,20 @@ proc test_sme_core_file { id_start id_end } {
continue
}
- if ![runto_main] {
- untested "could not run to main"
- return -1
- }
-
# Check if we are talking to a remote target. If so, bail out,
# as right now remote targets can't communicate vector length (vl
# or svl) changes to gdb via the RSP. When this restriction is
# lifted, we can remove this guard.
- if {[gdb_is_target_remote]} {
+ if {[gdb_protocol_is_remote]} {
unsupported "aarch64 sve/sme tests not supported for remote targets"
return -1
}
+ if ![runto_main] {
+ untested "could not run to main"
+ return -1
+ }
+
generate_sme_core_files ${executable} ${binfile} $id $state $vl $svl
}
}
diff --git a/gdb/testsuite/gdb.arch/aarch64-sme-regs-available.exp.tcl b/gdb/testsuite/gdb.arch/aarch64-sme-regs-available.exp.tcl
index 450cb87021e..569d0b71340 100644
--- a/gdb/testsuite/gdb.arch/aarch64-sme-regs-available.exp.tcl
+++ b/gdb/testsuite/gdb.arch/aarch64-sme-regs-available.exp.tcl
@@ -18,6 +18,19 @@
load_lib aarch64-scalable.exp
+require is_aarch64_target
+require allow_aarch64_sve_tests
+require allow_aarch64_sme_tests
+
+# Check if we are talking to a remote target. If so, bail out, as
+# right now remote targets can't communicate vector length (vl or svl)
+# changes to gdb via the RSP. When this restriction is lifted, we can
+# remove this guard.
+if {[gdb_protocol_is_remote]} {
+ unsupported "aarch64 sve/sme tests not supported for remote targets"
+ return -1
+}
+
#
# Cycle through all ZA registers and pseudo-registers and validate that their
# contents are available for vector length SVL.
@@ -160,14 +173,6 @@ proc test_sme_registers_available { id_start id_end } {
return -1
}
- # Check if we are talking to a remote target. If so, bail out, as right now
- # remote targets can't communicate vector length (vl or svl) changes to gdb
- # via the RSP. When this restriction is lifted, we can remove this guard.
- if {[gdb_is_target_remote]} {
- unsupported "aarch64 sve/sme tests not supported for remote targets"
- return -1
- }
-
gdb_test_no_output "set print repeats 1"
set prctl_breakpoint "stop 1"
@@ -255,8 +260,4 @@ proc test_sme_registers_available { id_start id_end } {
}
}
-require is_aarch64_target
-require allow_aarch64_sve_tests
-require allow_aarch64_sme_tests
-
test_sme_registers_available $id_start $id_end
diff --git a/gdb/testsuite/gdb.arch/aarch64-sme-regs-sigframe.exp.tcl b/gdb/testsuite/gdb.arch/aarch64-sme-regs-sigframe.exp.tcl
index d79bd3969c9..8b61ddaafd6 100644
--- a/gdb/testsuite/gdb.arch/aarch64-sme-regs-sigframe.exp.tcl
+++ b/gdb/testsuite/gdb.arch/aarch64-sme-regs-sigframe.exp.tcl
@@ -17,6 +17,19 @@
load_lib aarch64-scalable.exp
+require is_aarch64_target
+require allow_aarch64_sve_tests
+require allow_aarch64_sme_tests
+
+# Check if we are talking to a remote target. If so, bail out, as
+# right now remote targets can't communicate vector length (vl or svl)
+# changes to gdb via the RSP. When this restriction is lifted, we can
+# remove this guard.
+if {[gdb_protocol_is_remote]} {
+ unsupported "aarch64 sve/sme tests not supported for remote targets"
+ return -1
+}
+
#
# Validate the state of registers in the signal frame for various states.
#
@@ -39,14 +52,6 @@ proc test_sme_registers_sigframe { id_start id_end } {
return -1
}
- # Check if we are talking to a remote target. If so, bail out, as right now
- # remote targets can't communicate vector length (vl or svl) changes to gdb
- # via the RSP. When this restriction is lifted, we can remove this guard.
- if {[gdb_is_target_remote]} {
- unsupported "aarch64 sve/sme tests not supported for remote targets"
- return -1
- }
-
set sigill_breakpoint "stop before SIGILL"
set handler_breakpoint "handler"
gdb_breakpoint [gdb_get_line_number $sigill_breakpoint]
@@ -183,8 +188,4 @@ proc test_sme_registers_sigframe { id_start id_end } {
}
}
-require is_aarch64_target
-require allow_aarch64_sve_tests
-require allow_aarch64_sme_tests
-
test_sme_registers_sigframe $id_start $id_end
diff --git a/gdb/testsuite/gdb.arch/aarch64-sme-regs-unavailable.exp.tcl b/gdb/testsuite/gdb.arch/aarch64-sme-regs-unavailable.exp.tcl
index 51488527ca8..77ff66c49c4 100644
--- a/gdb/testsuite/gdb.arch/aarch64-sme-regs-unavailable.exp.tcl
+++ b/gdb/testsuite/gdb.arch/aarch64-sme-regs-unavailable.exp.tcl
@@ -120,19 +120,19 @@ proc test_sme_registers_unavailable { id_start id_end } {
}
set binfile [standard_output_file ${executable}]
- if ![runto_main] {
- untested "could not run to main"
- return -1
- }
-
# Check if we are talking to a remote target. If so, bail out, as right now
# remote targets can't communicate vector length (vl or svl) changes to gdb
# via the RSP. When this restriction is lifted, we can remove this guard.
- if {[gdb_is_target_remote]} {
+ if {[gdb_protocol_is_remote]} {
unsupported "aarch64 sve/sme tests not supported for remote targets"
return -1
}
+ if ![runto_main] {
+ untested "could not run to main"
+ return -1
+ }
+
gdb_test_no_output "set print repeats 1"
set prctl_breakpoint "stop 1"
diff --git a/gdb/testsuite/gdb.arch/aarch64-sme-sanity.exp b/gdb/testsuite/gdb.arch/aarch64-sme-sanity.exp
index 51b3d225cdd..9643b110c3d 100644
--- a/gdb/testsuite/gdb.arch/aarch64-sme-sanity.exp
+++ b/gdb/testsuite/gdb.arch/aarch64-sme-sanity.exp
@@ -40,6 +40,14 @@ require is_aarch64_target
require allow_aarch64_sve_tests
require allow_aarch64_sme_tests
+# Check if we are talking to a remote target. If so, bail out, as right now
+# remote targets can't communicate vector length (vl or svl) changes to gdb
+# via the RSP. When this restriction is lifted, we can remove this guard.
+if {[gdb_protocol_is_remote]} {
+ unsupported "aarch64 sve/sme tests not supported for remote targets"
+ return -1
+}
+
set compile_flags {"debug" "macros" "additional_flags=-march=armv8.5-a+sve"}
standard_testfile
if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}]} {
@@ -50,14 +58,6 @@ if {![runto_main]} {
return -1
}
-# Check if we are talking to a remote target. If so, bail out, as right now
-# remote targets can't communicate vector length (vl or svl) changes to gdb
-# via the RSP. When this restriction is lifted, we can remove this guard.
-if {[gdb_is_target_remote]} {
- unsupported "aarch64 sve/sme tests not supported for remote targets"
- return -1
-}
-
# Adjust the repeat count for the test.
gdb_test_no_output "set print repeats 1" "adjust repeat count"
diff --git a/gdb/testsuite/gdb.base/cond-eval-mode.exp b/gdb/testsuite/gdb.base/cond-eval-mode.exp
index cd1b78bf2ab..0e98b8307ca 100644
--- a/gdb/testsuite/gdb.base/cond-eval-mode.exp
+++ b/gdb/testsuite/gdb.base/cond-eval-mode.exp
@@ -58,7 +58,7 @@ gdb_test_multiple $test_target $test_target {
# We now know that the target supports target-side conditional
# evaluation. Now make sure we can force-disable the
# ConditionalBreakpoints RSP feature.
-if [gdb_is_target_remote] {
+if [gdb_protocol_is_remote] {
gdb_test \
"set remote conditional-breakpoints-packet off" \
"Support for the 'ConditionalBreakpoints' packet on the current remote target is set to \"off\"."
diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
index 8b284a8d93d..649126f141b 100644
--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -217,7 +217,7 @@ gdb_test "set dprintf-style foobar" "Undefined item: \"foobar\"." \
# Test that force-disabling the BreakpointCommands RSP feature works
# as expected. dprintf relies on support for target-side breakpoint
# commands --- use it as proxy.
-if [gdb_is_target_remote] {
+if [gdb_protocol_is_remote] {
gdb_test \
"set remote breakpoint-commands-packet off" \
"Support for the 'BreakpointCommands' packet on the current remote target is set to \"off\"."
diff --git a/gdb/testsuite/gdb.base/hbreak-in-shr-unsupported.exp b/gdb/testsuite/gdb.base/hbreak-in-shr-unsupported.exp
index 8b078a69d10..e90f352b272 100644
--- a/gdb/testsuite/gdb.base/hbreak-in-shr-unsupported.exp
+++ b/gdb/testsuite/gdb.base/hbreak-in-shr-unsupported.exp
@@ -41,8 +41,6 @@ if {![runto_main]} {
return -1
}
-set is_target_remote [gdb_is_target_remote]
-
# Get main breakpoint out of the way.
delete_breakpoints
@@ -51,7 +49,7 @@ gdb_test_no_output "set breakpoint always-inserted on"
# Force-disable Z1 packets, in case the target actually supports
# these.
-if {$is_target_remote} {
+if {[gdb_protocol_is_remote]} {
gdb_test \
"set remote Z-packet off" \
"Use of Z packets on the current remote target is set to \"off\"."
@@ -79,7 +77,7 @@ gdb_test_multiple "hbreak -q main" $test {
}
-re "Hardware assisted breakpoint.*at.* file .*$srcfile, line.*$gdb_prompt $" {
set supports_hbreak 1
- if {$is_target_remote} {
+ if {[gdb_protocol_is_remote]} {
# Z-packets have been force-disabled, so this shouldn't
# happen.
fail $test
diff --git a/gdb/testsuite/gdb.mi/mi-nonstop.exp b/gdb/testsuite/gdb.mi/mi-nonstop.exp
index 922f5ea0a76..609fbec0e9f 100644
--- a/gdb/testsuite/gdb.mi/mi-nonstop.exp
+++ b/gdb/testsuite/gdb.mi/mi-nonstop.exp
@@ -126,7 +126,7 @@ mi_gdb_test "-thread-select 2" "\\^done.*" "select first worker thread"
mi_gdb_test "-gdb-set --thread 3 variable exit_first_thread=1" ".*\\^done" "ask the second thread to exit"
set test "wait for thread exit"
-if { [mi_is_target_remote] } {
+if { [gdb_protocol_is_remote] } {
# The remote protocol doesn't have support for thread exit
# notifications.
unsupported $test
diff --git a/gdb/testsuite/gdb.python/py-evsignal.exp b/gdb/testsuite/gdb.python/py-evsignal.exp
index aa87cb42fbd..83b351f8f39 100644
--- a/gdb/testsuite/gdb.python/py-evsignal.exp
+++ b/gdb/testsuite/gdb.python/py-evsignal.exp
@@ -13,8 +13,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-if {[target_info gdb_protocol] == "remote"
- || [target_info gdb_protocol] == "extended-remote"} {
+if {[gdb_protocol_is_remote]} {
# RuntimeError: Could not find event thread
kfail "python/12966" "Signal Thread 3"
return -1
diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
index 4ca670e270f..73a4124fab4 100644
--- a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
+++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
@@ -52,7 +52,6 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
}
runto_main
-set target_remote [gdb_is_target_remote]
if [supports_process_record] {
# Activate process record/replay.
diff --git a/gdb/testsuite/gdb.threads/break-while-running.exp b/gdb/testsuite/gdb.threads/break-while-running.exp
index aa56af9ac62..4bec753c235 100644
--- a/gdb/testsuite/gdb.threads/break-while-running.exp
+++ b/gdb/testsuite/gdb.threads/break-while-running.exp
@@ -53,7 +53,7 @@ proc test { update_thread_list always_inserted non_stop } {
# RSP, we can't issue commands until the target replies to vCont.
# Not an issue with the non-stop RSP variant, which has a
# non-blocking vCont.
- if {$non_stop=="off" && [gdb_is_target_remote]} {
+ if {$non_stop=="off" && [gdb_protocol_is_remote]} {
return -1
}
diff --git a/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp b/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp
index 15780adc118..2a9320a6914 100644
--- a/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp
+++ b/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp
@@ -120,7 +120,7 @@ proc run_test { spawn_inferior } {
# In both cases the stop arrives while GDB is processing the
# detach, however, for remote targets GDB doesn't report the stop,
# while for local targets GDB does report the stop.
- if {![gdb_is_target_remote]} {
+ if {![gdb_protocol_is_remote]} {
set stop_re "\\\[Thread.*exited\\\]\r\n"
} else {
set stop_re ""
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp b/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
index a4c50d1c1f1..e1bc6feea46 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
@@ -52,7 +52,7 @@ proc do_test { non_stop cond_bp_target } {
# Whether it's known that the test fails.
set should_kfail 0
- if {![gdb_is_target_remote]} {
+ if {![gdb_protocol_is_remote]} {
set should_kfail 1
} else {
if {!$cond_bp_target} {
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index cc9f77f2514..fb55153bfcb 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -288,16 +288,16 @@ proc tracepoint_install_in_trace_disabled { trace_type } {
global pcreg
global gdb_prompt
+ # This test only makes sense with remote targets.
+ if ![gdb_protocol_is_remote] {
+ return
+ }
+
clean_restart ${testfile}
if ![runto_main] {
return -1
}
- # This test only makes sense with the remote target.
- if ![gdb_is_target_remote] {
- return
- }
-
gdb_test_no_output "delete break 1"
# Set a tracepoint we'll never meet. Just to avoid the
diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
index 7f74a5a45cb..9b100ced8f5 100644
--- a/gdb/testsuite/gdb.trace/ftrace.exp
+++ b/gdb/testsuite/gdb.trace/ftrace.exp
@@ -189,7 +189,7 @@ proc test_fast_tracepoints {} {
# fast tracepoints RSP feature, and confirm fast tracepoints
# can no longer be downloaded.
set test "fast tracepoint could not be downloaded with the feature disabled"
- if [gdb_is_target_remote] {
+ if [gdb_protocol_is_remote] {
gdb_test "set remote fast-tracepoints-packet off"
gdb_test_multiple "tstart" $test {
diff --git a/gdb/testsuite/gdb.trace/qtro.exp b/gdb/testsuite/gdb.trace/qtro.exp
index 3693f249e26..60f73d7e5ef 100644
--- a/gdb/testsuite/gdb.trace/qtro.exp
+++ b/gdb/testsuite/gdb.trace/qtro.exp
@@ -20,6 +20,10 @@
load_lib trace-support.exp
+# Check whether we're testing with the remote or extended-remote
+# targets.
+require gdb_protocol_is_remote
+
standard_testfile
if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug nopie}]} {
@@ -30,12 +34,7 @@ if ![runto_main] {
return -1
}
-# Check whether we're testing with the remote or extended-remote
-# targets, and whether the target supports tracepoints.
-
-if ![gdb_is_target_remote] {
- return -1
-}
+# Check whether the target supports tracepoints.
if ![gdb_target_supports_trace] {
unsupported "current target does not support trace"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index c072a4502b4..f37d54b16be 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4700,7 +4700,8 @@ proc gdb_is_target_remote_prompt { prompt_regexp } {
#
# This is meant to be used on testcases that connect to targets
# different from the default board protocol. For most tests, you can
-# check whether gdb_protocol is "remote" or "extended-remote" instead.
+# check whether gdb_protocol is "remote" or "extended-remote" instead
+# (or call gdb_protocol_is_remote for either).
#
# NOTE: GDB must be running BEFORE this procedure is called!
@@ -4731,6 +4732,14 @@ proc gdb_protocol_is_native { } {
return [expr {[target_info gdb_protocol] == ""}]
}
+# Returns true if gdb_protocol is either "remote" or
+# "extended-remote".
+
+proc gdb_protocol_is_remote { } {
+ return [expr {[target_info gdb_protocol] == "remote"
+ || [target_info gdb_protocol] == "extended-remote"}]
+}
+
# Like istarget, but checks a list of targets.
proc is_any_target {args} {
foreach targ $args {
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote
2024-04-19 15:13 ` [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote Pedro Alves
@ 2024-04-19 18:56 ` Tom Tromey
2024-04-23 12:30 ` Pedro Alves
2024-04-22 8:30 ` Aktemur, Tankut Baris
1 sibling, 1 reply; 55+ messages in thread
From: Tom Tromey @ 2024-04-19 18:56 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> This is similar to the previous patch, but for gdb_protocol_is_remote.
Pedro> gdb_is_target_remote and its MI cousin mi_is_target_remote, use "maint
Pedro> print target-stack", which is unnecessary when checking whether
Pedro> gdb_protocol is "remote" or "extended-remote" would do. Checking
Pedro> gdb_protocol is more efficient, and can be done before starting GDB
Pedro> and running to main, unlike gdb_is_target_remote/mi_is_target_remote.
Pedro> This adds a new gdb_protocol_is_remote procedure, and uses it in place
Pedro> of gdb_is_target_remote/mi_is_target_remote throughout.
Pedro> There are no uses of gdb_is_target_remote/mi_is_target_remote left
Pedro> after this. Those will be eliminated in a following patch.
Pedro> In some spots, we no longer need to defer the check until after
Pedro> starting GDB, so the patch adjusts accordingly.
Makes sense to me, I have a question though.
Pedro> +# Check if we are talking to a remote target. If so, bail out, as
Pedro> +# right now remote targets can't communicate vector length (vl or svl)
Pedro> +# changes to gdb via the RSP. When this restriction is lifted, we can
Pedro> +# remove this guard.
Pedro> +if {[gdb_protocol_is_remote]} {
Pedro> + unsupported "aarch64 sve/sme tests not supported for remote targets"
Pedro> + return -1
Pedro> +}
Seems like this spot could just use "require !gdb_protocol_is_remote" --
any reason why you didn't do this?
Tom
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote
2024-04-19 18:56 ` Tom Tromey
@ 2024-04-23 12:30 ` Pedro Alves
0 siblings, 0 replies; 55+ messages in thread
From: Pedro Alves @ 2024-04-23 12:30 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi!
On 2024-04-19 19:56, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>
> Pedro> This is similar to the previous patch, but for gdb_protocol_is_remote.
> Pedro> gdb_is_target_remote and its MI cousin mi_is_target_remote, use "maint
> Pedro> print target-stack", which is unnecessary when checking whether
> Pedro> gdb_protocol is "remote" or "extended-remote" would do. Checking
> Pedro> gdb_protocol is more efficient, and can be done before starting GDB
> Pedro> and running to main, unlike gdb_is_target_remote/mi_is_target_remote.
>
> Pedro> This adds a new gdb_protocol_is_remote procedure, and uses it in place
> Pedro> of gdb_is_target_remote/mi_is_target_remote throughout.
>
> Pedro> There are no uses of gdb_is_target_remote/mi_is_target_remote left
> Pedro> after this. Those will be eliminated in a following patch.
>
> Pedro> In some spots, we no longer need to defer the check until after
> Pedro> starting GDB, so the patch adjusts accordingly.
>
> Makes sense to me, I have a question though.
>
> Pedro> +# Check if we are talking to a remote target. If so, bail out, as
> Pedro> +# right now remote targets can't communicate vector length (vl or svl)
> Pedro> +# changes to gdb via the RSP. When this restriction is lifted, we can
> Pedro> +# remove this guard.
> Pedro> +if {[gdb_protocol_is_remote]} {
> Pedro> + unsupported "aarch64 sve/sme tests not supported for remote targets"
> Pedro> + return -1
> Pedro> +}
>
> Seems like this spot could just use "require !gdb_protocol_is_remote" --
> any reason why you didn't do this?
Really no reason. I've done that now locally, in the three spots I could do it.
I also simplified the comment, as most of it is obvious/redundant:
+# Remote targets can't communicate vector length (vl or svl) changes
+# to GDB via the RSP.
+require !gdb_protocol_is_remote
^ permalink raw reply [flat|nested] 55+ messages in thread
* RE: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote
2024-04-19 15:13 ` [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote Pedro Alves
2024-04-19 18:56 ` Tom Tromey
@ 2024-04-22 8:30 ` Aktemur, Tankut Baris
2024-04-23 12:47 ` Pedro Alves
1 sibling, 1 reply; 55+ messages in thread
From: Aktemur, Tankut Baris @ 2024-04-22 8:30 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On Friday, April 19, 2024 5:14 PM, Pedro Alves wrote:
> Subject: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote
Similar to the previous patch, "gdb_target_is_remote" shall be spelled
"gdb_is_target_remote".
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index c072a4502b4..f37d54b16be 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4700,7 +4700,8 @@ proc gdb_is_target_remote_prompt { prompt_regexp } {
> #
> # This is meant to be used on testcases that connect to targets
> # different from the default board protocol. For most tests, you can
> -# check whether gdb_protocol is "remote" or "extended-remote" instead.
> +# check whether gdb_protocol is "remote" or "extended-remote" instead
> +# (or call gdb_protocol_is_remote for either).
> #
> # NOTE: GDB must be running BEFORE this procedure is called!
>
> @@ -4731,6 +4732,14 @@ proc gdb_protocol_is_native { } {
> return [expr {[target_info gdb_protocol] == ""}]
> }
>
> +# Returns true if gdb_protocol is either "remote" or
> +# "extended-remote".
> +
> +proc gdb_protocol_is_remote { } {
> + return [expr {[target_info gdb_protocol] == "remote"
> + || [target_info gdb_protocol] == "extended-remote"}]
> +}
> +
How about using `eq`, since this is string comparison? Also in the previous
patch in the comparison against "".
Regards
-Baris
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote
2024-04-22 8:30 ` Aktemur, Tankut Baris
@ 2024-04-23 12:47 ` Pedro Alves
2024-04-24 13:48 ` Aktemur, Tankut Baris
0 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-23 12:47 UTC (permalink / raw)
To: Aktemur, Tankut Baris, gdb-patches
On 2024-04-22 09:30, Aktemur, Tankut Baris wrote:
> On Friday, April 19, 2024 5:14 PM, Pedro Alves wrote:
>> Subject: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote
>
> Similar to the previous patch, "gdb_target_is_remote" shall be spelled
> "gdb_is_target_remote".
Thanks. I fixed that locally.
>
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index c072a4502b4..f37d54b16be 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -4700,7 +4700,8 @@ proc gdb_is_target_remote_prompt { prompt_regexp } {
>> #
>> # This is meant to be used on testcases that connect to targets
>> # different from the default board protocol. For most tests, you can
>> -# check whether gdb_protocol is "remote" or "extended-remote" instead.
>> +# check whether gdb_protocol is "remote" or "extended-remote" instead
>> +# (or call gdb_protocol_is_remote for either).
>> #
>> # NOTE: GDB must be running BEFORE this procedure is called!
>>
>> @@ -4731,6 +4732,14 @@ proc gdb_protocol_is_native { } {
>> return [expr {[target_info gdb_protocol] == ""}]
>> }
>>
>> +# Returns true if gdb_protocol is either "remote" or
>> +# "extended-remote".
>> +
>> +proc gdb_protocol_is_remote { } {
>> + return [expr {[target_info gdb_protocol] == "remote"
>> + || [target_info gdb_protocol] == "extended-remote"}]
>> +}
>> +
>
> How about using `eq`, since this is string comparison? Also in the previous
> patch in the comparison against "".
I just find that == reads a little bit better. There is no risk
that this ends up doing a numerical comparison, which would be the reason to
use string eq. Note that we use == when comparing with gdb_protocol pretty
much everywhere throughout the testsuite.
^ permalink raw reply [flat|nested] 55+ messages in thread* RE: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote
2024-04-23 12:47 ` Pedro Alves
@ 2024-04-24 13:48 ` Aktemur, Tankut Baris
0 siblings, 0 replies; 55+ messages in thread
From: Aktemur, Tankut Baris @ 2024-04-24 13:48 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On Tuesday, April 23, 2024 2:47 PM, Pedro Alves wrote:
> >> +# Returns true if gdb_protocol is either "remote" or
> >> +# "extended-remote".
> >> +
> >> +proc gdb_protocol_is_remote { } {
> >> + return [expr {[target_info gdb_protocol] == "remote"
> >> + || [target_info gdb_protocol] == "extended-remote"}]
> >> +}
> >> +
> >
> > How about using `eq`, since this is string comparison? Also in the previous
> > patch in the comparison against "".
>
> I just find that == reads a little bit better. There is no risk
> that this ends up doing a numerical comparison, which would be the reason to
> use string eq. Note that we use == when comparing with gdb_protocol pretty
> much everywhere throughout the testsuite.
I also think `==` is more readable than `eq`. As a background motivation of my
question, I was actually seeking if there is a coding rule about the use of ==
vs eq in the testsuite. Thanks for clarifying!
-Baris
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 11/12] Eliminate gdb_is_target_remote / gdb_is_target_native & friends
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (9 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:57 ` Tom Tromey
2024-04-19 15:13 ` [PATCH 12/12] Fix gdb.base/attach.exp --pid test skipping on native-extended-gdbserver Pedro Alves
2024-04-26 20:25 ` [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
After the previous patches, gdb_is_target_remote,
gdb_is_target_native, and mi_is_target_remote aren't used anywhere.
This commit eliminates them, along with now unnecessary helpers.
Change-Id: I54f9ae1f5aed3f640e5758731cf4954e6dbb1bee
---
gdb/testsuite/lib/gdb.exp | 68 --------------------------------
gdb/testsuite/lib/mi-support.exp | 9 -----
2 files changed, 77 deletions(-)
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f37d54b16be..03ca8f9a63a 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4657,74 +4657,6 @@ proc have_longjmp_probe {} {
return $have_probe
}
-# Helper for gdb_is_target_* procs. TARGET_NAME is the name of the target
-# we're looking for (used to build the test name). TARGET_STACK_REGEXP
-# is a regexp that will match the output of "maint print target-stack" if
-# the target in question is currently pushed. PROMPT_REGEXP is a regexp
-# matching the expected prompt after the command output.
-#
-# NOTE: GDB must be running BEFORE this procedure is called!
-
-proc gdb_is_target_1 { target_name target_stack_regexp prompt_regexp } {
- global gdb_spawn_id
-
- # Throw a Tcl error if gdb isn't already started.
- if {![info exists gdb_spawn_id]} {
- error "gdb_is_target_1 called with no running gdb instance"
- }
-
- set test "probe for target ${target_name}"
- gdb_test_multiple "maint print target-stack" $test \
- -prompt "$prompt_regexp" {
- -re "${target_stack_regexp}${prompt_regexp}" {
- pass $test
- return 1
- }
- -re "$prompt_regexp" {
- pass $test
- }
- }
- return 0
-}
-
-# Helper for gdb_is_target_remote where the expected prompt is variable.
-#
-# NOTE: GDB must be running BEFORE this procedure is called!
-
-proc gdb_is_target_remote_prompt { prompt_regexp } {
- return [gdb_is_target_1 "remote" ".*emote target using gdb-specific protocol.*" $prompt_regexp]
-}
-
-# Check whether we're testing with the remote or extended-remote
-# targets.
-#
-# This is meant to be used on testcases that connect to targets
-# different from the default board protocol. For most tests, you can
-# check whether gdb_protocol is "remote" or "extended-remote" instead
-# (or call gdb_protocol_is_remote for either).
-#
-# NOTE: GDB must be running BEFORE this procedure is called!
-
-proc gdb_is_target_remote { } {
- global gdb_prompt
-
- return [gdb_is_target_remote_prompt "$gdb_prompt $"]
-}
-
-# Check whether we're testing with the native target.
-#
-# This is meant to be used on testcases that connect to targets
-# different from the default board protocol. For most tests, you can
-# check whether gdb_protocol is the empty string instead.
-#
-# NOTE: GDB must be running BEFORE this procedure is called!
-
-proc gdb_is_target_native { } {
- global gdb_prompt
-
- return [gdb_is_target_1 "native" ".*native \\(Native process\\).*" "$gdb_prompt $"]
-}
-
# Returns true if gdb_protocol is empty, indicating use of the native
# target.
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index b3a27efb155..aa0f9df6c3a 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2863,15 +2863,6 @@ proc mi_skip_libstdcxx_probe_tests {} {
return [skip_libstdcxx_probe_tests_prompt "$mi_gdb_prompt$"]
}
-# Check whether we're testing with the remote or extended-remote
-# targets.
-
-proc mi_is_target_remote {} {
- global mi_gdb_prompt
-
- return [gdb_is_target_remote_prompt "$mi_gdb_prompt"]
-}
-
# Retrieve the value of EXP in the inferior, represented in format
# specified in FMT (using "printFMT"). DEFAULT is used as fallback if
# print fails. TEST is the test message to use. It can be omitted,
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH 12/12] Fix gdb.base/attach.exp --pid test skipping on native-extended-gdbserver
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (10 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 11/12] Eliminate gdb_is_target_remote / gdb_is_target_native & friends Pedro Alves
@ 2024-04-19 15:13 ` Pedro Alves
2024-04-19 18:59 ` Tom Tromey
2024-04-26 20:25 ` [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
12 siblings, 1 reply; 55+ messages in thread
From: Pedro Alves @ 2024-04-19 15:13 UTC (permalink / raw)
To: gdb-patches
When testing with the native-extended-gdbserver board,
gdb.base/attach.exp shows a couple failures, like so:
Running /home/pedro/gdb/src/gdb/testsuite/gdb.base/attach.exp ...
FAIL: gdb.base/attach.exp: do_command_attach_tests: gdb_spawn_attach_cmdline: start gdb with --pid
FAIL: gdb.base/attach.exp: do_command_attach_tests: gdb_spawn_attach_cmdline: info thread (no thread)
From gdb.log:
builtin_spawn /home/pedro/gdb/build/gdb/testsuite/../../gdb/gdb -nw -nx -q -iex set height 0 -iex set width 0 -data-directory /home/pedro/gdb/build
/gdb/data-directory -iex set auto-connect-native-target off -iex set sysroot -quiet --pid=2115260
Don't know how to attach. Try "help target".
(gdb) FAIL: gdb.base/attach.exp: do_command_attach_tests: gdb_spawn_attach_cmdline: start gdb with --pid
There is a check for [isnative] to skip the test on anything but
target native, but that is the wrong check. native-extended-gdbserver
is "isnative". Fix it by using a gdb_protocol check instead.
Change-Id: I37ee730b8d6f1913b12c118838f511bd1c0b3768
---
gdb/testsuite/gdb.base/attach.exp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 637f287f59e..831e11f96a6 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -445,7 +445,9 @@ proc_with_prefix do_command_attach_tests {} {
global gdb_prompt
global binfile
- if {![isnative]} {
+ # The --pid option is used to attach to a process using the native
+ # target.
+ if { ![gdb_protocol_is_native] } {
unsupported "command attach test"
return 0
}
--
2.43.2
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more
2024-04-19 15:13 [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more Pedro Alves
` (11 preceding siblings ...)
2024-04-19 15:13 ` [PATCH 12/12] Fix gdb.base/attach.exp --pid test skipping on native-extended-gdbserver Pedro Alves
@ 2024-04-26 20:25 ` Pedro Alves
12 siblings, 0 replies; 55+ messages in thread
From: Pedro Alves @ 2024-04-26 20:25 UTC (permalink / raw)
To: gdb-patches
On 2024-04-19 16:13, Pedro Alves wrote:
> Pedro Alves (12):
> Document conventions for describing packet syntax
> Centralize documentation of error and empty RSP responses
> Document "E.MESSAGE" RSP errors
> Windows: Fix run/attach hang after bad run/attach
> Fix "run" failure handling with GDBserver
> Improve vRun error reporting
> Fix "attach" failure handling with GDBserver
> gdbserver: Fix vAttach response when attaching is not supported
> gdb_target_is_native -> gdb_protocol_is_native
> gdb_target_is_remote -> gdb_protocol_is_remote
> Eliminate gdb_is_target_remote / gdb_is_target_native & friends
> Fix gdb.base/attach.exp --pid test skipping on
> native-extended-gdbserver
>
FYI, I've now merged this.
^ permalink raw reply [flat|nested] 55+ messages in thread