-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
[lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id #108281
Conversation
…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.
@llvm/pr-subscribers-lldb Author: Felipe de Azevedo Piovezan (felipepiovezan) ChangesIf multiple breakpoints are hit at the same time, multiple stop reasons are reported, one per breakpoint. Currently, Full diff: https://github.com/llvm/llvm-project/pull/108281.diff 1 Files Affected:
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
|
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks correct.
There was a problem hiding this 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.
…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)
…le_reasons [cherry-pick][lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_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.