From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id olDlKzRrkmdfnRgAWB0awg (envelope-from ) for ; Thu, 23 Jan 2025 11:15:48 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=Jl+SOORF; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 9CA101E100; Thu, 23 Jan 2025 11:15:48 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 63EDC1E08E for ; Thu, 23 Jan 2025 11:15:47 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7BDAE3858406 for ; Thu, 23 Jan 2025 16:15:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7BDAE3858406 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1737648946; bh=Fz7cEUkhTInzsup56JuLsz6ayPGBoA8xSBJ3+sxD+sw=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Jl+SOORFA0NK1jRbbQN0gX1W4wbsdwEa4GoH65DAsyidxPB64d591q+p9IhhpSEDD YAJB/LMV40WLupc4ZxV3DPBbHe8ipLdj/MrXe7zxuEldG8NcVdDEEgmU8RGaNNo6q3 iTFY7Qq5Z42Jf+8J/F0D8vbvG4uTGx1IHl1FwFtA= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id E3A943858D3C for ; Thu, 23 Jan 2025 16:14:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E3A943858D3C ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E3A943858D3C ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737648891; cv=none; b=h2UuvnuG8faewzhnEvkykeJE7APWHlxnH5GQfjp4cPAzwMBZF3+yK+gmVdE1gSedqZtlHAWFgB1zNqSj80EJeHUsROTfpwFY9zdqCQuRiZszUt8f7cPfVLo9nN3pHmZ44zVk1v9H4f2tcBOZI1ADkAANIDYDW+uWFpa/qKZZqcQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737648891; c=relaxed/simple; bh=LIav65+inE/e7aAgAV6auAAfFN7q4qtONfply4ym3gA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=CmlWGq9ljLiadY8vh6OOyzWDn/+pc+kaKthy0xCuF2onYRB4ux99ZgwSTxCBEHBUMLPXONRSqTKqXwE9juezWv5HbDZ5bULK3x9qDnXyKnfh36aKiLnKb0sT69v9Iw9tKh7kXW6cc1pUzi1FWey3kqWm0qqrRuXpY9NJ7Hxf73I= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E3A943858D3C 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-613-mIWFFjABNh2PV6s0i2C7aA-1; Thu, 23 Jan 2025 11:14:49 -0500 X-MC-Unique: mIWFFjABNh2PV6s0i2C7aA-1 X-Mimecast-MFC-AGG-ID: mIWFFjABNh2PV6s0i2C7aA Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-385d6ee042eso843185f8f.0 for ; Thu, 23 Jan 2025 08:14:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737648888; x=1738253688; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Fz7cEUkhTInzsup56JuLsz6ayPGBoA8xSBJ3+sxD+sw=; b=CpcbhY8nfawaW7za5y6ZD4rW7c8NfdF5Jpyol3MJ4Dz88UILfvSCbOCY1yExgwI/hK HBT1FUPXLKAVSc+0JnySzjiTlmm9LB3den9Zn4MIWBas+667k4CJ8cstbYpguelNbdYM 2etD+H5JrE8x3IJtjq47r3trfVhntq+I8YB+qLA8qOzj0reqicJKNJ0KKSknzmP0wIXy n2AnKsaFqARsE35aQ4UfYMavkr2V/91T+i/XmO3kyQ8SwJtKXXQLYLB70wY3SZfAwaiW URXs1Zued+lQ6FoxiGkndY4V3KSoMECVRJknGrB8Z1L7ap288OmcoywrPo2a+Otdjnjz e6Vg== X-Forwarded-Encrypted: i=1; AJvYcCUq0bmm0QPXLpaxC7dmj9lqJ5pGR9TFzG8oFYmyJQsLFhNJiKqzwkP8G5sLpJYvcYkTO5U=@sourceware.org X-Gm-Message-State: AOJu0YwF3JtAagrVLFsKzybSpbvlGWp3r2OnGu6AlDbdC5B/n/PuIOwR pzaZ7DAJ8AEQVAJWMaGgC757URK8tG+7PrLJD/opVjw6SbXP91Wti2i3n1ZmDAC/kPbqqVgr1/P 2otJhNgspNow8PGM8QuUB81tuYLYzJO9Vhj3nPRyijV0Cqft5dv48uvjR X-Gm-Gg: ASbGncs3TjvgL+K9IKyRMiDL0oBI3j43nJQ6EJqI9nusXb35qiMLXe84tUHXOfOrUlF ZDaDqo+DeYKtlQ1i1IafY2tlvUKZJajhp3fb/WGIWyEVJcHqCQWnoc7bYVyvOtasnmq2uz7JkT4 aMVq/Eff5tKEQ5gy1m5Yf25/sQfUYMFGgXuHL4Bdrf8u9i9V5VtiFsQQcRoshWRIELBY1OR+m0j bQq+ySeNJBJzK2ji9mUiOq07SSrEsPI/M8FTaN9KW1vjoUTarPWcUV04JTeNrx3GShN3BkJN/OD NXaRLXemPCtsxSaIAtk= X-Received: by 2002:a5d:6d86:0:b0:385:faf5:eba6 with SMTP id ffacd0b85a97d-38bf5655a3dmr22477265f8f.1.1737648887870; Thu, 23 Jan 2025 08:14:47 -0800 (PST) X-Google-Smtp-Source: AGHT+IHkHhz68eB5zC4KGHa/RKWn4vqOkL0x3aZDeEKzs+Tj8JCxS2OYT2td1aUOz+5+Ky5ByvQr6Q== X-Received: by 2002:a5d:6d86:0:b0:385:faf5:eba6 with SMTP id ffacd0b85a97d-38bf5655a3dmr22477240f8f.1.1737648887419; Thu, 23 Jan 2025 08:14:47 -0800 (PST) Received: from localhost (44.226.159.143.dyn.plus.net. [143.159.226.44]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38c2a1764c8sm99066f8f.3.2025.01.23.08.14.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jan 2025 08:14:47 -0800 (PST) To: "Aktemur, Tankut Baris" , "robert@ocallahan.org" , "gdb@sourceware.org" Subject: RE: Incompatible implementat ion of 'x' packet in GDB vs LLDB In-Reply-To: References: Date: Thu, 23 Jan 2025 16:14:45 +0000 Message-ID: <87a5bhmrre.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 195XEKD6-YKeKAuUZINbH8vzLLtKFsF6uUjJA88_tpc_1737648888 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Andrew Burgess via Gdb Reply-To: Andrew Burgess Errors-To: gdb-bounces~public-inbox=simark.ca@sourceware.org Sender: "Gdb" "Aktemur, Tankut Baris via Gdb" writes: > On Thursday, January 23, 2025 8:13 AM, Robert O'Callahan wrote: >> On Thu, 23 Jan 2025 at 08:57, Robert O'Callahan >> wrote: >> >> > GDB (client) 16.1 started sending the gdbserver 'x' packet. It follows the >> > documentation [1] and expects a leading 'b' in the response [2]. >> > Unfortunately LLDB has supported this packet for quite a long time [3] and >> > does not expect a leading 'b' in the response. We added support for this >> > packet to rr last year and followed LLDB's format because it was the only >> > user of the packet at that time. So GDB 16.1 doesn't work with rr. [4] >> > >> > I realize that compatibility between GDB and LLDB flavoured gdbserver >> > protocols is not a priority for either team, but until now it has actually >> > worked in practice --- rr hasn't needed a client mode switch. We can add >> > one, but it will be unfortunate if GDB 16.1 and later is incompatible for >> > anyone who's installed the latest rr since May 2024. >> > >> > Could you make a GDB 16.1 point release that removes the 'b'? AFAIK it >> > serves no purpose. >> > >> >> It has been pointed out that if you want to return different error codes >> then you need the 'b'. Is that the rationale? > > Hello Rob, > > Essentially, yes. In case of an error, the server responds with an 'E' packet. > To be able to distinguish an error packet from binary data, 'E' would have to be > added to the list of escaped characters. Having the 'b' marker avoids that. > > Additionally, when the response is empty, per RSP, it means the packet is unsupported. > So, in case of a zero-length request, the 'b' marker could help us distinguish the > unsupported case from an actual zero-response. > LLDB doc says > > To test if this packet is available, send a addr/len of 0: > > x0,0 > > You will get an OK response if it is supported. > The reply will be the data requested in 8-bit binary data format. > > How does LLDB distinguish an "OK" response, an empty binary data, an error, > and an unsupported case? These were not clear to me from the docs. > Is the x0,0 query special-cased? > > For the record, the 'x' packet series were discussed in > > 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 also am not entirely sure how lldb's implementation can work in all cases given their description. For now at least, I still think we made the right choice with our implementation, though I guess if we'd known we were diverging from llvm we might have picked a different letter for packet in order to make it completely different. I have two thoughts for things we could do to possibly help with compatibility here though: 1. Hide the 'x' packet behind a feature flag. So instead of using the empty reply to indicate 'x' packet support, we ask remote targets to send 'gdb-x+' in their qSupported reply. If a remote target sends this to GDB then we will assume that they support GDB's style of 'x' packet, and can make use of the packet. Otherwise, it's 'm' packets all the way. 2. The other option would be to probe for 'x' packet support as a separate action. Right now probing is done the first time we send an 'x' packet. The packet is sent, and if we get back the empty packet, then we know 'x' packets are supported. But if instead, we sent a zero length read, then by GDB's rules we should get back a lone 'b'. By lldb's rules we should get back .... maybe "OK" ... or maybe the empty packet? It's not really clear. But the important thing is that neither of those replies are 'b'. So the first time we consider sending an 'x' packet, we send 'xADDR,0', if we get back 'b', then we know that we have GDB style support, otherwise, we disable the 'x' packet, and fall back to the 'm' packet. Approach #1 is pretty straight forward, but I wanted to check what approach #2 would look like, so I drafted the patch below. This patch is pretty rough right now, but can easily be cleaned up. For testing this patch I hacked up gdbserver so it no longer added the 'b' prefix, then I tried connecting with GDB. I see GDB send the zero length probe, then switch back to 'm' packets. Questions: + Do people think we should change GDB to improve compatibility? My thinking is yes, so long as the cost isn't too high. + Do people prefer approach #1 or #2? I'm leaning slightly more to #2 right now, but not massively so. If people prefer #1 I can write the patch for that instead. Thanks, Andrew --- diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index b65124d807f..e92227d3803 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 not data. @value{GDBN} sends a zero length +@samp{x} packet to probe for support. + Reply: @table @samp @item b @var{XX@dots{}} diff --git a/gdb/remote.c b/gdb/remote.c index 64622dbfcdf..a332fcd6f95 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -9683,17 +9683,55 @@ 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 + auto send_request = [this, rs, memaddr] (char format, int len) -> void { char *buffer = rs->buf.data (); *buffer++ = format; buffer += hexnumstr (buffer, (ULONGEST) memaddr); *buffer++ = ','; - buffer += hexnumstr (buffer, (ULONGEST) todo_units); + buffer += hexnumstr (buffer, (ULONGEST) len); *buffer = '\0'; putpkt (rs->buf); }; + /* Unfortunately, lldb and GDB disagree on the reply format for the 'x' + packet. GDB expects a 'b' as the first byte of a successful packet, + while lldb does not. There are good reasons why GDB uses the 'b' + prefix; it's not possible to distinguish between an empty + not-supported reply and a failed read empty reply, unless there is a + prefix. + + As lldb "got there first", and there are clients in the wild that + support the lldb style reply, we probe with a zero length read. If + this returns a single 'b' then we know that the 'x' packet is + supported. + + If we get back an error packet (starts with 'E') then we cannot know + if a successful read would start with a 'b' or not. We'll probe again + next time. */ + if (m_features.packet_support (PACKET_x) == PACKET_SUPPORT_UNKNOWN) + { + send_request ('x', 0); + int packet_len = getpkt (&rs->buf); + + /* Catastrophic error fetching packet. */ + if (packet_len < 0) + return TARGET_XFER_E_IO; + + /* 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 memory read is supported by target"); + m_features.m_protocol_packets[PACKET_x].support = PACKET_ENABLE; + } + else + { + remote_debug_printf ("binary memory read NOT supported by target"); + m_features.m_protocol_packets[PACKET_x].support = PACKET_DISABLE; + } + } + /* 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 again using 'm'. */ @@ -9703,32 +9741,11 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, else packet_format = 'x'; - send_request (packet_format); + send_request (packet_format, todo_units); 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;