From: Andrew Burgess Date: Fri, 24 Jan 2025 16:12:55 +0000 (+0000) Subject: gdb/remote: add 'binary-upload' feature to guard 'x' packet use X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9b381fd11184d1932c0eaefd83ee4ea5c7c280f9;p=thirdparty%2Fbinutils-gdb.git gdb/remote: add 'binary-upload' feature to guard 'x' packet use This mailing list discussion: https://inbox.sourceware.org/gdb-patches/CAOp6jLYD0g-GUsx7jhO3g8H_4pHkB6dkh51cbyDT-5yMfQwu+A@mail.gmail.com highlighted the following issue with GDB's 'x' packet implementation. Unfortunately, LLDB also has an 'x' packet, but their implementation is different to GDB's and so targets that have implemented LLDB's 'x' packet are incompatible with GDB. The above thread is specifically about the 'rr' tool, but there could be other remote targets out there that have this problem. The difference between LLDB and GDB is that GDB expects a 'b' prefix on the reply data, while LLDB does not. The 'b' is important as it allows GDB to distinguish between an empty reply (which will be a 'b' prefix with no trailing data) and an unsupported packet (which will be a completely empty packet). It is not clear to me how LLDB distinguishes these two cases. See for discussion of the 'x' packet: https://inbox.sourceware.org/gdb-patches/cover.1710343840.git.tankut.baris.aktemur@intel.com/#r with the part specific to the 'b' marker in: https://inbox.sourceware.org/gdb-patches/87msq82ced.fsf@redhat.com/ I propose that we add a new feature 'binary-upload' which can be reported by a stub in its qSupported reply. By default this feature is "off", meaning GDB will not use the 'x' packet unless a stub advertises this feature. I have updated gdbserver to send 'binary-upload+', and when I examine the gdbserver log I can see this feature being sent back, and then GDB will use the 'x' packet. When connecting to an older gdbserver, the feature is not sent, and GDB does not try to use the 'x' packet at all. I also built the latest version of `rr` and tested using current HEAD of master, where I see problems like this: (rr) x/10i main 0x401106
: Cannot access memory at address 0x401106 Then tested using this patched version of GDB, and now I see: (rr) x/10i main 0x401106
: push %rbp 0x401107 : mov %rsp,%rbp 0x40110a : mov 0x2f17(%rip),%rax # 0x404028 ... etc ... and looking in the remote log I see GDB is now using the 'm' packet instead of the 'x' packet. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32593 Reviewed-By: Eli Zaretskii Reviewed-By: Tankut Baris Aktemur --- diff --git a/gdb/NEWS b/gdb/NEWS index 3db90719917..e678de55ce6 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -52,6 +52,14 @@ show riscv numeric-register-names ** New constant PARAM_COLOR represents color type of a value of a object. Parameter's value is instance. +* New remote packets + +binary-upload in qSupported reply + If the stub sends back 'binary-upload+' in it's qSupported reply, + then GDB will, where possible, make use of the 'x' packet. If the + stub doesn't report this feature supported, then GDB will not use + the 'x' packet. + *** Changes in GDB 16 * Support for Nios II targets has been removed as this architecture diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 2909fc71510..ff9fe298be3 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -43555,6 +43555,10 @@ and @var{length} is a multiple of the word size, the stub is free to use byte accesses, or not. For this reason, this packet may not be suitable for accessing memory-mapped I/O devices. +@value{GDBN} will only use this packet if the stub reports the +@samp{binary-upload} feature is supported in its @samp{qSupported} +reply (@pxref{qSupported}). + Reply: @table @samp @item b @var{XX@dots{}} @@ -45202,6 +45206,11 @@ These are the currently defined stub features and their properties: @tab @samp{+} @tab No +@item @samp{binary-upload} +@tab No +@tab @samp{-} +@tab No + @end multitable These are the currently defined stub features, in more detail: @@ -45448,6 +45457,9 @@ The remote stub supports replying with an error in a 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. + +@item binary-upload +The remote stub supports the @samp{x} packet (@pxref{x packet}). @end table @item qSymbol:: diff --git a/gdb/remote.c b/gdb/remote.c index 64622dbfcdf..f3687fbaa70 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -5847,6 +5847,7 @@ static const struct protocol_feature remote_protocol_features[] = { PACKET_memory_tagging_feature }, { "error-message", PACKET_ENABLE, remote_supported_packet, PACKET_accept_error_message }, + { "binary-upload", PACKET_DISABLE, remote_supported_packet, PACKET_x }, }; static char *remote_support_xml; diff --git a/gdbserver/server.cc b/gdbserver/server.cc index c1b18cc947e..0ad27d5e830 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -2757,7 +2757,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) "PacketSize=%x;QPassSignals+;QProgramSignals+;" "QStartupWithShell+;QEnvironmentHexEncoded+;" "QEnvironmentReset+;QEnvironmentUnset+;" - "QSetWorkingDir+", + "QSetWorkingDir+;binary-upload+", PBUFSIZ - 1); if (target_supports_catch_syscall ())