> On Apr 20, 2021, at 11:39 PM, Pavel Labath via lldb-dev
> wrote:
>
> I am very happy to see this effort and I fully encourage it.
Completely agree. Thanks for Cc:'ing me Pavel, I hadn't seen Michał's thread.
>
> On 20/04/2021 09:13, Michał Górny via lldb-dev wrote:
>> On Mon, 2021-04-19 at 16:29 -0700, Greg Clayton wrote:
I think the first blocker towards this project are existing
implementation bugs in LLDB. For example, the vFile implementation is
documented as using incorrect data encoding and open flags. This is not
something that can be trivially fixed without breaking compatibility
between different versions of LLDB.
>>>
>>> We should just fix this bug in LLDB in both LLDB's logic and lldb-server
>>> IMHO. We typically distribute both "lldb" and "lldb-server" together so
>>> this shouldn't be a huge problem.
>> Hmm, I've focused on this because I recall hearing that OSX users
>> sometimes run new client against system server... but now I realized
>> this isn't relevant to LLGS ;-). Still, I'm happy to do things
>> the right way if people feel like it's needed, or the easy way if it's
>> not.
>
> The vFile packets are, used in the "platform" mode of the connection (which,
> btw, is also something that gdb does not have), and that is implemented by
> lldb-server on all hosts (although I think apple may have some custom
> platform implementations as well). In any case though, changing flag values
> on the client will affect all servers that it communicates with, regardless
> of the platform.
>
> At one point, Jason cared enough about this to add a warning about not
> changing these constants to the code. I'd suggest checking with him whether
> this is still relevant.
>
> Or just going with your proposed solution, which sounds perfectly reasonable
> to me
The main backwards compatibility issue for Apple is that lldb needs to talk to
old debugservers on iOS devices, where debugserver can't be updated. I know of
three protocol bugs we have today:
vFile:open flags
vFile:pread/pwrite base https://bugs.llvm.org/show_bug.cgi?id=47820
A packet base https://bugs.llvm.org/show_bug.cgi?id=42471
debugserver doesn't implement vFile packets. So for those, we only need to
worry about lldb/lldb-server/lldb-platform.
lldb-platform is a freestanding platform packets stub I wrote for Darwin
systems a while back. Real smol, it doesn't link to/use any llvm/lldb code. I
never upstreamed it because it doesn't really fit in with llvm/lldb projects in
any way and it's not super interesting, it is very smol and simple. I was
tired of tracking down complicated bugs and wanted easier bugs. It implements
the vFile packets; it only does the platform packets and runs debugserver for
everything else.
Technically a modern lldb could need to communicate with an old lldb-platform,
but it's much more of a corner case and I'm not super worried about it, we can
deal with that inside Apple (that is, I can be responsible for worrying about
it.)
For vFile:open and vFile:pread/pwrite, I say we just change them in
lldb/lldb-server and it's up to me to change them in lldb-platform at the same
time.
For the A packet, debugserver is using base 10,
errno = 0;
arglen = strtoul(buf, &c, 10);
if (errno != 0 && arglen == 0) {
return HandlePacket_ILLFORMED(__FILE__, __LINE__, p,
"arglen not a number on 'A' pkt");
}
[..]
errno = 0;
argnum = strtoul(buf, &c, 10);
if (errno != 0 && argnum == 0) {
return HandlePacket_ILLFORMED(__FILE__, __LINE__, p,
"argnum not a number on 'A' pkt");
}
as does lldb,
packet.PutChar('A');
for (size_t i = 0, n = argv.size(); i < n; ++i) {
arg = argv[i];
const int arg_len = strlen(arg);
if (i > 0)
packet.PutChar(',');
packet.Printf("%i,%i,", arg_len * 2, (int)i);
packet.PutBytesAsRawHex8(arg, arg_len);
and lldb-server,
// Decode the decimal argument string length. This length is the number of
// hex nibbles in the argument string value.
const uint32_t arg_len = packet.GetU32(UINT32_MAX);
if (arg_len == UINT32_MAX)
success = false;
else {
// Make sure the argument hex string length is followed by a comma
if (packet.GetChar() != ',')
success = false;
else {
// Decode the argument index. We ignore this really because who would
// really send down the arguments in a random order???
const uint32_t arg_idx = packet.GetU32(UINT32_MAX);
uint32_t StringExtractor::GetU32(uint32_t fail_value, int base) {
if (m_index < m_packet.size()) {
char *end = nullptr;
const char *start = m_packet.c_str();
const char *cstr = start + m_index;
uint32_t result = static_cast(::strtoul(cstr, &end, base));
where 'base' defaults to 0 which strtoul treats as base 10 unless the number
starts with 0x.
The A pa
