This revision was automatically updated to reflect the committed changes.
Closed by commit rGeaeb8ddd4a9d: [LLDB] add arch-specific watchpoint behavior
defaults to lldb (authored by jasonmolenda).
Changed prior to commit:
https://reviews.llvm.org/D143215?vs=495976&id=497401#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143215/new/
https://reviews.llvm.org/D143215
Files:
lldb/include/lldb/Target/Process.h
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectWatchpoint.cpp
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Process.cpp
lldb/source/Target/StopInfo.cpp
lldb/source/Target/Target.cpp
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ 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<uint32_t> 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;
Index: lldb/source/Target/StopInfo.cpp
===================================================================
--- lldb/source/Target/StopInfo.cpp
+++ 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
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2355,6 +2355,23 @@
return error;
}
+bool Process::GetWatchpointReportedAfter() {
+ if (std::optional<bool> 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() ||
+ 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) {
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ 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<uint32_t> GetWatchpointSlotCount() override;
llvm::Expected<TraceSupportedResponse> TraceSupported() override;
@@ -172,7 +172,7 @@
llvm::Expected<std::vector<uint8_t>>
TraceGetBinaryData(const TraceGetBinaryDataRequest &request) override;
- Status GetWatchpointSupportInfo(uint32_t &num, bool &after) override;
+ std::optional<bool> DoGetWatchpointReportedAfter() override;
bool StartNoticingNewThreads() override;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ 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<uint32_t> 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<bool> ProcessGDBRemote::DoGetWatchpointReportedAfter() {
+ return m_gdb_comm.GetWatchpointReportedAfter();
}
Status ProcessGDBRemote::DoDeallocateMemory(lldb::addr_t addr) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ 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<uint32_t> GetWatchpointSlotCount();
- Status GetWatchpointSupportInfo(uint32_t &num, bool &after,
- const ArchSpec &arch);
-
- Status GetWatchpointsTriggerAfterInstruction(bool &after,
- const ArchSpec &arch);
+ std::optional<bool> GetWatchpointReportedAfter();
const ArchSpec &GetHostArchitecture();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1780,16 +1780,12 @@
return error;
}
-Status GDBRemoteCommunicationClient::GetWatchpointSupportInfo(uint32_t &num) {
- Status error;
-
+std::optional<uint32_t> 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<uint32_t> 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<bool> 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) {
Index: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
+++ 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);
Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ 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<uint32_t> GetWatchpointSlotCount() override;
Status EnableWatchpoint(Watchpoint *wp, bool notify = true) override;
Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ 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<uint32_t> ProcessWindows::GetWatchpointSlotCount() {
+ return RegisterContextWindows::GetNumHardwareBreakpointSlots();
}
Status ProcessWindows::EnableWatchpoint(Watchpoint *wp, bool notify) {
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===================================================================
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ 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<uint32_t> 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();
Index: lldb/source/API/SBProcess.cpp
===================================================================
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -972,7 +972,12 @@
if (process_sp) {
std::lock_guard<std::recursive_mutex> guard(
process_sp->GetTarget().GetAPIMutex());
- sb_error.SetError(process_sp->GetWatchpointSupportInfo(num));
+ std::optional<uint32_t> 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");
}
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -17,6 +17,7 @@
#include <list>
#include <memory>
#include <mutex>
+#include <optional>
#include <string>
#include <unordered_set>
#include <vector>
@@ -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<uint32_t> 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<bool> DoGetWatchpointReportedAfter() {
+ return std::nullopt;
+ }
+
lldb::StateType GetPrivateState();
/// The "private" side of resuming a process. This doesn't alter the state
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
