[lldb] Implement CLI support for reverse-continue

[rocallahan]

This introduces the options "-F/--forward" and "-R/--reverse" to process continue.

These only work if you're running with a gdbserver backend that supports reverse execution, such as rr. For testing we rely on the fake reverse-execution functionality in lldbreverse.py.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[rocallahan]

I suppose we need to add documentation for this but I'm not sure where...

[rocallahan]

@jimingham@labath Here's a PR adding process continue --reverse and --forward. I guess it needs to add documentation somewhere under docs? I'm not sure where.

[vogelsgesang]

I suppose we need to add documentation for this but I'm not sure where...

Afaict, your code change should already be sufficient such that help processor continue also lists the new -F and -R flags.

In addition, I guess we would eventually want to also document more generally how to use rr with lldb (how to start the rr replay, how to connect lldb to it, etc.) such that end users are able to discover this feature. However, I guess before doing so, we would like to also implement reverse single- stepping etc.?

[rocallahan]

In addition, I guess we would eventually want to also document more generally how to use rr with lldb (how to start the rr replay, how to connect lldb to it, etc.) such that end users are able to discover this feature. However, I guess before doing so, we would like to also implement reverse single- stepping etc.?

Yes, but actually I think this documentation can be in rr.

Reverse-continue is 80% of what people do in practice so just advertising "lldb supports reverse-continue with rr now" would be OK.

reverse-continue on a non-reverse-capable process gives a decent error message

Added.

process continue --forward also work for non-reverse-capable processes

Added.

We might want to add a test case for this error condition?

Added.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Robert O'Callahan (rocallahan)

Changes

This introduces the options "-F/--forward" and "-R/--reverse" to process continue.

These only work if you're running with a gdbserver backend that supports reverse execution, such as rr. For testing we rely on the fake reverse-execution functionality in lldbreverse.py.

---

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

6 Files Affected:

• (modified) lldb/source/Commands/CommandObjectProcess.cpp (+23-1)
• (modified) lldb/source/Commands/Options.td (+4)
• (added) lldb/test/API/commands/process/reverse-continue/Makefile (+3)
• (added) lldb/test/API/commands/process/reverse-continue/TestReverseContinue.py (+66)
• (added) lldb/test/API/commands/process/reverse-continue/TestReverseContinueNotSupported.py (+51)
• (added) lldb/test/API/commands/process/reverse-continue/main.c (+12)

diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 654dfa83ea444..bed81ef0fa7cd 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp+++ b/lldb/source/Commands/CommandObjectProcess.cpp@@ -468,7 +468,23 @@ class CommandObjectProcessContinue : public CommandObjectParsed { case 'b': m_run_to_bkpt_args.AppendArgument(option_arg); m_any_bkpts_specified = true; - break;+ break;+ case 'F':+ if (m_base_direction == lldb::RunDirection::eRunReverse) {+ error = Status::FromErrorString(+ "cannot specify both 'forward' and 'reverse'");+ break;+ }+ m_base_direction = lldb::RunDirection::eRunForward;+ break;+ case 'R':+ if (m_base_direction == lldb::RunDirection::eRunForward) {+ error = Status::FromErrorString(+ "cannot specify both 'forward' and 'reverse'");+ break;+ }+ m_base_direction = lldb::RunDirection::eRunReverse;+ break; default: llvm_unreachable("Unimplemented option"); } @@ -479,6 +495,7 @@ class CommandObjectProcessContinue : public CommandObjectParsed { m_ignore = 0; m_run_to_bkpt_args.Clear(); m_any_bkpts_specified = false; + m_base_direction = std::nullopt; } llvm::ArrayRef<OptionDefinition> GetDefinitions() override { @@ -488,6 +505,7 @@ class CommandObjectProcessContinue : public CommandObjectParsed { uint32_t m_ignore = 0; Args m_run_to_bkpt_args; bool m_any_bkpts_specified = false; + std::optional<lldb::RunDirection> m_base_direction; }; void DoExecute(Args &command, CommandReturnObject &result) override { @@ -654,6 +672,10 @@ class CommandObjectProcessContinue : public CommandObjectParsed { } } + if (m_options.m_base_direction.has_value()) {+ process->SetBaseDirection(*m_options.m_base_direction);+ }+ const uint32_t iohandler_id = process->GetIOHandlerID(); StreamString stream; diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index cc579d767eb06..e8c340b85aa92 100644 --- a/lldb/source/Commands/Options.td+++ b/lldb/source/Commands/Options.td@@ -744,6 +744,10 @@ let Command = "process continue" in { Arg<"BreakpointIDRange">, Desc<"Specify a breakpoint to continue to, temporarily " "ignoring other breakpoints. Can be specified more than once. " "The continue action will be done synchronously if this option is specified.">; + def thread_continue_forward : Option<"forward", "F">, Group<3>,+ Desc<"Execute in forward direction">;+ def thread_continue_reverse : Option<"reverse", "R">, Group<3>,+ Desc<"Execute in reverse direction">; } let Command = "process detach" in { diff --git a/lldb/test/API/commands/process/reverse-continue/Makefile b/lldb/test/API/commands/process/reverse-continue/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null+++ b/lldb/test/API/commands/process/reverse-continue/Makefile@@ -0,0 +1,3 @@+C_SOURCES := main.c++include Makefile.rulesdiff --git a/lldb/test/API/commands/process/reverse-continue/TestReverseContinue.py b/lldb/test/API/commands/process/reverse-continue/TestReverseContinue.py new file mode 100644 index 0000000000000..c04d2b9d4b5a5 --- /dev/null+++ b/lldb/test/API/commands/process/reverse-continue/TestReverseContinue.py@@ -0,0 +1,66 @@+"""+Test the "process continue --reverse" and "--forward" options.+"""+++import lldb+from lldbsuite.test.lldbtest import *+from lldbsuite.test.decorators import *+from lldbsuite.test.gdbclientutils import *+from lldbsuite.test.lldbreverse import ReverseTestBase+from lldbsuite.test import lldbutil+++class TestReverseContinue(ReverseTestBase):+ @skipIfRemote+ def test_reverse_continue(self):+ target, _, _ = self.setup_recording()++ # Set breakpoint and reverse-continue+ trigger_bkpt = target.BreakpointCreateByName("trigger_breakpoint", None)+ self.assertTrue(trigger_bkpt.GetNumLocations() > 0)+ self.expect(+ "process continue --reverse",+ substrs=["stop reason = breakpoint {0}.1".format(trigger_bkpt.GetID())],+ )+ # `process continue` should preserve current base direction.+ self.expect(+ "process continue",+ STOPPED_DUE_TO_HISTORY_BOUNDARY,+ substrs=["stopped", "stop reason = history boundary"],+ )+ self.expect(+ "process continue --forward",+ substrs=["stop reason = breakpoint {0}.1".format(trigger_bkpt.GetID())],+ )++ def setup_recording(self):+ """+ Record execution of code between "start_recording" and "stop_recording" breakpoints.++ Returns with the target stopped at "stop_recording", with recording disabled,+ ready to reverse-execute.+ """+ self.build()+ target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))+ process = self.connect(target)++ # Record execution from the start of the function "start_recording"+ # to the start of the function "stop_recording". We want to keep the+ # interval that we record as small as possible to minimize the run-time+ # of our single-stepping recorder.+ start_recording_bkpt = target.BreakpointCreateByName("start_recording", None)+ self.assertTrue(start_recording_bkpt.GetNumLocations() > 0)+ initial_threads = lldbutil.continue_to_breakpoint(process, start_recording_bkpt)+ self.assertEqual(len(initial_threads), 1)+ target.BreakpointDelete(start_recording_bkpt.GetID())+ self.start_recording()+ stop_recording_bkpt = target.BreakpointCreateByName("stop_recording", None)+ self.assertTrue(stop_recording_bkpt.GetNumLocations() > 0)+ lldbutil.continue_to_breakpoint(process, stop_recording_bkpt)+ target.BreakpointDelete(stop_recording_bkpt.GetID())+ self.stop_recording()++ self.dbg.SetAsync(False)++ return target, process, initial_threadsdiff --git a/lldb/test/API/commands/process/reverse-continue/TestReverseContinueNotSupported.py b/lldb/test/API/commands/process/reverse-continue/TestReverseContinueNotSupported.py new file mode 100644 index 0000000000000..440e908eca344 --- /dev/null+++ b/lldb/test/API/commands/process/reverse-continue/TestReverseContinueNotSupported.py@@ -0,0 +1,51 @@+"""+Test the "process continue --reverse" and "--forward" options+when reverse-continue is not supported.+"""+++import lldb+from lldbsuite.test.lldbtest import *+from lldbsuite.test.decorators import *+from lldbsuite.test import lldbutil+++class TestReverseContinueNotSupported(TestBase):+ def test_reverse_continue_not_supported(self):+ target = self.connect()++ # Set breakpoint and reverse-continue+ trigger_bkpt = target.BreakpointCreateByName("trigger_breakpoint", None)+ self.assertTrue(trigger_bkpt, VALID_BREAKPOINT)+ # `process continue --forward` should work.+ self.expect(+ "process continue --forward",+ substrs=["stop reason = breakpoint {0}.1".format(trigger_bkpt.GetID())],+ )+ self.expect(+ "process continue --reverse",+ error=True,+ substrs=["target does not support reverse-continue"],+ )++ def test_reverse_continue_forward_and_reverse(self):+ self.connect()++ self.expect(+ "process continue --forward --reverse",+ error=True,+ substrs=["cannot specify both 'forward' and 'reverse'"],+ )++ def connect(self):+ self.build()+ exe = self.getBuildArtifact("a.out")+ target = self.dbg.CreateTarget(exe)+ self.assertTrue(target, VALID_TARGET)++ main_bkpt = target.BreakpointCreateByName("main", None)+ self.assertTrue(main_bkpt, VALID_BREAKPOINT)++ process = target.LaunchSimple(None, None, self.get_process_working_directory())+ self.assertTrue(process, PROCESS_IS_VALID)+ return targetdiff --git a/lldb/test/API/commands/process/reverse-continue/main.c b/lldb/test/API/commands/process/reverse-continue/main.c new file mode 100644 index 0000000000000..ccec2bb27658d --- /dev/null+++ b/lldb/test/API/commands/process/reverse-continue/main.c@@ -0,0 +1,12 @@+static void start_recording() {}++static void trigger_breakpoint() {}++static void stop_recording() {}++int main() {+ start_recording();+ trigger_breakpoint();+ stop_recording();+ return 0;+}

[vogelsgesang]

Yes, but actually I think this documentation can be in rr.

Agree, we should probably have the bulk of the documentation in the rr repo. Not sure if we want to put it online right away, or if we wait to wait for lldb-21 to go out the door first. (Not sure how accustomed rr users are to building lldb from source)

Inside the LLVM project, we probably want to add a release note for this change, in the lldb section in llvm/docs/ReleaseNotes.md. Would be great I'd you could add a short bullet point there as part of this PR

[rocallahan]

or if we wait to wait for lldb-21 to go out the door first

I'll definitely wait for the next lldb release before updating the rr docs.

Would be great I'd you could add a short bullet point there

Done.

[rocallahan]

Thanks for the feedback, folks... if you're happy with it, don't forget to give it the thumbs-up :-)

[rocallahan]

@jimingham@DavidSpickett can one of you approve this please?

[JDevlieghere]

LGTM

[rocallahan]

Thanks. Can this be merged now?

[luporl]

It seems this broke lldb-aarch64-windows bot: https://lab.llvm.org/buildbot/#/builders/141/builds/8149

[DavidSpickett]

Sod's law that the supposedly simple test would be the problem.

I'll figure this out, I've disabled it on Windows for now.

[DavidSpickett]

Likely won't find the fix today, but I suspect that the Windows process class just doesn't know that it doesn't support reverse execution:

(lldb) process continue --reverse Process 3108 resuming (lldb) process status Process 3108 stopped 

The process being stopped is probably due to us failing to continue properly, rather than something actually happening.

[labath]

Probably because windows uses a different (in-proces) implementation.