[Lldb-commits] [lldb] ebaa8b1 - [lldb] Don't use hardware index to determine whether a breakpoint site is hardware
Tatyana Krasnukha via lldb-commits
lldb-commits at lists.llvm.org
Wed Jul 29 11:27:45 PDT 2020
Author: Tatyana Krasnukha
Date: 2020-07-29T21:27:24+03:00
New Revision: ebaa8b1c60749883c6449a7c16096f1c40ccf4bc
URL: https://github.com/llvm/llvm-project/commit/ebaa8b1c60749883c6449a7c16096f1c40ccf4bc
DIFF: https://github.com/llvm/llvm-project/commit/ebaa8b1c60749883c6449a7c16096f1c40ccf4bc.diff
LOG: [lldb] Don't use hardware index to determine whether a breakpoint site is hardware
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.
Differential Revision: https://reviews.llvm.org/D84257
Added:
Modified:
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
lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 3fc571eaa292..04b2b8906899 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -76,6 +76,12 @@ class BreakpointLocation
/// \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);
diff --git a/lldb/include/lldb/Breakpoint/BreakpointSite.h b/lldb/include/lldb/Breakpoint/BreakpointSite.h
index 5ce17f511db4..6ddab5dc1460 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ b/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 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
/// \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; }
diff --git a/lldb/include/lldb/Breakpoint/StoppointLocation.h b/lldb/include/lldb/Breakpoint/StoppointLocation.h
index 4d6ca044ccc4..4392c7b84f9e 100644
--- a/lldb/include/lldb/Breakpoint/StoppointLocation.h
+++ b/lldb/include/lldb/Breakpoint/StoppointLocation.h
@@ -40,9 +40,7 @@ class StoppointLocation {
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; }
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 93d54c051ee5..eae1c1e033ad 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -68,6 +68,14 @@ Breakpoint &BreakpointLocation::GetBreakpoint() { return m_owner; }
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;
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp
index df73c6a17230..481c7b7eba8c 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -95,7 +95,10 @@ void Watchpoint::SetWatchSpec(const std::string &str) {
// 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; }
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 1fed8e064267..8dea8b980985 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3204,14 +3204,8 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
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;
}
diff --git a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
index 09db3ffd3ecd..01bf33693a23 100644
--- a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
+++ b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
@@ -17,27 +17,6 @@ class HardwareBreakpointMultiThreadTestCase(HardwareBreakpointTestBase):
def does_not_support_hw_breakpoints(self):
return not super().supports_hw_breakpoints()
- # LLDB on linux supports hardware breakpoints for arm and aarch64
- # architectures.
- @skipUnlessPlatform(oslist=['linux'])
- @skipTestIfFn(does_not_support_hw_breakpoints)
- def test_hw_break_set_delete_multi_thread_linux(self):
- self.build()
- self.setTearDownCleanup()
- self.break_multi_thread('delete', False) # llvm.org/PR44659
-
- # LLDB on linux supports hardware breakpoints for arm and aarch64
- # architectures.
- @skipUnlessPlatform(oslist=['linux'])
- @skipTestIfFn(does_not_support_hw_breakpoints)
- def test_hw_break_set_disable_multi_thread_linux(self):
- self.build()
- self.setTearDownCleanup()
- self.break_multi_thread('disable', False) # llvm.org/PR44659
-
- # LLDB on darwin supports hardware breakpoints for x86_64 and i386
- # architectures.
- @skipUnlessDarwin
@skipIfOutOfTreeDebugserver
@skipTestIfFn(does_not_support_hw_breakpoints)
def test_hw_break_set_delete_multi_thread_macos(self):
@@ -45,9 +24,6 @@ def test_hw_break_set_delete_multi_thread_macos(self):
self.setTearDownCleanup()
self.break_multi_thread('delete')
- # LLDB on darwin supports hardware breakpoints for x86_64 and i386
- # architectures.
- @skipUnlessDarwin
@skipIfOutOfTreeDebugserver
@skipTestIfFn(does_not_support_hw_breakpoints)
def test_hw_break_set_disable_multi_thread_macos(self):
@@ -55,7 +31,6 @@ def test_hw_break_set_disable_multi_thread_macos(self):
self.setTearDownCleanup()
self.break_multi_thread('disable')
-
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
@@ -65,7 +40,7 @@ def setUp(self):
self.first_stop = line_number(
self.source, 'Starting thread creation with hardware breakpoint set')
- def break_multi_thread(self, removal_type, check_hw_bp=True):
+ def break_multi_thread(self, removal_type):
"""Test that lldb hardware breakpoints work for multiple threads."""
self.runCmd("file " + self.getBuildArtifact("a.out"),
CURRENT_EXECUTABLE_SET)
@@ -109,10 +84,9 @@ def break_multi_thread(self, removal_type, check_hw_bp=True):
# Continue the loop and test that we are stopped 4 times.
count += 1
- if check_hw_bp:
- # Check the breakpoint list.
- self.expect("breakpoint list", substrs=['hw_break_function', 'hardware'])
- self.expect("breakpoint list -v", substrs=['function = hw_break_function', 'hardware = true'])
+ # Check the breakpoint list.
+ self.expect("breakpoint list", substrs=['hw_break_function', 'hardware'])
+ self.expect("breakpoint list -v", substrs=['function = hw_break_function', 'hardware = true'])
if removal_type == 'delete':
self.runCmd("settings set auto-confirm true")
More information about the lldb-commits
mailing list