* [PATCH] Add "error_message+" feature to qSupported
@ 2024-04-27 8:52 Alexandra Hájková
2024-04-27 10:03 ` Eli Zaretskii
2024-04-29 14:17 ` Tom Tromey
0 siblings, 2 replies; 3+ messages in thread
From: Alexandra Hájková @ 2024-04-27 8:52 UTC (permalink / raw)
To: gdb-patches; +Cc: ahajkova
From: Alexandra Hájková <ahajkova@redhat.com>
Check if the gdbserver GDB is communicating with supports
responding with the error message in an E.message format to GDB's
packets.
Add a new 'error_message' feature to the qSupported packet. When GDB
and gdbserver support this feature then gdbserver is able to send
errors in the E.message format for the qRcmd and m packets.
Update qRcmd packet and m packets documentation as qRcmd newly
accepts errors in a E.message format.
Previously these two packets didn't support E.message style errors.
---
gdb/doc/gdb.texinfo | 27 ++++++++++++++++++++++
gdb/remote.c | 56 +++++++++++++++++++++++++++++++++++++++++++--
gdbserver/server.cc | 6 +++++
gdbserver/server.h | 5 ++++
4 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 92731c510b2..c589c05db58 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42888,6 +42888,11 @@ 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
+@item E.errtext
+Alternatively, an error can be sent as a string. This reply is only valid
+if the 'accept-error-message' feature (@pxref{accept-error-message}) is enabled.
@end table
Unlike most packets, this packet does not support
@@ -44398,6 +44403,13 @@ 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 E.errtext
+Alternatively, an error can be sent as a string. This reply is only valid
+if the 'accept-error-message' feature (@pxref{accept-error-message}) is enabled.
+@item @w{}
@end table
Unlike most packets, this packet does not support
@@ -44549,6 +44561,17 @@ including @samp{exec-events+} in its @samp{qSupported} reply.
@item vContSupported
This feature indicates whether @value{GDBN} wants to know the
supported actions in the reply to @samp{vCont?} packet.
+
+@anchor{accept-error-message}
+@item accept-error-message
+This feature indicates whether @value{GDBN} supports accepting
+a reply to a request in an @code{E.message} format including
+@samp{accept-error-message+} in its @samp{qSupported} reply. This
+feature only covers the @code{qRcmd} and @code{m} packets, and exists for
+backwards compatibility reasons. Those packets, historically, didn't support
+@code{E.message}. New packets should be written to support
+@code{E.message} regardless of this feature being true or not.
+
@end table
Stubs should ignore any unknown values for
@@ -45065,6 +45088,10 @@ inspected, if @samp{qIsAddressTagged} (@pxref{qIsAddressTagged}) packet
is not supported by the stub. Access to the @file{/proc/@var{pid}/smaps}
file is done via @samp{vFile} requests.
+@item accept-error-message
+The remote stub supports replying with an error in a
+@code{E.message} format.
+
@end table
@item qSymbol::
diff --git a/gdb/remote.c b/gdb/remote.c
index d425e558422..1cc445a0de3 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -377,6 +377,18 @@ enum {
/* Support for the qIsAddressTagged packet. */
PACKET_qIsAddressTagged,
+ /* Support for accepting error message in a E.message format.
+ This allows every remote packet to return E.message.
+
+ This feature only exists to fix a backwards compatibility issue
+ with the qRcmd and m packets. Historically, these two packets didn't
+ support E.message style errors, but when this feature is on
+ these two packets can receive E.message style errors.
+
+ All new packets should be written to always accepts E.message style
+ errors, and so they should not need to check for this feature. */
+ PACKET_accept_error_message,
+
PACKET_MAX
};
@@ -798,6 +810,21 @@ struct remote_features
bool remote_memory_tagging_p () const
{ return packet_support (PACKET_memory_tagging_feature) == PACKET_ENABLE; }
+ /* Returns true if accepting E.message in the packet response is supported,
+ false otherwise.
+
+ Don't call this function. This function, and the feature it wraps
+ around only exists to fix a backwards compatibility issue with the
+ qRcmd and m packets. Historically, these two packets didn't
+ support E.message style errors, but when this feature is on (this
+ function returns true) these two packets can receive E.message style
+ errors.
+
+ All new packets should be written to always accepts E.message style
+ errors, and so they should not need to check for this feature. */
+ bool accept_error_message_p () const
+ { return packet_support (PACKET_accept_error_message) == PACKET_ENABLE; }
+
/* Reset all packets back to "unknown support". Called when opening a
new connection to a remote target. */
void reset_all_packet_configs_support ();
@@ -1389,6 +1416,20 @@ class remote_target : public process_stratum_target
bool start_remote_1 (int from_tty, int extended_p);
+ /* Don't call this function. This function, and the feature it wraps
+ around only exists to fix a backwards compatibility issue with the
+ qRcmd and m packets. Historically, these two packets didn't
+ support E.message style errors, but when this feature is on (this
+ function returns true) these two packets can receive E.message style
+ errors.
+
+ All new packets should be written to always accepts E.message style
+ errors, and so they should not need to check for this feature. */
+ bool supports_error_message ()
+ {
+ return m_features.accept_error_message_p ();
+ }
+
/* The remote state. Don't reference this directly. Use the
get_remote_state method instead. */
remote_state m_remote_state;
@@ -5815,6 +5856,8 @@ static const struct protocol_feature remote_protocol_features[] = {
{ "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed },
{ "memory-tagging", PACKET_DISABLE, remote_supported_packet,
PACKET_memory_tagging_feature },
+ { "accept-error-message", PACKET_DISABLE, remote_supported_packet,
+ PACKET_accept_error_message },
};
static char *remote_support_xml;
@@ -5933,6 +5976,10 @@ remote_target::remote_query_supported ()
!= PACKET_DISABLE))
remote_query_supported_append (&q, remote_support_xml);
+ if (m_features.packet_set_cmd_state (PACKET_accept_error_message)
+ != AUTO_BOOLEAN_FALSE)
+ remote_query_supported_append (&q, "error_message+");
+
q = "qSupported:" + q;
putpkt (q.c_str ());
@@ -9649,7 +9696,8 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr,
*p = '\0';
putpkt (rs->buf);
getpkt (&rs->buf);
- packet_result result = packet_check_result (rs->buf, false);
+ packet_result result = packet_check_result (rs->buf,
+ supports_error_message ());
if (result.status () == PACKET_ERROR)
return TARGET_XFER_E_IO;
/* Reply describes memory byte by byte, each byte encoded as two hex
@@ -11996,7 +12044,8 @@ remote_target::rcmd (const char *command, struct ui_file *outbuf)
remote_console_output (buf + 1); /* 'O' message from stub. */
continue;
}
- packet_result result = packet_check_result (buf, false);
+ packet_result result = packet_check_result (buf,
+ supports_error_message ());
if (strcmp (buf, "OK") == 0)
break;
else if (result.status () == PACKET_UNKNOWN)
@@ -16255,6 +16304,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (PACKET_qIsAddressTagged,
"qIsAddressTagged", "memory-tagging-address-check", 0);
+ add_packet_config_cmd (PACKET_accept_error_message,
+ "accept-error-message", "accept-error-message", 0);
+
/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 789af36d9a4..3aac361f39d 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -2710,6 +2710,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_memory_tagging ())
cs.memory_tagging_feature = true;
}
+ else if (feature == "error_message+")
+ cs.error_message_supported = true;
else
{
/* Move the unknown features all together. */
@@ -2839,6 +2841,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_memory_tagging ())
strcat (own_buf, ";memory-tagging+");
+ if (cs.error_message_supported)
+ strcat (own_buf, ";accept-error-message+");
+
/* Reinitialize components as needed for the new connection. */
hostio_handle_new_gdb_connection ();
target_handle_new_gdb_connection ();
@@ -4375,6 +4380,7 @@ captured_main (int argc, char *argv[])
cs.hwbreak_feature = 0;
cs.vCont_supported = 0;
cs.memory_tagging_feature = false;
+ cs.error_message_supported = false;
remote_open (port);
diff --git a/gdbserver/server.h b/gdbserver/server.h
index 0074818c6df..c39241c960d 100644
--- a/gdbserver/server.h
+++ b/gdbserver/server.h
@@ -192,6 +192,11 @@ struct client_state
/* If true, memory tagging features are supported. */
bool memory_tagging_feature = false;
+ /* If true then E.message style errors are supported everywhere,
+ including for the qRcmd and m packet. When false E.message errors
+ are not supported with qRcmd and m packets, but are still supported
+ everywhere else. This is for backward compatibility reasons. */
+ bool error_message_supported = false;
};
client_state &get_client_state ();
--
2.44.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Add "error_message+" feature to qSupported
2024-04-27 8:52 [PATCH] Add "error_message+" feature to qSupported Alexandra Hájková
@ 2024-04-27 10:03 ` Eli Zaretskii
2024-04-29 14:17 ` Tom Tromey
1 sibling, 0 replies; 3+ messages in thread
From: Eli Zaretskii @ 2024-04-27 10:03 UTC (permalink / raw)
To: Alexandra Hájková; +Cc: gdb-patches
> From: Alexandra Hájková <ahajkova@khirnov.net>
> Cc: ahajkova@redhat.com
> Date: Sat, 27 Apr 2024 10:52:26 +0200
>
> From: Alexandra Hájková <ahajkova@redhat.com>
>
> Check if the gdbserver GDB is communicating with supports
> responding with the error message in an E.message format to GDB's
> packets.
>
> Add a new 'error_message' feature to the qSupported packet. When GDB
> and gdbserver support this feature then gdbserver is able to send
> errors in the E.message format for the qRcmd and m packets.
>
> Update qRcmd packet and m packets documentation as qRcmd newly
> accepts errors in a E.message format.
> Previously these two packets didn't support E.message style errors.
> ---
> gdb/doc/gdb.texinfo | 27 ++++++++++++++++++++++
> gdb/remote.c | 56 +++++++++++++++++++++++++++++++++++++++++++--
> gdbserver/server.cc | 6 +++++
> gdbserver/server.h | 5 ++++
> 4 files changed, 92 insertions(+), 2 deletions(-)
Thanks.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 92731c510b2..c589c05db58 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -42888,6 +42888,11 @@ 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
> +@item E.errtext
> +Alternatively, an error can be sent as a string. This reply is only valid
Two spaces between sentences, please (here and elsewhere in the
patch).
> +if the 'accept-error-message' feature (@pxref{accept-error-message}) is enabled.
^^^^^^^^^^^^^^^^^^^^^^
This should be @code{accept-error-message} instead of the quotes.
> +Alternatively, an error can be sent as a string. This reply is only valid
> +if the 'accept-error-message' feature (@pxref{accept-error-message}) is enabled.
^^^^^^^^^^^^^^^^^^^^^^
Likewise.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Add "error_message+" feature to qSupported
2024-04-27 8:52 [PATCH] Add "error_message+" feature to qSupported Alexandra Hájková
2024-04-27 10:03 ` Eli Zaretskii
@ 2024-04-29 14:17 ` Tom Tromey
1 sibling, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2024-04-29 14:17 UTC (permalink / raw)
To: Alexandra Hájková; +Cc: gdb-patches, ahajkova
>>>>> Alexandra Hájková <ahajkova@khirnov.net> writes:
> +@item E @var{NN}
> +@var{NN} is errno
I don't think it's correct to say this is errno. For one thing, errno
values aren't interchangeable across systems... but also I don't think
gdb has ever really treated them that way.
> + if (m_features.packet_set_cmd_state (PACKET_accept_error_message)
> + != AUTO_BOOLEAN_FALSE)
> + remote_query_supported_append (&q, "error_message+");
Looking at the existing features, I think it's more normal to use "-" to
separate words, not "_"; or alternatively "camel case". Personally I
perfer "-".
> + if (cs.error_message_supported)
> + strcat (own_buf, ";accept-error-message+");
It seems strange for the response to not just be "error-message+".
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-29 14:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-27 8:52 [PATCH] Add "error_message+" feature to qSupported Alexandra Hájková
2024-04-27 10:03 ` Eli Zaretskii
2024-04-29 14:17 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).