diff --git a/lldb/docs/resources/lldbgdbremote.md b/lldb/docs/resources/lldbgdbremote.md index 3c7f21c83abe9..703f70f9ac1a8 100644 --- a/lldb/docs/resources/lldbgdbremote.md +++ b/lldb/docs/resources/lldbgdbremote.md @@ -632,6 +632,62 @@ running, then an error message is returned. do live tracing. Specifically, the name of the plug-in should match the name of the tracing technology returned by this packet. +## jMultiBreakpoint + +This packet allows setting and removing multiple breakpoints in one go. It +lists multiple `Z` and `z` packets using a JSON array of strings. +Formally: + +``` +$jMultiBreakpoint:{"breakpoint_requests" : ["request"[,"request"]*]} +``` + +Where each `request` is one of: + +``` +* z0,addr,kind +* z1,addr,kind +* z2,addr,kind +* z3,addr,kind +* z4,addr,kind +* Z0,addr,kind[;cond_list…][;cmds:persist,cmd_list…] +* Z1,addr,kind[;cond_list…][;cmds:persist,cmd_list…] +* Z2,addr,kind +* Z3,addr,kind +* Z4,addr,kind +``` + +Each field has the same meaning as the corresponding packet in the GDB Remote +Protocol. + +For example, the packet below is a request to set one breakpoint and to remove +two others: + +``` +$jMultiBreakpoint: {"breakpoint_requests": ["Z0,1025783e8,4", "z0,1025783ec,4", "z0,1025783e8,4"]} +``` + +The same address may be specified multiple times. + +The stub must execute the sequence of `request`s in the order they +appear in the `jMultiBreakpoint` packet. This is not an atomic operation: +individual requests may fail, and the stub must process subsequent requests +regardless of previous failures. + +The reply consists of a JSON dictionary with a single entry, `results`, which +is an array of strings, with the same contents allowed by a reply to a `z` or +`Z` packet. + +``` +{"results": ["OK", "E03", "OK"]} +``` + +A stub that supports this packet must include `jMultiBreakpoint+` in the reply +to `qSupported`. + +**Priority To Implement:** Low. This is a performance optimization, reducing +the number of packets sent when manipulating breakpoints. + ## jThreadExtendedInfo This packet, which takes its arguments as JSON and sends its reply as diff --git a/lldb/include/lldb/Breakpoint/BreakpointSite.h b/lldb/include/lldb/Breakpoint/BreakpointSite.h index e189ed77e261b..e40eb8e3545c1 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointSite.h +++ b/lldb/include/lldb/Breakpoint/BreakpointSite.h @@ -77,20 +77,6 @@ class BreakpointSite : public std::enable_shared_from_this, lldb::addr_t *intersect_addr, size_t *intersect_size, size_t *opcode_offset) const; - /// Tells whether the current breakpoint site is enabled or not - /// - /// This is a low-level enable bit for the breakpoint sites. If a - /// breakpoint site has no enabled constituents, it should just get removed. - /// This enable/disable is for the low-level target code to enable and disable - /// breakpoint sites when single stepping, etc. - bool IsEnabled() const; - - /// Sets whether the current breakpoint site is enabled or not - /// - /// \param[in] enabled - /// \b true if the breakpoint is enabled, \b false otherwise. - void SetEnabled(bool enabled); - /// Enquires of the breakpoint locations that produced this breakpoint site /// whether we should stop at this location. /// @@ -223,6 +209,12 @@ class BreakpointSite : public std::enable_shared_from_this, size_t RemoveConstituent(lldb::break_id_t break_id, lldb::break_id_t break_loc_id); + /// Sets whether the current breakpoint site is enabled or not. + /// + /// \param[in] enabled + /// \b true if the breakpoint is enabled, \b false otherwise. + void SetEnabled(bool enabled); + BreakpointSite::Type m_type; ///< The type of this breakpoint site. uint8_t m_saved_opcode[8]; ///< The saved opcode bytes if this breakpoint site ///uses trap opcodes. diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index b58a96e564cde..4fdf19615dbef 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -117,6 +117,7 @@ class ProcessProperties : public Properties { Args GetAlwaysRunThreadNames() const; FollowForkMode GetFollowForkMode() const; bool TrackMemoryCacheChanges() const; + bool GetUseDelayedBreakpoints() const; protected: Process *m_process; // Can be nullptr for global ProcessProperties @@ -2236,6 +2237,9 @@ class Process : public std::enable_shared_from_this, // Process Breakpoints size_t GetSoftwareBreakpointTrapOpcode(BreakpointSite *bp_site); + enum class BreakpointAction { Enable, Disable }; + +protected: virtual Status EnableBreakpointSite(BreakpointSite *bp_site) { return Status::FromErrorStringWithFormatv( "error: {0} does not support enabling breakpoints", GetPluginName()); @@ -2246,6 +2250,24 @@ class Process : public std::enable_shared_from_this, "error: {0} does not support disabling breakpoints", GetPluginName()); } + /// Compare BreakpointSiteSPs by ID, so that iteration order is independent + /// of pointer addresses. + struct SiteIDCmp { + bool operator()(const lldb::BreakpointSiteSP &lhs, + const lldb::BreakpointSiteSP &rhs) const { + return lhs->GetID() < rhs->GetID(); + } + }; + using BreakpointSiteToActionMap = + std::map; + + virtual llvm::Error + UpdateBreakpointSites(const BreakpointSiteToActionMap &site_to_action); + +public: + llvm::Error ExecuteBreakpointSiteAction(BreakpointSite &site, + Process::BreakpointAction action); + // This is implemented completely using the lldb::Process API. Subclasses // don't need to implement this function unless the standard flow of read // existing opcode, write breakpoint opcode, verify breakpoint opcode doesn't @@ -2274,6 +2296,17 @@ class Process : public std::enable_shared_from_this, Status EnableBreakpointSiteByID(lldb::user_id_t break_id); + bool IsBreakpointSiteEnabled(const BreakpointSite &site); + + bool IsBreakpointSitePhysicallyEnabled(const BreakpointSite &site); + + /// Reports whether this process should delay physically enabling/disabling + /// breakpoints until the process is about to resume. The default honors the + /// user-facing `target.process.use-delayed-breakpoints` setting. + virtual bool ShouldUseDelayedBreakpoints() const { + return GetUseDelayedBreakpoints(); + } + // BreakpointLocations use RemoveConstituentFromBreakpointSite to remove // themselves from the constituent's list of this breakpoint sites. void RemoveConstituentFromBreakpointSite(lldb::user_id_t site_id, @@ -3535,6 +3568,21 @@ void PruneThreadPlans(); /// GetExtendedCrashInformation. StructuredData::DictionarySP m_crash_info_dict_sp; + struct DelayedBreakpointCache { + void Enqueue(lldb::BreakpointSiteSP site, BreakpointAction action); + void RemoveSite(lldb::BreakpointSiteSP site) { + m_site_to_action.erase(site); + } + void Clear() { m_site_to_action.clear(); } + + BreakpointSiteToActionMap m_site_to_action; + }; + + DelayedBreakpointCache m_delayed_breakpoints; + std::recursive_mutex m_delayed_breakpoints_mutex; + + llvm::Error FlushDelayedBreakpoints(); + size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size, uint8_t *buf) const; @@ -3616,6 +3664,13 @@ void PruneThreadPlans(); void SetAddressableBitMasks(AddressableBits bit_masks); + // Updates the state of site. + // This should be used by derived Process classes after they have changed the + // state of a site. + void SetBreakpointSiteEnabled(BreakpointSite &site, bool is_enabled = true) { + site.SetEnabled(is_enabled); + } + private: Status DestroyImpl(bool force_kill); diff --git a/lldb/include/lldb/Utility/GDBRemote.h b/lldb/include/lldb/Utility/GDBRemote.h index ab700c00474ae..473df93191833 100644 --- a/lldb/include/lldb/Utility/GDBRemote.h +++ b/lldb/include/lldb/Utility/GDBRemote.h @@ -43,6 +43,9 @@ class StreamGDBRemote : public StreamString { /// Number of bytes written. // TODO: Convert this function to take ArrayRef int PutEscapedBytes(const void *s, size_t src_len); + + /// Equivalent to PutEscapedBytes(str.data(), str.size()); + int PutEscapedBytes(llvm::StringRef str); }; /// GDB remote packet as used by the GDB remote communication history. Packets diff --git a/lldb/include/lldb/Utility/Stream.h b/lldb/include/lldb/Utility/Stream.h index 37bcdc9924171..8277aae0e9554 100644 --- a/lldb/include/lldb/Utility/Stream.h +++ b/lldb/include/lldb/Utility/Stream.h @@ -225,6 +225,16 @@ class Stream { /// in one statement. Stream &operator<<(char ch); + /// Output the result of a formatv expression to the stream. + /// + /// \param[in] obj + /// A formatv_object_base produced by llvm::formatv(). + /// + /// \return + /// A reference to this class so multiple things can be streamed + /// in one statement. + Stream &operator<<(const llvm::formatv_object_base &obj); + Stream &operator<<(uint8_t uval) = delete; Stream &operator<<(uint16_t uval) = delete; Stream &operator<<(uint32_t uval) = delete; @@ -350,8 +360,10 @@ class Stream { size_t PrintfVarArg(const char *format, va_list args); + /// Forwards the arguments to llvm::formatv and writes to the stream. + /// FIXME: instead of this API, consider using llvm::formatv directly. template void Format(const char *format, Args &&... args) { - PutCString(llvm::formatv(format, std::forward(args)...).str()); + *this << llvm::formatv(format, std::forward(args)...); } /// Output a quoted C string value to the stream. diff --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h index dd468ef5bddef..f822dc13739e4 100644 --- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h +++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h @@ -171,6 +171,7 @@ class StringExtractorGDBRemote : public StringExtractor { eServerPacketType_jLLDBTraceStop, eServerPacketType_jLLDBTraceGetState, eServerPacketType_jLLDBTraceGetBinaryData, + eServerPacketType_jMultiBreakpoint, eServerPacketType_qMemTags, // read memory tags eServerPacketType_QMemTags, // write memory tags diff --git a/lldb/packages/Python/lldbsuite/test/lldbreverse.py b/lldb/packages/Python/lldbsuite/test/lldbreverse.py index d9a8daba3772d..50b97b5597f3a 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbreverse.py +++ b/lldb/packages/Python/lldbsuite/test/lldbreverse.py @@ -5,6 +5,7 @@ from lldbsuite.test.gdbclientutils import * from lldbsuite.test.lldbgdbproxy import * import lldbgdbserverutils +import json import re @@ -132,6 +133,10 @@ def respond(self, packet): if reply == "OK": self.update_breakpoints(packet) return reply + if packet.startswith("jMultiBreakpoint:"): + reply = self.pass_through(packet) + self.update_multi_breakpoints(packet, reply) + return reply return GDBProxyTestBase.respond(self, packet) def start_recording(self): @@ -161,6 +166,23 @@ def update_breakpoints(self, packet): else: self.breakpoints[t].discard((addr, kind)) + def update_multi_breakpoints(self, packet, reply): + # Remove the final ], which is binary-escaping the }. + packet = packet[:-1] + reply = reply[:-1] + body = packet[len("jMultiBreakpoint:") :] + requests = json.loads(body)["breakpoint_requests"] + try: + results = json.loads(reply)["results"] + except (ValueError, KeyError): + # Empty/unsupported/error reply: nothing was installed. + return + if len(requests) != len(results): + raise ValueError("jMultiBreakpoint response count mismatch") + for inner_packet, result in zip(requests, results): + if result == "OK": + self.update_breakpoints(inner_packet) + def breakpoint_triggered_at(self, pc): if any(addr == pc for addr, kind in self.breakpoints[SOFTWARE_BREAKPOINTS]): return True diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 26d949ffbf5e4..002b01b4998f8 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -18,6 +18,7 @@ import lldb from . import lldbtest_config from . import configuration +from lldbsuite.test.gdbclientutils import escape_binary # How often failed simulator process launches are retried. SIMULATOR_RETRY = 3 @@ -1936,3 +1937,24 @@ def ignore_swift_stdlib_when_stepping(platform, tester): "settings set " "target.process.thread.step-avoid-libraries {}".format(lib_name) ) + +# Binary escapes `packet_str`, sends it to the remote and returns the reply. +def send_packet_get_reply(test, packet_str): + packet_str = escape_binary(packet_str) + test.runCmd(f"proc plugin packet send '{packet_str}'", check=False) + # The output is of the form: + # packet: + # response: + output = test.res.GetOutput() + reply = output.split("\n") + packet = reply[0].strip() + response = reply[1].strip() + + test.assertTrue(packet.startswith("packet: "), output) + test.assertTrue(response.startswith("response: "), output) + return response[len("response: ") :].strip() + + +def get_qsupported_capabilities(test): + reply = send_packet_get_reply(test, "qSupported") + return reply.strip().split(";") 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 5b973a9d926e5..5d669854472b1 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 @@ -925,6 +925,7 @@ def add_qSupported_packets(self, client_features=[]): "SupportedWatchpointTypes", "SupportedCompressions", "MultiMemRead", + "jMultiBreakpoint", ] def parse_qSupported_response(self, context): diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp b/lldb/source/Breakpoint/BreakpointSite.cpp index 8639379afe1df..584ff1ff52c07 100644 --- a/lldb/source/Breakpoint/BreakpointSite.cpp +++ b/lldb/source/Breakpoint/BreakpointSite.cpp @@ -135,8 +135,6 @@ const uint8_t *BreakpointSite::GetSavedOpcodeBytes() const { return &m_saved_opcode[0]; } -bool BreakpointSite::IsEnabled() const { return m_enabled; } - void BreakpointSite::SetEnabled(bool enabled) { m_enabled = enabled; } void BreakpointSite::AddConstituent(const BreakpointLocationSP &constituent) { diff --git a/lldb/source/Core/UserSettingsController.cpp b/lldb/source/Core/UserSettingsController.cpp index 5408d64b40647..206b2072ddaf2 100644 --- a/lldb/source/Core/UserSettingsController.cpp +++ b/lldb/source/Core/UserSettingsController.cpp @@ -55,7 +55,7 @@ void Properties::DumpAllPropertyValues(const ExecutionContext *exe_ctx, bool is_json) { if (is_json) { llvm::json::Value json = m_collection_sp->ToJSON(exe_ctx); - strm.Printf("%s", llvm::formatv("{0:2}", json).str().c_str()); + strm << llvm::formatv("{0:2}", json); } else m_collection_sp->DumpValue(exe_ctx, strm, dump_mask); } diff --git a/lldb/source/Interpreter/OptionValueProperties.cpp b/lldb/source/Interpreter/OptionValueProperties.cpp index cb71bffa8fe25..dbcaaf8add490 100644 --- a/lldb/source/Interpreter/OptionValueProperties.cpp +++ b/lldb/source/Interpreter/OptionValueProperties.cpp @@ -364,11 +364,9 @@ Status OptionValueProperties::DumpPropertyValue(const ExecutionContext *exe_ctx, if (dump_mask & ~eDumpOptionName) strm.PutChar(' '); } - if (is_json) { - strm.Printf( - "%s", - llvm::formatv("{0:2}", value_sp->ToJSON(exe_ctx)).str().c_str()); - } else + if (is_json) + strm << llvm::formatv("{0:2}", value_sp->ToJSON(exe_ctx)); + else value_sp->DumpValue(exe_ctx, strm, dump_mask); } return error; diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp index 3c26afa3d4861..b8219efe53a74 100644 --- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp @@ -635,9 +635,9 @@ Status ProcessKDP::EnableBreakpointSite(BreakpointSite *bp_site) { if (m_comm.LocalBreakpointsAreSupported()) { Status error; - if (!bp_site->IsEnabled()) { + if (!IsBreakpointSitePhysicallyEnabled(*bp_site)) { if (m_comm.SendRequestBreakpoint(true, bp_site->GetLoadAddress())) { - bp_site->SetEnabled(true); + SetBreakpointSiteEnabled(*bp_site); bp_site->SetType(BreakpointSite::eExternal); } else { return Status::FromErrorString("KDP set breakpoint failed"); @@ -651,15 +651,15 @@ Status ProcessKDP::EnableBreakpointSite(BreakpointSite *bp_site) { Status ProcessKDP::DisableBreakpointSite(BreakpointSite *bp_site) { if (m_comm.LocalBreakpointsAreSupported()) { Status error; - if (bp_site->IsEnabled()) { + if (IsBreakpointSitePhysicallyEnabled(*bp_site)) { BreakpointSite::Type bp_type = bp_site->GetType(); if (bp_type == BreakpointSite::eExternal) { if (m_destroy_in_process && m_comm.IsRunning()) { // We are trying to destroy our connection and we are running - bp_site->SetEnabled(false); + SetBreakpointSiteEnabled(*bp_site, false); } else { if (m_comm.SendRequestBreakpoint(false, bp_site->GetLoadAddress())) - bp_site->SetEnabled(false); + SetBreakpointSiteEnabled(*bp_site, false); else return Status::FromErrorString("KDP remove breakpoint failed"); } diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp index d374742b5722c..5fbc1f62a065f 100644 --- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp +++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp @@ -636,7 +636,7 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException( addr_t pc = reg_ctx_sp->GetPC(); BreakpointSiteSP bp_site_sp = process_sp->GetBreakpointSiteList().FindByAddress(pc); - if (bp_site_sp && bp_site_sp->IsEnabled()) + if (bp_site_sp && process_sp->IsBreakpointSitePhysicallyEnabled(*bp_site_sp)) thread.SetThreadStoppedAtUnexecutedBP(pc); switch (exc_type) { @@ -771,7 +771,8 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException( if (!bp_site_sp && reg_ctx_sp) { bp_site_sp = process_sp->GetBreakpointSiteList().FindByAddress(pc); } - if (bp_site_sp && bp_site_sp->IsEnabled()) { + if (bp_site_sp && + process_sp->IsBreakpointSitePhysicallyEnabled(*bp_site_sp)) { // We've hit this breakpoint, whether it was intended for this thread // or not. Clear this in the Tread object so we step past it on resume. thread.SetThreadHitBreakpointSite(); @@ -865,7 +866,8 @@ bool StopInfoMachException::WasContinueInterrupted(Thread &thread) { // We have a hardware breakpoint -- this is the kernel bug. auto &bp_site_list = process_sp->GetBreakpointSiteList(); for (auto &site : bp_site_list.Sites()) { - if (site->IsHardware() && site->IsEnabled()) { + if (site->IsHardware() && + process_sp->IsBreakpointSitePhysicallyEnabled(*site)) { LLDB_LOGF(log, "Thread stopped with insn-step completed mach exception but " "thread was not stepping; there is a hardware breakpoint set."); diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp index 4417eafcae125..c1a8a185214ff 100644 --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -414,7 +414,7 @@ void ProcessWindows::RefreshStateAfterStop() { // If we're at a BreakpointSite, mark this as an Unexecuted Breakpoint. // We'll clear that state if we've actually executed the breakpoint. BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc)); - if (site && site->IsEnabled()) + if (site && IsBreakpointSitePhysicallyEnabled(*site)) stop_thread->SetThreadStoppedAtUnexecutedBP(pc); switch (active_exception->GetExceptionCode()) { diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h index d3e3f9a5ed0ce..e26177147c224 100644 --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h @@ -44,6 +44,8 @@ class ProcessWindows : public Process, public ProcessDebugger { Status EnableBreakpointSite(BreakpointSite *bp_site) override; Status DisableBreakpointSite(BreakpointSite *bp_site) override; + bool ShouldUseDelayedBreakpoints() const override { return false; } + Status DoDetach(bool keep_stopped) override; Status DoLaunch(Module *exe_module, ProcessLaunchInfo &launch_info) override; Status DoAttachToProcessWithID( diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp index 406fa06ea011a..fa927b74d56fa 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp @@ -9,6 +9,7 @@ #include "GDBRemoteClientBase.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/ErrorExtras.h" #include "lldb/Target/UnixSignals.h" #include "lldb/Utility/LLDBAssert.h" @@ -194,6 +195,22 @@ GDBRemoteClientBase::SendPacketAndWaitForResponse( return SendPacketAndWaitForResponseNoLock(payload, response, sync_on_timeout); } +llvm::Expected +GDBRemoteClientBase::SendPacketAndExpectResponse( + llvm::StringRef payload, std::chrono::seconds interrupt_timeout) { + StringExtractorGDBRemote response; + GDBRemoteCommunication::PacketResult packet_result = + SendPacketAndWaitForResponse(payload, response, interrupt_timeout); + if (packet_result != GDBRemoteCommunication::PacketResult::Success) + return llvm::createStringErrorV("failed to send packet: '{0}'", payload); + + if (response.IsUnsupportedResponse()) + return llvm::createStringErrorV("unsupported response: '{0}'", + response.GetStringRef()); + + return std::move(response); +} + GDBRemoteCommunication::PacketResult GDBRemoteClientBase::ReadPacketWithOutputSupport( StringExtractorGDBRemote &response, Timeout timeout, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h index 9c17a8c1de057..65738491c21dd 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h @@ -74,6 +74,11 @@ class GDBRemoteClientBase : public GDBRemoteCommunication, public Broadcaster { std::chrono::seconds interrupt_timeout, llvm::function_ref output_callback); + /// Wrapper around SendPacketAndWaitForResponse that returns an `Expected`. + llvm::Expected SendPacketAndExpectResponse( + llvm::StringRef payload, + std::chrono::seconds interrupt_timeout = std::chrono::seconds(0)); + class Lock { public: // If interrupt_timeout == 0 seconds, only take the lock if the target is diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 354de8b765a6d..ebf3e2b452978 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -217,6 +217,12 @@ bool GDBRemoteCommunicationClient::GetMultiMemReadSupported() { return m_supports_multi_mem_read == eLazyBoolYes; } +bool GDBRemoteCommunicationClient::GetMultiBreakpointSupported() { + if (m_supports_multi_breakpoint == eLazyBoolCalculate) + GetRemoteQSupported(); + return m_supports_multi_breakpoint == eLazyBoolYes; +} + bool GDBRemoteCommunicationClient::QueryNoAckModeSupported() { if (m_supports_not_sending_acks == eLazyBoolCalculate) { m_send_acks = true; @@ -346,6 +352,7 @@ void GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) { m_supported_async_json_packets_sp.reset(); m_supports_jModulesInfo = true; m_supports_multi_mem_read = eLazyBoolCalculate; + m_supports_multi_breakpoint = eLazyBoolCalculate; } // These flags should be reset when we first connect to a GDB server and when @@ -373,6 +380,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() { m_supports_reverse_continue = eLazyBoolNo; m_supports_reverse_step = eLazyBoolNo; m_supports_multi_mem_read = eLazyBoolNo; + m_supports_multi_breakpoint = eLazyBoolNo; m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if // not, we assume no limit @@ -434,6 +442,8 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() { m_supports_reverse_step = eLazyBoolYes; else if (x == "MultiMemRead+") m_supports_multi_mem_read = eLazyBoolYes; + else if (x == "jMultiBreakpoint+") + m_supports_multi_breakpoint = eLazyBoolYes; // Look for a list of compressions in the features list e.g. // qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib- // deflate,lzma diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h index c337066f43257..22406673a36df 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -344,6 +344,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { bool GetMultiMemReadSupported(); + bool GetMultiBreakpointSupported(); + LazyBool SupportsAllocDeallocMemory() // const { // Uncomment this to have lldb pretend the debug server doesn't respond to @@ -579,6 +581,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { LazyBool m_supports_reverse_continue = eLazyBoolCalculate; LazyBool m_supports_reverse_step = eLazyBoolCalculate; LazyBool m_supports_multi_mem_read = eLazyBoolCalculate; + LazyBool m_supports_multi_breakpoint = eLazyBoolCalculate; bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1, m_supports_qUserName : 1, m_supports_qGroupName : 1, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index 89d2730cfccd0..5ca425256a8b9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include "GDBRemoteCommunicationServerLLGS.h" #include "lldb/Host/ConnectionFileDescriptor.h" @@ -212,6 +213,9 @@ void GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() { RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_jLLDBTraceGetBinaryData, &GDBRemoteCommunicationServerLLGS::Handle_jLLDBTraceGetBinaryData); + RegisterMemberFunctionHandler( + StringExtractorGDBRemote::eServerPacketType_jMultiBreakpoint, + &GDBRemoteCommunicationServerLLGS::Handle_jMultiBreakpoint); RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_g, &GDBRemoteCommunicationServerLLGS::Handle_g); @@ -2824,155 +2828,146 @@ GDBRemoteCommunicationServerLLGS::Handle_qMemoryRegionInfo( return SendPacketNoLock(response.GetString()); } -GDBRemoteCommunication::PacketResult -GDBRemoteCommunicationServerLLGS::Handle_Z(StringExtractorGDBRemote &packet) { +namespace { +struct UseBreakpoint { + bool want_hardware = false; +}; +struct UseWatchpoint { + uint32_t flags; + static constexpr bool want_hardware = true; +}; +struct InvalidStoppoint {}; + +std::variant +getBreakpointKind(GDBStoppointType stoppoint_type) { + switch (stoppoint_type) { + case eBreakpointSoftware: + return UseBreakpoint{/*want_hardware*/ false}; + case eBreakpointHardware: + return UseBreakpoint{/*want_hardware*/ true}; + case eWatchpointWrite: + return UseWatchpoint{/*flags*/ 1}; + case eWatchpointRead: + return UseWatchpoint{/*flags*/ 2}; + case eWatchpointReadWrite: + return UseWatchpoint{/*flags*/ 3}; + case eStoppointInvalid: + return InvalidStoppoint(); + } + llvm_unreachable("unhandled GDBStoppointType"); +} +} // namespace + +GDBRemoteCommunicationServerLLGS::BreakpointResult +GDBRemoteCommunicationServerLLGS::ExecuteSetBreakpoint( + llvm::StringRef packet_str) { // Ensure we have a process. if (!m_current_process || (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) { Log *log = GetLog(LLDBLog::Process); LLDB_LOG(log, "failed, no process available"); - return SendErrorResponse(0x15); + return BreakpointError{0x15}; } + StringExtractorGDBRemote packet(packet_str); + // Parse out software or hardware breakpoint or watchpoint requested. packet.SetFilePos(strlen("Z")); if (packet.GetBytesLeft() < 1) - return SendIllFormedResponse( - packet, "Too short Z packet, missing software/hardware specifier"); - - bool want_breakpoint = true; - bool want_hardware = false; - uint32_t watch_flags = 0; + return BreakpointIllFormed{ + "Too short Z packet, missing software/hardware specifier"}; const GDBStoppointType stoppoint_type = GDBStoppointType(packet.GetS32(eStoppointInvalid)); - switch (stoppoint_type) { - case eBreakpointSoftware: - want_hardware = false; - want_breakpoint = true; - break; - case eBreakpointHardware: - want_hardware = true; - want_breakpoint = true; - break; - case eWatchpointWrite: - watch_flags = 1; - want_hardware = true; - want_breakpoint = false; - break; - case eWatchpointRead: - watch_flags = 2; - want_hardware = true; - want_breakpoint = false; - break; - case eWatchpointReadWrite: - watch_flags = 3; - want_hardware = true; - want_breakpoint = false; - break; - case eStoppointInvalid: - return SendIllFormedResponse( - packet, "Z packet had invalid software/hardware specifier"); - } + std::variant bp_variant = + getBreakpointKind(stoppoint_type); + if (std::holds_alternative(bp_variant)) + return BreakpointIllFormed{ + "Z packet had invalid software/hardware specifier"}; if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',') - return SendIllFormedResponse( - packet, "Malformed Z packet, expecting comma after stoppoint type"); + return BreakpointIllFormed{ + "Malformed Z packet, expecting comma after stoppoint type"}; // Parse out the stoppoint address. if (packet.GetBytesLeft() < 1) - return SendIllFormedResponse(packet, "Too short Z packet, missing address"); + return BreakpointIllFormed{"Too short Z packet, missing address"}; const lldb::addr_t addr = packet.GetHexMaxU64(false, 0); if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',') - return SendIllFormedResponse( - packet, "Malformed Z packet, expecting comma after address"); + return BreakpointIllFormed{ + "Malformed Z packet, expecting comma after address"}; // Parse out the stoppoint size (i.e. size hint for opcode size). const uint32_t size = packet.GetHexMaxU32(false, std::numeric_limits::max()); if (size == std::numeric_limits::max()) - return SendIllFormedResponse( - packet, "Malformed Z packet, failed to parse size argument"); + return BreakpointIllFormed{ + "Malformed Z packet, failed to parse size argument"}; - if (want_breakpoint) { - // Try to set the breakpoint. + // Try to set a breakpoint. + if (auto *bp_kind = std::get_if(&bp_variant)) { const Status error = - m_current_process->SetBreakpoint(addr, size, want_hardware); + m_current_process->SetBreakpoint(addr, size, bp_kind->want_hardware); if (error.Success()) - return SendOKResponse(); + return BreakpointOK(); Log *log = GetLog(LLDBLog::Breakpoints); LLDB_LOG(log, "pid {0} failed to set breakpoint: {1}", m_current_process->GetID(), error); - return SendErrorResponse(0x09); - } else { - // Try to set the watchpoint. - const Status error = m_current_process->SetWatchpoint( - addr, size, watch_flags, want_hardware); - if (error.Success()) - return SendOKResponse(); - Log *log = GetLog(LLDBLog::Watchpoints); - LLDB_LOG(log, "pid {0} failed to set watchpoint: {1}", - m_current_process->GetID(), error); - return SendErrorResponse(0x09); - } + return BreakpointError{0x09}; + } + + // Try to set a watchpoint. + auto wp_kind = std::get(bp_variant); + const Status error = m_current_process->SetWatchpoint( + addr, size, wp_kind.flags, wp_kind.want_hardware); + if (error.Success()) + return BreakpointOK(); + Log *log = GetLog(LLDBLog::Watchpoints); + LLDB_LOG(log, "pid {0} failed to set watchpoint: {1}", + m_current_process->GetID(), error); + return BreakpointError{0x09}; } -GDBRemoteCommunication::PacketResult -GDBRemoteCommunicationServerLLGS::Handle_z(StringExtractorGDBRemote &packet) { +GDBRemoteCommunicationServerLLGS::BreakpointResult +GDBRemoteCommunicationServerLLGS::ExecuteRemoveBreakpoint( + llvm::StringRef packet_str) { // Ensure we have a process. if (!m_current_process || (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) { Log *log = GetLog(LLDBLog::Process); LLDB_LOG(log, "failed, no process available"); - return SendErrorResponse(0x15); + return BreakpointError{0x15}; } + StringExtractorGDBRemote packet(packet_str); + // Parse out software or hardware breakpoint or watchpoint requested. packet.SetFilePos(strlen("z")); if (packet.GetBytesLeft() < 1) - return SendIllFormedResponse( - packet, "Too short z packet, missing software/hardware specifier"); - - bool want_breakpoint = true; - bool want_hardware = false; + return BreakpointIllFormed{ + "Too short z packet, missing software/hardware specifier"}; const GDBStoppointType stoppoint_type = GDBStoppointType(packet.GetS32(eStoppointInvalid)); - switch (stoppoint_type) { - case eBreakpointHardware: - want_breakpoint = true; - want_hardware = true; - break; - case eBreakpointSoftware: - want_breakpoint = true; - break; - case eWatchpointWrite: - want_breakpoint = false; - break; - case eWatchpointRead: - want_breakpoint = false; - break; - case eWatchpointReadWrite: - want_breakpoint = false; - break; - default: - return SendIllFormedResponse( - packet, "z packet had invalid software/hardware specifier"); - } + std::variant bp_variant = + getBreakpointKind(stoppoint_type); + if (std::holds_alternative(bp_variant)) + return BreakpointIllFormed{ + "z packet had invalid software/hardware specifier"}; if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',') - return SendIllFormedResponse( - packet, "Malformed z packet, expecting comma after stoppoint type"); + return BreakpointIllFormed{ + "Malformed z packet, expecting comma after stoppoint type"}; // Parse out the stoppoint address. if (packet.GetBytesLeft() < 1) - return SendIllFormedResponse(packet, "Too short z packet, missing address"); + return BreakpointIllFormed{"Too short z packet, missing address"}; const lldb::addr_t addr = packet.GetHexMaxU64(false, 0); if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',') - return SendIllFormedResponse( - packet, "Malformed z packet, expecting comma after address"); + return BreakpointIllFormed{ + "Malformed z packet, expecting comma after address"}; /* // Parse out the stoppoint size (i.e. size hint for opcode size). @@ -2983,26 +2978,121 @@ GDBRemoteCommunicationServerLLGS::Handle_z(StringExtractorGDBRemote &packet) { size argument"); */ - if (want_breakpoint) { - // Try to clear the breakpoint. + // Try to clear the breakpoint. + if (auto *bp_kind = std::get_if(&bp_variant)) { const Status error = - m_current_process->RemoveBreakpoint(addr, want_hardware); + m_current_process->RemoveBreakpoint(addr, bp_kind->want_hardware); if (error.Success()) - return SendOKResponse(); + return BreakpointOK(); Log *log = GetLog(LLDBLog::Breakpoints); LLDB_LOG(log, "pid {0} failed to remove breakpoint: {1}", m_current_process->GetID(), error); - return SendErrorResponse(0x09); - } else { - // Try to clear the watchpoint. - const Status error = m_current_process->RemoveWatchpoint(addr); - if (error.Success()) - return SendOKResponse(); - Log *log = GetLog(LLDBLog::Watchpoints); - LLDB_LOG(log, "pid {0} failed to remove watchpoint: {1}", - m_current_process->GetID(), error); - return SendErrorResponse(0x09); + return BreakpointError{0x09}; + } + // Try to clear the watchpoint. + const Status error = m_current_process->RemoveWatchpoint(addr); + if (error.Success()) + return BreakpointOK(); + Log *log = GetLog(LLDBLog::Watchpoints); + LLDB_LOG(log, "pid {0} failed to remove watchpoint: {1}", + m_current_process->GetID(), error); + return BreakpointError{0x09}; +} + +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::SendBreakpointResponse( + StringExtractorGDBRemote &packet, const BreakpointResult &result) { + return std::visit( + [&](auto &&arg) { + using T = std::decay_t; + static_assert(std::is_same_v || + std::is_same_v || + std::is_same_v, + "non-exhaustive visitor!"); + if constexpr (std::is_same_v) + return SendOKResponse(); + else if constexpr (std::is_same_v) + return SendErrorResponse(arg.error_code); + else + return SendIllFormedResponse(packet, arg.message.c_str()); + }, + result); +} + +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::Handle_Z(StringExtractorGDBRemote &packet) { + return SendBreakpointResponse(packet, + ExecuteSetBreakpoint(packet.GetStringRef())); +} + +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::Handle_z(StringExtractorGDBRemote &packet) { + return SendBreakpointResponse(packet, + ExecuteRemoveBreakpoint(packet.GetStringRef())); +} + +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::Handle_jMultiBreakpoint( + StringExtractorGDBRemote &packet) { + llvm::StringRef packet_str = packet.GetStringRef(); + if (!packet_str.consume_front("jMultiBreakpoint:")) + return SendIllFormedResponse(packet, + "Invalid jMultiBreakpoint packet prefix"); + + llvm::Expected parsed = llvm::json::parse(packet_str); + if (!parsed) { + llvm::consumeError(parsed.takeError()); + return SendIllFormedResponse(packet, + "jMultiBreakpoint did not contain valid JSON"); } + llvm::json::Object *request_dict = parsed->getAsObject(); + if (!request_dict) + return SendIllFormedResponse( + packet, "jMultiBreakpoint did not contain a JSON dictionary"); + + llvm::json::Array *request_array = + request_dict->getArray("breakpoint_requests"); + if (!request_array) + return SendIllFormedResponse( + packet, + "jMultiBreakpoint did not contain a valid 'breakpoint_requests' field"); + + llvm::json::Array reply_array; + for (const llvm::json::Value &value : *request_array) { + std::optional request = value.getAsString(); + if (!request) + return SendIllFormedResponse(packet, + "jMultiBreakpoint had a non-string entry"); + BreakpointResult result = request->starts_with("Z") + ? ExecuteSetBreakpoint(*request) + : ExecuteRemoveBreakpoint(*request); + std::visit( + [&](const auto &arg) { + using T = std::decay_t; + static_assert(std::is_same_v || + std::is_same_v || + std::is_same_v, + "non-exhaustive visitor!"); + if constexpr (std::is_same_v) + reply_array.push_back("OK"); + else if constexpr (std::is_same_v) + reply_array.push_back( + llvm::formatv("E{0:X-2}", arg.error_code).str()); + else + reply_array.push_back("E03"); + }, + result); + } + + llvm::json::Object dict; + dict.try_emplace("results", std::move(reply_array)); + + StreamString stream; + stream.AsRawOstream() << llvm::json::Value(std::move(dict)); + StringRef response_str = stream.GetString(); + StreamGDBRemote response; + response.PutEscapedBytes(response_str.data(), response_str.size()); + return SendPacketNoLock(response.GetString()); } GDBRemoteCommunication::PacketResult @@ -4221,6 +4311,7 @@ std::vector GDBRemoteCommunicationServerLLGS::HandleFeatures( "QListThreadsInStopReply+", "qXfer:features:read+", "QNonStop+", + "jMultiBreakpoint+", }); // report server-only features diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h index 646b6a102abf6..9c41f5e302181 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -271,6 +271,8 @@ class GDBRemoteCommunicationServerLLGS PacketResult Handle_T(StringExtractorGDBRemote &packet); + PacketResult Handle_jMultiBreakpoint(StringExtractorGDBRemote &packet); + void SetCurrentThreadID(lldb::tid_t tid); lldb::tid_t GetCurrentThreadID() const; @@ -302,6 +304,28 @@ class GDBRemoteCommunicationServerLLGS private: llvm::Expected> BuildTargetXml(); + struct BreakpointOK {}; + struct BreakpointIllFormed { + std::string message; + }; + struct BreakpointError { + uint8_t error_code; + }; + + using BreakpointResult = + std::variant; + + /// Core logic for a Z (set breakpoint/watchpoint) request. + BreakpointResult ExecuteSetBreakpoint(llvm::StringRef packet_str); + + /// Core logic for a z (remove breakpoint/watchpoint) request. + BreakpointResult ExecuteRemoveBreakpoint(llvm::StringRef packet_str); + + /// Convert a BreakpointResult into a PacketResult, sending the appropriate + /// response. + PacketResult SendBreakpointResponse(StringExtractorGDBRemote &packet, + const BreakpointResult &result); + void HandleInferiorState_Exited(NativeProcessProtocol *process); void HandleInferiorState_Stopped(NativeProcessProtocol *process); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 0eb7648affd2e..4d70233e7f6c9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1795,7 +1795,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo( addr_t pc = thread_sp->GetRegisterContext()->GetPC(); BreakpointSiteSP bp_site_sp = thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc); - if (bp_site_sp && bp_site_sp->IsEnabled()) + if (bp_site_sp && IsBreakpointSitePhysicallyEnabled(*bp_site_sp)) thread_sp->SetThreadStoppedAtUnexecutedBP(pc); if (exc_type != 0) { @@ -1979,7 +1979,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo( // BreakpointSites in any other location, but we can't know for // sure what happened so it's a reasonable default. if (bp_site_sp) { - if (bp_site_sp->IsEnabled()) + if (IsBreakpointSitePhysicallyEnabled(*bp_site_sp)) thread_sp->SetThreadHitBreakpointSite(); if (bp_site_sp->ValidForThisThread(*thread_sp)) { @@ -3274,8 +3274,93 @@ size_t ProcessGDBRemote::PutSTDIN(const char *src, size_t src_len, return 0; } +/// Enable a single breakpoint site by trying Z0 (software), then Z1 +/// (hardware), then manual memory write as a last resort. +llvm::Error ProcessGDBRemote::DoEnableBreakpointSite(BreakpointSite &bp_site) { + Log *log = GetLog(GDBRLog::Breakpoints); + const addr_t addr = bp_site.GetLoadAddress(); + const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(&bp_site); + auto &gdb_comm = GetGDBRemote(); + + // SupportsGDBStoppointPacket always returns true unless a previously sent + // packet failed. As such, query the function before AND after sending the + // packet. + if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware) && + !bp_site.HardwareRequired()) { + uint8_t error_no = gdb_comm.SendGDBStoppointTypePacket( + eBreakpointSoftware, true, addr, bp_op_size, GetInterruptTimeout()); + if (error_no == 0) { + SetBreakpointSiteEnabled(bp_site); + bp_site.SetType(BreakpointSite::eExternal); + return llvm::Error::success(); + } + if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) { + if (error_no != UINT8_MAX) + return llvm::createStringErrorV( + "error sending the breakpoint request: {0}", error_no); + return llvm::createStringError("error sending the breakpoint request"); + } + LLDB_LOG(log, "Software breakpoints are unsupported"); + } + + // Like above, this is also queried twice. + if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) { + uint8_t error_no = gdb_comm.SendGDBStoppointTypePacket( + eBreakpointHardware, true, addr, bp_op_size, GetInterruptTimeout()); + if (error_no == 0) { + SetBreakpointSiteEnabled(bp_site); + bp_site.SetType(BreakpointSite::eHardware); + return llvm::Error::success(); + } + if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) { + if (error_no != UINT8_MAX) + return llvm::createStringErrorV( + "error sending the hardware breakpoint request: {0} " + "(hardware breakpoint resources might be exhausted or unavailable)", + error_no); + return llvm::createStringError( + "error sending the hardware breakpoint request " + "(hardware breakpoint resources might be exhausted or unavailable)"); + } + LLDB_LOG(log, "Hardware breakpoints are unsupported"); + } + + if (bp_site.HardwareRequired()) + return llvm::createStringError("hardware breakpoints are not supported"); + + return EnableSoftwareBreakpoint(&bp_site).takeError(); +} + +/// Disable a single breakpoint site directly by sending the appropriate +/// z packet or restoring the original instruction. +llvm::Error ProcessGDBRemote::DoDisableBreakpointSite(BreakpointSite &bp_site) { + const addr_t addr = bp_site.GetLoadAddress(); + const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(&bp_site); + auto &gdb_comm = GetGDBRemote(); + + switch (bp_site.GetType()) { + case BreakpointSite::eSoftware: { + Status error = DisableSoftwareBreakpoint(&bp_site); + if (error.Fail()) + return error.takeError(); + break; + } + case BreakpointSite::eHardware: + if (gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, false, addr, + bp_op_size, GetInterruptTimeout())) + return llvm::createStringError("unknown error"); + break; + case BreakpointSite::eExternal: + if (gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false, addr, + bp_op_size, GetInterruptTimeout())) + return llvm::createStringError("unknown error"); + break; + } + SetBreakpointSiteEnabled(bp_site, false); + return llvm::Error::success(); +} + Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) { - Status error; assert(bp_site != nullptr); // Get logging info @@ -3292,115 +3377,18 @@ Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) { site_id, (uint64_t)addr); // Breakpoint already exists and is enabled - if (bp_site->IsEnabled()) { + if (IsBreakpointSitePhysicallyEnabled(*bp_site)) { LLDB_LOGF(log, "ProcessGDBRemote::EnableBreakpointSite (size_id = %" PRIu64 ") address = 0x%" PRIx64 " -- SUCCESS (already enabled)", site_id, (uint64_t)addr); - return error; - } - - // Get the software breakpoint trap opcode size - const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(bp_site); - - // SupportsGDBStoppointPacket() simply checks a boolean, indicating if this - // breakpoint type is supported by the remote stub. These are set to true by - // default, and later set to false only after we receive an unimplemented - // response when sending a breakpoint packet. This means initially that - // unless we were specifically instructed to use a hardware breakpoint, LLDB - // will attempt to set a software breakpoint. HardwareRequired() also queries - // a boolean variable which indicates if the user specifically asked for - // hardware breakpoints. If true then we will skip over software - // breakpoints. - if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware) && - (!bp_site->HardwareRequired())) { - // Try to send off a software breakpoint packet ($Z0) - uint8_t error_no = m_gdb_comm.SendGDBStoppointTypePacket( - eBreakpointSoftware, true, addr, bp_op_size, GetInterruptTimeout()); - if (error_no == 0) { - // The breakpoint was placed successfully - bp_site->SetEnabled(true); - bp_site->SetType(BreakpointSite::eExternal); - return error; - } - - // SendGDBStoppointTypePacket() will return an error if it was unable to - // set this breakpoint. We need to differentiate between a error specific - // to placing this breakpoint or if we have learned that this breakpoint - // type is unsupported. To do this, we must test the support boolean for - // this breakpoint type to see if it now indicates that this breakpoint - // type is unsupported. If they are still supported then we should return - // with the error code. If they are now unsupported, then we would like to - // fall through and try another form of breakpoint. - if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) { - if (error_no != UINT8_MAX) - error = Status::FromErrorStringWithFormat( - "error: %d sending the breakpoint request", error_no); - else - error = Status::FromErrorString("error sending the breakpoint request"); - return error; - } - - // We reach here when software breakpoints have been found to be - // unsupported. For future calls to set a breakpoint, we will not attempt - // to set a breakpoint with a type that is known not to be supported. - LLDB_LOGF(log, "Software breakpoints are unsupported"); - - // So we will fall through and try a hardware breakpoint - } - - // The process of setting a hardware breakpoint is much the same as above. - // We check the supported boolean for this breakpoint type, and if it is - // thought to be supported then we will try to set this breakpoint with a - // hardware breakpoint. - if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) { - // Try to send off a hardware breakpoint packet ($Z1) - uint8_t error_no = m_gdb_comm.SendGDBStoppointTypePacket( - eBreakpointHardware, true, addr, bp_op_size, GetInterruptTimeout()); - if (error_no == 0) { - // The breakpoint was placed successfully - bp_site->SetEnabled(true); - bp_site->SetType(BreakpointSite::eHardware); - return error; - } - - // Check if the error was something other then an unsupported breakpoint - // type - if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) { - // Unable to set this hardware breakpoint - if (error_no != UINT8_MAX) - error = Status::FromErrorStringWithFormat( - "error: %d sending the hardware breakpoint request " - "(hardware breakpoint resources might be exhausted or unavailable)", - error_no); - else - error = Status::FromErrorString( - "error sending the hardware breakpoint request " - "(hardware breakpoint resources " - "might be exhausted or unavailable)"); - return error; - } - - // We will reach here when the stub gives an unsupported response to a - // hardware breakpoint - LLDB_LOGF(log, "Hardware breakpoints are unsupported"); - - // Finally we will falling through to a #trap style breakpoint - } - - // Don't fall through when hardware breakpoints were specifically requested - if (bp_site->HardwareRequired()) { - error = Status::FromErrorString("hardware breakpoints are not supported"); - return error; + return Status(); } - // As a last resort we want to place a manual breakpoint. An instruction is - // placed into the process memory using memory write packets. - return EnableSoftwareBreakpoint(bp_site); + return Status::FromError(DoEnableBreakpointSite(*bp_site)); } Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) { - Status error; assert(bp_site != nullptr); addr_t addr = bp_site->GetLoadAddress(); user_id_t site_id = bp_site->GetID(); @@ -3410,42 +3398,15 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) { ") addr = 0x%8.8" PRIx64, site_id, (uint64_t)addr); - if (bp_site->IsEnabled()) { - const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(bp_site); - - BreakpointSite::Type bp_type = bp_site->GetType(); - switch (bp_type) { - case BreakpointSite::eSoftware: - error = DisableSoftwareBreakpoint(bp_site); - break; - - case BreakpointSite::eHardware: - if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, false, - addr, bp_op_size, - GetInterruptTimeout())) - error = Status::FromErrorString("unknown error"); - break; - - case BreakpointSite::eExternal: { - if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false, - addr, bp_op_size, - GetInterruptTimeout())) - error = Status::FromErrorString("unknown error"); - } break; - } - if (error.Success()) - bp_site->SetEnabled(false); - } else { + if (!IsBreakpointSitePhysicallyEnabled(*bp_site)) { LLDB_LOGF(log, "ProcessGDBRemote::DisableBreakpointSite (site_id = %" PRIu64 ") addr = 0x%8.8" PRIx64 " -- SUCCESS (already disabled)", site_id, (uint64_t)addr); - return error; + return Status(); } - if (error.Success()) - error = Status::FromErrorString("unknown error"); - return error; + return Status::FromError(DoDisableBreakpointSite(*bp_site)); } // Pre-requisite: wp != NULL. @@ -6055,7 +6016,7 @@ CommandObject *ProcessGDBRemote::GetPluginCommandObject() { void ProcessGDBRemote::DidForkSwitchSoftwareBreakpoints(bool enable) { GetBreakpointSiteList().ForEach([this, enable](BreakpointSite *bp_site) { - if (bp_site->IsEnabled() && + if (IsBreakpointSitePhysicallyEnabled(*bp_site) && (bp_site->GetType() == BreakpointSite::eSoftware || bp_site->GetType() == BreakpointSite::eExternal)) { m_gdb_comm.SendGDBStoppointTypePacket( @@ -6068,7 +6029,7 @@ void ProcessGDBRemote::DidForkSwitchSoftwareBreakpoints(bool enable) { void ProcessGDBRemote::DidForkSwitchHardwareTraps(bool enable) { if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) { GetBreakpointSiteList().ForEach([this, enable](BreakpointSite *bp_site) { - if (bp_site->IsEnabled() && + if (IsBreakpointSitePhysicallyEnabled(*bp_site) && bp_site->GetType() == BreakpointSite::eHardware) { m_gdb_comm.SendGDBStoppointTypePacket( eBreakpointHardware, enable, bp_site->GetLoadAddress(), @@ -6229,3 +6190,173 @@ void ProcessGDBRemote::DidExec() { } Process::DidExec(); } + +llvm::Error ProcessGDBRemote::UpdateBreakpointSitesNotBatched( + const BreakpointSiteToActionMap &site_to_action) { + llvm::Error joined = llvm::Error::success(); + for (auto &[site, action] : site_to_action) { + llvm::Error error = action == Process::BreakpointAction::Enable + ? DoEnableBreakpointSite(*site) + : DoDisableBreakpointSite(*site); + joined = llvm::joinErrors(std::move(joined), std::move(error)); + } + return joined; +} + +/// Parse a MultiBreakpoint response into per-request results. +/// Returns a vector of results: std::nullopt means OK, a uint8_t value is the +/// error code from an Exx response. +static llvm::SmallVector> +ParseMultiBreakpointResponse(llvm::StringRef response_str) { + llvm::SmallVector> results; + + StructuredData::ObjectSP parsed = StructuredData::ParseJSON(response_str); + StructuredData::Dictionary *dict = + parsed ? parsed->GetAsDictionary() : nullptr; + StructuredData::Array *array = nullptr; + if (dict) + dict->GetValueForKeyAsArray("results", array); + if (!array) + return results; + + array->ForEach([&results](StructuredData::Object *object) -> bool { + llvm::StringRef token; + if (auto *string = object->GetAsString()) + token = string->GetValue(); + if (token == "OK") { + results.push_back(std::nullopt); + return true; + } + if (token.size() != 3 || !token.starts_with("E")) { + results.push_back(uint8_t(0xff)); + return true; + } + uint8_t error_code = 0; + if (token.drop_front(1).getAsInteger(16, error_code)) + results.push_back(0xff); + else + results.push_back(error_code); + return true; + }); + return results; +} + +/// Determine the GDB stoppoint type for a breakpoint site by checking which +/// packet types the remote supports (for insertions), or by checking the site +/// type (for deletions). +static std::optional +GetStoppointType(BreakpointSite &site, bool insert, + GDBRemoteCommunicationClient &gdb_comm) { + if (insert) { + if (!site.HardwareRequired() && + gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) + return eBreakpointSoftware; + if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) + return eBreakpointHardware; + return std::nullopt; + } + + switch (site.GetType()) { + case BreakpointSite::eExternal: + return eBreakpointSoftware; + case BreakpointSite::eHardware: + return eBreakpointHardware; + case BreakpointSite::eSoftware: + return std::nullopt; + } + llvm_unreachable("unhandled BreakpointSite type"); +} + +namespace { +struct BreakpointPacketInfo { + BreakpointSite &site; + size_t trap_opcode_size; + GDBStoppointType type; + bool is_enable; +}; + +std::string to_string(const BreakpointPacketInfo &info) { + char packet = info.is_enable ? 'Z' : 'z'; + return llvm::formatv("{0}{1},{2:x-},{3:x-}", packet, + static_cast(info.type), info.site.GetLoadAddress(), + info.trap_opcode_size) + .str(); +} +} // namespace + +llvm::Error ProcessGDBRemote::UpdateBreakpointSites( + const BreakpointSiteToActionMap &site_to_action) { + if (site_to_action.empty()) + return llvm::Error::success(); + if (!m_gdb_comm.GetMultiBreakpointSupported()) + return UpdateBreakpointSitesNotBatched(site_to_action); + + Log *log = GetLog(GDBRLog::Breakpoints); + + std::vector breakpoint_infos; + for (auto [site, action] : site_to_action) { + size_t trap_opcode_size = GetSoftwareBreakpointTrapOpcode(site.get()); + std::optional type = + GetStoppointType(*site, action == BreakpointAction::Enable, m_gdb_comm); + + if (!type) { + LLDB_LOG(log, "MultiBreakpoint: site {0} at {1:x} can't be batched", + site->GetID(), site->GetLoadAddress()); + return UpdateBreakpointSitesNotBatched(site_to_action); + } + + breakpoint_infos.push_back( + {*site, trap_opcode_size, *type, action == BreakpointAction::Enable}); + } + + StreamString stream; + stream << "jMultiBreakpoint:"; + + auto args_array = std::make_shared(); + for (auto &bp_info : breakpoint_infos) + args_array->AddStringItem(to_string(bp_info)); + + StructuredData::Dictionary packet_dict; + packet_dict.AddItem("breakpoint_requests", args_array); + packet_dict.Dump(stream, false); + + StreamGDBRemote escaped_stream; + escaped_stream.PutEscapedBytes(stream.GetString()); + llvm::Expected response = + m_gdb_comm.SendPacketAndExpectResponse(escaped_stream.GetString(), + GetInterruptTimeout()); + + if (!response) { + LLDB_LOG_ERROR(log, response.takeError(), "jMultiBreakpoint failed: {0}"); + return UpdateBreakpointSitesNotBatched(site_to_action); + } + + llvm::SmallVector> results = + ParseMultiBreakpointResponse(response->GetStringRef()); + + // This is a protocol violation, do nothing. + if (results.size() != breakpoint_infos.size()) + return llvm::createStringErrorV( + "MultiBreakpoint response count mismatch (expected {0}, got {1})", + site_to_action.size(), results.size()); + + llvm::Error joined = llvm::Error::success(); + for (auto [error_code, bp_info] : + llvm::zip_equal(results, breakpoint_infos)) { + BreakpointSite &site = bp_info.site; + if (error_code) { + auto error = llvm::createStringErrorV( + "MultiBreakpoint: site {0} at {1:x} failed with E{2}", + bp_info.site.GetID(), bp_info.site.GetLoadAddress(), error_code); + joined = llvm::joinErrors(std::move(joined), std::move(error)); + continue; + } + SetBreakpointSiteEnabled(site, bp_info.is_enable); + if (bp_info.is_enable) + site.SetType(bp_info.type == eBreakpointHardware + ? BreakpointSite::eHardware + : BreakpointSite::eExternal); + } + + return joined; +} diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 61c20067622e2..d43b1bb296226 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -170,6 +170,9 @@ class ProcessGDBRemote : public Process, // Process Breakpoints Status EnableBreakpointSite(BreakpointSite *bp_site) override; + llvm::Error UpdateBreakpointSites( + const BreakpointSiteToActionMap &site_to_action) override; + Status DisableBreakpointSite(BreakpointSite *bp_site) override; // Process Watchpoints @@ -450,6 +453,17 @@ class ProcessGDBRemote : public Process, std::map m_thread_id_to_used_usec_map; uint64_t m_last_signals_version = 0; + /// Enable a single breakpoint site by trying Z0 (software), then Z1 + /// (hardware), then manual memory write as a last resort. + llvm::Error DoEnableBreakpointSite(BreakpointSite &bp_site); + + /// Disable a single breakpoint site directly by sending the appropriate + /// z packet or restoring the original instruction. + llvm::Error DoDisableBreakpointSite(BreakpointSite &bp_site); + + llvm::Error UpdateBreakpointSitesNotBatched( + const BreakpointSiteToActionMap &site_to_action); + static bool NewThreadNotifyBreakpointHit(void *baton, StoppointCallbackContext *context, lldb::user_id_t break_id, diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp index b359aad199161..dc3433ebfc4aa 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp @@ -271,7 +271,7 @@ size_t ScriptedProcess::DoWriteMemory(lldb::addr_t vm_addr, const void *buf, Status ScriptedProcess::EnableBreakpointSite(BreakpointSite *bp_site) { assert(bp_site != nullptr); - if (bp_site->IsEnabled()) { + if (IsBreakpointSitePhysicallyEnabled(*bp_site)) { return {}; } diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp index f1d39a85cb18e..d11d134295579 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp @@ -313,9 +313,10 @@ bool ScriptedThread::CalculateStopInfo() { // if we CreateStopReasonWithBreakpointSiteID. if (RegisterContextSP reg_ctx_sp = GetRegisterContext()) { addr_t pc = reg_ctx_sp->GetPC(); + ProcessSP proc = GetProcess(); if (BreakpointSiteSP bp_site_sp = - GetProcess()->GetBreakpointSiteList().FindByAddress(pc)) - if (bp_site_sp->IsEnabled()) + proc->GetBreakpointSiteList().FindByAddress(pc)) + if (proc->IsBreakpointSitePhysicallyEnabled(*bp_site_sp)) SetThreadStoppedAtUnexecutedBP(pc); } diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 90d727b17df67..f256e327cd181 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -84,6 +84,17 @@ using namespace lldb; using namespace lldb_private; using namespace std::chrono; +void Process::DelayedBreakpointCache::Enqueue(lldb::BreakpointSiteSP site, + BreakpointAction action) { + auto [previous, inserted] = m_site_to_action.insert({site, action}); + // New site or already enqueued for the same action. + if (inserted || previous->second == action) + return; + // Previously enqueued for the opposite action, don't update the site. + m_site_to_action.erase(previous); + assert(site->m_enabled == (action == BreakpointAction::Enable)); +} + class ProcessOptionValueProperties : public Cloneable { public: @@ -335,6 +346,12 @@ bool ProcessProperties::GetStopOnExec() const { idx, g_process_properties[idx].default_uint_value != 0); } +bool ProcessProperties::GetUseDelayedBreakpoints() const { + const uint32_t idx = ePropertyUseDelayedBreakpoints; + return GetPropertyAtIndexAs( + idx, g_process_properties[idx].default_uint_value != 0); +} + std::chrono::seconds ProcessProperties::GetUtilityExpressionTimeout() const { const uint32_t idx = ePropertyUtilityExpressionTimeout; uint64_t value = GetPropertyAtIndexAs( @@ -1722,8 +1739,8 @@ Process::GetBreakpointSiteList() const { void Process::DisableAllBreakpointSites() { m_breakpoint_site_list.ForEach([this](BreakpointSite *bp_site) -> void { - // bp_site->SetEnabled(true); - DisableBreakpointSite(bp_site); + llvm::consumeError( + ExecuteBreakpointSiteAction(*bp_site, BreakpointAction::Disable)); }); } @@ -1740,8 +1757,9 @@ Status Process::DisableBreakpointSiteByID(lldb::user_id_t break_id) { Status error; BreakpointSiteSP bp_site_sp = m_breakpoint_site_list.FindByID(break_id); if (bp_site_sp) { - if (bp_site_sp->IsEnabled()) - error = DisableBreakpointSite(bp_site_sp.get()); + if (IsBreakpointSiteEnabled(*bp_site_sp)) + error = Status::FromError( + ExecuteBreakpointSiteAction(*bp_site_sp, BreakpointAction::Disable)); } else { error = Status::FromErrorStringWithFormat( "invalid breakpoint site ID: %" PRIu64, break_id); @@ -1750,12 +1768,40 @@ Status Process::DisableBreakpointSiteByID(lldb::user_id_t break_id) { return error; } +llvm::Error Process::ExecuteBreakpointSiteAction(BreakpointSite &site, + BreakpointAction action) { + auto site_sp = site.shared_from_this(); + std::unique_lock guard(m_delayed_breakpoints_mutex); + + // Ignore requests that won't change the Site status. + if (IsBreakpointSiteEnabled(*site_sp) == (action == BreakpointAction::Enable)) + return llvm::Error::success(); + + if (ShouldUseDelayedBreakpoints()) { + m_delayed_breakpoints.Enqueue(site_sp, action); + return llvm::Error::success(); + } + + m_delayed_breakpoints.RemoveSite(site_sp); + guard.unlock(); + + switch (action) { + case BreakpointAction::Enable: + return EnableBreakpointSite(site_sp.get()).takeError(); + case BreakpointAction::Disable: + return DisableBreakpointSite(site_sp.get()).takeError(); + } + + llvm_unreachable("Unhandled BreakpointAction"); +} + Status Process::EnableBreakpointSiteByID(lldb::user_id_t break_id) { Status error; BreakpointSiteSP bp_site_sp = m_breakpoint_site_list.FindByID(break_id); if (bp_site_sp) { - if (!bp_site_sp->IsEnabled()) - error = EnableBreakpointSite(bp_site_sp.get()); + if (!IsBreakpointSiteEnabled(*bp_site_sp)) + error = Status::FromError( + ExecuteBreakpointSiteAction(*bp_site_sp, BreakpointAction::Enable)); } else { error = Status::FromErrorStringWithFormat( "invalid breakpoint site ID: %" PRIu64, break_id); @@ -1763,13 +1809,26 @@ Status Process::EnableBreakpointSiteByID(lldb::user_id_t break_id) { return error; } -lldb::break_id_t -Process::CreateBreakpointSite(const BreakpointLocationSP &constituent, - bool use_hardware) { - addr_t load_addr = LLDB_INVALID_ADDRESS; +bool Process::IsBreakpointSiteEnabled(const BreakpointSite &site) { + std::lock_guard guard(m_delayed_breakpoints_mutex); + + // `site` won't be mutated, but the cache stores mutable pointers. + auto it = m_delayed_breakpoints.m_site_to_action.find( + const_cast(site).shared_from_this()); + + // If no actions are delayed, use the current state of the site. + if (it == m_delayed_breakpoints.m_site_to_action.end()) + return site.m_enabled; + + return it->second == BreakpointAction::Enable; +} - bool show_error = true; - switch (GetState()) { +bool Process::IsBreakpointSitePhysicallyEnabled(const BreakpointSite &site) { + return site.m_enabled; +} + +static bool ShouldShowError(Process &process) { + switch (process.GetState()) { case eStateInvalid: case eStateUnloaded: case eStateConnected: @@ -1777,80 +1836,133 @@ Process::CreateBreakpointSite(const BreakpointLocationSP &constituent, case eStateLaunching: case eStateDetached: case eStateExited: - show_error = false; - break; - + return false; case eStateStopped: case eStateRunning: case eStateStepping: case eStateCrashed: case eStateSuspended: - show_error = IsAlive(); - break; + return process.IsAlive(); } + llvm_unreachable("unhandled process state"); +} +static addr_t ComputeConstituentLoadAddress(BreakpointLocation &constituent, + Process &proc) { // Reset the IsIndirect flag here, in case the location changes from pointing - // to a indirect symbol to a regular symbol. - constituent->SetIsIndirect(false); + // from an indirect symbol to a regular symbol. + constituent.SetIsIndirect(false); - if (constituent->ShouldResolveIndirectFunctions()) { - Symbol *symbol = constituent->GetAddress().CalculateSymbolContextSymbol(); - if (symbol && symbol->IsIndirect()) { - Status error; - Address symbol_address = symbol->GetAddress(); - load_addr = ResolveIndirectFunction(&symbol_address, error); - if (!error.Success() && show_error) { - GetTarget().GetDebugger().GetAsyncErrorStream()->Printf( - "warning: failed to resolve indirect function at 0x%" PRIx64 - " for breakpoint %i.%i: %s\n", - symbol->GetLoadAddress(&GetTarget()), - constituent->GetBreakpoint().GetID(), constituent->GetID(), - error.AsCString() ? error.AsCString() : "unknown error"); - return LLDB_INVALID_BREAK_ID; - } - Address resolved_address(load_addr); - load_addr = resolved_address.GetOpcodeLoadAddress(&GetTarget()); - constituent->SetIsIndirect(true); - } else - load_addr = constituent->GetAddress().GetOpcodeLoadAddress(&GetTarget()); - } else - load_addr = constituent->GetAddress().GetOpcodeLoadAddress(&GetTarget()); + Target &target = proc.GetTarget(); - if (load_addr != LLDB_INVALID_ADDRESS) { - BreakpointSiteSP bp_site_sp; + if (!constituent.ShouldResolveIndirectFunctions()) + return constituent.GetAddress().GetOpcodeLoadAddress(&target); - // Look up this breakpoint site. If it exists, then add this new - // constituent, otherwise create a new breakpoint site and add it. + const Symbol *symbol = + constituent.GetAddress().CalculateSymbolContextSymbol(); + if (!symbol || !symbol->IsIndirect()) + return constituent.GetAddress().GetOpcodeLoadAddress(&target); - bp_site_sp = m_breakpoint_site_list.FindByAddress(load_addr); + // An indirect symbol is involved. + Status error; + Address symbol_address = symbol->GetAddress(); + addr_t load_addr = proc.ResolveIndirectFunction(&symbol_address, error); + + if (!error.Success() && ShouldShowError(proc)) { + target.GetDebugger().GetAsyncErrorStream()->Printf( + "warning: failed to resolve indirect function at 0x%" PRIx64 + " for breakpoint %i.%i: %s\n", + symbol->GetLoadAddress(&target), constituent.GetBreakpoint().GetID(), + constituent.GetID(), + error.AsCString() ? error.AsCString() : "unknown error"); + // FIXME: ShouldShowError must only guard the error message. + // FIXME: Use diagnostics instead of printing "warning" to the async output. + return LLDB_INVALID_ADDRESS; + } - if (bp_site_sp) { - bp_site_sp->AddConstituent(constituent); - constituent->SetBreakpointSite(bp_site_sp); - return bp_site_sp->GetID(); - } else { - bp_site_sp.reset( - new BreakpointSite(constituent, load_addr, use_hardware)); - if (bp_site_sp) { - Status error = EnableBreakpointSite(bp_site_sp.get()); - if (error.Success()) { - constituent->SetBreakpointSite(bp_site_sp); - return m_breakpoint_site_list.Add(bp_site_sp); - } else { - if (show_error || use_hardware) { - // Report error for setting breakpoint... - GetTarget().GetDebugger().GetAsyncErrorStream()->Printf( - "warning: failed to set breakpoint site at 0x%" PRIx64 - " for breakpoint %i.%i: %s\n", - load_addr, constituent->GetBreakpoint().GetID(), - constituent->GetID(), - error.AsCString() ? error.AsCString() : "unknown error"); - } - } - } - } + Address resolved_address(load_addr); + constituent.SetIsIndirect(true); + return resolved_address.GetOpcodeLoadAddress(&target); +} + +llvm::Error Process::FlushDelayedBreakpoints() { + std::unique_lock guard(m_delayed_breakpoints_mutex); + + // Clear the cache in m_delayed_breakpoints so it can't affect the actual + // enabling of breakpoints. For example, if `EnableSoftwareBreakpoint` is + // called outside of FlushDelayedBreakpoints, it needs to check the delayed + // breakpoints and possibly early return. However, when called from + // FlushDelayedBreakpoints, the queue better be empty so that no early returns + // take place. + auto site_to_action = std::move(m_delayed_breakpoints.m_site_to_action); + m_delayed_breakpoints.m_site_to_action.clear(); + + guard.unlock(); + // Use a copy of the cache so that iteration is safe. + return UpdateBreakpointSites(site_to_action); +} + +llvm::Error Process::UpdateBreakpointSites( + const BreakpointSiteToActionMap &site_to_action) { + llvm::Error error = llvm::Error::success(); + for (auto [site, action] : site_to_action) { + Status new_error = action == BreakpointAction::Enable + ? EnableBreakpointSite(site.get()) + : DisableBreakpointSite(site.get()); + error = llvm::joinErrors(std::move(error), new_error.takeError()); + } + return error; +} + +lldb::break_id_t +Process::CreateBreakpointSite(const BreakpointLocationSP &constituent, + bool use_hardware) { + addr_t load_addr = ComputeConstituentLoadAddress(*constituent, *this); + + if (load_addr == LLDB_INVALID_ADDRESS) + return LLDB_INVALID_BREAK_ID; + + // Look up this breakpoint site. If it exists, then add this new + // constituent, otherwise create a new breakpoint site and add it. + if (BreakpointSiteSP bp_site_sp = + m_breakpoint_site_list.FindByAddress(load_addr)) { + bp_site_sp->AddConstituent(constituent); + constituent->SetBreakpointSite(bp_site_sp); + return bp_site_sp->GetID(); + } + + BreakpointSiteSP bp_site_sp( + new BreakpointSite(constituent, load_addr, use_hardware)); + + bool bp_from_address = + constituent->GetBreakpoint().GetResolver()->GetResolverTy() == + BreakpointResolver::ResolverTy::AddressResolver; + bool should_be_eager = use_hardware || bp_from_address; + + // If this breakpoint must be eager, flush the breakpoint queue in case there + // is an interaction between the sites in the queue and this new site. + if (should_be_eager) + if (llvm::Error E = FlushDelayedBreakpoints()) + LLDB_LOG_ERROR( + GetLog(LLDBLog::Breakpoints), std::move(E), + "eager breakpoint requested, but failed to flush breakpoints: {0}"); + + auto error = should_be_eager ? EnableBreakpointSite(bp_site_sp.get()) + : Status::FromError(ExecuteBreakpointSiteAction( + *bp_site_sp, BreakpointAction::Enable)); + if (error.Success()) { + constituent->SetBreakpointSite(bp_site_sp); + return m_breakpoint_site_list.Add(bp_site_sp); + } + + if (ShouldShowError(*this) || use_hardware) { + // Report error for setting breakpoint... + GetTarget().GetDebugger().GetAsyncErrorStream()->Printf( + "warning: failed to set breakpoint site at 0x%" PRIx64 + " for breakpoint %i.%i: %s\n", + load_addr, constituent->GetBreakpoint().GetID(), constituent->GetID(), + error.AsCString() ? error.AsCString() : "unknown error"); } - // We failed to enable the breakpoint return LLDB_INVALID_BREAK_ID; } @@ -1862,7 +1974,8 @@ void Process::RemoveConstituentFromBreakpointSite( if (num_constituents == 0) { // Don't try to disable the site if we don't have a live process anymore. if (IsAlive()) - DisableBreakpointSite(bp_site_sp.get()); + llvm::consumeError( + ExecuteBreakpointSiteAction(*bp_site_sp, BreakpointAction::Disable)); m_breakpoint_site_list.RemoveByAddress(bp_site_sp->GetLoadAddress()); } } @@ -1912,7 +2025,7 @@ Status Process::EnableSoftwareBreakpoint(BreakpointSite *bp_site) { LLDB_LOGF( log, "Process::EnableSoftwareBreakpoint (site_id = %d) addr = 0x%" PRIx64, bp_site->GetID(), (uint64_t)bp_addr); - if (bp_site->IsEnabled()) { + if (IsBreakpointSiteEnabled(*bp_site)) { LLDB_LOGF( log, "Process::EnableSoftwareBreakpoint (site_id = %d) addr = 0x%" PRIx64 @@ -1956,7 +2069,7 @@ Status Process::EnableSoftwareBreakpoint(BreakpointSite *bp_site) { error) == bp_opcode_size) { if (::memcmp(bp_opcode_bytes, verify_bp_opcode_bytes, bp_opcode_size) == 0) { - bp_site->SetEnabled(true); + SetBreakpointSiteEnabled(*bp_site); bp_site->SetType(BreakpointSite::eSoftware); LLDB_LOGF(log, "Process::EnableSoftwareBreakpoint (site_id = %d) " @@ -1998,7 +2111,7 @@ Status Process::DisableSoftwareBreakpoint(BreakpointSite *bp_site) { if (bp_site->IsHardware()) { error = Status::FromErrorString("Breakpoint site is a hardware breakpoint."); - } else if (bp_site->IsEnabled()) { + } else if (IsBreakpointSiteEnabled(*bp_site)) { const size_t break_op_size = bp_site->GetByteSize(); const uint8_t *const break_op = bp_site->GetTrapOpcodeBytes(); if (break_op_size > 0) { @@ -2040,7 +2153,7 @@ Status Process::DisableSoftwareBreakpoint(BreakpointSite *bp_site) { if (::memcmp(bp_site->GetSavedOpcodeBytes(), verify_opcode, break_op_size) == 0) { // SUCCESS - bp_site->SetEnabled(false); + SetBreakpointSiteEnabled(*bp_site, false); LLDB_LOGF(log, "Process::DisableSoftwareBreakpoint (site_id = %d) " "addr = 0x%" PRIx64 " -- SUCCESS", @@ -3605,6 +3718,9 @@ Status Process::PrivateResume() { "Process::PrivateResume PreResumeActions failed, not resuming."); } else { m_mod_id.BumpResumeID(); + if (auto E = FlushDelayedBreakpoints()) + LLDB_LOG_ERROR(log, std::move(E), + "Failed to update some delayed breakpoints: {0}"); error = DoResume(direction); if (error.Success()) { DidResume(); @@ -3811,6 +3927,10 @@ Status Process::Detach(bool keep_stopped) { m_thread_list.DiscardThreadPlans(); DisableAllBreakpointSites(); + if (auto error = FlushDelayedBreakpoints()) + LLDB_LOG_ERROR( + GetLog(LLDBLog::Process), std::move(error), + "Failed to update some delayed breakpoints during detach: {0}"); error = DoDetach(keep_stopped); if (error.Success()) { @@ -3881,6 +4001,10 @@ Status Process::DestroyImpl(bool force_kill) { // doing this now. m_thread_list.DiscardThreadPlans(); DisableAllBreakpointSites(); + if (auto error = FlushDelayedBreakpoints()) + LLDB_LOG_ERROR( + GetLog(LLDBLog::Process), std::move(error), + "Failed to update some delayed breakpoints during destroy: {0}"); } error = DoDestroy(); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 0d63f37945b63..36baa34510d95 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -269,6 +269,7 @@ void Target::CleanupProcess() { m_breakpoint_list.ClearAllBreakpointSites(); m_internal_breakpoint_list.ClearAllBreakpointSites(); ResetBreakpointHitCounts(); + llvm::consumeError(m_process_sp->FlushDelayedBreakpoints()); // Disable watchpoints just on the debugger side. std::unique_lock lock; this->GetWatchpointList().GetListMutex(lock); diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index ca43be2ed05c6..62146ce507c60 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -392,6 +392,11 @@ let Definition = "process" in { Desc<"A list of thread names. Threads with any of these names will " "always be resumed when the process resumes, even when other " "threads are suspended during single-stepping operations.">; + def UseDelayedBreakpoints + : Property<"use-delayed-breakpoints", "Boolean">, + DefaultTrue, + Desc<"Specify whether updating breakpoints may be delayed until the " + "process is about to resume.">; } let Definition = "platform" in { diff --git a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp index 3602527a9231b..5626ebb286764 100644 --- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp +++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp @@ -119,8 +119,9 @@ bool ThreadPlanStepOverBreakpoint::DoWillResume(StateType resume_state, if (current_plan) { BreakpointSiteSP bp_site_sp( m_process.GetBreakpointSiteList().FindByAddress(m_breakpoint_addr)); - if (bp_site_sp && bp_site_sp->IsEnabled()) { - m_process.DisableBreakpointSite(bp_site_sp.get()); + if (bp_site_sp && m_process.IsBreakpointSiteEnabled(*bp_site_sp)) { + llvm::consumeError(m_process.ExecuteBreakpointSiteAction( + *bp_site_sp, Process::BreakpointAction::Disable)); m_reenabled_breakpoint_site = false; } } @@ -158,7 +159,8 @@ void ThreadPlanStepOverBreakpoint::ReenableBreakpointSite() { BreakpointSiteSP bp_site_sp( m_process.GetBreakpointSiteList().FindByAddress(m_breakpoint_addr)); if (bp_site_sp) { - m_process.EnableBreakpointSite(bp_site_sp.get()); + llvm::consumeError(m_process.ExecuteBreakpointSiteAction( + *bp_site_sp, Process::BreakpointAction::Enable)); } } } diff --git a/lldb/source/Target/TraceDumper.cpp b/lldb/source/Target/TraceDumper.cpp index 1da3fad0e7df8..8f0027d67f65d 100644 --- a/lldb/source/Target/TraceDumper.cpp +++ b/lldb/source/Target/TraceDumper.cpp @@ -156,9 +156,10 @@ class OutputWriterCLI : public TraceDumper::OutputWriter { m_s.Format(" {0}: ", item.id); if (m_options.show_timestamps) { - m_s.Format("[{0}] ", item.timestamp - ? formatv("{0:3} ns", *item.timestamp).str() - : "unavailable"); + if (item.timestamp) + m_s << formatv("[{0:3} ns]", *item.timestamp); + else + m_s << "[unavailable]"; } if (item.event) { diff --git a/lldb/source/Utility/GDBRemote.cpp b/lldb/source/Utility/GDBRemote.cpp index 276b1276f4e6d..0fa53420261fe 100644 --- a/lldb/source/Utility/GDBRemote.cpp +++ b/lldb/source/Utility/GDBRemote.cpp @@ -25,6 +25,10 @@ StreamGDBRemote::StreamGDBRemote(uint32_t flags, uint32_t addr_size, StreamGDBRemote::~StreamGDBRemote() = default; +int StreamGDBRemote::PutEscapedBytes(llvm::StringRef str) { + return PutEscapedBytes(str.data(), str.size()); +} + int StreamGDBRemote::PutEscapedBytes(const void *s, size_t src_len) { int bytes_written = 0; const uint8_t *src = static_cast(s); diff --git a/lldb/source/Utility/Stream.cpp b/lldb/source/Utility/Stream.cpp index 89dce9fb0e1f7..264da9167bb97 100644 --- a/lldb/source/Utility/Stream.cpp +++ b/lldb/source/Utility/Stream.cpp @@ -183,6 +183,12 @@ Stream &Stream::operator<<(const void *p) { return *this; } +// Stream the result of a formatv expression to this stream. +Stream &Stream::operator<<(const llvm::formatv_object_base &obj) { + obj.format(m_forwarder); + return *this; +} + // Get the current indentation level unsigned Stream::GetIndentLevel() const { return m_indent_level; } diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp index c5755b2733605..25cf12d1aaae0 100644 --- a/lldb/source/Utility/StringExtractorGDBRemote.cpp +++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -331,6 +331,8 @@ StringExtractorGDBRemote::GetServerPacketType() const { return eServerPacketType_jLLDBTraceGetState; if (PACKET_STARTS_WITH("jLLDBTraceGetBinaryData:")) return eServerPacketType_jLLDBTraceGetBinaryData; + if (PACKET_STARTS_WITH("jMultiBreakpoint:")) + return eServerPacketType_jMultiBreakpoint; break; case 'v': diff --git a/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/Makefile b/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/TestDelayedBreakpoint.py b/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/TestDelayedBreakpoint.py new file mode 100644 index 0000000000000..0d55e4e03f2e9 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/TestDelayedBreakpoint.py @@ -0,0 +1,79 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +import os + + +@skipIfWindows +class TestDelayedBreakpoint(TestBase): + def test(self): + self.build() + logfile = os.path.join(self.getBuildDir(), "log.txt") + self.runCmd(f"log enable -f {logfile} gdb-remote packets") + + target, process, _, _ = lldbutil.run_to_source_breakpoint( + self, "main", lldb.SBFileSpec("main.c") + ) + + self.runCmd(f"proc plugin packet send BEFORE_BPS", check=False) + + breakpoint = target.BreakpointCreateByLocation("main.c", 1) + self.assertGreater(breakpoint.GetNumResolvedLocations(), 0) + + self.runCmd(f"proc plugin packet send AFTER_BPS", check=False) + + lldbutil.continue_to_breakpoint(process, breakpoint) + self.runCmd(f"log disable all") + + self.runCmd(f"proc plugin packet send AFTER_CONTINUE", check=False) + + self.assertTrue(os.path.exists(logfile)) + log_text = open(logfile).read() + + log_before_continue = log_text.split("BEFORE_BPS", 1)[-1].split("AFTER_BPS", 1)[ + 0 + ] + self.assertNotIn("send packet: $Z", log_before_continue) + self.assertNotIn("send packet: $z", log_before_continue) + + log_after = log_text.split("AFTER_BPS", 1)[-1].split("AFTER_CONTINUE", 1)[0] + if "jMultiBreakpoint+" in lldbutil.get_qsupported_capabilities(self): + self.assertIn("send packet: $jMultiBreakpoint", log_after) + else: + self.assertIn("send packet: $Z", log_after) + + def test_eager_breakpoints(self): + self.build() + logfile = os.path.join(self.getBuildDir(), "log.txt") + self.runCmd(f"log enable -f {logfile} gdb-remote packets") + + target, process, _, _ = lldbutil.run_to_source_breakpoint( + self, "main", lldb.SBFileSpec("main.c") + ) + + bp1 = target.BreakpointCreateByLocation("main.c", 1) + self.runCmd("proc plugin packet send BEGIN_EAGER", check=False) + # Create an address breakpoint to trigger eager breakpoints. + fake_address = 0x1234567 + target.BreakpointCreateByAddress(fake_address) + self.runCmd("proc plugin packet send END_EAGER", check=False) + + self.assertTrue(os.path.exists(logfile)) + log = ( + open(logfile) + .read() + .split("BEGIN_EAGER")[1] + .split("END_EAGER")[0] + .splitlines() + ) + breakpoint_lines = [line for line in log if "send packet: $Z" in line] + breakpoint_lines += [ + line for line in log if "send packet: $jMultiBreakpoint" in line + ] + breakpoint_lines = "".join(breakpoint_lines) + + bp_addresses = [f"{loc.GetLoadAddress():x}" for loc in bp1.locations] + bp_addresses += [f"{fake_address:x}"] + for addr in bp_addresses: + self.assertIn(addr, breakpoint_lines) diff --git a/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/main.c b/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/main.c new file mode 100644 index 0000000000000..a9086f89af86a --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/delayed_breakpoints/main.c @@ -0,0 +1,7 @@ +int break_on_me() { + int i = 10; // breakhere + i++; + return i; // secondbreak +} + +int main() { return break_on_me(); } diff --git a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py index a2a078e96bd59..c88dd0f942d36 100644 --- a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py +++ b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py @@ -12,38 +12,21 @@ from lldbsuite.test.gdbclientutils import * -@skipUnlessDarwin # Remove once lldbsever support is implemented. +@skipIfWindows # No server on Windows. @skipIfOutOfTreeDebugserver # Runs on systems where we can always predict the software break size @skipIf(archs=no_match(["x86_64", "arm64", "aarch64"])) class TestMultiBreakpoint(TestBase): - def send_packet(self, packet_str): - packet_str = escape_binary(packet_str) - self.runCmd(f"process plugin packet send '{packet_str}'", check=False) - output = self.res.GetOutput() - reply = output.split("\n") - # The output is of the form: - # packet: - # response: - packet_line = None - response_line = None - for line in reply: - line = line.strip() - if line.startswith("packet:"): - packet_line = line - elif line.startswith("response:"): - response_line = line - self.assertIsNotNone(packet_line, f'No "packet:" line in output: {output}') - self.assertIsNotNone(response_line, f'No "response:" line in output: {output}') - return response_line[len("response:") :].strip() - def check_invalid_packet(self, packet_str): - reply = self.send_packet(packet_str) + reply = lldbutil.send_packet_get_reply(self, packet_str) if reply.startswith("E"): return else: self.assertMultiResponse(reply, ["error"]) + def send_packet(self, packet_str): + return lldbutil.send_packet_get_reply(self, packet_str) + def assertMultiResponse(self, reply, expected): """Assert a JSON-array multi-response matches the expected pattern. @@ -84,8 +67,8 @@ def test_multi_breakpoint(self): ) # Verify the server advertises jMultiBreakpoint support. - reply = self.send_packet("qSupported") - self.assertIn("jMultiBreakpoint+", reply) + capabilities = lldbutil.get_qsupported_capabilities(self) + self.assertIn("jMultiBreakpoint+", capabilities) addr_a = self.get_function_address("func_a") addr_b = self.get_function_address("func_b") diff --git a/lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py b/lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py index 6caa5f85c2848..ab6e952fb7dd1 100644 --- a/lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py +++ b/lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py @@ -11,23 +11,13 @@ @skipUnlessDarwin @skipIfOutOfTreeDebugserver class TestCase(TestBase): - def send_process_packet(self, packet_str): - self.runCmd(f"proc plugin packet send {packet_str}", check=False) - # The output is of the form: - # packet: - # response: - reply = self.res.GetOutput().split("\n") - packet = reply[0].strip() - response = reply[1].strip() - - self.assertTrue(packet.startswith("packet: ")) - self.assertTrue(response.startswith("response: ")) - return response[len("response: ") :] - def check_invalid_packet(self, packet_str): - reply = self.send_process_packet("packet_str") + reply = lldbutil.send_packet_get_reply(self, "packet_str") self.assertEqual(reply, "E03") + def send_process_packet(self, packet_str): + return lldbutil.send_packet_get_reply(self, packet_str) + def test_packets(self): self.build() source_file = lldb.SBFileSpec("main.c") @@ -35,8 +25,8 @@ def test_packets(self): self, "break here", source_file ) - reply = self.send_process_packet("qSupported") - self.assertIn("MultiMemRead+", reply) + capabilities = lldbutil.get_qsupported_capabilities(self) + self.assertIn("MultiMemRead+", capabilities) mem_address_var = thread.frames[0].FindVariable("memory") self.assertTrue(mem_address_var) diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp index 966b37e09ee55..175201deda018 100644 --- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -665,3 +665,25 @@ TEST_F(GDBRemoteCommunicationClientTest, MultiMemReadNotSupported) { ASSERT_FALSE(client.GetMultiMemReadSupported()); async_result.wait(); } + +TEST_F(GDBRemoteCommunicationClientTest, MultiBreakpointdSupported) { + std::future async_result = std::async(std::launch::async, [&] { + StringExtractorGDBRemote qSupported_packet_request; + server.GetPacket(qSupported_packet_request); + server.SendPacket("jMultiBreakpoint+;"); + return true; + }); + ASSERT_TRUE(client.GetMultiBreakpointSupported()); + async_result.wait(); +} + +TEST_F(GDBRemoteCommunicationClientTest, MultiBreakpointdNotSupported) { + std::future async_result = std::async(std::launch::async, [&] { + StringExtractorGDBRemote qSupported_packet_request; + server.GetPacket(qSupported_packet_request); + server.SendPacket(";"); + return true; + }); + ASSERT_FALSE(client.GetMultiBreakpointSupported()); + async_result.wait(); +} diff --git a/lldb/unittests/Utility/StreamTest.cpp b/lldb/unittests/Utility/StreamTest.cpp index 959097dc52f98..cb36d798b0fe4 100644 --- a/lldb/unittests/Utility/StreamTest.cpp +++ b/lldb/unittests/Utility/StreamTest.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/Utility/StreamString.h" +#include "llvm/Support/FormatVariadic.h" #include "gtest/gtest.h" using namespace lldb_private; @@ -416,6 +417,14 @@ TEST_F(StreamTest, ShiftOperatorStrings) { EXPECT_EQ("cstring\nllvm::StringRef\n", TakeValue()); } +TEST_F(StreamTest, ShiftOperatorFormatv) { + s << llvm::formatv("x{0}y", 42); + EXPECT_EQ("x42y", TakeValue()); + + s << llvm::formatv("{0} {1}", "hello", "world") << '!'; + EXPECT_EQ("hello world!", TakeValue()); +} + TEST_F(StreamTest, ShiftOperatorPtr) { // This test is a bit tricky because pretty much everything related to // pointer printing seems to lead to UB or IB. So let's make the most basic