From patchwork Thu Jan 23 19:36:45 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 105321 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 3083C3858D26 for ; Thu, 23 Jan 2025 19:37:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3083C3858D26 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=CKsT21PM X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 1A1D03858D26 for ; Thu, 23 Jan 2025 19:36:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1A1D03858D26 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1A1D03858D26 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737661013; cv=none; b=BPBavKd3LwrTbyUxVqtmdLBauseW4qVtXOySvzdP+/sXoRybEwmz0qLVQeBtIikgv4ODyMHV74PkgghbjzkVLI9fWhdeu6F8bGHsA5minN9K5HhfbA/ZmA0ROPme+kvAxQ0ygff7ZE5HSWmvnJ35Xm59B//GplLXkm/A/jumjcM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737661013; c=relaxed/simple; bh=mQIOoilH7yy2LXQSxHUT1Mc1XJmHxDGPMIUsiXnqfS8=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Q0Lk0HCaSu0TX0hu1C0D4+tfFX+wKKG5mBaVCiG2yAPutjSbJyATsYOGdnNYOPmxd5EkvF2zf/LMKWhWmDcQiYstyX+1N/aLJMbCvkNMRu9xWxTH+WMiYduK961o4CrVdv3/VQ+uPnAWE8oZVNF4YtDOwUsrP4lOGQYhvLjxssY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1A1D03858D26 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1737661012; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=OoJz94u1p7oKYYQGc7LvH0p3eudSjDg4H9UlgGOEwvM=; b=CKsT21PMb9FjKJRihqC3I0LiKp6ZnsqlNchSx0VfySid4PqHQDvLwFFt3Ok/GWF8KvSJyf JKIKUL9lxW0CYi5VsTU/v7cwt4qZJ1vEM1tcQjPGOxZ+thi5PRdlbFmty1Ty1YoLkAnNXH xSjyis4drF/MaTNtooUjDPHvuT+ZZJU= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-648-N32NjeYRP0iKfdCKqXQ5sg-1; Thu, 23 Jan 2025 14:36:51 -0500 X-MC-Unique: N32NjeYRP0iKfdCKqXQ5sg-1 X-Mimecast-MFC-AGG-ID: N32NjeYRP0iKfdCKqXQ5sg Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-388d1f6f3b2so613155f8f.0 for ; Thu, 23 Jan 2025 11:36:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737661010; x=1738265810; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OoJz94u1p7oKYYQGc7LvH0p3eudSjDg4H9UlgGOEwvM=; b=k5ndovwGKvxDgFebY/Bk2J9QfhZqQibrUyMQVD5jbizqEDAj9IMm8G612+1t4DkFbV 7S78RK2BkBBXZz0hvIgBXbYn8aK8Dov6F46mkFWO8Jd18W7J0UFxocu+JqyauzyGg79R zWL6luMkxM2JegixPDN6b75Q7tH/KrHUdBeNsffEt6YaHVIOLXhY36wIjjOKUmRsyfIy JsqXWqx6NpsJGyWdM9FXFwwUtRdYH51c6rZsxcIEDs+k5d6eizddgyYKYl43EIZz3u8P dyaa3BpvhJyJbuKTic7IHE7L2rtsRk8osDJVmEHTL0InZvU2qa8id5ksJiGZso6IW62v Lyxw== X-Gm-Message-State: AOJu0Yy0pIHL5hvuWK9rANvPh7BTUbMco/Wv24S76oT2f2F3iGksHtCi ShCezcgDRWbBm1+u2371kLXRY/FuRV6ZTxOI52G8qZXDmtDynXXzqpqgpVdX8VedEjMg2bXZdIy FyZOjfmkJ4Yvj6rsWdfkA9BjCX8lOXSz/3Vae2l6EABKQ8sOnnFF2tNPLuwsjK+B1k0wFqsbcFh +wxq8gLjvCg4MkdcN7J4tRjQ1WU+8+dPw5dBMS1uyBPa0= X-Gm-Gg: ASbGncsJux49uuYBUU+dkrm/h7s+Fqgkq8/E8aapOIU3RVQIHy+y3uzAfHV02JJHxaD NF6/wlmADc0uNfsH1ifaaciU/XBKkB7yb3NjO/q4jyP9QKPKArjM0Hhwul0+h2iXLwSjObdhwZ0 GEHlu0zJXNwonMG41FqmYF/z+04wiotfCfgBi2MAm+R2Gkasb6wQNKcDb06OTIB6vlDZXU4cQb3 lpAMGJ0VnLHFO0dfS1Ow78g5FubspE8KEPoWjU11gDulW/AGdeomJsMFHGNiybrqT05uNqGjY2D N34FYnVChBsmyj8nGv8= X-Received: by 2002:a05:6000:178e:b0:385:fd24:3303 with SMTP id ffacd0b85a97d-38bf55c5f54mr24665061f8f.0.1737661009610; Thu, 23 Jan 2025 11:36:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IErXbKZ4KW5H6xZAK8mZ/OFkciOyVyCzWaQ2RpDmCr7AFe5+RTb/pTLtHPie0LCs5Obkm9kUg== X-Received: by 2002:a05:6000:178e:b0:385:fd24:3303 with SMTP id ffacd0b85a97d-38bf55c5f54mr24665043f8f.0.1737661009132; Thu, 23 Jan 2025 11:36:49 -0800 (PST) Received: from localhost (44.226.159.143.dyn.plus.net. [143.159.226.44]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-438bd573245sm1366125e9.33.2025.01.23.11.36.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jan 2025 11:36:48 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: tankut.baris.aktemur@intel.com, Robert O'Callahan , Andrew Burgess Subject: [PATCH] gdb/remote: probe for 'x' packet support with a zero length request Date: Thu, 23 Jan 2025 19:36:45 +0000 Message-ID: <3183bc4875167009274313529badbb1685042c76.1737660927.git.aburgess@redhat.com> X-Mailer: git-send-email 2.47.1 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: vUjOYujVErOrJUvqiis_d8X0BmAY8Ie6gt5h8WCvOI0_1737661010 X-Mimecast-Originator: redhat.com content-type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-13.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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 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 start probing for 'x' packet support by sending a zero length 'x' packet read. We already do something similar for the 'X' packet (see remote_target::check_binary_download in remote.c). By sending a zero length read, a remote target that supports 'x' packets, and implements GDB's approach, will reply with a single 'b' character. If we get back anything else then GDB can just disable 'x' packet support and fall back to the slower 'm' packet. In the future remote targets can, if they want, check to see if the client is GDB or LLDB and then modify their handling of 'x' packets in order to support both debuggers. I have not modified gdbserver at all. I don't know if it is possible to connect LLDB to gdbserver (I'm guessing not), but if a user tried such a thing, and LLDB sent an 'x' packet, then gdbserver will send back a reply with a 'b' prefix, which is going to confuse LLDB. Until a user complains about this then I'm going to assume that LLDB and gdbserver is not a supported combination. I have added a paragraph to the docs to explain that GDB probes using a zero length read. I feel this is important so that targets don't assume zero length accesses are an error. Plus the text makes it clear how GDB expects the access to be handled. For testing I have tried connecting to gdbserver and checked the packet log. I can see the zero length probe, then GDB continues using 'x' packets as normal. I then modified gdbserver to stop it sending back the 'b' prefix. Now I see the zero length probe, and after that GDB switches to using the 'm' packet instead and 'show remote binary-upload-packet' shows the packet as disabled. 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 now see GDB probe with a zero length access and then fall-back to using 'm' packets. Reviewed-By: Eli Zaretskii --- gdb/doc/gdb.texinfo | 4 ++ gdb/remote.c | 106 ++++++++++++++++++++++++++++++-------------- 2 files changed, 77 insertions(+), 33 deletions(-) base-commit: 95db53e5a03dacef594430f28281748f3111c1f6 diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index b65124d807f..79403a85db1 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -43531,6 +43531,10 @@ Packets use byte accesses, or not. For this reason, this packet may not be suitable for accessing memory-mapped I/O devices. +If @var{length} is zero then the reply should contain the leading +@samp{b} prefix and no data. @value{GDBN} sends a zero length +@samp{x} packet to probe for support of this packet type. + Reply: @table @samp @item b @var{XX@dots{}} diff --git a/gdb/remote.c b/gdb/remote.c index 64622dbfcdf..bd849c0cbbd 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1329,6 +1329,7 @@ class remote_target : public process_stratum_target void set_remote_traceframe (); void check_binary_download (CORE_ADDR addr); + void check_binary_upload (CORE_ADDR addr); target_xfer_status remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, @@ -9411,6 +9412,65 @@ remote_target::check_binary_download (CORE_ADDR addr) } } +/* Similar to check_binary_download, determine whether the remote target + supports binary uploading ('x' packet). This is accomplished by sending + a no-op memory read of zero length to the target at the specified + address ADDR. + + Unfortunately we cannot just send the complete packet (with the actual + length) due to divergence between GDB and LLDB. Both debuggers support + the 'x' packet, but disagree about the expected reply format. GDB + requires a 'b' prefix on the returned data in order to distinguish + between an unsupported packet (empty), and a failed, or zero length read + ('b' followed by no data). It's not clear how LLDB distinguishes these + two cases. However, there are remote targets in the wild that use + LLDB's reply format, which is not compatible with GDB. For those + targets we will fall back to the slower 'm' packet. + + Send the zero length read request. If we don't get back a packet + containing just 'b', then we assume that 'x' packets are not + supported. */ + +void +remote_target::check_binary_upload (CORE_ADDR addr) +{ + struct remote_state *rs = get_remote_state (); + + switch (m_features.packet_support (PACKET_x)) + { + case PACKET_DISABLE: + break; + case PACKET_ENABLE: + break; + case PACKET_SUPPORT_UNKNOWN: + { + char *p = rs->buf.data (); + *p++ = 'x'; + p += hexnumstr (p, (ULONGEST) addr); + *p++ = ','; + p += hexnumstr (p, (ULONGEST) 0); + *p = '\0'; + + putpkt (rs->buf); + getpkt (&rs->buf); + + /* If we get back a lone 'b' with no data then the remote replied as + expected. Anything else and we just disable the 'x' packet. */ + if (rs->buf[0] == 'b' && rs->buf[1] == '\0') + { + remote_debug_printf ("binary uploading is supported by target"); + m_features.m_protocol_packets[PACKET_x].support = PACKET_ENABLE; + } + else + { + remote_debug_printf ("binary uploading NOT supported by target"); + m_features.m_protocol_packets[PACKET_x].support = PACKET_DISABLE; + } + break; + } + } +} + /* Helper function to resize the payload in order to try to get a good alignment. We try to write an amount of data such that the next write will start on an address aligned on REMOTE_ALIGN_WRITES. */ @@ -9682,17 +9742,9 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, memaddr = remote_address_masked (memaddr); - /* Construct "m/x"","". */ - auto send_request = [this, rs, memaddr, todo_units] (char format) -> void - { - char *buffer = rs->buf.data (); - *buffer++ = format; - buffer += hexnumstr (buffer, (ULONGEST) memaddr); - *buffer++ = ','; - buffer += hexnumstr (buffer, (ULONGEST) todo_units); - *buffer = '\0'; - putpkt (rs->buf); - }; + /* Check whether the target supports binary uploading. */ + check_binary_upload (memaddr); + gdb_assert (m_features.packet_support (PACKET_x) != PACKET_SUPPORT_UNKNOWN); /* Determine which packet format to use. The target's support for 'x' may be unknown. We just try. If it doesn't work, we try @@ -9703,32 +9755,20 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, else packet_format = 'x'; - send_request (packet_format); + /* Construct "m/x"","". */ + char *buffer = rs->buf.data (); + *buffer++ = packet_format; + buffer += hexnumstr (buffer, (ULONGEST) memaddr); + *buffer++ = ','; + buffer += hexnumstr (buffer, (ULONGEST) todo_units); + *buffer = '\0'; + putpkt (rs->buf); + + /* Fetch and process the reply. */ int packet_len = getpkt (&rs->buf); if (packet_len < 0) return TARGET_XFER_E_IO; - if (m_features.packet_support (PACKET_x) == PACKET_SUPPORT_UNKNOWN) - { - if (rs->buf[0] == '\0') - { - remote_debug_printf ("binary uploading NOT supported by target"); - m_features.m_protocol_packets[PACKET_x].support = PACKET_DISABLE; - - /* Try again using 'm'. */ - packet_format = 'm'; - send_request (packet_format); - packet_len = getpkt (&rs->buf); - if (packet_len < 0) - return TARGET_XFER_E_IO; - } - else - { - remote_debug_printf ("binary uploading supported by target"); - m_features.m_protocol_packets[PACKET_x].support = PACKET_ENABLE; - } - } - packet_result result = packet_check_result (rs->buf); if (result.status () == PACKET_ERROR) return TARGET_XFER_E_IO;