Skip to content

Commit

Permalink
Add "error_message+" feature to qSupported
Browse files Browse the repository at this point in the history 
Add a new 'error_message' feature to the qSupported packet. When GDB
supports this feature then gdbserver is able to send
errors in the E.errtext format for the qRcmd and m packets.

Update qRcmd packet and m packets documentation as qRcmd newly
accepts errors in a E.errtext format.
Previously these two packets didn't support E.errtext style errors.

Approved-By: Tom Tromey <[email protected]>
Approved-By: Andrew Burgess <[email protected]>
  • Loading branch information
@sasshka
sasshka committed Jun 12, 2024
1 parent 7065d0a commit ddb3f3d
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 34 deletions.
31 changes: 29 additions & 2 deletions gdb/doc/gdb.texinfo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42970,7 +42970,9 @@ server was able to read only part of the region of memory.

Unlike most packets, this packet does not support
@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
error reply}).
error reply}) by default. Stubs should be careful to only send such a
reply if @value{GDBN} reported support for it with the
@code{error-message} feature (@pxref{error-message}).

@item M @var{addr},@var{length}:@var{XX@dots{}}
@cindex @samp{M} packet
Expand Down Expand Up @@ -44480,7 +44482,9 @@ A command response with the hex encoded output string @var{OUTPUT}.

Unlike most packets, this packet does not support
@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
error reply}).
error reply}) by default. Stubs should be careful to only send such a
reply if @value{GDBN} reported support for it with the
@code{error-message} feature (@pxref{error-message}).

(Note that the @code{qRcmd} packet's name is separated from the
command by a @samp{,}, not a @samp{:}, contrary to the naming
Expand Down Expand Up @@ -44627,6 +44631,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{error-message}
@item error-message
This feature indicates whether @value{GDBN} supports accepting a reply
in @samp{E.@var{errtext}} format (@xref{textual error reply}) from the
@samp{qRcmd} and @samp{m} packets. These packets, historically,
didn't support @samp{E.@var{errtext}}, and older versions of
@value{GDBN} didn't expect to see a reply in this format.

New packets should be written to support @samp{E.@var{errtext}}
regardless of this feature being true or not.
@end table

Stubs should ignore any unknown values for
Expand Down Expand Up @@ -44910,6 +44925,11 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No

@item @samp{error-message}
@tab No
@tab @samp{+}
@tab No

@end multitable

These are the currently defined stub features, in more detail:
Expand Down Expand Up @@ -45143,6 +45163,13 @@ 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 error-message
The remote stub supports replying with an error in a
@samp{E.@var{errtext}} (@xref{textual error reply}) format from the
@samp{m} and @samp{qRcmd} packets. It is not usually necessary to
send this feature back to @value{GDBN} in the @samp{qSupported} reply,
@value{GDBN} will always support @samp{E.@var{errtext}} format replies
if it sent the @samp{error-message} feature.
@end table

@item qSymbol::
Expand Down
78 changes: 46 additions & 32 deletions gdb/remote.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,18 @@ enum {
/* Support for the qIsAddressTagged packet. */
PACKET_qIsAddressTagged,

/* Support for accepting error message in a E.errtext format.
This allows every remote packet to return E.errtext.
This feature only exists to fix a backwards compatibility issue
with the qRcmd and m packets. Historically, these two packets didn't
support E.errtext style errors, but when this feature is on
these two packets can receive E.errtext style errors.
All new packets should be written to always accept E.errtext style
errors, and so they should not need to check for this feature. */
PACKET_accept_error_message,

PACKET_MAX
};

Expand Down Expand Up @@ -2502,10 +2514,9 @@ add_packet_config_cmd (const unsigned int which_packet, const char *name,
PACKET_ERROR case.
An error packet can always take the form Exx (where xx is a hex
code). When ACCEPT_MSG is true error messages can also take the
form E.msg (where msg is any arbitrary string). */
code). */
static packet_result
packet_check_result (const char *buf, bool accept_msg)
packet_check_result (const char *buf)
{
if (buf[0] != '\0')
{
Expand All @@ -2520,17 +2531,14 @@ packet_check_result (const char *buf, bool accept_msg)
/* Not every request accepts an error in a E.msg form.
Some packets accepts only Enn, in this case E. is not
defined and E. is treated as PACKET_OK. */
if (accept_msg)
/* Always treat "E." as an error. This will be used for
more verbose error messages, such as E.memtypes. */
if (buf[0] == 'E' && buf[1] == '.')
{
/* Always treat "E." as an error. This will be used for
more verbose error messages, such as E.memtypes. */
if (buf[0] == 'E' && buf[1] == '.')
{
if (buf[2] != '\0')
return packet_result::make_textual_error (buf + 2);
else
return packet_result::make_textual_error ("no error provided");
}
if (buf[2] != '\0')
return packet_result::make_textual_error (buf + 2);
else
return packet_result::make_textual_error ("no error provided");
}

/* The packet may or may not be OK. Just assume it is. */
Expand All @@ -2544,9 +2552,9 @@ packet_check_result (const char *buf, bool accept_msg)
}

static packet_result
packet_check_result (const gdb::char_vector &buf, bool accept_msg)
packet_check_result (const gdb::char_vector &buf)
{
return packet_check_result (buf.data (), accept_msg);
return packet_check_result (buf.data ());
}

packet_result
Expand All @@ -2559,7 +2567,7 @@ remote_features::packet_ok (const char *buf, const int which_packet)
&& config->support == PACKET_DISABLE)
internal_error (_("packet_ok: attempt to use a disabled packet"));

packet_result result = packet_check_result (buf, true);
packet_result result = packet_check_result (buf);
switch (result.status ())
{
case PACKET_OK:
Expand Down Expand Up @@ -5820,6 +5828,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 },
{ "error-message", PACKET_ENABLE, remote_supported_packet,
PACKET_accept_error_message },
};

static char *remote_support_xml;
Expand Down Expand Up @@ -5938,6 +5948,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 ());

Expand Down Expand Up @@ -8892,7 +8906,7 @@ remote_target::send_g_packet ()
xsnprintf (rs->buf.data (), get_remote_packet_size (), "g");
putpkt (rs->buf);
getpkt (&rs->buf);
packet_result result = packet_check_result (rs->buf, true);
packet_result result = packet_check_result (rs->buf);
if (result.status () == PACKET_ERROR)
error (_("Could not read registers; remote failure reply '%s'"),
result.err_msg ());
Expand Down Expand Up @@ -9202,7 +9216,7 @@ remote_target::store_registers_using_G (const struct regcache *regcache)
bin2hex (regs, p, rsa->sizeof_g_packet);
putpkt (rs->buf);
getpkt (&rs->buf);
packet_result pkt_status = packet_check_result (rs->buf, true);
packet_result pkt_status = packet_check_result (rs->buf);
if (pkt_status.status () == PACKET_ERROR)
error (_("Could not write registers; remote failure reply '%s'"),
pkt_status.err_msg ());
Expand Down Expand Up @@ -9654,7 +9668,7 @@ 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);
if (result.status () == PACKET_ERROR)
return TARGET_XFER_E_IO;
/* Reply describes memory byte by byte, each byte encoded as two hex
Expand Down Expand Up @@ -9809,7 +9823,7 @@ remote_target::remote_send_printf (const char *format, ...)
rs->buf[0] = '\0';
getpkt (&rs->buf);

return packet_check_result (rs->buf, true).status ();
return packet_check_result (rs->buf).status ();
}

/* Flash writing can take quite some time. We'll set
Expand Down Expand Up @@ -12002,7 +12016,7 @@ remote_target::rcmd (const char *command, struct ui_file *outbuf)
remote_console_output (buf + 1, outbuf);
continue;
}
packet_result result = packet_check_result (buf, false);
packet_result result = packet_check_result (buf);
switch (result.status ())
{
case PACKET_UNKNOWN:
Expand Down Expand Up @@ -15682,7 +15696,7 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
getpkt (&rs->buf);

/* Verify if the request was successful. */
return packet_check_result (rs->buf, true).status () == PACKET_OK;
return packet_check_result (rs->buf).status () == PACKET_OK;
}

/* Implement the "is_address_tagged" target_ops method. */
Expand Down Expand Up @@ -15883,29 +15897,26 @@ static void
test_packet_check_result ()
{
std::string buf = "E.msg";
packet_result result = packet_check_result (buf.data (), true);
packet_result result = packet_check_result (buf.data ());

SELF_CHECK (result.status () == PACKET_ERROR);
SELF_CHECK (strcmp(result.err_msg (), "msg") == 0);

result = packet_check_result ("E01", true);
result = packet_check_result ("E01");
SELF_CHECK (result.status () == PACKET_ERROR);
SELF_CHECK (strcmp(result.err_msg (), "01") == 0);

SELF_CHECK (packet_check_result ("E1", true).status () == PACKET_OK);
SELF_CHECK (packet_check_result ("E1").status () == PACKET_OK);

SELF_CHECK (packet_check_result ("E000", true).status () == PACKET_OK);
SELF_CHECK (packet_check_result ("E000").status () == PACKET_OK);

result = packet_check_result ("E.", true);
result = packet_check_result ("E.");
SELF_CHECK (result.status () == PACKET_ERROR);
SELF_CHECK (strcmp(result.err_msg (), "no error provided") == 0);

SELF_CHECK (packet_check_result ("some response", true).status () == PACKET_OK);
SELF_CHECK (packet_check_result ("some response").status () == PACKET_OK);

SELF_CHECK (packet_check_result ("", true).status () == PACKET_UNKNOWN);

result = packet_check_result ("E.msg", false);
SELF_CHECK (result.status () == PACKET_OK);
SELF_CHECK (packet_check_result ("").status () == PACKET_UNKNOWN);
}
} // namespace selftests
#endif /* GDB_SELF_TEST */
Expand Down Expand Up @@ -16274,6 +16285,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,
"error-message", "error-message", 0);

/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
Expand Down
3 changes: 3 additions & 0 deletions gdbserver/server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2699,6 +2699,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. */
Expand Down Expand Up @@ -4364,6 +4366,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);

Expand Down
5 changes: 5 additions & 0 deletions gdbserver/server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ struct client_state
/* If true, memory tagging features are supported. */
bool memory_tagging_feature = false;

/* If true then E.errtext style errors are supported everywhere,
including for the qRcmd and m packet. When false E.errtext 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 ();
Expand Down

0 comments on commit ddb3f3d

Please sign in to comment.