[Lldb-commits] [PATCH] D89227: [lldb] Note difference in vFile:pread/pwrite format for lldb

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D89227#2327066 <https://reviews.llvm.org/D89227#2327066>, @DavidSpickett wrote:

>> Given how often the '0x' prefix on base16 numbers is dropped in the remote serial protocol
>
> Now you mention it, I thought I knew how strtoull worked but I don't. (I assumed it was some best effort that would roll back if somehow the digits didn't fit the assumed base)
>
> This is what a gdb pread looks like:
>
>   getpkt ("vFile:pread:6,3fff,9ab473c8");  [no ack sent]
>
> So in fact it is doing exactly what you said and not including the `0x`.
>
>> What do you think?
>
> Yes I think sending hex with the `0x` is the clearest indicator for implementers of their own lldb-servers. However gdb-server doesn't expect the leading `0x` (I don't see the spec mandating it either way) so it doesn't solve that side, if that is a goal.
>
> So would it make more sense to change lldb to send unprefixed hex like gdb does but update lldb-server to accept either? (by trying base 10 then base 16). Especially if implementers are going to prefer the packet specification over reading packet logs from lldb.
>
> That's not perfect either, an older lldb-server with a new lldb client could happen to parse the number as base 10 but you'll get a unexpected result. (at least adding `0x` would give you a hard error there) Do we commit to that kind of compatibility?

Hmm... interesting. Given that lldb-servers already accept a 0x encoding I think that changing the client to send it that way would be a strict improvement (a noop for lldb-server and an hard error instead of something random for gdb-server).
Then we might also try to rectify the situation via some steps like:

- if it's important for new lldb-server to communicate with old lldb, then wait a while
- change lldb-server to parse everything as hex, regardless of the 0x prefix
- wait a while (I'm guessing that having new lldb communicate with old lldb-server *is* important)
- change lldb to drop the 0x prefix



In D89227#2327068 <https://reviews.llvm.org/D89227#2327068>, @DavidSpickett wrote:

> And I'm saying lldb-server and not that and debug-server because if I'm honest, I don't really know what the latter is. I assume this applies to both though.

debugserver is an apple-only implementation of the remote stub. However, I think that may be a red herring, because in lldb land the vFile packets are handled by lldb-server platform, which is used even on apple platforms (IIUC).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89227/new/

https://reviews.llvm.org/D89227