jasonmolenda wrote:
> > @AlexK0 if you have a setup to build and run the testsuite on windows,
> > could you try it with this patch some time when you're able? I can't see
> > this being merged for at least another 5-6 days, there's no rush. You can
> > download the unidiff style diff of the patch from
> > https://patch-diff.githubusercontent.com/raw/llvm/llvm-project/pull/96260.diff
>
> @jasonmolenda I checked the tests with VS2022/x86-64/win11 in a debug build
> with assertions enabled. Unfortunately, the main branch is a bit unstable,
> and some tests constantly fail :( . Nonetheless, I noticed one new failure
> with the applied patch:
>
> `functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py`
>
These TestConcurrent / TestConsecutiveBreakpoints tests are great for finding
corner cases with these changes, thanks so much. I was a little uncertain
about one part of my ProcessWindows change, where the pc is *past* the
breakpoint when a software breakpoint instruction is used, on Windows. For a
moment, I thought, "oh, I don't need to record that we hit the breakpoint
because we're already past it and we don't need to instruction past it" but of
course that was wrong -- ProcessWindows::RefreshStateAfterStop decrements the
$pc value by the size of its breakpoint instruction, which is necessary because
e.g. the size of an x86 breakpoint instruction is 1 byte (0xcc) but x86_64
instructions can be between 1 to 15 bytes long, so stopping 1 byte after the
BreakpointSite means we are possibly in the middle of the real instruction. We
must set the pc to the BreakpointSite address and use lldb's normal logic.
Anyway, tl;dr, I believe this will fix it:
```
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -442,6 +442,7 @@ void ProcessWindows::RefreshStateAfterStop() {
m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
site->GetID());
+ stop_thread->SetThreadHitBreakpointAtAddr(pc);
if (site->ValidForThisThread(*stop_thread)) {
LLDB_LOG(log,
"Breakpoint site {0} is valid for this thread ({1:x}), "
```
I'll push it right now. Sorry for my confusion, and thanks again for looking
at it, this was a big help.
https://github.com/llvm/llvm-project/pull/96260
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
