[PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more

[PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more

[Pedro Alves]

This is a "down the rabbit hole" series.

It started out as wanting to fix the hangs that you see on native
Windows when "attach" or "run" fails.  A subsequent "run" or "attach"
hangs/deadlocks forever.

Then I wrote tests for those.  And... they hit GDBserver bugs...

... so fixed the GDBserver bugs.

... but I want to be able to report textual errors to GDB using
E.MESSAGE responses.

... but GDB doesn't print the textual error in the case of "run"
failing.  But wait, textual errors in the RSP aren't documented...

... so I'm fixing that too.

... and then I notice gdb.base/attach.exp has a couple FAILs that
shouldn't be there, the tests should be skipped.  There is already a
similar test in the same file that is skipped, I should just copy the
predicate it uses to skip the test.  Oh wait, that is unnecessarily
heavy.  Let's fix that.

... but better fix that throughout the whole testsuite.

On the textual errors in RSP issue.  I am talking about the E.MESSAGE
responses.  I've been aware of those since forever, and all along I
thought they were documented, up until more recently, the fact that
most other people weren't familiar when them surprised me, at some
Cauldron GDB-related talk.  Recently, Sandra handed me CodeSourcery's
local GDB patches, to see if I could upstream any that relates to
Windows debugging, and there I saw the local patch that CS had been
carrying for many years that documents the "E.NAME.MESSAGE" response,
like so:

 +@item E.@var{name}@r{[}.@var{message}@r{]}
 +An error has occurred; @var{name} is the name of the error.  The name
 +may contain letters, numbers, and @samp{-} characters.  If present,
 +@var{message} is an error message, encoded using the escaped eight-bit
 +conventions for binary data described above.

... and also implements the GDB-side of parsing that.

Ahhh!  That _must_ have been were I first became aware of this "E."
response, back when I was at CS.  And this explains why we have this
comment upstream, too:

	  /* Always treat "E." as an error.  This will be used for
	     more verbose error messages, such as E.memtypes.  */

The CS patch goes on to say:

  +The protocol uses the following error names:
  +
  +@table @samp
  +
  +@item fatal
  +A fatal error has occurred; the stub will be unable to interact
  +further with @value{GDBN}.  Fatal errors should always include a
  +message explaining their cause.
  +
  +Any command may return this error.
  +
  +@item memtype
  +The memory addressed is of the wrong type for the given command.  For
  +example, a @samp{vFlashWrite} command applied to non-flash memory
  +elicits an @samp{E.memtype} error response.
  +
  +@end table
  +@end table

Ah, yeah, that matches.  vFlashWrite was contributed upstream by CS
(by Dan) back in 2006, in commit a76d924dffcb, here:

  https://sourceware.org/pipermail/gdb-patches/2006-September/047286.html

And that is exactly the patch that first made GDB understand "E.",
including that "E.memtypes" comment mentioned above.

OK, so seems like Dan/CS intended to upstream the "E.name.message"
format completely at some point, but never did.  And then, because GDB
prints whatever follows the "E." as the error message, GDBserver
started responding to errors with "E.MESSAGE" throughout and nowadays
it's kind of pervasive.

I think the ship for "E.NAME.MESSAGE" with binary encoded MESSAGE has
sailed though.  The format would work because "NAME" in E.NAME.MESSAGE
can't have a period.  So to find the message part, you'd look for the
second period.  But GDBserver has many instances of error messages
with two periods (and no "NAME" component), like e.g.:

 server.cc:      strcpy (own_buf, "E.Must select a single thread.");
 server.cc:      strcpy (own_buf, "E.No such thread.");
 server.cc:      sprintf (own_buf, "E.%s", exception.what ());
 server.cc:      strcpy (own_buf, "E.Must select a single thread.");
 server.cc:      strcpy (own_buf, "E.No such thread.");

Also, I can't find anywhere in GDB upstream or in CS's local patches
that handled "memtype" or "fatal" specially.  It seems like really the
NAME part in E.NAME.MESSAGE in CS-style error responses never found a
real use case.

So seems to me that best we can do is just document what we've been
using in practice for probably a decade and a half already, which is
just "E.ASCII_MESSAGE".  That is what this series does (among other
things).

Tested on x86_64 GNU/Linux {native, gdbserver, extended-gdbserver} and
Cygwin.

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

 gdb/doc/gdb.texinfo                           | 264 ++++--------------
 gdb/remote.c                                  |  67 ++++-
 .../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/attach-fail-twice.c    |  27 ++
 gdb/testsuite/gdb.base/attach-fail-twice.exp  |  94 +++++++
 gdb/testsuite/gdb.base/attach.exp             |  23 +-
 gdb/testsuite/gdb.base/cond-eval-mode.exp     |   2 +-
 gdb/testsuite/gdb.base/dprintf.exp            |   2 +-
 gdb/testsuite/gdb.base/foll-exec-mode.exp     |   4 +-
 .../gdb.base/hbreak-in-shr-unsupported.exp    |   6 +-
 gdb/testsuite/gdb.base/load-command.exp       |  11 +-
 gdb/testsuite/gdb.base/run-fail-twice.c       |  20 ++
 gdb/testsuite/gdb.base/run-fail-twice.exp     |  63 +++++
 gdb/testsuite/gdb.mi/mi-nonstop.exp           |   2 +-
 gdb/testsuite/gdb.multi/stop-all-on-exit.exp  |  16 +-
 gdb/testsuite/gdb.python/py-evsignal.exp      |   3 +-
 gdb/testsuite/gdb.python/py-inferior.exp      |   2 +-
 .../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.threads/threads-after-exec.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                     |  62 +---
 gdb/testsuite/lib/mi-support.exp              |   9 -
 gdb/windows-nat.c                             |  35 ++-
 gdbserver/server.cc                           |  54 ++--
 33 files changed, 468 insertions(+), 420 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/attach-fail-twice.c
 create mode 100644 gdb/testsuite/gdb.base/attach-fail-twice.exp
 create mode 100644 gdb/testsuite/gdb.base/run-fail-twice.c
 create mode 100644 gdb/testsuite/gdb.base/run-fail-twice.exp


base-commit: 0e6747d2a638693ad2f20e7929c8364913c87279
-- 
2.43.2

[PATCH 01/12] Document conventions for describing packet syntax

[Pedro Alves]

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

[PATCH 02/12] Centralize documentation of error and empty RSP responses

[Pedro Alves]

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

[PATCH 03/12] Document "E.MESSAGE" RSP errors

[Pedro Alves]

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

[PATCH 04/12] Windows: Fix run/attach hang after bad run/attach

[Pedro Alves]

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

[PATCH 05/12] Fix "run" failure handling with GDBserver

[Pedro Alves]

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

[PATCH 06/12] Improve vRun error reporting

[Pedro Alves]

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

[PATCH 07/12] Fix "attach" failure handling with GDBserver

[Pedro Alves]

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

[PATCH 08/12] gdbserver: Fix vAttach response when attaching is not supported

[Pedro Alves]

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

[PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native

[Pedro Alves]

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

[PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote

[Pedro Alves]

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

[PATCH 11/12] Eliminate gdb_is_target_remote / gdb_is_target_native & friends

[Pedro Alves]

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

[PATCH 12/12] Fix gdb.base/attach.exp --pid test skipping on native-extended-gdbserver

[Pedro Alves]

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

Re: [PATCH 01/12] Document conventions for describing packet syntax

[Eli Zaretskii]

> 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>

Re: [PATCH 02/12] Centralize documentation of error and empty RSP responses

[Eli Zaretskii]

> 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>

Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors

[Eli Zaretskii]

> 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>

Re: [PATCH 01/12] Document conventions for describing packet syntax

[Eli Zaretskii]

> 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.

Re: [PATCH 02/12] Centralize documentation of error and empty RSP responses

[Eli Zaretskii]

> 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.

Re: [PATCH 04/12] Windows: Fix run/attach hang after bad run/attach

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> This commit fixes both the "attach" and "run" paths, and the latter
Pedro> both the Cygwin and MinGW paths.  The tests mentioned above now pass
Pedro> on Cygwin.  Confirmed the fixes manually for MinGW GDB.

Looks good to me.  Thank you.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 05/12] Fix "run" failure handling with GDBserver

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> This commit fixes GDBserver by catching the error locally in
Pedro> handle_v_run.

Looks good.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 06/12] Improve vRun error reporting

[Tom Tromey]

>>>>> "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

Re: [PATCH 07/12] Fix "attach" failure handling with GDBserver

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> This commit fixes GDBserver by catching the error locally in
Pedro> handle_v_attach.

Pedro> Note we now let pid == 0 pass down to attach_inferior.  That is so we
Pedro> get a useful textual error message to report to GDB.

Pedro> This fixes a couple KFAILs in gdb.base/attach.exp.  Still, I thought
Pedro> it would be useful to add a new testcase specifically for this
Pedro> scenario, in case gdb.base/attach.exp is ever split and stops trying
Pedro> to attach again after a failed attach, with the same GDB session.

Pedro> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19558
Pedro> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31554

Makes sense to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 08/12] gdbserver: Fix vAttach response when attaching is not supported

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> In practice, this shouldn't trigger, as vAttach support is supposed to
Pedro> be reported via qSupported.  But it doesn't hurt to be pedantic here.

Makes sense, thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native

[Tom Tromey]

>>>>> "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

Re: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote

[Tom Tromey]

>>>>> "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

Re: [PATCH 11/12] Eliminate gdb_is_target_remote / gdb_is_target_native & friends

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> After the previous patches, gdb_is_target_remote,
Pedro> gdb_is_target_native, and mi_is_target_remote aren't used anywhere.
Pedro> This commit eliminates them, along with now unnecessary helpers.

Thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 12/12] Fix gdb.base/attach.exp --pid test skipping on native-extended-gdbserver

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> There is a check for [isnative] to skip the test on anything but
Pedro> target native, but that is the wrong check.  native-extended-gdbserver
Pedro> is "isnative".  Fix it by using a gdb_protocol check instead.

Looks good.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

RE: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native

[Aktemur, Tankut Baris]

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

RE: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote

[Aktemur, Tankut Baris]

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

Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors

[Andrew Burgess]

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

Re: [PATCH 06/12] Improve vRun error reporting

[Alexandra Petlanova Hajkova]

[-- 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.

Re: [PATCH 02/12] Centralize documentation of error and empty RSP responses

[Pedro Alves]

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

Re: [PATCH 01/12] Document conventions for describing packet syntax

[Pedro Alves]

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>
> 

Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors

[Pedro Alves]

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

Re: [PATCH 01/12] Document conventions for describing packet syntax

[Pedro Alves]

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.  

Re: [PATCH 02/12] Centralize documentation of error and empty RSP responses

[Eli Zaretskii]

> 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>

Re: [PATCH 01/12] Document conventions for describing packet syntax

[Eli Zaretskii]

> 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.

Re: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote

[Pedro Alves]

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

Re: [PATCH 09/12] gdb_target_is_native -> gdb_protocol_is_native

[Pedro Alves]

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.

Re: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote

[Pedro Alves]

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.

RE: [PATCH 10/12] gdb_target_is_remote -> gdb_protocol_is_remote

[Aktemur, Tankut Baris]

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

Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors

[Pedro Alves]

[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

Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors

[Eli Zaretskii]

> 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.

Re: [PATCH 00/12] Fix attach/run failure handling - gdbserver & Windows, document "E.MESSAGE" RSP errors, more

[Pedro Alves]

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.

Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors

[Andrew Burgess]

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