diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -1817,20 +1818,35 @@ virtual Status GetMemoryRegions(lldb_private::MemoryRegionInfos ®ion_list); - virtual Status GetWatchpointSupportInfo(uint32_t &num) { - Status error; - num = 0; - error.SetErrorString("Process::GetWatchpointSupportInfo() not supported"); - return error; + /// Get the number of watchpoints supported by this target. + /// + /// We may be able to determine the number of watchpoints available + /// on this target; retrieve this value if possible. + /// + /// This number may be less than the number of watchpoints a user + /// can specify. This is because a single user watchpoint may require + /// multiple watchpoint slots to implement. Due to the size + /// and/or alignment of objects. + /// + /// \return + /// Returns the number of watchpoints, if available. + virtual std::optional GetWatchpointSlotCount() { + return std::nullopt; } - virtual Status GetWatchpointSupportInfo(uint32_t &num, bool &after) { - Status error; - num = 0; - after = true; - error.SetErrorString("Process::GetWatchpointSupportInfo() not supported"); - return error; - } + /// Whether lldb will be notified about watchpoints after + /// the instruction has completed executing, or if the + /// instruction is rolled back and it is notified before it + /// executes. + /// The default behavior is "exceptions received after instruction + /// has executed", except for certain CPU architectures. + /// Process subclasses may override this if they have additional + /// information. + /// + /// \return + /// Returns true for targets where lldb is notified after + /// the instruction has completed executing. + bool GetWatchpointReportedAfter(); lldb::ModuleSP ReadModuleFromMemory(const FileSpec &file_spec, lldb::addr_t header_addr, @@ -2663,6 +2679,24 @@ return Status("Process::DoGetMemoryRegionInfo() not supported"); } + /// Provide an override value in the subclass for lldb's + /// CPU-based logic for whether watchpoint exceptions are + /// received before or after an instruction executes. + /// + /// If a Process subclass needs to override this architecture-based + /// result, it may do so by overriding this method. + /// + /// \return + /// No boolean returned means there is no override of the + /// default architecture-based behavior. + /// true is returned for targets where watchpoints are reported + /// after the instruction has completed. + /// false is returned for targets where watchpoints are reported + /// before the instruction executes. + virtual std::optional DoGetWatchpointReportedAfter() { + return std::nullopt; + } + lldb::StateType GetPrivateState(); /// The "private" side of resuming a process. This doesn't alter the state diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -972,7 +972,12 @@ if (process_sp) { std::lock_guard guard( process_sp->GetTarget().GetAPIMutex()); - sb_error.SetError(process_sp->GetWatchpointSupportInfo(num)); + std::optional actual_num = process_sp->GetWatchpointSlotCount(); + if (actual_num) { + num = *actual_num; + } else { + sb_error.SetErrorString("Unable to determine number of watchpoints"); + } } else { sb_error.SetErrorString("SBProcess is invalid"); } diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp --- a/lldb/source/Commands/CommandObjectWatchpoint.cpp +++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp @@ -209,13 +209,13 @@ Target *target = &GetSelectedTarget(); if (target->GetProcessSP() && target->GetProcessSP()->IsAlive()) { - uint32_t num_supported_hardware_watchpoints; - Status error = target->GetProcessSP()->GetWatchpointSupportInfo( - num_supported_hardware_watchpoints); - if (error.Success()) + std::optional num_supported_hardware_watchpoints = + target->GetProcessSP()->GetWatchpointSlotCount(); + + if (num_supported_hardware_watchpoints) result.AppendMessageWithFormat( "Number of supported hardware watchpoints: %u\n", - num_supported_hardware_watchpoints); + *num_supported_hardware_watchpoints); } const WatchpointList &watchpoints = target->GetWatchpointList(); diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h @@ -95,8 +95,7 @@ void OnDebugString(const std::string &string) override; void OnDebuggerError(const Status &error, uint32_t type) override; - Status GetWatchpointSupportInfo(uint32_t &num) override; - Status GetWatchpointSupportInfo(uint32_t &num, bool &after) override; + std::optional GetWatchpointSlotCount() override; Status EnableWatchpoint(Watchpoint *wp, bool notify = true) override; Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override; diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -835,15 +835,8 @@ } } -Status ProcessWindows::GetWatchpointSupportInfo(uint32_t &num) { - num = RegisterContextWindows::GetNumHardwareBreakpointSlots(); - return {}; -} - -Status ProcessWindows::GetWatchpointSupportInfo(uint32_t &num, bool &after) { - num = RegisterContextWindows::GetNumHardwareBreakpointSlots(); - after = RegisterContextWindows::DoHardwareBreakpointsTriggerAfter(); - return {}; +std::optional ProcessWindows::GetWatchpointSlotCount() { + return RegisterContextWindows::GetNumHardwareBreakpointSlots(); } Status ProcessWindows::EnableWatchpoint(Watchpoint *wp, bool notify) { diff --git a/lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h b/lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h --- a/lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h +++ b/lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h @@ -38,7 +38,6 @@ static constexpr uint32_t GetNumHardwareBreakpointSlots() { return NUM_HARDWARE_BREAKPOINT_SLOTS; } - static constexpr bool DoHardwareBreakpointsTriggerAfter() { return true; } bool AddHardwareBreakpoint(uint32_t slot, lldb::addr_t address, uint32_t size, bool read, bool write); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -194,13 +194,9 @@ Status GetMemoryRegionInfo(lldb::addr_t addr, MemoryRegionInfo &range_info); - Status GetWatchpointSupportInfo(uint32_t &num); + std::optional GetWatchpointSlotCount(); - Status GetWatchpointSupportInfo(uint32_t &num, bool &after, - const ArchSpec &arch); - - Status GetWatchpointsTriggerAfterInstruction(bool &after, - const ArchSpec &arch); + std::optional GetWatchpointReportedAfter(); const ArchSpec &GetHostArchitecture(); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -1780,16 +1780,12 @@ return error; } -Status GDBRemoteCommunicationClient::GetWatchpointSupportInfo(uint32_t &num) { - Status error; - +std::optional GDBRemoteCommunicationClient::GetWatchpointSlotCount() { if (m_supports_watchpoint_support_info == eLazyBoolYes) { - num = m_num_supported_hardware_watchpoints; - return error; + return m_num_supported_hardware_watchpoints; } - // Set num to 0 first. - num = 0; + std::optional num; if (m_supports_watchpoint_support_info != eLazyBoolNo) { StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse("qWatchpointSupportInfo:", response) == @@ -1797,15 +1793,13 @@ m_supports_watchpoint_support_info = eLazyBoolYes; llvm::StringRef name; llvm::StringRef value; - bool found_num_field = false; while (response.GetNameColonValue(name, value)) { if (name.equals("num")) { value.getAsInteger(0, m_num_supported_hardware_watchpoints); num = m_num_supported_hardware_watchpoints; - found_num_field = true; } } - if (!found_num_field) { + if (!num) { m_supports_watchpoint_support_info = eLazyBoolNo; } } else { @@ -1813,44 +1807,24 @@ } } - if (m_supports_watchpoint_support_info == eLazyBoolNo) { - error.SetErrorString("qWatchpointSupportInfo is not supported"); - } - return error; + return num; } -lldb_private::Status GDBRemoteCommunicationClient::GetWatchpointSupportInfo( - uint32_t &num, bool &after, const ArchSpec &arch) { - Status error(GetWatchpointSupportInfo(num)); - if (error.Success()) - error = GetWatchpointsTriggerAfterInstruction(after, arch); - return error; -} - -lldb_private::Status -GDBRemoteCommunicationClient::GetWatchpointsTriggerAfterInstruction( - bool &after, const ArchSpec &arch) { - Status error; - llvm::Triple triple = arch.GetTriple(); - - // we assume watchpoints will happen after running the relevant opcode and we - // only want to override this behavior if we have explicitly received a - // qHostInfo telling us otherwise - if (m_qHostInfo_is_valid != eLazyBoolYes) { - // On targets like MIPS and ppc64, watchpoint exceptions are always - // generated before the instruction is executed. The connected target may - // not support qHostInfo or qWatchpointSupportInfo packets. - after = !(triple.isMIPS() || triple.isPPC64()); - } else { - // For MIPS and ppc64, set m_watchpoints_trigger_after_instruction to - // eLazyBoolNo if it is not calculated before. - if (m_watchpoints_trigger_after_instruction == eLazyBoolCalculate && - (triple.isMIPS() || triple.isPPC64())) - m_watchpoints_trigger_after_instruction = eLazyBoolNo; +std::optional GDBRemoteCommunicationClient::GetWatchpointReportedAfter() { + if (m_qHostInfo_is_valid == eLazyBoolCalculate) + GetHostInfo(); - after = (m_watchpoints_trigger_after_instruction != eLazyBoolNo); + // Process determines this by target CPU, but allow for the + // remote stub to override it via the qHostInfo + // watchpoint_exceptions_received key, if it is present. + if (m_qHostInfo_is_valid == eLazyBoolYes) { + if (m_watchpoints_trigger_after_instruction == eLazyBoolNo) + return false; + if (m_watchpoints_trigger_after_instruction == eLazyBoolYes) + return true; } - return error; + + return std::nullopt; } int GDBRemoteCommunicationClient::SetSTDIN(const FileSpec &file_spec) { diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -159,7 +159,7 @@ Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override; - Status GetWatchpointSupportInfo(uint32_t &num) override; + std::optional GetWatchpointSlotCount() override; llvm::Expected TraceSupported() override; @@ -172,7 +172,7 @@ llvm::Expected> TraceGetBinaryData(const TraceGetBinaryDataRequest &request) override; - Status GetWatchpointSupportInfo(uint32_t &num, bool &after) override; + std::optional DoGetWatchpointReportedAfter() override; bool StartNoticingNewThreads() override; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2818,16 +2818,12 @@ return error; } -Status ProcessGDBRemote::GetWatchpointSupportInfo(uint32_t &num) { - - Status error(m_gdb_comm.GetWatchpointSupportInfo(num)); - return error; +std::optional ProcessGDBRemote::GetWatchpointSlotCount() { + return m_gdb_comm.GetWatchpointSlotCount(); } -Status ProcessGDBRemote::GetWatchpointSupportInfo(uint32_t &num, bool &after) { - Status error(m_gdb_comm.GetWatchpointSupportInfo( - num, after, GetTarget().GetArchitecture())); - return error; +std::optional ProcessGDBRemote::DoGetWatchpointReportedAfter() { + return m_gdb_comm.GetWatchpointReportedAfter(); } Status ProcessGDBRemote::DoDeallocateMemory(lldb::addr_t addr) { diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2355,6 +2355,24 @@ return error; } +bool Process::GetWatchpointReportedAfter() { + if (std::optional subclass_override = DoGetWatchpointReportedAfter()) + return *subclass_override; + + bool reported_after = true; + const ArchSpec &arch = GetTarget().GetArchitecture(); + if (!arch.IsValid()) + return reported_after; + llvm::Triple triple = arch.GetTriple(); + + if (triple.isMIPS() || triple.isPPC64() || triple.isRISCV()) + reported_after = false; + if (triple.isAArch64() || triple.isArmMClass() || triple.isARM()) + reported_after = false; + + return reported_after; +} + ModuleSP Process::ReadModuleFromMemory(const FileSpec &file_spec, lldb::addr_t header_addr, size_t size_to_read) { diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -823,16 +823,8 @@ // stop ProcessSP process_sp = exe_ctx.GetProcessSP(); - uint32_t num; - bool wp_triggers_after; + bool wp_triggers_after = process_sp->GetWatchpointReportedAfter(); - if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after) - .Success()) { - m_should_stop_is_valid = true; - m_should_stop = true; - return m_should_stop; - } - if (!wp_triggers_after) { // We have to step over the watchpoint before we know what to do: StopInfoWatchpointSP me_as_siwp_sp diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -790,19 +790,18 @@ } static bool CheckIfWatchpointsSupported(Target *target, Status &error) { - uint32_t num_supported_hardware_watchpoints; - Status rc = target->GetProcessSP()->GetWatchpointSupportInfo( - num_supported_hardware_watchpoints); + std::optional num_supported_hardware_watchpoints = + target->GetProcessSP()->GetWatchpointSlotCount(); // If unable to determine the # of watchpoints available, // assume they are supported. - if (rc.Fail()) + if (!num_supported_hardware_watchpoints) return true; if (num_supported_hardware_watchpoints == 0) { error.SetErrorStringWithFormat( "Target supports (%u) hardware watchpoint slots.\n", - num_supported_hardware_watchpoints); + *num_supported_hardware_watchpoints); return false; } return true;