https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/80376
>From 70a518030f2b23ca130a8d0ea667729d7985795c Mon Sep 17 00:00:00 2001 From: Jason Molenda <jmole...@apple.com> Date: Thu, 1 Feb 2024 17:46:03 -0800 Subject: [PATCH 1/7] [lldb] Add QSupported key to report watchpoint types supported debugserver on arm64 devices can manage both Byte Address Select watchpoints (1-8 bytes) and MASK watchpoints (8 bytes-2 gigabytes). This adds a SupportedWatchpointTypes key to the QSupported response from debugserver with a list of these, so lldb can take full advantage of them when creating larger regions with a single hardware watchpoint. Also add documentation for this, and two other lldb extensions, to the lldb-gdb-remote.txt documentation. Re-enable TestLargeWatchpoint.py on Darwin systems when testing with the in-tree built debugserver. I can remove the "in-tree built debugserver" in the future when this new key is handled by an Xcode debugserver. --- lldb/docs/lldb-gdb-remote.txt | 37 +++++++++++++++++++ .../tools/lldb-server/gdbremote_testcase.py | 2 + .../GDBRemoteCommunicationClient.cpp | 21 +++++++++++ .../gdb-remote/GDBRemoteCommunicationClient.h | 4 ++ .../Process/gdb-remote/ProcessGDBRemote.cpp | 11 +----- .../large-watchpoint/TestLargeWatchpoint.py | 5 --- lldb/tools/debugserver/source/RNBRemote.cpp | 30 ++++++++------- 7 files changed, 82 insertions(+), 28 deletions(-) diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt index 58269e4c2b688..8db2fbc47b165 100644 --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -38,7 +38,44 @@ read packet: + read packet: $OK#9a send packet: + +//---------------------------------------------------------------------- +// "QSupported" +// +// BRIEF +// Query the GDB remote server for features it supports +// +// PRIORITY TO IMPLEMENT +// Optional. +//---------------------------------------------------------------------- +QSupported is a standard GDB Remote Serial Protocol packet, but +there are several additions to the response that lldb can parse. +An example exchange: + +send packet: qSupported:xmlRegisters=i386,arm,mips,arc;multiprocess+;fork-events+;vfork-events+ + +read packet: qXfer:features:read+;PacketSize=20000;qEcho+;native-signals+;SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;SupportedWatchpointTypes =aarch64-mask,aarch64-bas; + +In this example, three lldb extensions are reported: + PacketSize=20000 + The base16 maximum packet size that the GDB Remote Serial stub + can handle. + SupportedCompressions=<item,item,...> + A list of compression types that the GDB Remote Serial stub can use to + compress packets when the QEnableCompression packet is used to request one + of them. + SupportedWatchpointTypes=<item,item,...> + A list of watchpoint types that this GDB Remote Serial stub can manage. + Currently defined names are: + x86_64 64-bit x86-64 watchpoints + (1, 2, 4, 8 byte watchpoints aligned to those amounts) + aarch64-bas AArch64 Byte Address Select watchpoints + (any number of contiguous bytes within a doubleword) + aarch64-mask AArch64 MASK watchpoints + (any power-of-2 region of memory from 8 to 2GB, aligned) + +lldb will default to sending power-of-2 watchpoints up to a pointer size +(void*) in the target process if nothing is specified. //---------------------------------------------------------------------- // "A" - launch args packet diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py index 3341b6e54a3bc..75522158b3221 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py @@ -921,6 +921,8 @@ def add_qSupported_packets(self, client_features=[]): "qSaveCore", "native-signals", "QNonStop", + "SupportedWatchpointTypes", + "SupportedCompressions", ] def parse_qSupported_response(self, context): diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 7bb4498418513..c625adc87cbd4 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -403,6 +403,22 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() { x.split(compressions, ','); if (!compressions.empty()) MaybeEnableCompression(compressions); + } else if (x.consume_front("SupportedWatchpointTypes=")) { + llvm::SmallVector<llvm::StringRef, 4> watchpoint_types; + x.split(watchpoint_types, ','); + m_watchpoint_types = + WatchpointHardwareFeature::eWatchpointHardwareFeatureUnknown; + for (auto wp_type : watchpoint_types) { + if (wp_type == "x86_64") + m_watchpoint_types |= + WatchpointHardwareFeature::eWatchpointHardwareX86; + if (wp_type == "aarch64-mask") + m_watchpoint_types |= + WatchpointHardwareFeature::eWatchpointHardwareArmMASK; + if (wp_type == "aarch64-bas") + m_watchpoint_types |= + WatchpointHardwareFeature::eWatchpointHardwareArmBAS; + } } else if (x.consume_front("PacketSize=")) { StringExtractorGDBRemote packet_response(x); m_max_packet_size = @@ -1828,6 +1844,11 @@ std::optional<uint32_t> GDBRemoteCommunicationClient::GetWatchpointSlotCount() { return num; } +WatchpointHardwareFeature +GDBRemoteCommunicationClient::GetSupportedWatchpointTypes() { + return m_watchpoint_types; +} + std::optional<bool> GDBRemoteCommunicationClient::GetWatchpointReportedAfter() { if (m_qHostInfo_is_valid == eLazyBoolCalculate) GetHostInfo(); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h index 866b0773d86d5..f25b3f9dd1a6d 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -199,6 +199,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { std::optional<bool> GetWatchpointReportedAfter(); + lldb::WatchpointHardwareFeature GetSupportedWatchpointTypes(); + const ArchSpec &GetHostArchitecture(); std::chrono::seconds GetHostDefaultPacketTimeout(); @@ -581,6 +583,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { lldb::tid_t m_curr_tid_run = LLDB_INVALID_THREAD_ID; uint32_t m_num_supported_hardware_watchpoints = 0; + lldb::WatchpointHardwareFeature m_watchpoint_types = + lldb::WatchpointHardwareFeature::eWatchpointHardwareFeatureUnknown; uint32_t m_low_mem_addressing_bits = 0; uint32_t m_high_mem_addressing_bits = 0; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 4e3447e767c35..629b191f3117a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -3156,16 +3156,7 @@ Status ProcessGDBRemote::EnableWatchpoint(WatchpointSP wp_sp, bool notify) { ArchSpec target_arch = GetTarget().GetArchitecture(); WatchpointHardwareFeature supported_features = - eWatchpointHardwareFeatureUnknown; - - // LWP_TODO: enable MASK watchpoint for arm64 debugserver - // when it reports that it supports them. - if (target_arch.GetTriple().getOS() == llvm::Triple::MacOSX && - target_arch.GetTriple().getArch() == llvm::Triple::aarch64) { -#if 0 - supported_features |= eWatchpointHardwareArmMASK; -#endif - } + m_gdb_comm.GetSupportedWatchpointTypes(); std::vector<WatchpointResourceSP> resources = WatchpointAlgorithms::AtomizeWatchpointRequest( diff --git a/lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py b/lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py index f7ceb47c0b615..c5e161497e628 100644 --- a/lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py +++ b/lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py @@ -25,11 +25,6 @@ def continue_and_report_stop_reason(self, process, iter_str): @skipIf(archs=no_match(["arm64", "arm64e", "aarch64"])) @skipUnlessDarwin - # LWP_TODO: until debugserver advertises that it supports - # MASK watchpoints, this test can't be enabled, lldb won't - # try to send watchpoints larger than 8 bytes. - @skipIfDarwin - # debugserver only gained the ability to watch larger regions # with this patch. @skipIfOutOfTreeDebugserver diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index 224ed033fc421..20384920823d2 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -3557,10 +3557,10 @@ static bool GetProcessNameFrom_vAttach(const char *&p, rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) { uint32_t max_packet_size = 128 * 1024; // 128KBytes is a reasonable max packet // size--debugger can always use less - char buf[256]; - snprintf(buf, sizeof(buf), - "qXfer:features:read+;PacketSize=%x;qEcho+;native-signals+", - max_packet_size); + std::stringstream reply; + reply << "qXfer:features:read+;PacketSize=" << std::hex << max_packet_size + << ";"; + reply << "qEcho+;native-signals+;"; bool enable_compression = false; (void)enable_compression; @@ -3574,15 +3574,19 @@ rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) { #endif if (enable_compression) { - strcat(buf, ";SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;" - "DefaultCompressionMinSize="); - char numbuf[16]; - snprintf(numbuf, sizeof(numbuf), "%zu", m_compression_minsize); - numbuf[sizeof(numbuf) - 1] = '\0'; - strcat(buf, numbuf); - } - - return SendPacket(buf); + reply << "SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;"; + reply << "DefaultCompressionMinSize=" << std::dec << m_compression_minsize + << ";"; + } + +#if (defined(__arm64__) || defined(__aarch64__)) + reply << "SupportedWatchpointTypes=aarch64-mask,aarch64-bas;"; +#endif +#if defined(__x86_64__) + reply << "SupportedWatchpointTypes=x86_64;"; +#endif + + return SendPacket(reply.str().c_str()); } static bool process_does_not_exist (nub_process_t pid) { >From 17e0b7b06cd0cf2fe4f9e6c1f338afb536ec7796 Mon Sep 17 00:00:00 2001 From: Jason Molenda <jmole...@apple.com> Date: Thu, 1 Feb 2024 19:10:53 -0800 Subject: [PATCH 2/7] Clarify 'size of pointer' language. --- lldb/docs/lldb-gdb-remote.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt index 8db2fbc47b165..a291f6bd150eb 100644 --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -74,8 +74,8 @@ In this example, three lldb extensions are reported: aarch64-mask AArch64 MASK watchpoints (any power-of-2 region of memory from 8 to 2GB, aligned) -lldb will default to sending power-of-2 watchpoints up to a pointer size -(void*) in the target process if nothing is specified. +lldb will default to sending power-of-2 watchpoints up to a pointer size, +`sizeof(void*)`, in the target process if nothing is specified. //---------------------------------------------------------------------- // "A" - launch args packet >From f2bf59ba6d13598babfaf31b2470ffa233f92fc6 Mon Sep 17 00:00:00 2001 From: Jason Molenda <ja...@molenda.com> Date: Fri, 2 Feb 2024 11:40:52 -0800 Subject: [PATCH 3/7] Address David's initial feedback comments --- lldb/docs/lldb-gdb-remote.txt | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt index a291f6bd150eb..f668ec0ce150c 100644 --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -54,18 +54,17 @@ An example exchange: send packet: qSupported:xmlRegisters=i386,arm,mips,arc;multiprocess+;fork-events+;vfork-events+ -read packet: qXfer:features:read+;PacketSize=20000;qEcho+;native-signals+;SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;SupportedWatchpointTypes =aarch64-mask,aarch64-bas; +read packet: qXfer:features:read+;PacketSize=20000;qEcho+;native-signals+;SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;SupportedWatchpointTypes=aarch64-mask,aarch64-bas; In this example, three lldb extensions are reported: + PacketSize=20000 - The base16 maximum packet size that the GDB Remote Serial stub - can handle. + The base16 maximum packet size that the stub can handle. SupportedCompressions=<item,item,...> - A list of compression types that the GDB Remote Serial stub can use to - compress packets when the QEnableCompression packet is used to request one - of them. + A list of compression types that the stub can use to compress packets + when the QEnableCompression packet is used to request one of them. SupportedWatchpointTypes=<item,item,...> - A list of watchpoint types that this GDB Remote Serial stub can manage. + A list of watchpoint types that this stub can manage. Currently defined names are: x86_64 64-bit x86-64 watchpoints (1, 2, 4, 8 byte watchpoints aligned to those amounts) @@ -73,9 +72,9 @@ In this example, three lldb extensions are reported: (any number of contiguous bytes within a doubleword) aarch64-mask AArch64 MASK watchpoints (any power-of-2 region of memory from 8 to 2GB, aligned) - -lldb will default to sending power-of-2 watchpoints up to a pointer size, -`sizeof(void*)`, in the target process if nothing is specified. + If nothing is specified, lldb will default to sending power-of-2 + watchpoints, up to a pointer size, `sizeof(void*)`, a reasonable + baseline assumption. //---------------------------------------------------------------------- // "A" - launch args packet >From 3917dceab86633e6bba908539e965925d6356ac4 Mon Sep 17 00:00:00 2001 From: Jason Molenda <ja...@molenda.com> Date: Fri, 2 Feb 2024 14:10:33 -0800 Subject: [PATCH 4/7] Word tweak in doc. --- lldb/docs/lldb-gdb-remote.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt index f668ec0ce150c..3384f7a4ecb5f 100644 --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -56,7 +56,7 @@ send packet: qSupported:xmlRegisters=i386,arm,mips,arc;multiprocess+;fork-events read packet: qXfer:features:read+;PacketSize=20000;qEcho+;native-signals+;SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;SupportedWatchpointTypes=aarch64-mask,aarch64-bas; -In this example, three lldb extensions are reported: +In this example, three lldb extensions are shown: PacketSize=20000 The base16 maximum packet size that the stub can handle. >From 353d1fd039ac15bb4cc7f17bcd935ac0bbde9ca4 Mon Sep 17 00:00:00 2001 From: Jason Molenda <ja...@molenda.com> Date: Fri, 2 Feb 2024 18:38:42 -0800 Subject: [PATCH 5/7] Move the WatchpointHardwareFeature to lldb-private-enumerations WatchpointHardwareFeature is set by the Process plugin to WatchpointAlgorithms::AtomizeWatchpointRequest to indicate which types of watchpoints this target/stub support. It's entirely internal and should be in lldb-private-enumerations.h. To make a bitfield enum work with the bitfield, & and |, operators without lots of casting, I need the LLDB_MARK_AS_BITMASK_ENUM() constexpr template stuff from lldb-enumerations.h. It might be better to put this and FLAGS_ENUM() in their own file, but I don't think I'm messing up with any layering violations by having lldb-private-enumerations.h include lldb-enumerations.h to get them so I'll start with this. --- .../lldb/Breakpoint/WatchpointAlgorithms.h | 4 +-- lldb/include/lldb/lldb-enumerations.h | 26 ------------------ lldb/include/lldb/lldb-private-enumerations.h | 27 +++++++++++++++++++ .../Breakpoint/WatchpointAlgorithms.cpp | 3 +-- .../GDBRemoteCommunicationClient.cpp | 12 +++------ .../gdb-remote/GDBRemoteCommunicationClient.h | 6 ++--- 6 files changed, 37 insertions(+), 41 deletions(-) diff --git a/lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h b/lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h index 8871e4e5e84e6..5727e4581e81f 100644 --- a/lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h +++ b/lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h @@ -11,7 +11,7 @@ #include "lldb/Breakpoint/WatchpointResource.h" #include "lldb/Utility/ArchSpec.h" -#include "lldb/lldb-public.h" +#include "lldb/lldb-private.h" #include <vector> @@ -59,7 +59,7 @@ class WatchpointAlgorithms { /// watchpoint cannot be set. static std::vector<lldb::WatchpointResourceSP> AtomizeWatchpointRequest( lldb::addr_t addr, size_t size, bool read, bool write, - lldb::WatchpointHardwareFeature supported_features, ArchSpec &arch); + WatchpointHardwareFeature supported_features, ArchSpec &arch); struct Region { lldb::addr_t addr; diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 50cbccba4d7c5..392d333c23a44 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -448,32 +448,6 @@ enum WatchpointWriteType { eWatchpointWriteTypeOnModify }; -/// The hardware and native stub capabilities for a given target, -/// for translating a user's watchpoint request into hardware -/// capable watchpoint resources. -FLAGS_ENUM(WatchpointHardwareFeature){ - /// lldb will fall back to a default that assumes the target - /// can watch up to pointer-size power-of-2 regions, aligned to - /// power-of-2. - eWatchpointHardwareFeatureUnknown = (1u << 0), - - /// Intel systems can watch 1, 2, 4, or 8 bytes (in 64-bit targets), - /// aligned naturally. - eWatchpointHardwareX86 = (1u << 1), - - /// ARM systems with Byte Address Select watchpoints - /// can watch any consecutive series of bytes up to the - /// size of a pointer (4 or 8 bytes), at a pointer-size - /// alignment. - eWatchpointHardwareArmBAS = (1u << 2), - - /// ARM systems with MASK watchpoints can watch any power-of-2 - /// sized region from 8 bytes to 2 gigabytes, aligned to that - /// same power-of-2 alignment. - eWatchpointHardwareArmMASK = (1u << 3), -}; -LLDB_MARK_AS_BITMASK_ENUM(WatchpointHardwareFeature) - /// Programming language type. /// /// These enumerations use the same language enumerations as the DWARF diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h index 5f1597200a83e..9e8ab56305bef 100644 --- a/lldb/include/lldb/lldb-private-enumerations.h +++ b/lldb/include/lldb/lldb-private-enumerations.h @@ -9,6 +9,7 @@ #ifndef LLDB_LLDB_PRIVATE_ENUMERATIONS_H #define LLDB_LLDB_PRIVATE_ENUMERATIONS_H +#include "lldb/lldb-enumerations.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/FormatProviders.h" #include "llvm/Support/raw_ostream.h" @@ -282,4 +283,30 @@ enum InterruptionControl : bool { DoNotAllowInterruption = false, }; +/// The hardware and native stub capabilities for a given target, +/// for translating a user's watchpoint request into hardware +/// capable watchpoint resources. +FLAGS_ENUM(WatchpointHardwareFeature){ + /// lldb will fall back to a default that assumes the target + /// can watch up to pointer-size power-of-2 regions, aligned to + /// power-of-2. + eWatchpointHardwareFeatureUnknown = (1u << 0), + + /// Intel systems can watch 1, 2, 4, or 8 bytes (in 64-bit targets), + /// aligned naturally. + eWatchpointHardwareX86 = (1u << 1), + + /// ARM systems with Byte Address Select watchpoints + /// can watch any consecutive series of bytes up to the + /// size of a pointer (4 or 8 bytes), at a pointer-size + /// alignment. + eWatchpointHardwareArmBAS = (1u << 2), + + /// ARM systems with MASK watchpoints can watch any power-of-2 + /// sized region from 8 bytes to 2 gigabytes, aligned to that + /// same power-of-2 alignment. + eWatchpointHardwareArmMASK = (1u << 3), +}; +LLDB_MARK_AS_BITMASK_ENUM(WatchpointHardwareFeature) + #endif // LLDB_LLDB_PRIVATE_ENUMERATIONS_H diff --git a/lldb/source/Breakpoint/WatchpointAlgorithms.cpp b/lldb/source/Breakpoint/WatchpointAlgorithms.cpp index 94f1dfffbf293..3caf29b04317f 100644 --- a/lldb/source/Breakpoint/WatchpointAlgorithms.cpp +++ b/lldb/source/Breakpoint/WatchpointAlgorithms.cpp @@ -27,8 +27,7 @@ WatchpointAlgorithms::AtomizeWatchpointRequest( std::vector<Region> entries; - if (supported_features & - WatchpointHardwareFeature::eWatchpointHardwareArmMASK) { + if (supported_features & eWatchpointHardwareArmMASK) { entries = PowerOf2Watchpoints(addr, size, /*min_byte_size*/ 1, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index c625adc87cbd4..6f8aa26228994 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -406,18 +406,14 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() { } else if (x.consume_front("SupportedWatchpointTypes=")) { llvm::SmallVector<llvm::StringRef, 4> watchpoint_types; x.split(watchpoint_types, ','); - m_watchpoint_types = - WatchpointHardwareFeature::eWatchpointHardwareFeatureUnknown; + m_watchpoint_types = eWatchpointHardwareFeatureUnknown; for (auto wp_type : watchpoint_types) { if (wp_type == "x86_64") - m_watchpoint_types |= - WatchpointHardwareFeature::eWatchpointHardwareX86; + m_watchpoint_types |= eWatchpointHardwareX86; if (wp_type == "aarch64-mask") - m_watchpoint_types |= - WatchpointHardwareFeature::eWatchpointHardwareArmMASK; + m_watchpoint_types |= eWatchpointHardwareArmMASK; if (wp_type == "aarch64-bas") - m_watchpoint_types |= - WatchpointHardwareFeature::eWatchpointHardwareArmBAS; + m_watchpoint_types |= eWatchpointHardwareArmBAS; } } else if (x.consume_front("PacketSize=")) { StringExtractorGDBRemote packet_response(x); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h index f25b3f9dd1a6d..bd2d3e232b464 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -199,7 +199,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { std::optional<bool> GetWatchpointReportedAfter(); - lldb::WatchpointHardwareFeature GetSupportedWatchpointTypes(); + WatchpointHardwareFeature GetSupportedWatchpointTypes(); const ArchSpec &GetHostArchitecture(); @@ -583,8 +583,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { lldb::tid_t m_curr_tid_run = LLDB_INVALID_THREAD_ID; uint32_t m_num_supported_hardware_watchpoints = 0; - lldb::WatchpointHardwareFeature m_watchpoint_types = - lldb::WatchpointHardwareFeature::eWatchpointHardwareFeatureUnknown; + WatchpointHardwareFeature m_watchpoint_types = + eWatchpointHardwareFeatureUnknown; uint32_t m_low_mem_addressing_bits = 0; uint32_t m_high_mem_addressing_bits = 0; >From 996d049366445719a2b8df41081fbf41d1b88c55 Mon Sep 17 00:00:00 2001 From: Jason Molenda <ja...@molenda.com> Date: Mon, 5 Feb 2024 13:20:58 -0800 Subject: [PATCH 6/7] Space between "base" and the number, consistently. Addressing feedback from David Spickett. --- lldb/docs/lldb-gdb-remote.txt | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt index 3384f7a4ecb5f..a97bb44053d68 100644 --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -59,7 +59,7 @@ read packet: qXfer:features:read+;PacketSize=20000;qEcho+;native-signals+;Suppor In this example, three lldb extensions are shown: PacketSize=20000 - The base16 maximum packet size that the stub can handle. + The base 16 maximum packet size that the stub can handle. SupportedCompressions=<item,item,...> A list of compression types that the stub can use to compress packets when the QEnableCompression packet is used to request one of them. @@ -630,7 +630,7 @@ read packet: <binary data>/E<error code>;AAAAAAAAA With LLDB, for register information, remote GDB servers can add support for the "qRegisterInfoN" packet where "N" is a zero based -base16 register number that must start at zero and increase by one +base 16 register number that must start at zero and increase by one for each register that is supported. The response is done in typical GDB remote fashion where a series of "KEY:VALUE;" pairs are returned. An example for the x86_64 registers is included below: @@ -1089,7 +1089,7 @@ Suggested key names: // 64-bit slices so it may be impossible to know until you're attached to a real // process to know what you're working with. // -// All numeric fields return base-16 numbers without any "0x" prefix. +// All numeric fields return base 16 numbers without any "0x" prefix. //---------------------------------------------------------------------- An i386 process: @@ -1121,7 +1121,7 @@ main-binary-uuid: is the UUID of a firmware type binary that the gdb stub knows main-binary-address: is the load address of the firmware type binary main-binary-slide: is the slide of the firmware type binary, if address isn't known -binary-addresses: A comma-separated list of binary load addresses base16. +binary-addresses: A comma-separated list of binary load addresses base 16. lldb will parse the binaries in memory to get UUIDs, then try to find the binaries & debug info by UUID. Intended for use with a small number of firmware type binaries where the @@ -1338,7 +1338,7 @@ tuples to return are: dirty-pages:[<hexaddr>][,<hexaddr]; // A list of memory pages within this // region that are "dirty" -- they have been modified. - // Page addresses are in base16. The size of a page can + // Page addresses are in base 16. The size of a page can // be found from the qHostInfo's page-size key-value. // // If the stub supports identifying dirty pages within a @@ -1621,7 +1621,7 @@ for this region. // describe why something stopped. // // For "reason:watchpoint", "description" is an ascii-hex -// encoded string with between one and three base10 numbers, +// encoded string with between one and three base 10 numbers, // space separated. The three numbers are // 1. watchpoint address. This address should always be within // a memory region lldb has a watchpoint on. @@ -1947,7 +1947,7 @@ for this region. // // jThreadExtendedInfo:{"thread":612910} // -// Because this is a JSON string, the thread number is provided in base10. +// Because this is a JSON string, the thread number is provided in base 10. // Additional key-value pairs may be provided by lldb to the gdb remote // stub. For instance, on some versions of macOS, lldb can read offset // information out of the system libraries. Using those offsets, debugserver @@ -2016,13 +2016,13 @@ for this region. // // $N<uncompressed payload>#00 // -// $C<size of uncompressed payload in base10>:<compressed payload>#00 +// $C<size of uncompressed payload in base 10>:<compressed payload>#00 // // Where "#00" is the actual checksum value if noack mode is not enabled. The checksum // value is for the "N<uncompressed payload>" or -// "C<size of uncompressed payload in base10>:<compressed payload>" bytes in the packet. +// "C<size of uncompressed payload in base 10>:<compressed payload>" bytes in the packet. // -// The size of the uncompressed payload in base10 is provided because it will simplify +// The size of the uncompressed payload in base 10 is provided because it will simplify // decompression if the final buffer size needed is known ahead of time. // // Compression on low-latency connections is unlikely to be an improvement. Particularly >From 94f119f7320dc19ef2fd6e55d0ff8336e031116d Mon Sep 17 00:00:00 2001 From: Jason Molenda <ja...@molenda.com> Date: Mon, 5 Feb 2024 13:26:24 -0800 Subject: [PATCH 7/7] Two more copyedit suggestions from David in the docs. --- lldb/docs/lldb-gdb-remote.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt index a97bb44053d68..76ac3f28d73b0 100644 --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -50,13 +50,15 @@ send packet: + QSupported is a standard GDB Remote Serial Protocol packet, but there are several additions to the response that lldb can parse. +They are not all listed here. + An example exchange: send packet: qSupported:xmlRegisters=i386,arm,mips,arc;multiprocess+;fork-events+;vfork-events+ read packet: qXfer:features:read+;PacketSize=20000;qEcho+;native-signals+;SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;SupportedWatchpointTypes=aarch64-mask,aarch64-bas; -In this example, three lldb extensions are shown: +In the example above, three lldb extensions are shown: PacketSize=20000 The base 16 maximum packet size that the stub can handle. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits