[llvm] [Dexter] Adapt to upcoming lldb stepping behavior (PR #108127)

[Jason Molenda via llvm-commits]

https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/108127

lldb will change how it reports stop reasons around breakpoints in the near future.  I landed an earlier version of this change and noticed debuginfo test failures on the CI bots due to the changes. I'm addressing the issues found by CI at
https://github.com/llvm/llvm-project/pull/105594 and will re-land once I've done all of them.

Currently, when lldb stops at a breakpoint instruction -- but has not yet executed the instruction -- it will overwrite the thread's Stop Reason with "breakpoint-hit".  This caused bugs when the original stop reason was important to the user - for instance, a watchpoint on an AArch64 system where we have to instruction-step past the watchpoint to find the new value.  Normally we would instruction step, fetch the new value, then report the user that a watchpoint has been hit with the old and new values.  But if the instruction after this access is a breakpoint site, we overwrite the "watchpoint hit" stop reason (and related actions) with "breakpoint hit".

dexter sets breakpoints on all source lines, then steps line-to-line, hitting the breakpoints.  But with this new behavior, we see two steps per source line:  The first step gets us to the start of the next line, with a "step completed" stop reason.  Then we step again and we execute the breakpoint instruction, stop with the pc the same, and report "breakpoint hit".  Now we can step a second time and move past the breakpoint.

I've changed the `step` method in LLDB.py to check if we step to a breakpoint site but have a "step completed" stop reason -- in which case we have this new breakpoint behavior, and we need to step a second time to actually hit the breakpoint like the debuginfo tests expect.

>From 5a87d8367495a2b54dc32418c96cb74fa5038dc4 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Tue, 10 Sep 2024 18:06:11 -0700
Subject: [PATCH] [debuginfo, dexter] Adapt to upcoming lldb stepping behavior

lldb will change how it reports stop reasons around breakpoints in
the near future.  I landed an earlier version of this change and
noticed debuginfo test failures on the CI bots due to the changes.
I'm addressing the issues found by CI at
https://github.com/llvm/llvm-project/pull/105594 and will re-land
once I've done all of them.

Currently, when lldb stops at a breakpoint instruction -- but has
not yet executed the instruction -- it will overwrite the thread's
Stop Reason with "breakpoint-hit".  This caused bugs when the
original stop reason was important to the user - for instance, a
watchpoint on an AArch64 system where we have to instruction-step
past the watchpoint to find the new value.  Normally we would
instruction step, fetch the new value, then report the user that a
watchpoint has been hit with the old and new values.  But if the
instruction after this access is a breakpoint site, we overwrite
the "watchpoint hit" stop reason (and related actions) with "breakpoint
hit".

dexter sets breakpoints on all source lines, then steps line-to-line,
hitting the breakpoints.  But with this new behavior, we see two
steps per source line:  The first step gets us to the start of the
next line, with a "step completed" stop reason.  Then we step again
and we execute the breakpoint instruction, stop with the pc the
same, and report "breakpoint hit".  Now we can step a second time
and move past the breakpoint.

I've changed the `step` method in LLDB.py to check if we step to a
breakpoint site but have a "step completed" stop reason -- in which
case we have this new breakpoint behavior, and we need to step a
second time to actually hit the breakpoint like the debuginfo tests
expect.
---
 .../dexter/dex/debugger/lldb/LLDB.py          | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
index 3944c1c4b009db..2307550aca047b 100644
--- a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
+++ b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
@@ -206,6 +206,33 @@ def launch(self, cmdline):
 
     def step(self):
         self._thread.StepInto()
+        stop_reason = self._thread.GetStopReason()
+        # If we (1) completed a step and (2) are sitting at a breakpoint,
+        # but (3) the breakpoint is not reported as the stop reason, then
+        # we'll need to step once more to hit the breakpoint.
+        #
+        # dexter sets breakpoints on every source line, then steps
+        # each source line. Older lldb's would overwrite the stop
+        # reason with "breakpoint hit" when we stopped at a breakpoint,
+        # even if the breakpoint hadn't been exectued yet.  One
+        # step per source line, hitting a breakpoint each time.
+        #
+        # But a more accurate behavior is that the step completes
+        # with step-completed stop reason, then when we step again,
+        # we execute the breakpoint and stop (with the pc the same) and
+        # a breakpoint-hit stop reason.  So we need to step twice per line.
+        if stop_reason == self._interface.eStopReasonPlanComplete:
+            stepped_to_breakpoint = False
+            pc = self._thread.GetFrameAtIndex(0).GetPC()
+            for bp in self._target.breakpoints:
+                for bploc in bp.locations:
+                    if (
+                        bploc.IsEnabled()
+                        and bploc.GetAddress().GetLoadAddress(self._target) == pc
+                    ):
+                        stepped_to_breakpoint = True
+            if stepped_to_breakpoint:
+                self._thread.StepInto()
 
     def go(self) -> ReturnCode:
         self._process.Continue()