https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/211495
The documentation for qSpeedTest says ``` send packet: qSpeedTest:response_size:response-size; read packet: data:<response data> response-size is a hex encoded unsigned number up to 64 bits in size. ``` debugserver implements qSpeedTest as per this documentation, but lldb sends the number in decimal (base 10), and lldb-server parses it as base 10. I changed lldb and lldb-server to base 16. This is a maintenance command used by lldb developers exclusivley, so IMO I'm not handling a migration for old/new servers or defining a new packet or key. I was running some packet transmission tests with debugserver and noticed debugserver was sending much larger packets than requested; that's the kind of failure you see when there is a mismatch. I audited all uses of `getAsInteger()` that pass a 0 for the radix, meaning auto-detect the radix, in the gdb-remote dir and changed nearly all of them to specify either base 10 or base 16. There were two places where we were using auto-sensing on a base 16 number (and no "0x" prefix), which would parse incorrectly if an [a-f] letter did not occur. There is one use of `getAsInteger(0,...)` that is still in place, for pasing the `qRegisterInfo` eh-frame/dwarf register numbers. Our documentation for these keys specifies that they may be base 10 or base 16 if prefixed with 0x. This is unlike anywhere else in gdb remote serial protocol, and smells like something I probably stuck in there 15+ years ago because I've never liked the ambiguity about number bases in the protocol and I made this half-hearted stab at encouraging use of "0x". It is always fun to see the inconsistencies as you review multiple packets. qProcessInfo returns keys like pid/gid/ppid in base 16. qfProcessInfo returns the same keys in base 10. qHostInfo returns cputype/cpusubtype (a Mach-O way of specifying a target cpu) in base 10, qProcessInfo returns the same keys in base 16. There's so many of these kinds of little inconsistencies :/ >From b2d516f0cafba1bddd52a490586fda5173e3f2f8 Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Thu, 23 Jul 2026 01:07:20 -0700 Subject: [PATCH] [lldb] Fix qSpeedTest radix mistake, make number parsings explicit The documentation for qSpeedTest says ``` send packet: qSpeedTest:response_size:response-size; read packet: data:<response data> response-size is a hex encoded unsigned number up to 64 bits in size. ``` debugserver implements qSpeedTest as per this documentation, but lldb sends the number in decimal (base 10), and lldb-server parses it that way too. I changed lldb and lldb-server to base 16. This is a maintenance command used by lldb developers exclusivley, so IMO I'm not handling a migration for old/new servers or defining a new packet or key. I was running some packet transmission tests with debugserver and noticed lldb was receiving much larger packets than it requested, that's the kind of failure you see when there is a mismatch. I audited all uses of `getAsInteger()` that pass a 0 for the radix, meaning auto-detect the radix, and changed nearly all of them to specify either base 10 or base 16. There were two cases where packets were sending a base 16 number without a "0x" prefix and the auto-sensing radix would do the wrong thing if an [a-f] letter didn't occur in the number. There is one use of `getAsInteger(0,...)` that is still in place, for pasing the `qRegisterInfo` eh-frame/dwarf register numbers. Our documentation for these keys specifies that they may be base 10 or base 16 if prefixed with 0x. This is unlike anywhere else in gdb remote serial protocol, and smells like something I probably stuck in there 15+ years ago because I've never liked the ambiguity about number bases in the protocol and I made this half-hearted stab at encouraging use of "0x". It is always fun to see the inconsistencies as you review multiple packets. qProcessInfo returns keys like pid/gid/ppid in base 16. qfProcessInfo returns the same keys in base 10. qHostInfo returns cputype/cpusubtype (a Mach-O way of specifying a target cpu) in base 10, qProcessInfo returns the same keys in base 16. There's so many of these kinds of little inconsistencies :/ --- .../GDBRemoteCommunicationClient.cpp | 40 +++++++++---------- .../GDBRemoteCommunicationServerCommon.cpp | 14 +++---- .../GDBRemoteCommunicationServerPlatform.cpp | 2 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 16 ++++---- 4 files changed, 36 insertions(+), 36 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index a3fe6661737a4..8ae50f1f83c3f 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -1179,7 +1179,7 @@ bool GDBRemoteCommunicationClient::GetGDBServerVersion() { } else if (name == "version") { llvm::StringRef major, minor; std::tie(major, minor) = value.split('.'); - if (!major.getAsInteger(0, m_gdb_server_version)) + if (!major.getAsInteger(10, m_gdb_server_version)) success = true; } } @@ -1347,11 +1347,11 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool force) { while (response.GetNameColonValue(name, value)) { if (name == "cputype") { // exception type in big endian hex - if (!value.getAsInteger(0, cpu)) + if (!value.getAsInteger(10, cpu)) ++num_keys_decoded; } else if (name == "cpusubtype") { // exception count in big endian hex - if (!value.getAsInteger(0, sub)) + if (!value.getAsInteger(10, sub)) ++num_keys_decoded; } else if (name == "arch") { arch_name = std::string(value); @@ -1391,17 +1391,17 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool force) { if (byte_order != eByteOrderInvalid) ++num_keys_decoded; } else if (name == "ptrsize") { - if (!value.getAsInteger(0, pointer_byte_size)) + if (!value.getAsInteger(10, pointer_byte_size)) ++num_keys_decoded; } else if (name == "addressing_bits") { - if (!value.getAsInteger(0, m_low_mem_addressing_bits)) { + if (!value.getAsInteger(10, m_low_mem_addressing_bits)) { ++num_keys_decoded; } } else if (name == "high_mem_addressing_bits") { - if (!value.getAsInteger(0, m_high_mem_addressing_bits)) + if (!value.getAsInteger(10, m_high_mem_addressing_bits)) ++num_keys_decoded; } else if (name == "low_mem_addressing_bits") { - if (!value.getAsInteger(0, m_low_mem_addressing_bits)) + if (!value.getAsInteger(10, m_low_mem_addressing_bits)) ++num_keys_decoded; } else if (name == "os_version" || name == "version") // Older debugserver binaries used @@ -1423,14 +1423,14 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool force) { ++num_keys_decoded; } else if (name == "default_packet_timeout") { uint32_t timeout_seconds; - if (!value.getAsInteger(0, timeout_seconds)) { + if (!value.getAsInteger(10, timeout_seconds)) { m_default_packet_timeout = seconds(timeout_seconds); SetPacketTimeout(m_default_packet_timeout); ++num_keys_decoded; } } else if (name == "vm-page-size") { int page_size; - if (!value.getAsInteger(0, page_size)) { + if (!value.getAsInteger(10, page_size)) { m_target_vm_page_size = page_size; ++num_keys_decoded; } @@ -2144,27 +2144,27 @@ bool GDBRemoteCommunicationClient::DecodeProcessInfoResponse( while (response.GetNameColonValue(name, value)) { if (name == "pid") { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - value.getAsInteger(0, pid); + value.getAsInteger(10, pid); process_info.SetProcessID(pid); } else if (name == "ppid") { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - value.getAsInteger(0, pid); + value.getAsInteger(10, pid); process_info.SetParentProcessID(pid); } else if (name == "uid") { uint32_t uid = UINT32_MAX; - value.getAsInteger(0, uid); + value.getAsInteger(10, uid); process_info.SetUserID(uid); } else if (name == "euid") { uint32_t uid = UINT32_MAX; - value.getAsInteger(0, uid); + value.getAsInteger(10, uid); process_info.SetEffectiveUserID(uid); } else if (name == "gid") { uint32_t gid = UINT32_MAX; - value.getAsInteger(0, gid); + value.getAsInteger(10, gid); process_info.SetGroupID(gid); } else if (name == "egid") { uint32_t gid = UINT32_MAX; - value.getAsInteger(0, gid); + value.getAsInteger(10, gid); process_info.SetEffectiveGroupID(gid); } else if (name == "triple") { StringExtractor extractor(value); @@ -2199,9 +2199,9 @@ bool GDBRemoteCommunicationClient::DecodeProcessInfoResponse( is_arg0 = false; } } else if (name == "cputype") { - value.getAsInteger(0, cpu); + value.getAsInteger(10, cpu); } else if (name == "cpusubtype") { - value.getAsInteger(0, sub); + value.getAsInteger(10, sub); } else if (name == "vendor") { vendor = std::string(value); } else if (name == "ostype") { @@ -2570,7 +2570,7 @@ bool GDBRemoteCommunicationClient::GetGroupName(uint32_t gid, static void MakeSpeedTestPacket(StreamString &packet, uint32_t send_size, uint32_t recv_size) { packet.Clear(); - packet.Printf("qSpeedTest:response_size:%i;data:", recv_size); + packet.Printf("qSpeedTest:response_size:%x;data:", recv_size); uint32_t bytes_left = send_size; while (bytes_left > 0) { if (bytes_left >= 26) { @@ -2772,9 +2772,9 @@ bool GDBRemoteCommunicationClient::LaunchGDBServer( llvm::StringRef value; while (response.GetNameColonValue(name, value)) { if (name == "port") - value.getAsInteger(0, port); + value.getAsInteger(10, port); else if (name == "pid") - value.getAsInteger(0, pid); + value.getAsInteger(10, pid); else if (name.compare("socket_name") == 0) { StringExtractor extractor(value); extractor.GetHexByteString(socket_name); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index d710dfec95873..2ebbb35a56bb6 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -365,32 +365,32 @@ GDBRemoteCommunicationServerCommon::Handle_qfProcessInfo( return SendErrorResponse(2); } else if (key == "pid") { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - if (value.getAsInteger(0, pid)) + if (value.getAsInteger(10, pid)) return SendErrorResponse(2); match_info.GetProcessInfo().SetProcessID(pid); } else if (key == "parent_pid") { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - if (value.getAsInteger(0, pid)) + if (value.getAsInteger(10, pid)) return SendErrorResponse(2); match_info.GetProcessInfo().SetParentProcessID(pid); } else if (key == "uid") { uint32_t uid = UINT32_MAX; - if (value.getAsInteger(0, uid)) + if (value.getAsInteger(10, uid)) return SendErrorResponse(2); match_info.GetProcessInfo().SetUserID(uid); } else if (key == "gid") { uint32_t gid = UINT32_MAX; - if (value.getAsInteger(0, gid)) + if (value.getAsInteger(10, gid)) return SendErrorResponse(2); match_info.GetProcessInfo().SetGroupID(gid); } else if (key == "euid") { uint32_t uid = UINT32_MAX; - if (value.getAsInteger(0, uid)) + if (value.getAsInteger(10, uid)) return SendErrorResponse(2); match_info.GetProcessInfo().SetEffectiveUserID(uid); } else if (key == "egid") { uint32_t gid = UINT32_MAX; - if (value.getAsInteger(0, gid)) + if (value.getAsInteger(10, gid)) return SendErrorResponse(2); match_info.GetProcessInfo().SetEffectiveGroupID(gid); } else if (key == "all_users") { @@ -480,7 +480,7 @@ GDBRemoteCommunicationServerCommon::Handle_qSpeedTest( bool success = packet.GetNameColonValue(key, value); if (success && key == "response_size") { uint32_t response_size = 0; - if (!value.getAsInteger(0, response_size)) { + if (!value.getAsInteger(16, response_size)) { if (response_size == 0) return SendOKResponse(); StreamString response; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index eaed4e6742824..0bcfea5e9c68e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -164,7 +164,7 @@ GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer( if (name == "port") { // Make the Optional valid so we can use its value port = 0; - value.getAsInteger(0, *port); + value.getAsInteger(10, *port); } } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 20fa9f03d0a07..7d4d870ccbbbc 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -540,10 +540,10 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) { } else if (name == "alt-name") { reg_info.alt_name.SetString(value); } else if (name == "bitsize") { - if (!value.getAsInteger(0, reg_info.byte_size)) + if (!value.getAsInteger(10, reg_info.byte_size)) reg_info.byte_size /= CHAR_BIT; } else if (name == "offset") { - value.getAsInteger(0, reg_info.byte_offset); + value.getAsInteger(10, reg_info.byte_offset); } else if (name == "encoding") { const Encoding encoding = Args::StringToEncoding(value); if (encoding != eEncodingInvalid) @@ -2515,7 +2515,7 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) { .Default(eQueueKindUnknown); queue_vars_valid = queue_kind != eQueueKindUnknown; } else if (key.compare("qserialnum") == 0) { - if (!value.getAsInteger(0, queue_serial_number)) + if (!value.getAsInteger(10, queue_serial_number)) queue_vars_valid = true; } else if (key.compare("reason") == 0) { reason = std::string(value); @@ -2541,7 +2541,7 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) { std::tie(addr_str, bytes_str) = value.split('='); if (!addr_str.empty() && !bytes_str.empty()) { lldb::addr_t mem_cache_addr = LLDB_INVALID_ADDRESS; - if (!addr_str.getAsInteger(0, mem_cache_addr)) { + if (!addr_str.getAsInteger(16, mem_cache_addr)) { StringExtractor bytes(bytes_str); const size_t byte_size = bytes.GetBytesLeft() / 2; WritableDataBufferSP data_buffer_sp( @@ -2594,17 +2594,17 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) { description = std::string(ostr.GetString()); } else if (key.compare("addressing_bits") == 0) { uint64_t addressing_bits; - if (!value.getAsInteger(0, addressing_bits)) { + if (!value.getAsInteger(10, addressing_bits)) { addressable_bits.SetAddressableBits(addressing_bits); } } else if (key.compare("low_mem_addressing_bits") == 0) { uint64_t addressing_bits; - if (!value.getAsInteger(0, addressing_bits)) { + if (!value.getAsInteger(10, addressing_bits)) { addressable_bits.SetLowmemAddressableBits(addressing_bits); } } else if (key.compare("high_mem_addressing_bits") == 0) { uint64_t addressing_bits; - if (!value.getAsInteger(0, addressing_bits)) { + if (!value.getAsInteger(10, addressing_bits)) { addressable_bits.SetHighmemAddressableBits(addressing_bits); } } else if (key == "added-binaries") { @@ -6022,7 +6022,7 @@ std::string ProcessGDBRemote::HarmonizeThreadIdsForProfileData( if (profileDataExtractor.GetNameColonValue(usec_name, usec_value)) { if (usec_name == "thread_used_usec") { has_used_usec = true; - usec_value.getAsInteger(0, curr_used_usec); + usec_value.getAsInteger(10, curr_used_usec); } else { // We didn't find what we want, it is probably an older version. Bail // out. _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
