[Dexter] Implement DexStepFunction and DexContinue #152721
Conversation
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp -- cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex-continue.cpp cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex_step_function.cpp
View the diff from clang-format here.diff --git a/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex-continue.cpp b/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex-continue.cpp
index bb51b85cb..0e9c36c22 100644
--- a/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex-continue.cpp
+++ b/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex-continue.cpp
@@ -21,8 +21,7 @@ int c(int) {
return 0;
}
-__attribute__((always_inline))
-int b(int) {
+__attribute__((always_inline)) int b(int) {
++g;
return c(g);
}
@@ -34,9 +33,7 @@ int a(int) {
return g;
}
-void f() {
- ++g;
-}
+void f() { ++g; }
int main() {
int x = a(g);
@@ -52,13 +49,20 @@ int main() {
// DexStepFunction('f')
// CHECK: ## BEGIN ##
-// CHECK-NEXT: . [0, "a(int)", "{{.*}}dex-continue.cpp", 31, 3, "StopReason.BREAKPOINT", "StepKind.FUNC", []]
-// CHECK-NEXT: . [1, "a(int)", "{{.*}}dex-continue.cpp", 32, 5, "StopReason.STEP", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT: . . . [2, "c(int)", "{{.*}}dex-continue.cpp", 16, 3, "StopReason.BREAKPOINT", "StepKind.FUNC", []]
-// CHECK-NEXT: . . . [3, "c(int)", "{{.*}}dex-continue.cpp", 17, 3, "StopReason.BREAKPOINT", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT: . . . [4, "c(int)", "{{.*}}dex-continue.cpp", 19, 3, "StopReason.BREAKPOINT", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT: . . . [5, "c(int)", "{{.*}}dex-continue.cpp", 20, 3, "StopReason.BREAKPOINT", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT: . [6, "a(int)", "{{.*}}dex-continue.cpp", 33, 3, "StopReason.BREAKPOINT", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT: . [8, "f()", "{{.*}}dex-continue.cpp", 38, 3, "StopReason.BREAKPOINT", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT: . [9, "f()", "{{.*}}dex-continue.cpp", 39, 1, "StopReason.STEP", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT: ## END (9 steps) ##
+// CHECK-NEXT: . [0, "a(int)", "{{.*}}dex-continue.cpp", 31, 3,
+// "StopReason.BREAKPOINT", "StepKind.FUNC", []] CHECK-NEXT: . [1, "a(int)",
+// "{{.*}}dex-continue.cpp", 32, 5, "StopReason.STEP",
+// "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT: . . . [2, "c(int)",
+// "{{.*}}dex-continue.cpp", 16, 3, "StopReason.BREAKPOINT", "StepKind.FUNC",
+// []] CHECK-NEXT: . . . [3, "c(int)", "{{.*}}dex-continue.cpp", 17, 3,
+// "StopReason.BREAKPOINT", "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT: . .
+// . [4, "c(int)", "{{.*}}dex-continue.cpp", 19, 3, "StopReason.BREAKPOINT",
+// "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT: . . . [5, "c(int)",
+// "{{.*}}dex-continue.cpp", 20, 3, "StopReason.BREAKPOINT",
+// "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT: . [6, "a(int)",
+// "{{.*}}dex-continue.cpp", 33, 3, "StopReason.BREAKPOINT",
+// "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT: . [8, "f()",
+// "{{.*}}dex-continue.cpp", 38, 3, "StopReason.BREAKPOINT",
+// "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT: . [9, "f()",
+// "{{.*}}dex-continue.cpp", 39, 1, "StopReason.STEP",
+// "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT: ## END (9 steps) ##
diff --git a/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex_step_function.cpp b/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex_step_function.cpp
index e7e666d0e..4aaabb47f 100644
--- a/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex_step_function.cpp
+++ b/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/control/dex_step_function.cpp
@@ -23,17 +23,19 @@ int a(int) {
return b(g);
}
-int main() {
- return a(g);
-}
+int main() { return a(g); }
// DexStepFunction('a')
// DexStepFunction('c')
// CHECK: ## BEGIN ##
-// CHECK-NEXT:. [0, "a(int)", "{{.*}}dex_step_function.cpp", 22, 3, "StopReason.BREAKPOINT", "StepKind.FUNC", []]
-// CHECK-NEXT:. [1, "a(int)", "{{.*}}dex_step_function.cpp", 23, 12, "StopReason.STEP", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT:. . . [2, "c(int)", "{{.*}}dex_step_function.cpp", 12, 3, "StopReason.BREAKPOINT", "StepKind.FUNC", []]
-// CHECK-NEXT:. . . [3, "c(int)", "{{.*}}dex_step_function.cpp", 13, 3, "StopReason.STEP", "StepKind.VERTICAL_FORWARD", []]
-// CHECK-NEXT:. [6, "a(int)", "{{.*}}dex_step_function.cpp", 23, 3, "StopReason.STEP", "StepKind.HORIZONTAL_BACKWARD", []]
-// CHECK-NEXT: ## END (5 steps) ##
+// CHECK-NEXT:. [0, "a(int)", "{{.*}}dex_step_function.cpp", 22, 3,
+// "StopReason.BREAKPOINT", "StepKind.FUNC", []] CHECK-NEXT:. [1, "a(int)",
+// "{{.*}}dex_step_function.cpp", 23, 12, "StopReason.STEP",
+// "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT:. . . [2, "c(int)",
+// "{{.*}}dex_step_function.cpp", 12, 3, "StopReason.BREAKPOINT",
+// "StepKind.FUNC", []] CHECK-NEXT:. . . [3, "c(int)",
+// "{{.*}}dex_step_function.cpp", 13, 3, "StopReason.STEP",
+// "StepKind.VERTICAL_FORWARD", []] CHECK-NEXT:. [6, "a(int)",
+// "{{.*}}dex_step_function.cpp", 23, 3, "StopReason.STEP",
+// "StepKind.HORIZONTAL_BACKWARD", []] CHECK-NEXT: ## END (5 steps) ##
|
|
✅ With the latest revision this PR passed the Python code formatter. |
f42e8d1 to
3c16976
Compare
18b6dc8 to
07cee24
Compare
| self.context.logger.warning( | ||
| f"Set leading breakpoint {id} at {bpr.function}" | ||
| ) |
There was a problem hiding this comment.
Probably don't need this log anymore!
|
|
||
| log_step = False |
There was a problem hiding this comment.
Possibly change this to record_step or similar, since this isn't what we'd usually refer to by "log"?
| step_info, self._watches, self.step_collection.commands | ||
| ) | ||
| self.step_collection.new_step(self.context, step_info) | ||
| backtrace = list([f.function for f in step_info.frames]) |
There was a problem hiding this comment.
| backtrace = list([f.function for f in step_info.frames]) | |
| backtrace = [f.function for f in step_info.frames] |
AFAICT the square-brackets already make this a list, no need for the explicit list.
| # Add this backtrace to the stack. While the current | ||
| # backtrace matches the top of the stack we'll step, | ||
| # and while there's a backtrace in the stack that | ||
| # is a subset of the current backtrack we'll step-out. |
There was a problem hiding this comment.
| # is a subset of the current backtrack we'll step-out. | |
| # is a subset of the current backtrace we'll step-out. |
|
|
||
| elif bpr.is_continue: | ||
| debugger_continue = True | ||
| self.debugger.add_breakpoint(bpr.path, bpr.range_to) |
There was a problem hiding this comment.
range_to is an optional argument right? I'd assume this should be conditional on bpr.range_to is not None
| for i, f in enumerate(reversed(step_function_backtraces[-1])): | ||
| if backtrace[-1 - i] != f: | ||
| match_subtrace = False | ||
| match_trace = False | ||
| break |
There was a problem hiding this comment.
| for i, f in enumerate(reversed(step_function_backtraces[-1])): | |
| if backtrace[-1 - i] != f: | |
| match_subtrace = False | |
| match_trace = False | |
| break | |
| if backtrace[-len(step_function_backtraces[-1]):] != step_function_backtraces[-1]: | |
| match_subtrace = False | |
| match_trace = False |
A possible alternative way of expressing this, YMMV on whether this is clearer!
There was a problem hiding this comment.
👍 also added a comment to hopefully make it clearer
3c16976 to
83608ac
Compare
07cee24 to
0605b1e
Compare
| self.context.logger.warning( | ||
| f"Set trailing breakpoint {id} at {line}" | ||
| ) |
There was a problem hiding this comment.
Also some unnecessary logging.
...oject-tests/debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py
Outdated
Show resolved
Hide resolved
Adding together in a single commit as their implementations are linked. Only supported for DAP debuggers. These two commands make it a bit easier to drive dexter: DexStepFunction tells dexter to step-next though a function and DexContinue tells dexter to continue (run free) from one breakpoint to another within the current DexStepFunction function. When the DexStepFunction function breakpoint is triggered, dexter sets an instruction breakpoint at the return-address. This is so that stepping can resume in any other DexStepFunctions deeps in the call stack.
4947499 to
d7c91f8
Compare
|
Thanks Rebased. I'll land this one once we move over to using lldb-dap (rather than lldb) in feature_tests. |
…erControllers/ConditionalController.py Co-authored-by: Stephen Tozer <stephen.tozer@sony.com>
...oject-tests/debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py
Outdated
Show resolved
Hide resolved
…erControllers/ConditionalController.py
We should only notify the receiver thread that we've got a response after we've handled it, else we risk race conditions read/writing the debugger_state.
|
I've added two commits to fix test failures -
@SLTozer does this still look good? |
|
@OCHyams This broke the x86 bot on green dragon: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/32349/ Can you take a look? |
It looks like the link is to a broken arm64 bot, or am I confused? I'm testing on x86 locally and the test passes for me. Looking at the output... it looks like maybe some of the breakpoints are either not triggering or dexter doesn't see them as having been triggered. It could be the code I added that skips unknown breakpoints returned by lldb-dap - @adrian-prantl I was seeing LLDB-DAP return In the mean time, I'm out of office now but please feel free to revert this if it's blocking. Otherwise I'll be able to get to it tomorrow morning (UK). |
Adding together in a single commit as their implementations are linked.
Only supported for DAP debuggers. These two commands make it a bit easier to
drive dexter: DexStepFunction tells dexter to step-next though a function and
DexContinue tells dexter to continue (run free) from one breakpoint to another
within the current DexStepFunction function.
When the DexStepFunction function breakpoint is triggered, dexter sets an
instruction breakpoint at the return-address. This is so that stepping can
resume in any other DexStepFunctions deeps in the call stack.