jasonmolenda updated this revision to Diff 494428.
jasonmolenda added a comment.
Ah, I wasn't paying close enough attention and got the logic for MIPS/PPC64
watchpoint exceptions backwards in my update to
GDBRemoteCommunicationClient::GetWatchpointReportedAfter, update patch for that.
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/include/lldb/lldb-defines.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/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/StopInfo.cpp
lldb/source/Target/Target.cpp
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -790,13 +790,12 @@
}
static bool CheckIfWatchpointsSupported(Target *target, Status &error) {
- uint32_t num_supported_hardware_watchpoints;
- Status rc = target->GetProcessSP()->GetWatchpointSupportInfo(
- num_supported_hardware_watchpoints);
+ 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 == LLDB_INVALID_WATCHPOINT_SLOTS)
return true;
if (num_supported_hardware_watchpoints == 0) {
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/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;
+ 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;
+ bool GetWatchpointReportedAfter() 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,13 @@
return error;
}
-Status ProcessGDBRemote::GetWatchpointSupportInfo(uint32_t &num) {
+uint32_t ProcessGDBRemote::GetWatchpointSlotCount() {
- Status error(m_gdb_comm.GetWatchpointSupportInfo(num));
- return error;
+ 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;
+bool ProcessGDBRemote::GetWatchpointReportedAfter() {
+ return m_gdb_comm.GetWatchpointReportedAfter(GetTarget().GetArchitecture());
}
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);
+ uint32_t GetWatchpointSlotCount();
- Status GetWatchpointSupportInfo(uint32_t &num, bool &after,
- const ArchSpec &arch);
-
- Status GetWatchpointsTriggerAfterInstruction(bool &after,
- const ArchSpec &arch);
+ bool GetWatchpointReportedAfter(const ArchSpec &arch);
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;
-
+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;
+ uint32_t num = LLDB_INVALID_WATCHPOINT_SLOTS;
if (m_supports_watchpoint_support_info != eLazyBoolNo) {
StringExtractorGDBRemote response;
if (SendPacketAndWaitForResponse("qWatchpointSupportInfo:", response) ==
@@ -1813,44 +1809,27 @@
}
}
- 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;
+bool GDBRemoteCommunicationClient::GetWatchpointReportedAfter(
+ const ArchSpec &arch) {
llvm::Triple triple = arch.GetTriple();
+ bool after = true;
+
+ if (triple.isMIPS() || triple.isPPC64())
+ after = false;
+ if (triple.isAArch64() || triple.isArmMClass() || triple.isARM())
+ after = false;
// 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;
+ if (m_qHostInfo_is_valid == eLazyBoolYes)
+ if (m_watchpoints_trigger_after_instruction == eLazyBoolNo)
+ after = false;
- after = (m_watchpoints_trigger_after_instruction != eLazyBoolNo);
- }
- return error;
+ return after;
}
int GDBRemoteCommunicationClient::SetSTDIN(const FileSpec &file_spec) {
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,8 @@
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;
+ uint32_t GetWatchpointSlotCount() override;
+ bool GetWatchpointReportedAfter() 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,12 @@
}
}
-Status ProcessWindows::GetWatchpointSupportInfo(uint32_t &num) {
- num = RegisterContextWindows::GetNumHardwareBreakpointSlots();
- return {};
+uint32_t ProcessWindows::GetWatchpointSlotCount() {
+ return RegisterContextWindows::GetNumHardwareBreakpointSlots();
}
-Status ProcessWindows::GetWatchpointSupportInfo(uint32_t &num, bool &after) {
- num = RegisterContextWindows::GetNumHardwareBreakpointSlots();
- after = RegisterContextWindows::DoHardwareBreakpointsTriggerAfter();
- return {};
+bool ProcessWindows::GetWatchpointReportedAfter() {
+ return RegisterContextWindows::DoHardwareBreakpointsTriggerAfter();
}
Status ProcessWindows::EnableWatchpoint(Watchpoint *wp, bool notify) {
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===================================================================
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -209,10 +209,10 @@
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())
+ uint32_t num_supported_hardware_watchpoints =
+ target->GetProcessSP()->GetWatchpointSlotCount();
+
+ if (num_supported_hardware_watchpoints != LLDB_INVALID_WATCHPOINT_SLOTS)
result.AppendMessageWithFormat(
"Number of supported hardware watchpoints: %u\n",
num_supported_hardware_watchpoints);
Index: lldb/source/API/SBProcess.cpp
===================================================================
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -967,7 +967,11 @@
if (process_sp) {
std::lock_guard<std::recursive_mutex> guard(
process_sp->GetTarget().GetAPIMutex());
- sb_error.SetError(process_sp->GetWatchpointSupportInfo(num));
+ num = process_sp->GetWatchpointSlotCount();
+ if (num == LLDB_INVALID_WATCHPOINT_SLOTS) {
+ sb_error.SetErrorString("Unable to determine number of watchpoints");
+ num = 0;
+ }
} else {
sb_error.SetErrorString("SBProcess is invalid");
}
Index: lldb/include/lldb/lldb-defines.h
===================================================================
--- lldb/include/lldb/lldb-defines.h
+++ lldb/include/lldb/lldb-defines.h
@@ -40,6 +40,7 @@
#define LLDB_BREAK_ID_IS_INTERNAL(bid) ((bid) < 0)
// Watchpoints
+#define LLDB_INVALID_WATCHPOINT_SLOTS UINT32_MAX
#define LLDB_INVALID_WATCH_ID 0
#define LLDB_WATCH_ID_IS_VALID(uid) ((uid) != (LLDB_INVALID_WATCH_ID))
#define LLDB_WATCH_TYPE_READ (1u << 0)
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -1817,20 +1817,29 @@
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.
+ ///
+ /// One user specified watchpoint may require multiple hardware
+ /// watchpoints, e.g. a larger object, or an unaligned object.
+ /// A target stub may not allow the user to set that many distinct
+ /// watchpoints if one of these is true.
+ ///
+ /// \return
+ /// Returns the number of watchpoints, or LLDB_INVALID_WATCHPOINT_SLOTS
+ /// if unknown.
+ virtual uint32_t GetWatchpointSlotCount() {
+ return LLDB_INVALID_WATCHPOINT_SLOTS;
}
- 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.
+ ///
+ /// \return
+ /// Returns true for targets where lldb is notified after
+ /// the instruction has completed executing.
+ virtual bool GetWatchpointReportedAfter() { return true; }
lldb::ModuleSP ReadModuleFromMemory(const FileSpec &file_spec,
lldb::addr_t header_addr,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
