[lldb/qemu] Add support for pty redirection

labath · labath · commit 5c4cb323e86a · 2021-12-06T15:03:21.000+01:00
Lldb uses a pty to read/write to the standard input and output of the debugged process. For host processes this would be automatically set up by Target::FinalizeFileActions. The Qemu platform is in a unique position of not really being a host platform, but not being remote either. It reports IsHost() = false, but it is sufficiently host-like that we can use the usual pty mechanism. This patch adds the necessary glue code to enable pty redirection. It includes a small refactor of Target::FinalizeFileActions and ProcessLaunchInfo::SetUpPtyRedirection to reduce the amount of boilerplate that would need to be copied. I will note that qemu is not able to separate output from the emulated program from the output of the emulator itself, so the two will arrive intertwined. Normally this should not be a problem since qemu should not produce any output during regular operation, but some output can slip through in case of errors. This situation should be pretty obvious (to a human), and it is the best we can do anyway. For testing purposes, and inspired by lldb-server tests, I have extended the mock emulator with the ability "program" the behavior of the "emulated" program via command-line arguments. Differential Revision: https://reviews.llvm.org/D114796
diff --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp b/lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -212,6 +212,14 @@ void ProcessLaunchInfo::SetDetachOnError(bool enable) {
 
 llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
   Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS);
+
+  bool stdin_free = GetFileActionForFD(STDIN_FILENO) == nullptr;
+  bool stdout_free = GetFileActionForFD(STDOUT_FILENO) == nullptr;
+  bool stderr_free = GetFileActionForFD(STDERR_FILENO) == nullptr;
+  bool any_free = stdin_free || stdout_free || stderr_free;
+  if (!any_free)
+    return llvm::Error::success();
+
   LLDB_LOG(log, "Generating a pty to use for stdin/out/err");
 
   int open_flags = O_RDWR | O_NOCTTY;
@@ -226,19 +234,13 @@ llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
 
   const FileSpec secondary_file_spec(m_pty->GetSecondaryName());
 
-  // Only use the secondary tty if we don't have anything specified for
-  // input and don't have an action for stdin
-  if (GetFileActionForFD(STDIN_FILENO) == nullptr)
+  if (stdin_free)
     AppendOpenFileAction(STDIN_FILENO, secondary_file_spec, true, false);
 
-  // Only use the secondary tty if we don't have anything specified for
-  // output and don't have an action for stdout
-  if (GetFileActionForFD(STDOUT_FILENO) == nullptr)
+  if (stdout_free)
     AppendOpenFileAction(STDOUT_FILENO, secondary_file_spec, false, true);
 
-  // Only use the secondary tty if we don't have anything specified for
-  // error and don't have an action for stderr
-  if (GetFileActionForFD(STDERR_FILENO) == nullptr)
+  if (stderr_free)
     AppendOpenFileAction(STDERR_FILENO, secondary_file_spec, false, true);
   return llvm::Error::success();
 }
diff --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -126,6 +126,11 @@ lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
   launch_info.SetMonitorProcessCallback(ProcessLaunchInfo::NoOpMonitorCallback,
                                         false);
 
+  // This is automatically done for host platform in
+  // Target::FinalizeFileActions, but we're not a host platform.
+  llvm::Error Err = launch_info.SetUpPtyRedirection();
+  LLDB_LOG_ERROR(log, std::move(Err), "SetUpPtyRedirection failed: {0}");
+
   error = Host::LaunchProcess(launch_info);
   if (error.Fail())
     return nullptr;
@@ -134,6 +139,7 @@ lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
       launch_info.GetListener(),
       process_gdb_remote::ProcessGDBRemote::GetPluginNameStatic(), nullptr,
       true);
+
   ListenerSP listener_sp =
       Listener::MakeListener("lldb.platform_qemu_user.debugprocess");
   launch_info.SetHijackListener(listener_sp);
@@ -143,6 +149,11 @@ lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
   if (error.Fail())
     return nullptr;
 
+  if (launch_info.GetPTY().GetPrimaryFileDescriptor() !=
+      PseudoTerminal::invalid_fd)
+    process_sp->SetSTDIOFileDescriptor(
+        launch_info.GetPTY().ReleasePrimaryFileDescriptor());
+
   process_sp->WaitForProcessToStop(llvm::None, nullptr, false, listener_sp);
   return process_sp;
 }
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
@@ -3321,8 +3321,7 @@ void Target::FinalizeFileActions(ProcessLaunchInfo &info) {
                  err_file_spec);
       }
 
-      if (default_to_use_pty &&
-          (!in_file_spec || !out_file_spec || !err_file_spec)) {
+      if (default_to_use_pty) {
         llvm::Error Err = info.SetUpPtyRedirection();
         LLDB_LOG_ERROR(log, std::move(Err), "SetUpPtyRedirection failed: {0}");
       }
diff --git a/lldb/test/API/qemu/TestQemuLaunch.py b/lldb/test/API/qemu/TestQemuLaunch.py
@@ -6,6 +6,7 @@
 import stat
 import sys
 from textwrap import dedent
+import lldbsuite.test.lldbutil
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.decorators import *
 from lldbsuite.test.gdbclientutils import *
@@ -46,7 +47,7 @@ def test_basic_launch(self):
         self.build()
         exe = self.getBuildArtifact()
 
-        # Create a target using out platform
+        # Create a target using our platform
         error = lldb.SBError()
         target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
         self.assertSuccess(error)
@@ -55,7 +56,7 @@ def test_basic_launch(self):
         # "Launch" the process. Our fake qemu implementation will pretend it
         # immediately exited.
         process = target.LaunchSimple(
-                [self.getBuildArtifact("state.log"), "arg2", "arg3"], None, None)
+                ["dump:" + self.getBuildArtifact("state.log")], None, None)
         self.assertIsNotNone(process)
         self.assertEqual(process.GetState(), lldb.eStateExited)
         self.assertEqual(process.GetExitStatus(), 0x47)
@@ -64,7 +65,84 @@ def test_basic_launch(self):
         with open(self.getBuildArtifact("state.log")) as s:
             state = json.load(s)
         self.assertEqual(state["program"], self.getBuildArtifact())
-        self.assertEqual(state["rest"], ["arg2", "arg3"])
+        self.assertEqual(state["args"],
+                ["dump:" + self.getBuildArtifact("state.log")])
+
+    def test_stdio_pty(self):
+        self.build()
+        exe = self.getBuildArtifact()
+
+        # Create a target using our platform
+        error = lldb.SBError()
+        target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
+        self.assertSuccess(error)
+
+        info = lldb.SBLaunchInfo([
+            "stdin:stdin",
+            "stdout:STDOUT CONTENT\n",
+            "stderr:STDERR CONTENT\n",
+            "dump:" + self.getBuildArtifact("state.log"),
+            ])
+
+        listener = lldb.SBListener("test_stdio")
+        info.SetListener(listener)
+
+        self.dbg.SetAsync(True)
+        process = target.Launch(info, error)
+        self.assertSuccess(error)
+        lldbutil.expect_state_changes(self, listener, process,
+                [lldb.eStateRunning])
+
+        process.PutSTDIN("STDIN CONTENT\n")
+
+        lldbutil.expect_state_changes(self, listener, process,
+                [lldb.eStateExited])
+
+        # Echoed stdin, stdout and stderr. With a pty we cannot split standard
+        # output and error.
+        self.assertEqual(process.GetSTDOUT(1000),
+                "STDIN CONTENT\r\nSTDOUT CONTENT\r\nSTDERR CONTENT\r\n")
+        with open(self.getBuildArtifact("state.log")) as s:
+            state = json.load(s)
+        self.assertEqual(state["stdin"], "STDIN CONTENT\n")
+
+    def test_stdio_redirect(self):
+        self.build()
+        exe = self.getBuildArtifact()
+
+        # Create a target using our platform
+        error = lldb.SBError()
+        target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
+        self.assertSuccess(error)
+
+        info = lldb.SBLaunchInfo([
+            "stdin:stdin",
+            "stdout:STDOUT CONTENT",
+            "stderr:STDERR CONTENT",
+            "dump:" + self.getBuildArtifact("state.log"),
+            ])
+
+        info.AddOpenFileAction(0, self.getBuildArtifact("stdin.txt"),
+                True, False)
+        info.AddOpenFileAction(1, self.getBuildArtifact("stdout.txt"),
+                False, True)
+        info.AddOpenFileAction(2, self.getBuildArtifact("stderr.txt"),
+                False, True)
+
+        with open(self.getBuildArtifact("stdin.txt"), "w") as f:
+            f.write("STDIN CONTENT")
+
+        process = target.Launch(info, error)
+        self.assertSuccess(error)
+        self.assertEqual(process.GetState(), lldb.eStateExited)
+
+        with open(self.getBuildArtifact("stdout.txt")) as f:
+            self.assertEqual(f.read(), "STDOUT CONTENT")
+        with open(self.getBuildArtifact("stderr.txt")) as f:
+            self.assertEqual(f.read(), "STDERR CONTENT")
+        with open(self.getBuildArtifact("state.log")) as s:
+            state = json.load(s)
+        self.assertEqual(state["stdin"], "STDIN CONTENT")
 
     def test_bad_emulator_path(self):
         self.set_emulator_setting("emulator-path",
diff --git a/lldb/test/API/qemu/qemu.py b/lldb/test/API/qemu/qemu.py
@@ -1,36 +1,63 @@
-from textwrap import dedent
 import argparse
 import socket
 import json
+import sys
 
 import use_lldb_suite
 from lldbsuite.test.gdbclientutils import *
 
+_description = """\
+Implements a fake qemu for testing purposes. The executable program
+is not actually run. Instead a very basic mock process is presented
+to lldb. This allows us to check the invocation parameters.
+
+The behavior of the emulated "process" is controlled via its command line
+arguments, which should take the form of key:value pairs. Currently supported
+actions are:
+- dump: Dump the state of the emulator as a json dictionary. <value> specifies
+  the target filename.
+- stdout: Write <value> to program stdout.
+- stderr: Write <value> to program stderr.
+- stdin: Read a line from stdin and store it in the emulator state. <value>
+  specifies the dictionary key.
+"""
+
 class MyResponder(MockGDBServerResponder):
+    def __init__(self, state):
+        super().__init__()
+        self._state = state
+
     def cont(self):
+        for a in self._state["args"]:
+            action, data = a.split(":", 1)
+            if action == "dump":
+                with open(data, "w") as f:
+                    json.dump(self._state, f)
+            elif action == "stdout":
+                sys.stdout.write(data)
+            elif action == "stderr":
+                sys.stderr.write(data)
+            elif action == "stdin":
+                self._state[data] = sys.stdin.readline()
+            else:
+                print("Unknown action: %r\n" % a)
+                return "X01"
         return "W47"
 
 class FakeEmulator(MockGDBServer):
-    def __init__(self, addr):
+    def __init__(self, addr, state):
         super().__init__(UnixServerSocket(addr))
-        self.responder = MyResponder()
+        self.responder = MyResponder(state)
 
 def main():
-    parser = argparse.ArgumentParser(description=dedent("""\
-            Implements a fake qemu for testing purposes. The executable program
-            is not actually run. Instead a very basic mock process is presented
-            to lldb. The emulated program must accept at least one argument.
-            This should be a path where the emulator will dump its state. This
-            allows us to check the invocation parameters.
-            """))
+    parser = argparse.ArgumentParser(description=_description,
+            formatter_class=argparse.RawDescriptionHelpFormatter)
     parser.add_argument('-g', metavar="unix-socket", required=True)
     parser.add_argument('program', help="The program to 'emulate'.")
-    parser.add_argument('state_file', help="Where to dump the emulator state.")
-    parsed, rest = parser.parse_known_args()
-    with open(parsed.state_file, "w") as f:
-        json.dump({"program":parsed.program, "rest":rest}, f)
+    parser.add_argument("args", nargs=argparse.REMAINDER)
+    args = parser.parse_args()
 
-    emulator = FakeEmulator(parsed.g)
+    emulator = FakeEmulator(args.g, vars(args))
     emulator.run()
 
 if __name__ == "__main__":

Original file line number	Diff line number	Diff line change
`@@ -3321,8 +3321,7 @@ void Target::FinalizeFileActions(ProcessLaunchInfo &info) {`
`3321`	`3321`	`err_file_spec);`
`3322`	`3322`	`}`
`3323`	`3323`
`3324`		`- if (default_to_use_pty &&`
`3325`		`- (!in_file_spec \|\| !out_file_spec \|\| !err_file_spec)) {`
	`3324`	`+ if (default_to_use_pty) {`
`3326`	`3325`	`llvm::Error Err = info.SetUpPtyRedirection();`
`3327`	`3326`	`LLDB_LOG_ERROR(log, std::move(Err), "SetUpPtyRedirection failed: {0}");`
`3328`	`3327`	`}`