From patchwork Tue May 21 16:18:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alexandra_H=C3=A1jkov=C3=A1?= X-Patchwork-Id: 90619 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CFD59384AB47 for ; Tue, 21 May 2024 16:18:48 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail0.khirnov.net (mail0.khirnov.net [IPv6:2001:67c:1138:4304::3]) by sourceware.org (Postfix) with ESMTPS id 3909A3858D1E for ; Tue, 21 May 2024 16:18:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3909A3858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=khirnov.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=khirnov.net ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3909A3858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:67c:1138:4304::3 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716308305; cv=none; b=lMgDCPK46Am0yd0zsIrbNurcAYTY3clJFYLjmyMdkZoF8JI5BL07rPfIYlLcpFz/N7nWZePI3aMR4edjJUKLnyuilPRVTz+CAPP0rkrGO9P0hH4SBm3AngrGh8FsRNeUptdX7m7Tm/5Sm2jhJ30RXrKGAb8hw2L2w6DOdspExaY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716308305; c=relaxed/simple; bh=1HoITwqSEfaV9+Rxo088wRJR705zTsiewzKie5fm8kQ=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=A9IzvTmZwnTpWqPtWtVh68Hb1LqPnksbgAJXJNhhb+U6C1pxWCOhvAwEMrXq8BaDpjH/k7/BJ4cDB6UlS9Xc5wIYLpHeWbuqKbtCjBQ58hQT4xcQKSGHr+sUBDPoP6XnZrx+pKZ6zWL6Au4RGcLOy/kZJBpivB8/Dq2uj+izNSk= ARC-Authentication-Results: i=1; server2.sourceware.org Authentication-Results: mail0.khirnov.net; dkim=pass (2048-bit key; unprotected) header.d=khirnov.net header.i=@khirnov.net header.a=rsa-sha256 header.s=mail header.b=XMJzEa1l; dkim-atps=neutral Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 88471240DAC; Tue, 21 May 2024 18:18:19 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavis, port 10024) with ESMTP id N9NdRe0RZfGw; Tue, 21 May 2024 18:18:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=khirnov.net; s=mail; t=1716308295; bh=1HoITwqSEfaV9+Rxo088wRJR705zTsiewzKie5fm8kQ=; h=From:To:Cc:Subject:Date:From; b=XMJzEa1lCkAp30Ame1G3jXRq82WpskP7/3zUS4fgJXGvGwE2mLWXngmZ0TTOvnQXx qZESGG+Inuy3l5oqSZEfjoGvG0RJojLztrsG/aAWH9mjFoWT3pn5LUNAx0Q3gDIoyl b+c7A58jLwgo8A1rnckacm2IBCwLfAc3xKd+gUvhXEYK/4da5tPneva4DN/7wQsGse ohRkVKtdA6widMGUkq0UlcQ51mFr9Kv30pUNxCULQuC8z0ebN1sGtBZ0pDtcYJkxT/ LcSD3VITH7AJQvj+55fin0cCk1Kh+heqw+ALxck+wV3mIO7EiPpsKmEVtVUu8RMhJE YQb+djsXhrHxQ== Received: from fedora.khirnov.net (ip-244-183.pel.cz [IPv6:2002:b061:f0a:201::d5e2:f4b7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "sasshka SMTP cert", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id BB2F42404E5; Tue, 21 May 2024 18:18:15 +0200 (CEST) Received: from fedora.redhat.com (localhost [127.0.0.1]) by fedora.khirnov.net (Postfix) with ESMTP id BE2B295A0FC; Tue, 21 May 2024 18:18:12 +0200 (CEST) From: =?utf-8?q?Alexandra_H=C3=A1jkov=C3=A1?= To: gdb-patches@sourceware.org Cc: ahajkova@redhat.com, Tom Tromey Subject: [PATCH v3] Add "error_message+" feature to qSupported Date: Tue, 21 May 2024 18:18:10 +0200 Message-ID: <20240521161812.146008-1-ahajkova@khirnov.net> X-Mailer: git-send-email 2.45.0 MIME-Version: 1.0 X-Spam-Status: No, score=-13.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, JMQ_SPF_NEUTRAL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org From: Alexandra Hájková Check if the gdbserver GDB is communicating with supports responding with the error message in an E.errtext format to GDB's packets. 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 --- v3: - Improved documentation. - Simplified the handling of the feature, GDB send the feature request to the gdbserver, so gdbserver know it can always reply with 'E.errtext', but I dropped sending a response if gdbserver is going to be sending errors in such a way. GDB can handle both versions of the errors now and do not need to know if gdbserver supports E.errtext response. gdb/doc/gdb.texinfo | 31 +++++++++++++++++-- gdb/remote.c | 75 +++++++++++++++++++++++++++------------------ gdbserver/server.cc | 3 ++ gdbserver/server.h | 5 +++ 4 files changed, 82 insertions(+), 32 deletions(-) diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 61f91ef4ad6..b15dca84cb9 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -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 @@ -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 @@ -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 @@ -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: @@ -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:: diff --git a/gdb/remote.c b/gdb/remote.c index 42b446c7e27..d4ddd3b2998 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.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 }; @@ -2503,7 +2515,7 @@ add_packet_config_cmd (const unsigned int which_packet, const char *name, code). When ACCEPT_MSG is true error messages can also take the form E.msg (where msg is any arbitrary string). */ static packet_result -packet_check_result (const char *buf, bool accept_msg) +packet_check_result (const char *buf) { if (buf[0] != '\0') { @@ -2518,17 +2530,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. */ @@ -2542,9 +2551,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 @@ -2557,7 +2566,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: @@ -5818,6 +5827,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; @@ -5936,6 +5947,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 ()); @@ -8890,7 +8905,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 ()); @@ -9200,7 +9215,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 ()); @@ -9652,7 +9667,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 @@ -9807,7 +9822,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 @@ -12000,7 +12015,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: @@ -15672,7 +15687,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. */ @@ -15873,29 +15888,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 */ @@ -16264,6 +16276,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. */ { diff --git a/gdbserver/server.cc b/gdbserver/server.cc index 789af36d9a4..c306d51e848 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. */ @@ -4375,6 +4377,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 ();