labath created this revision.
labath added reviewers: mgorny, rupprecht.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

Uses our existing "error string" extension to provide a better
indication of why the launch failed (the client does not make use of the
error yet).

Also, fix the way we obtain the launch error message (make sure we read
the whole message, and skip trailing garbage), and reduce the size of
TestLldbGdbServer by splitting some tests into a separate file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133352

Files:
  lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1205,128 +1205,6 @@
             self.assertEqual(read_value, expected_reg_values[thread_index])
             thread_index += 1
 
-    @skipIfWindows # No pty support to test any inferior output
-    @add_test_categories(["llgs"])
-    def test_launch_via_A(self):
-        self.build()
-        exe_path = self.getBuildArtifact("a.out")
-        args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"]
-        hex_args = [binascii.b2a_hex(x.encode()).decode() for x in args]
-
-        server = self.connect_to_debug_monitor()
-        self.assertIsNotNone(server)
-        self.do_handshake()
-        # NB: strictly speaking we should use %x here but this packet
-        # is deprecated, so no point in changing lldb-server's expectations
-        self.test_sequence.add_log_lines(
-            ["read packet: $A %d,0,%s,%d,1,%s,%d,2,%s,%d,3,%s#00" %
-             tuple(itertools.chain.from_iterable(
-                 [(len(x), x) for x in hex_args])),
-             "send packet: $OK#00",
-             "read packet: $c#00",
-             "send packet: $W00#00"],
-            True)
-        context = self.expect_gdbremote_sequence()
-        self.assertEqual(context["O_content"],
-                         b'arg1\r\narg2\r\narg3\r\n')
-
-    @skipIfWindows # No pty support to test any inferior output
-    @add_test_categories(["llgs"])
-    def test_launch_via_vRun(self):
-        self.build()
-        exe_path = self.getBuildArtifact("a.out")
-        args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"]
-        hex_args = [binascii.b2a_hex(x.encode()).decode() for x in args]
-
-        server = self.connect_to_debug_monitor()
-        self.assertIsNotNone(server)
-        self.do_handshake()
-        self.test_sequence.add_log_lines(
-            ["read packet: $vRun;%s;%s;%s;%s#00" % tuple(hex_args),
-             {"direction": "send",
-              "regex": r"^\$T([0-9a-fA-F]+)"},
-             "read packet: $c#00",
-             "send packet: $W00#00"],
-            True)
-        context = self.expect_gdbremote_sequence()
-        self.assertEqual(context["O_content"],
-                         b'arg1\r\narg2\r\narg3\r\n')
-
-    @add_test_categories(["llgs"])
-    def test_launch_via_vRun_no_args(self):
-        self.build()
-        exe_path = self.getBuildArtifact("a.out")
-        hex_path = binascii.b2a_hex(exe_path.encode()).decode()
-
-        server = self.connect_to_debug_monitor()
-        self.assertIsNotNone(server)
-        self.do_handshake()
-        self.test_sequence.add_log_lines(
-            ["read packet: $vRun;%s#00" % (hex_path,),
-             {"direction": "send",
-              "regex": r"^\$T([0-9a-fA-F]+)"},
-             "read packet: $c#00",
-             "send packet: $W00#00"],
-            True)
-        self.expect_gdbremote_sequence()
-
-    @skipIfWindows # No pty support to test any inferior output
-    @add_test_categories(["llgs"])
-    def test_QEnvironment(self):
-        self.build()
-        exe_path = self.getBuildArtifact("a.out")
-        env = {"FOO": "test", "BAR": "a=z"}
-        args = [exe_path, "print-env:FOO", "print-env:BAR"]
-        hex_args = [binascii.b2a_hex(x.encode()).decode() for x in args]
-
-        server = self.connect_to_debug_monitor()
-        self.assertIsNotNone(server)
-        self.do_handshake()
-
-        for key, value in env.items():
-            self.test_sequence.add_log_lines(
-                ["read packet: $QEnvironment:%s=%s#00" % (key, value),
-                 "send packet: $OK#00"],
-                True)
-        self.test_sequence.add_log_lines(
-            ["read packet: $vRun;%s#00" % (";".join(hex_args),),
-             {"direction": "send",
-              "regex": r"^\$T([0-9a-fA-F]+)"},
-             "read packet: $c#00",
-             "send packet: $W00#00"],
-            True)
-        context = self.expect_gdbremote_sequence()
-        self.assertEqual(context["O_content"], b"test\r\na=z\r\n")
-
-    @skipIfWindows # No pty support to test any inferior output
-    @add_test_categories(["llgs"])
-    def test_QEnvironmentHexEncoded(self):
-        self.build()
-        exe_path = self.getBuildArtifact("a.out")
-        env = {"FOO": "test", "BAR": "a=z", "BAZ": "a*}#z"}
-        args = [exe_path, "print-env:FOO", "print-env:BAR", "print-env:BAZ"]
-        hex_args = [binascii.b2a_hex(x.encode()).decode() for x in args]
-
-        server = self.connect_to_debug_monitor()
-        self.assertIsNotNone(server)
-        self.do_handshake()
-
-        for key, value in env.items():
-            hex_enc = binascii.b2a_hex(("%s=%s" % (key, value)).encode()).decode()
-            self.test_sequence.add_log_lines(
-                ["read packet: $QEnvironmentHexEncoded:%s#00" % (hex_enc,),
-                 "send packet: $OK#00"],
-                True)
-        self.test_sequence.add_log_lines(
-            ["read packet: $vRun;%s#00" % (";".join(hex_args),),
-             {"direction": "send",
-              "regex": r"^\$T([0-9a-fA-F]+)"},
-             "read packet: $c#00",
-             "send packet: $W00#00"],
-            True)
-        context = self.expect_gdbremote_sequence()
-        self.assertEqual(context["O_content"], b"test\r\na=z\r\na*}#z\r\n")
-
     @skipUnlessPlatform(oslist=["freebsd", "linux"])
     @add_test_categories(["llgs"])
     def test_qXfer_siginfo_read(self):
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py
===================================================================
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py
@@ -0,0 +1,154 @@
+import itertools
+
+import gdbremote_testcase
+import lldbgdbserverutils
+from lldbsuite.support import seven
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class GdbRemoteLaunchTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+    @skipIfWindows # No pty support to test any inferior output
+    @add_test_categories(["llgs"])
+    def test_launch_via_A(self):
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"]
+        hex_args = [seven.hexlify(x) for x in args]
+
+        server = self.connect_to_debug_monitor()
+        self.assertIsNotNone(server)
+        self.do_handshake()
+        # NB: strictly speaking we should use %x here but this packet
+        # is deprecated, so no point in changing lldb-server's expectations
+        self.test_sequence.add_log_lines(
+            ["read packet: $A %d,0,%s,%d,1,%s,%d,2,%s,%d,3,%s#00" %
+             tuple(itertools.chain.from_iterable(
+                 [(len(x), x) for x in hex_args])),
+             "send packet: $OK#00",
+             "read packet: $c#00",
+             "send packet: $W00#00"],
+            True)
+        context = self.expect_gdbremote_sequence()
+        self.assertEqual(context["O_content"],
+                         b'arg1\r\narg2\r\narg3\r\n')
+
+    @skipIfWindows # No pty support to test any inferior output
+    @add_test_categories(["llgs"])
+    def test_launch_via_vRun(self):
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"]
+        hex_args = [seven.hexlify(x) for x in args]
+
+        server = self.connect_to_debug_monitor()
+        self.assertIsNotNone(server)
+        self.do_handshake()
+        self.test_sequence.add_log_lines(
+            ["read packet: $vRun;%s;%s;%s;%s#00" % tuple(hex_args),
+             {"direction": "send",
+              "regex": r"^\$T([0-9a-fA-F]+)"},
+             "read packet: $c#00",
+             "send packet: $W00#00"],
+            True)
+        context = self.expect_gdbremote_sequence()
+        self.assertEqual(context["O_content"],
+                         b'arg1\r\narg2\r\narg3\r\n')
+
+    @add_test_categories(["llgs"])
+    def test_launch_via_vRun_no_args(self):
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        hex_path = seven.hexlify(exe_path)
+
+        server = self.connect_to_debug_monitor()
+        self.assertIsNotNone(server)
+        self.do_handshake()
+        self.test_sequence.add_log_lines(
+            ["read packet: $vRun;%s#00" % (hex_path,),
+             {"direction": "send",
+              "regex": r"^\$T([0-9a-fA-F]+)"},
+             "read packet: $c#00",
+             "send packet: $W00#00"],
+            True)
+        self.expect_gdbremote_sequence()
+
+    @add_test_categories(["llgs"])
+    def test_launch_failure_via_vRun(self):
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        hex_path = seven.hexlify(exe_path)
+
+        server = self.connect_to_debug_monitor()
+        self.assertIsNotNone(server)
+        self.do_handshake()
+        self.test_sequence.add_log_lines(
+            ["read packet: $QEnableErrorStrings#00",
+             "send packet: $OK#00",
+             "read packet: $vRun;%s#00" % hex_path,
+             {"direction": "send", "regex": r"^\$E..;([0-9a-fA-F]+)#",
+                 "capture": {1: "msg"},
+             }],
+            True)
+        with open(exe_path, "ab") as exe_for_writing:
+            context = self.expect_gdbremote_sequence()
+        self.assertEqual(seven.unhexlify(context.get("msg")),
+                "execve failed: Text file busy")
+
+    @skipIfWindows # No pty support to test any inferior output
+    @add_test_categories(["llgs"])
+    def test_QEnvironment(self):
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        env = {"FOO": "test", "BAR": "a=z"}
+        args = [exe_path, "print-env:FOO", "print-env:BAR"]
+        hex_args = [seven.hexlify(x) for x in args]
+
+        server = self.connect_to_debug_monitor()
+        self.assertIsNotNone(server)
+        self.do_handshake()
+
+        for key, value in env.items():
+            self.test_sequence.add_log_lines(
+                ["read packet: $QEnvironment:%s=%s#00" % (key, value),
+                 "send packet: $OK#00"],
+                True)
+        self.test_sequence.add_log_lines(
+            ["read packet: $vRun;%s#00" % (";".join(hex_args),),
+             {"direction": "send",
+              "regex": r"^\$T([0-9a-fA-F]+)"},
+             "read packet: $c#00",
+             "send packet: $W00#00"],
+            True)
+        context = self.expect_gdbremote_sequence()
+        self.assertEqual(context["O_content"], b"test\r\na=z\r\n")
+
+    @skipIfWindows # No pty support to test any inferior output
+    @add_test_categories(["llgs"])
+    def test_QEnvironmentHexEncoded(self):
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        env = {"FOO": "test", "BAR": "a=z", "BAZ": "a*}#z"}
+        args = [exe_path, "print-env:FOO", "print-env:BAR", "print-env:BAZ"]
+        hex_args = [seven.hexlify(x) for x in args]
+
+        server = self.connect_to_debug_monitor()
+        self.assertIsNotNone(server)
+        self.do_handshake()
+
+        for key, value in env.items():
+            hex_enc = seven.hexlify("%s=%s" % (key, value))
+            self.test_sequence.add_log_lines(
+                ["read packet: $QEnvironmentHexEncoded:%s#00" % (hex_enc,),
+                 "send packet: $OK#00"],
+                True)
+        self.test_sequence.add_log_lines(
+            ["read packet: $vRun;%s#00" % (";".join(hex_args),),
+             {"direction": "send",
+              "regex": r"^\$T([0-9a-fA-F]+)"},
+             "read packet: $c#00",
+             "send packet: $W00#00"],
+            True)
+        context = self.expect_gdbremote_sequence()
+        self.assertEqual(context["O_content"], b"test\r\na=z\r\na*}#z\r\n")
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3517,19 +3517,17 @@
               arg.c_str());
   }
 
-  if (!argv.empty()) {
-    m_process_launch_info.GetExecutableFile().SetFile(
-        m_process_launch_info.GetArguments()[0].ref(), FileSpec::Style::native);
-    m_process_launch_error = LaunchProcess();
-    if (m_process_launch_error.Success()) {
-      assert(m_current_process);
-      return SendStopReasonForState(*m_current_process,
-                                    m_current_process->GetState(),
-                                    /*force_synchronous=*/true);
-    }
-    LLDB_LOG(log, "failed to launch exe: {0}", m_process_launch_error);
-  }
-  return SendErrorResponse(8);
+  if (argv.empty())
+    return SendErrorResponse(Status("No arguments"));
+  m_process_launch_info.GetExecutableFile().SetFile(
+      m_process_launch_info.GetArguments()[0].ref(), FileSpec::Style::native);
+  m_process_launch_error = LaunchProcess();
+  if (m_process_launch_error.Fail())
+    return SendErrorResponse(m_process_launch_error);
+  assert(m_current_process);
+  return SendStopReasonForState(*m_current_process,
+                                m_current_process->GetState(),
+                                /*force_synchronous=*/true);
 }
 
 GDBRemoteCommunication::PacketResult
Index: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
===================================================================
--- lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -282,10 +282,20 @@
   // parent process
 
   pipe.CloseWriteFileDescriptor();
-  char buf[1000];
-  int r = read(pipe.GetReadFileDescriptor(), buf, sizeof buf);
-
-  if (r == 0)
+  llvm::SmallString<0> buf;
+  buf.resize_for_overwrite(1000);
+  char *pos = buf.begin();
+
+  ssize_t r = 0;
+  do {
+    pos += r;
+    r = llvm::sys::RetryAfterSignal(-1, read, pipe.GetReadFileDescriptor(), pos,
+                                    buf.end() - pos);
+  } while (r > 0);
+  assert(r != -1);
+
+  buf.resize(pos - buf.begin());
+  if (buf.empty())
     return HostProcess(pid); // No error. We're done.
 
   error.SetErrorString(buf);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to