Skip to content

[lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id #108281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

felipepiovezan
Copy link
Contributor

If multiple breakpoints are hit at the same time, multiple stop reasons are reported, one per breakpoint.

Currently, get_threads_stopped_at_breakpoint_id only checks the first such reason.

…kpoint_id

If multiple breakpoints are hit at the same time, multiple stop reasons are
reported, one per breakpoint.

Currently, `get_threads_stopped_at_breakpoint_id` only checks the first such
reason.
@llvmbot
Copy link
Member

llvmbot commented Sep 11, 2024

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

If multiple breakpoints are hit at the same time, multiple stop reasons are reported, one per breakpoint.

Currently, get_threads_stopped_at_breakpoint_id only checks the first such reason.


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

1 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/lldbutil.py (+10-3)
diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index 629565b38ca1e6..660a3c085a908a 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -773,9 +773,16 @@ def get_threads_stopped_at_breakpoint_id(process, bpid):
         return threads
 
     for thread in stopped_threads:
-        # Make sure we've hit our breakpoint...
-        break_id = thread.GetStopReasonDataAtIndex(0)
-        if break_id == bpid:
+        # Make sure we've hit our breakpoint.
+        # From the docs of GetStopReasonDataAtIndex: "Breakpoint stop reasons
+        # will have data that consists of pairs of breakpoint IDs followed by
+        # the breakpoint location IDs".
+        # Iterate over all such pairs looking for `bpid`.
+        break_ids = [
+            thread.GetStopReasonDataAtIndex(idx)
+            for idx in range(0, thread.GetStopReasonDataCount(), 2)
+        ]
+        if bpid in break_ids:
             threads.append(thread)
 
     return threads

@felipepiovezan
Copy link
Contributor Author

felipepiovezan commented Sep 11, 2024

I only hit this bug in a downstream test. It seems we don't directly test lldbutil, but none of the tests regress with this change, so we should be confident it is correct.

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.

Looks correct.

Copy link
Collaborator

@jimingham jimingham left a comment

Choose a reason for hiding this comment

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

Yup, that's better.

@felipepiovezan felipepiovezan merged commit 7e74472 into llvm:main Sep 12, 2024
9 checks passed
@felipepiovezan felipepiovezan deleted the felipe/breakpoint_multiple_reasons branch September 12, 2024 14:02
felipepiovezan added a commit to felipepiovezan/llvm-project that referenced this pull request Sep 12, 2024
…kpoint_id (llvm#108281)

If multiple breakpoints are hit at the same time, multiple stop reasons
are reported, one per breakpoint.

Currently, `get_threads_stopped_at_breakpoint_id` only checks the first
such reason.

(cherry picked from commit 7e74472)
felipepiovezan added a commit to swiftlang/llvm-project that referenced this pull request Sep 12, 2024
…le_reasons

[cherry-pick][lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id (llvm#108281)
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.

4 participants