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") _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
