[lldb] [gdb-remote] Implement vRun packet

[mgorny]

Summary

Implement the simpler vRun packet and prefer it over the A packet.
Unlike the latter, it tranmits command-line arguments without redundant
indices and lengths.

[mgorny]

Include server tests.

[mgorny]

Correct the vRun packet to return a stop reason rather than 'OK' to match GDB behavior. Add initial client tests.

[mgorny]

Handle unsupported qLaunchSuccess gracefully on the client. This effectively makes it possible to launch programs via gdbserver.

Enhance the client tests to verify that the process has actually 'started'.

[labath]

I like this. I am saddened to see that neither method supports setting a "fake" argv[0], but that's not a problem of this patch,,,

[mgorny]

While technically this patch doesn't depend on these two, applying them out of order would cause unnecessary rebase hell.

lldb/include/lldb/Utility/StringExtractorGDBRemote.h

enum ServerPacketType {
     eServerPacketType_jModulesInfo,
 
     eServerPacketType_vAttach,
     eServerPacketType_vAttachWait,
     eServerPacketType_vAttachOrWait,
     eServerPacketType_vAttachName,
     eServerPacketType_vCont,
     eServerPacketType_vCont_actions, // vCont?
+    eServerPacketType_vRun,
 
     eServerPacketType_stop_reason, // '?'
 
     eServerPacketType_c,
     eServerPacketType_C,
     eServerPacketType_D,
     eServerPacketType_g,
     eServerPacketType_G,

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

protected:
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,
       m_supports_qThreadStopInfo : 1, m_supports_z0 : 1, m_supports_z1 : 1,
       m_supports_z2 : 1, m_supports_z3 : 1, m_supports_z4 : 1,
       m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
       m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
       m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
       m_supports_jModulesInfo : 1, m_supports_vFileSize : 1,
-      m_supports_vFileMode : 1, m_supports_vFileExists : 1;
+      m_supports_vFileMode : 1, m_supports_vFileExists : 1,
+      m_supports_vRun : 1;
 
   /// Current gdb remote protocol process identifier for all other operations
   lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
   /// Current gdb remote protocol process identifier for continue, step, etc
   lldb::pid_t m_curr_pid_run = LLDB_INVALID_PROCESS_ID;
   /// Current gdb remote protocol thread identifier for all other operations
   lldb::tid_t m_curr_tid = LLDB_INVALID_THREAD_ID;
   /// Current gdb remote protocol thread identifier for continue, step, etc

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

: GDBRemoteClientBase("gdb-remote.client", "gdb-remote.client.rx_packet"),
       m_supports_qUserName(true), m_supports_qGroupName(true),
       m_supports_qThreadStopInfo(true), m_supports_z0(true),
       m_supports_z1(true), m_supports_z2(true), m_supports_z3(true),
       m_supports_z4(true), m_supports_QEnvironment(true),
       m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
       m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
       m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
       m_supports_vFileSize(true), m_supports_vFileMode(true),
-      m_supports_vFileExists(true),
+      m_supports_vFileExists(true), m_supports_vRun(true),
 
       m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
       m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
       m_qSupported_response(), m_supported_async_json_packets_sp(),
       m_qXfer_memory_map() {}
 
 // Destructor
 GDBRemoteCommunicationClient::~GDBRemoteCommunicationClient() {

 
 bool GDBRemoteCommunicationClient::GetLaunchSuccess(std::string &error_str) {
   error_str.clear();
   StringExtractorGDBRemote response;
   if (SendPacketAndWaitForResponse("qLaunchSuccess", response) ==
       PacketResult::Success) {
     if (response.IsOKResponse())
       return true;
+    // GDB does not implement qLaunchSuccess -- but if we used vRun,
+    // then we already received a successful launch indication via stop
+    // reason.
+    if (response.IsUnsupportedResponse() && m_supports_vRun)
+      return true;
     if (response.GetChar() == 'E') {
       // A string the describes what failed when launching...
       error_str = std::string(response.GetStringRef().substr(1));
     } else {
       error_str.assign("unknown error occurred launching process");
     }
   } else {
     error_str.assign("timed out waiting for app to launch");
if (!exe_path.empty()) {
     argv.push_back(exe_path.c_str());
     for (uint32_t i = 1; (arg = launch_args.GetArgumentAtIndex(i)) != nullptr;
          ++i) {
       if (arg)
         argv.push_back(arg);
     }
   }
   if (!argv.empty()) {
+    // try vRun first
+    if (m_supports_vRun) {
+      StreamString packet;
+      packet.PutCString("vRun");
+      for (const char *arg : argv) {
+        packet.PutChar(';');
+        packet.PutBytesAsRawHex8(arg, strlen(arg));
+      }
+
+      StringExtractorGDBRemote response;
+      if (SendPacketAndWaitForResponse(packet.GetString(), response) !=
+          PacketResult::Success)
+        return -1;
+
+      if (response.IsErrorResponse()) {
+        uint8_t error = response.GetError();
+        if (error)
+          return error;
+        return -1;
+      }
+      // vRun replies with a stop reason packet
+      // FIXME: right now we just discard the packet and LLDB queries
+      // for stop reason again
+      if (!response.IsUnsupportedResponse())
+        return 0;
+
+      m_supports_vRun = false;
+    }
+
+    // fallback to A
     StreamString packet;
     packet.PutChar('A');
     for (size_t i = 0, n = argv.size(); i < n; ++i) {
       arg = argv[i];
       const int arg_len = strlen(arg);
       if (i > 0)
         packet.PutChar(',');
       packet.Printf("%i,%i,", arg_len * 2, (int)i);

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

protected:
   PacketResult Handle_vAttach(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vAttachWait(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qVAttachOrWaitSupported(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vAttachOrWait(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vRun(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_D(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qThreadStopInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_jThreadsInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qWatchpointSupportInfo(StringExtractorGDBRemote &packet);
 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

RegisterMemberFunctionHandler(
       &GDBRemoteCommunicationServerLLGS::Handle_vAttachOrWait);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_vCont,
       &GDBRemoteCommunicationServerLLGS::Handle_vCont);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_vCont_actions,
       &GDBRemoteCommunicationServerLLGS::Handle_vCont_actions);
   RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_vRun,
+      &GDBRemoteCommunicationServerLLGS::Handle_vRun);
+  RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_x,
       &GDBRemoteCommunicationServerLLGS::Handle_memory_read);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_Z,
                                 &GDBRemoteCommunicationServerLLGS::Handle_Z);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_z,
                                 &GDBRemoteCommunicationServerLLGS::Handle_z);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_QPassSignals,
if (error.Fail()) {
     return SendErrorResponse(error);
   }
 
   // Notify we attached by sending a stop packet.
   return SendStopReasonForState(m_current_process->GetState());
 }
 
 GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_vRun(
+    StringExtractorGDBRemote &packet) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  llvm::StringRef s = packet.GetStringRef();
+  if (!s.consume_front("vRun;"))
+    return SendErrorResponse(8);
+
+  llvm::SmallVector<llvm::StringRef, 16> argv;
+  s.split(argv, ';');
+
+  for (llvm::StringRef hex_arg : argv) {
+    StringExtractor arg_ext{hex_arg};
+    std::string arg;
+    arg_ext.GetHexByteString(arg);
+    m_process_launch_info.GetArguments().AppendArgument(arg);
+    LLDB_LOGF(log, "LLGSPacketHandler::%s added arg: \"%s\"", __FUNCTION__,
+              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())
+      return SendStopReasonForState(m_current_process->GetState());
+    LLDB_LOG(log, "failed to launch exe: {0}", m_process_launch_error);
+  }
+  return SendErrorResponse(8);
+}
+
+GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) {
   StopSTDIOForwarding();
 
   lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
 
   // Consume the ';' after D.
   packet.SetFilePos(1);
   if (packet.GetBytesLeft()) {

lldb/source/Utility/StringExtractorGDBRemote.cpp

if (PACKET_STARTS_WITH("vFile:")) {
       if (PACKET_STARTS_WITH("vAttachOrWait;"))
         return eServerPacketType_vAttachOrWait;
       if (PACKET_STARTS_WITH("vAttachName;"))
         return eServerPacketType_vAttachName;
       if (PACKET_STARTS_WITH("vCont;"))
         return eServerPacketType_vCont;
       if (PACKET_MATCHES("vCont?"))
         return eServerPacketType_vCont_actions;
+      if (PACKET_STARTS_WITH("vRun;"))
+        return eServerPacketType_vRun;
     }
     break;
   case '_':
     switch (packet_cstr[1]) {
     case 'M':
       return eServerPacketType__M;
 
     case 'm':

lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

def test_launch_fail(self):
 
             def qfThreadInfo(self):
                 return "OK" # No threads.
 
             # Then, when we are asked to attach, error out.
             def A(self, packet):
                 return "E47"
 
-        self.runCmd("log enable gdb-remote packets")
         self.server.responder = MyResponder()
 
         target = self.createTarget("a.yaml")
         process = self.connect(target)
         lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, [lldb.eStateConnected])
 
         error = lldb.SBError()
         target.Launch(lldb.SBListener(), None, None, None, None, None,
class TestGDBRemoteClient(GDBRemoteTestBase):
     def for_each_gpr(self, process, operation):
         registers = process.GetThreadAtIndex(0).GetFrameAtIndex(0).GetRegisters()
         self.assertGreater(registers.GetSize(), 0)
         regSet = registers[0]
         numChildren = regSet.GetNumChildren()
         self.assertGreater(numChildren, 0)
         for i in range(numChildren):
             operation(regSet.GetChildAtIndex(i))
+
+    def test_launch_A(self):
+        class MyResponder(MockGDBServerResponder):
+            def __init__(self, *args, **kwargs):
+                self.started = False
+                return super().__init__(*args, **kwargs)
+
+            def qC(self):
+                if self.started:
+                    return "QCp10.10"
+                else:
+                    return "E42"
+
+            def qfThreadInfo(self):
+                if self.started:
+                    return "mp10.10"
+                else:
+                   return "E42"
+
+            def qsThreadInfo(self):
+                return "l"
+
+            def A(self, packet):
+                self.started = True
+                return "OK"
+
+            def qLaunchSuccess(self):
+                if self.started:
+                    return "OK"
+                return "E42"
+
+        self.server.responder = MyResponder()
+
+        target = self.createTarget("a.yaml")
+        exe_path = self.getBuildArtifact("a")
+        exe_hex = binascii.b2a_hex(exe_path.encode()).decode()
+        process = self.connect(target)
+        lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+                                      [lldb.eStateConnected])
+
+        target.Launch(lldb.SBListener(),
+                      ["arg1", "arg2", "arg3"],  # argv
+                      [],  # envp
+                      None,  # stdin_path
+                      None,  # stdout_path
+                      None,  # stderr_path
+                      None,  # working_directory
+                      0,  # launch_flags
+                      True,  # stop_at_entry
+                      lldb.SBError())  # error
+        self.assertTrue(process, PROCESS_IS_VALID)
+        self.assertEqual(process.GetProcessID(), 16)
+
+        self.assertPacketLogContains([
+          "A%d,0,%s,8,1,61726731,8,2,61726732,8,3,61726733" % (
+              len(exe_hex), exe_hex),
+        ])
+
+    def test_launch_vRun(self):
+        class MyResponder(MockGDBServerResponder):
+            def __init__(self, *args, **kwargs):
+                self.started = False
+                return super().__init__(*args, **kwargs)
+
+            def qC(self):
+                if self.started:
+                    return "QCp10.10"
+                else:
+                    return "E42"
+
+            def qfThreadInfo(self):
+                if self.started:
+                    return "mp10.10"
+                else:
+                   return "E42"
+
+            def qsThreadInfo(self):
+                return "l"
+
+            def vRun(self, packet):
+                self.started = True
+                return "T13"
+
+            def A(self, packet):
+                return "E28"
+
+        self.server.responder = MyResponder()
+
+        target = self.createTarget("a.yaml")
+        exe_path = self.getBuildArtifact("a")
+        exe_hex = binascii.b2a_hex(exe_path.encode()).decode()
+        process = self.connect(target)
+        lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+                                      [lldb.eStateConnected])
+
+        process = target.Launch(lldb.SBListener(),
+                                ["arg1", "arg2", "arg3"],  # argv
+                                [],  # envp
+                                None,  # stdin_path
+                                None,  # stdout_path
+                                None,  # stderr_path
+                                None,  # working_directory
+                                0,  # launch_flags
+                                True,  # stop_at_entry
+                                lldb.SBError())  # error
+        self.assertTrue(process, PROCESS_IS_VALID)
+        self.assertEqual(process.GetProcessID(), 16)
+
+        self.assertPacketLogContains([
+          "vRun;%s;61726731;61726732;61726733" % (exe_hex,)
+        ])

lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

def respond(self, packet):
         if packet == "qsProcessInfo":
             return self.qsProcessInfo()
         if packet.startswith("qfProcessInfo"):
             return self.qfProcessInfo(packet)
         if packet.startswith("qPathComplete:"):
             return self.qPathComplete()
         if packet.startswith("vFile:"):
             return self.vFile(packet)
+        if packet.startswith("vRun;"):
+            return self.vRun(packet)
+        if packet.startswith("qLaunchSuccess"):
+            return self.qLaunchSuccess()
 
         return self.other(packet)
 
     def qsProcessInfo(self):
         return "E04"
 
     def qfProcessInfo(self, packet):
         return "E04"
def qMemoryRegionInfo(self, addr):
         return ""
 
     def qPathComplete(self):
         return ""
 
     def vFile(self, packet):
         return ""
 
+    def vRun(self, packet):
+        return ""
+
+    def qLaunchSuccess(self):
+        return ""
+
     """
     Raised when we receive a packet for which there is no default action.
     Override the responder class to implement behavior suitable for the test at
     hand.
     """
     class UnexpectedPacketException(Exception):
         pass
 

lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

 """
 Test case for testing the gdbremote protocol.
 
 Tests run against debugserver and lldb-server (llgs).
 lldb-server tests run where the lldb-server exe is
 available.
 
 This class will be broken into smaller test case classes by
 gdb remote packet functional areas.  For now it contains
 the initial set of tests implemented.
 """
 
+import binascii
+import itertools
+
 import unittest2
 import gdbremote_testcase
 import lldbgdbserverutils
 from lldbsuite.support import seven
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.lldbdwarf import *
 from lldbsuite.test import lldbutil, lldbplatformutil
def test_P_and_p_thread_suffix_work(self):
             p_response = context.get("p_response")
             self.assertIsNotNone(p_response)
             read_value = lldbgdbserverutils.unpack_register_hex_unsigned(
                 endian, p_response)
 
             # Make sure we read back what we wrote.
             self.assertEqual(read_value, expected_reg_values[thread_index])
             thread_index += 1
+
+    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')
+
+    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()
+
+    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()