tatyana-krasnukha created this revision.
tatyana-krasnukha added reviewers: clayborg, labath, JDevlieghere.
tatyana-krasnukha added a project: LLDB.
Herald added subscribers: lldb-commits, arphaman.
Most process plugins (if not all) don't set hardware index for breakpoints.
They even are not able to determine this index.
This patch makes StoppointLocation::IsHardware pure virtual and lets
BreakpointSite override it using more accurate BreakpointSite::Type.
It also adds assertions to be sure that a breakpoint site is hardware when this
is required.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84257
Files:
lldb/include/lldb/Breakpoint/BreakpointLocation.h
lldb/include/lldb/Breakpoint/BreakpointSite.h
lldb/include/lldb/Breakpoint/StoppointLocation.h
lldb/source/Breakpoint/BreakpointLocation.cpp
lldb/source/Breakpoint/Watchpoint.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
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
@@ -3205,14 +3205,8 @@
break;
case BreakpointSite::eExternal: {
- GDBStoppointType stoppoint_type;
- if (bp_site->IsHardware())
- stoppoint_type = eBreakpointHardware;
- else
- stoppoint_type = eBreakpointSoftware;
-
- if (m_gdb_comm.SendGDBStoppointTypePacket(stoppoint_type, false, addr,
- bp_op_size))
+ if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false,
+ addr, bp_op_size))
error.SetErrorToGenericError();
} break;
}
Index: lldb/source/Breakpoint/Watchpoint.cpp
===================================================================
--- lldb/source/Breakpoint/Watchpoint.cpp
+++ lldb/source/Breakpoint/Watchpoint.cpp
@@ -95,7 +95,10 @@
// Override default impl of StoppointLocation::IsHardware() since m_is_hardware
// member field is more accurate.
-bool Watchpoint::IsHardware() const { return m_is_hardware; }
+bool Watchpoint::IsHardware() const {
+ lldbassert(m_is_hardware || !HardwareRequired());
+ return m_is_hardware;
+}
bool Watchpoint::IsWatchVariable() const { return m_is_watch_variable; }
Index: lldb/source/Breakpoint/BreakpointLocation.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointLocation.cpp
+++ lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -68,6 +68,14 @@
Target &BreakpointLocation::GetTarget() { return m_owner.GetTarget(); }
+bool BreakpointLocation::IsHardware() const {
+ if (m_bp_site_sp)
+ return m_bp_site_sp->IsHardware();
+
+ // If breakpoint location is not resolved yet, it cannot be hardware.
+ return false;
+}
+
bool BreakpointLocation::IsEnabled() const {
if (!m_owner.IsEnabled())
return false;
Index: lldb/include/lldb/Breakpoint/StoppointLocation.h
===================================================================
--- lldb/include/lldb/Breakpoint/StoppointLocation.h
+++ lldb/include/lldb/Breakpoint/StoppointLocation.h
@@ -40,9 +40,7 @@
bool HardwareRequired() const { return m_hardware; }
- virtual bool IsHardware() const {
- return m_hardware_index != LLDB_INVALID_INDEX32;
- }
+ virtual bool IsHardware() const = 0;
virtual bool ShouldStop(StoppointCallbackContext *context) { return true; }
Index: lldb/include/lldb/Breakpoint/BreakpointSite.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ lldb/include/lldb/Breakpoint/BreakpointSite.h
@@ -15,6 +15,7 @@
#include "lldb/Breakpoint/BreakpointLocationCollection.h"
#include "lldb/Breakpoint/StoppointLocation.h"
+#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/UserID.h"
#include "lldb/lldb-forward.h"
@@ -184,6 +185,12 @@
/// \b false otherwise.
bool IsInternal() const;
+ bool IsHardware() const override {
+ lldbassert(BreakpointSite::Type::eHardware == GetType() ||
+ !HardwareRequired());
+ return BreakpointSite::Type::eHardware == GetType();
+ }
+
BreakpointSite::Type GetType() const { return m_type; }
void SetType(BreakpointSite::Type type) { m_type = type; }
Index: lldb/include/lldb/Breakpoint/BreakpointLocation.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -76,6 +76,12 @@
/// \b true if the breakpoint is enabled, \b false if disabled.
bool IsEnabled() const;
+ /// Check if it is a hardware breakpoint.
+ ///
+ /// \return
+ /// \b true if the breakpoint is a harware breakpoint.
+ bool IsHardware() const override;
+
/// If \a auto_continue is \b true, set the breakpoint to continue when hit.
void SetAutoContinue(bool auto_continue);
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
