Re: Incompatible implementat ion of 'x' packet in GDB vs LLDB

On 1/23/25 11:23, Robert O'Callahan wrote:
> On Thu, 23 Jan 2025 at 21:13, Aktemur, Tankut Baris <
> [email protected]> wrote:
> 
>> 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?
>>
> 
> Yes, both on the sending side and the receiving side:
> Send:
> https://github.com/llvm/llvm-project/blob/cb714e74cc0efd5bfdb3e5e80978239425bd83d4/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp#L723
> Receive:
> https://github.com/llvm/llvm-project/blob/2e6cc79f816d942ab09d6a310cd925c1da148aa9/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp#L2517
> Not ideal, although it gets the job done.
> 
> For the record, the 'x' packet series were discussed in
>>
>>
>> https://inbox.sourceware.org/gdb-patches/[email protected]/#r
>>
>> with the part specific to the 'b' marker in
>>
>>   https://inbox.sourceware.org/gdb-patches/[email protected]/
> 
> 
> Thanks. From the point of view of an implementer trying to serve both
> debuggers, it's unfortunate that no-one raised the question of what LLDB
> had done... maybe next time someone could skim the LLDB protocol doc (and
> vice versa on their side).

I understand the pain. Sorry this has caused a bit of a hiccup in terms of support.

We can probably look into ways to make this smoother to integrate (or a workaround
to check if we're talking to GDB or LLDB), as we're not actively trying to break
stuff. It is just an unfortunate lack of community cooperation/coordination.

With that said, at least some of us have been aware of the gap between LLDB's RSP
implementation and GDB's. I recall discussing some of this years ago and warning
about it. But at the time there was a bit of a "us x them" feeling going on between
GNU and LLVM (mostly clang x gcc). So very little cooperation went into it.

As a result, you have a significantly different RSP implementation for LLDB that
doesn't make sense for GDB to follow without proper coordination/justification.

Given GDB is probably deemed the source of the RSP implementation and documentation,
I think things created for the RSP should be contributed back to the protocol for the
sake of making the lives of debugging server implementors easier across the board.

Maybe moving forward we could plan to converge the two variations of the RSP, or
just call them different and have a way to tell them apart (I don't think that's
ideal though).

Really ideally, we should probably ditch the RSP mechanism and go for a better
protocol. One can dream.

> 
> The LLDB approach is a bit distasteful so I understand why you wouldn't
> want to follow it. But rr users who upgrade to gdb 16.1 before they update
> rr are going to have a bad time, especially because it manifests as rr+gdb
> just being mysteriously broken. GDB 16.1 was only just released and already
> three users have reported the bug to us [1].
> 
> The least hacky fix I can think of that you could do to help us would be to
> do the "x0,0" query thing to detect if the packet is supported and if so,
> whether it's LLDB or GDB flavour and use that.
> 
> Whatever you do or don't do, for the future in rr I think we'll have to
> push "LLDB vs GDB mode" deeper into our protocol handling and make sure any
> new features are only enabled for the client that we have tested them with.
> It may be even more complicated than that because there are other clients
> like delve, although hopefully they can stick to e.g. the GDB mode.
> 
> Rob
> 
> [1] https://github.com/rr-debugger/rr/issues/3901