Skip to content

[lldb] Fix the hardware breakpoint decorator#146609

Merged
JDevlieghere merged 1 commit intollvm:mainfrom
JDevlieghere:fix-hw-bp-decorator
Jul 2, 2025
Merged

[lldb] Fix the hardware breakpoint decorator#146609
JDevlieghere merged 1 commit intollvm:mainfrom
JDevlieghere:fix-hw-bp-decorator

Conversation

@JDevlieghere
Copy link
Member

A decorator to skip or XFAIL a test takes effect when the function that's passed in returns a reason string. The wrappers around hw_breakpoints_supported were doing that incorrectly by inverting (calling not) on the result, turning it into a boolean, which means the test is always skipped.

A decorator to skip or XFAIL a test takes effect when the function
that's passed in returns a reason string. The wrappers around
hw_breakpoints_supported were doing that incorrectly by inverting
(calling `not`) on the result, turning it into a boolean, which means
the test is always skipped.
@llvmbot
Copy link
Member

llvmbot commented Jul 1, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

A decorator to skip or XFAIL a test takes effect when the function that's passed in returns a reason string. The wrappers around hw_breakpoints_supported were doing that incorrectly by inverting (calling not) on the result, turning it into a boolean, which means the test is always skipped.


Full diff: https://github.com/llvm/llvm-project/pull/146609.diff

4 Files Affected:

  • (modified) lldb/test/API/functionalities/breakpoint/hardware_breakpoints/base.py (+10)
  • (modified) lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py (+2-5)
  • (modified) lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py (+4-4)
  • (modified) lldb/test/API/functionalities/breakpoint/hardware_breakpoints/write_memory_with_hw_breakpoint/TestWriteMemoryWithHWBreakpoint.py (+2-3)
diff --git a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/base.py b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/base.py
index 0ab5dd0f910f2..0ad903befc65e 100644
--- a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/base.py
+++ b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/base.py
@@ -14,5 +14,15 @@ def supports_hw_breakpoints(self):
         self.runCmd("breakpoint set -b main --hardware")
         self.runCmd("run")
         if "stopped" in self.res.GetOutput():
+            return True
+        return False
+
+    def hw_breakpoints_supported(self):
+        if self.supports_hw_breakpoints():
             return "Hardware breakpoints are supported"
         return None
+
+    def hw_breakpoints_unsupported(self):
+        if not self.supports_hw_breakpoints():
+            return "Hardware breakpoints are unsupported"
+        return None
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 1a0515aa04c07..4632c3bed1899 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
@@ -12,16 +12,13 @@
 
 
 class HardwareBreakpointMultiThreadTestCase(HardwareBreakpointTestBase):
-    def does_not_support_hw_breakpoints(self):
-        return not super().supports_hw_breakpoints()
-
-    @skipTestIfFn(does_not_support_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.hw_breakpoints_unsupported)
     def test_hw_break_set_delete_multi_thread_macos(self):
         self.build()
         self.setTearDownCleanup()
         self.break_multi_thread("delete")
 
-    @skipTestIfFn(does_not_support_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.hw_breakpoints_unsupported)
     def test_hw_break_set_disable_multi_thread_macos(self):
         self.build()
         self.setTearDownCleanup()
diff --git a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
index 89d57683a8007..a8c9cdeea9362 100644
--- a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
+++ b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
@@ -26,7 +26,7 @@ def test_breakpoint(self):
         breakpoint = target.BreakpointCreateByLocation("main.c", 1)
         self.assertTrue(breakpoint.IsHardware())
 
-    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.hw_breakpoints_supported)
     def test_step_range(self):
         """Test stepping when hardware breakpoints are required."""
         self.build()
@@ -49,7 +49,7 @@ def test_step_range(self):
             "Could not create hardware breakpoint for thread plan", error.GetCString()
         )
 
-    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.hw_breakpoints_supported)
     def test_step_out(self):
         """Test stepping out when hardware breakpoints are required."""
         self.build()
@@ -71,7 +71,7 @@ def test_step_out(self):
             "Could not create hardware breakpoint for thread plan", error.GetCString()
         )
 
-    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.hw_breakpoints_supported)
     def test_step_over(self):
         """Test stepping over when hardware breakpoints are required."""
         self.build()
@@ -91,7 +91,7 @@ def test_step_over(self):
 
     # Was reported to sometimes pass on certain hardware.
     @skipIf(oslist=["linux"], archs=["arm$"])
-    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.hw_breakpoints_supported)
     def test_step_until(self):
         """Test stepping until when hardware breakpoints are required."""
         self.build()
diff --git a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/write_memory_with_hw_breakpoint/TestWriteMemoryWithHWBreakpoint.py b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/write_memory_with_hw_breakpoint/TestWriteMemoryWithHWBreakpoint.py
index e4e25ae877ded..c82ae24a6d9ab 100644
--- a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/write_memory_with_hw_breakpoint/TestWriteMemoryWithHWBreakpoint.py
+++ b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/write_memory_with_hw_breakpoint/TestWriteMemoryWithHWBreakpoint.py
@@ -12,10 +12,9 @@
 
 
 class WriteMemoryWithHWBreakpoint(HardwareBreakpointTestBase):
-    def does_not_support_hw_breakpoints(self):
-        return not super().supports_hw_breakpoints()
 
-    @skipTestIfFn(does_not_support_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skip
     def test_copy_memory_with_hw_break(self):
         self.build()
         exe = self.getBuildArtifact("a.out")


@skipTestIfFn(does_not_support_hw_breakpoints)
@skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
@skip
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is broken which went unnoticed because of the decorator.

Copy link
Collaborator

@jasonmolenda jasonmolenda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@JDevlieghere JDevlieghere merged commit a87b27f into llvm:main Jul 2, 2025
9 checks passed
@JDevlieghere JDevlieghere deleted the fix-hw-bp-decorator branch July 2, 2025 01:01
igorkudrin added a commit that referenced this pull request Nov 20, 2025
If `HardwareBreakpointTestBase.supports_hw_breakpoints()` returns False,
`SimpleHWBreakpointTest.does_not_support_hw_breakpoints()` returns None,
so the test runs and fails. However, it should be skipped instead.

The test was added in #146602, while `supports_hw_breakpoints()` was
changed in #146609, which was landed earlier despite having a bigger
number.
aadeshps-mcw pushed a commit to aadeshps-mcw/llvm-project that referenced this pull request Nov 26, 2025
)

If `HardwareBreakpointTestBase.supports_hw_breakpoints()` returns False,
`SimpleHWBreakpointTest.does_not_support_hw_breakpoints()` returns None,
so the test runs and fails. However, it should be skipped instead.

The test was added in llvm#146602, while `supports_hw_breakpoints()` was
changed in llvm#146609, which was landed earlier despite having a bigger
number.
Priyanshu3820 pushed a commit to Priyanshu3820/llvm-project that referenced this pull request Nov 26, 2025
)

If `HardwareBreakpointTestBase.supports_hw_breakpoints()` returns False,
`SimpleHWBreakpointTest.does_not_support_hw_breakpoints()` returns None,
so the test runs and fails. However, it should be skipped instead.

The test was added in llvm#146602, while `supports_hw_breakpoints()` was
changed in llvm#146609, which was landed earlier despite having a bigger
number.
augusto2112 pushed a commit to augusto2112/llvm-project that referenced this pull request Dec 3, 2025
)

If `HardwareBreakpointTestBase.supports_hw_breakpoints()` returns False,
`SimpleHWBreakpointTest.does_not_support_hw_breakpoints()` returns None,
so the test runs and fails. However, it should be skipped instead.

The test was added in llvm#146602, while `supports_hw_breakpoints()` was
changed in llvm#146609, which was landed earlier despite having a bigger
number.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants