[Lldb-commits] [lldb] eaeb8dd - [LLDB] add arch-specific watchpoint behavior defaults to lldb
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 14 11:35:48 PST 2023
Author: Jason Molenda
Date: 2023-02-14T11:35:39-08:00
New Revision: eaeb8ddd4a9d3799470479a532e721d017f22a70
URL: https://github.com/llvm/llvm-project/commit/eaeb8ddd4a9d3799470479a532e721d017f22a70
DIFF: https://github.com/llvm/llvm-project/commit/eaeb8ddd4a9d3799470479a532e721d017f22a70.diff
LOG: [LLDB] add arch-specific watchpoint behavior defaults to lldb
lldb was originally designed to get the watchpoint exception behavior
from the gdb remote serial protocol stub -- exceptions are either
received before the instruction executes, or after the instruction
has executed. This behavior was reported via two lldb extensions
to gdb RSP, so generic remote stubs like gdbserver or a JTAG stub,
would not tell lldb which behavior was correct, and it would default
to "exceptions are received after the instruction has executed".
Two architectures hard coded their correct "exceptions before
instruction" behavior, to work around this issue.
Most architectures have a fixed behavior of watchpoint exceptions,
and we can center that information in lldb. We can allow a remote
stub to override the default behavior via our packet extensions
if it's needed on a specific target.
This patch also separates the fetching of the number of watchpoints
from whether exceptions are before/after the insn. Currently if
lldb couldn't fetch the number of watchpoints (not really needed), it
also wouldn't get when exceptions are received, and watchpoint
handling would fail. lldb doesn't actually use the number of
watchpoints for anything beyond printing it to the user.
Differential Revision: https://reviews.llvm.org/D143215
rdar://101426626
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 9862b6e57212..3ffacb52299b 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/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 @@ class Process : public std::enable_shared_from_this<Process>,
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 @@ void PruneThreadPlans();
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
diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index 5c8f17fa97fb..ca473175f18f 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -972,7 +972,12 @@ SBProcess::GetNumSupportedHardwareWatchpoints(lldb::SBError &sb_error) const {
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");
}
diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index 8df8aca04e4a..7aa961463418 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -209,13 +209,13 @@ class CommandObjectWatchpointList : public CommandObjectParsed {
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();
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index a84dd62357de..6befec9f8654 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -835,15 +835,8 @@ void ProcessWindows::OnDebuggerError(const Status &error, uint32_t type) {
}
}
-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) {
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
index bf9e5fe7a633..ed083b9e78b4 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -95,8 +95,7 @@ class ProcessWindows : public Process, public ProcessDebugger {
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;
diff --git a/lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h b/lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
index ac412d74f9fa..95f9bf6a481a 100644
--- a/lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
+++ b/lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
@@ -38,7 +38,6 @@ class RegisterContextWindows : public lldb_private::RegisterContext {
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.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 6e47e5b3e3d1..a1c1aa5e0f29 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1780,16 +1780,12 @@ Status GDBRemoteCommunicationClient::LoadQXferMemoryMap() {
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 @@ Status GDBRemoteCommunicationClient::GetWatchpointSupportInfo(uint32_t &num) {
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 @@ Status GDBRemoteCommunicationClient::GetWatchpointSupportInfo(uint32_t &num) {
}
}
- 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) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 45ea2f2589c1..b32e09cf8e76 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -194,13 +194,9 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
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();
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index de8f2df52f23..7b083e1478db 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2818,16 +2818,12 @@ Status ProcessGDBRemote::DoGetMemoryRegionInfo(addr_t load_addr,
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) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 83f0adfe35af..39065e5f87d3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -159,7 +159,7 @@ class ProcessGDBRemote : public Process,
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 @@ class ProcessGDBRemote : public Process,
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;
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index f6c53e6d4c2d..8bfebe3c8ff6 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2355,6 +2355,23 @@ Status Process::DeallocateMemory(addr_t ptr) {
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) {
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 2c61216ee53d..9fdb29f9e427 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -823,16 +823,8 @@ class StopInfoWatchpoint : public StopInfo {
// 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
index 14c7ff85ec3e..2b3b92a3cad4 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -790,19 +790,18 @@ bool Target::ProcessIsValid() {
}
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;
More information about the lldb-commits
mailing list