mgorny updated this revision to Diff 366090. mgorny added a comment. 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'. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107931/new/ https://reviews.llvm.org/D107931 Files: lldb/include/lldb/Utility/StringExtractorGDBRemote.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h lldb/source/Utility/StringExtractorGDBRemote.cpp lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.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 @@ -10,6 +10,9 @@ the initial set of tests implemented. """ +import binascii +import itertools + import unittest2 import gdbremote_testcase import lldbgdbserverutils @@ -1246,3 +1249,61 @@ # 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() Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py =================================================================== --- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py +++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py @@ -183,6 +183,10 @@ 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) @@ -293,6 +297,12 @@ 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 Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py =================================================================== --- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py +++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py @@ -57,7 +57,6 @@ def A(self, packet): return "E47" - self.runCmd("log enable gdb-remote packets") self.server.responder = MyResponder() target = self.createTarget("a.yaml") @@ -148,3 +147,114 @@ 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,) + ]) Index: lldb/source/Utility/StringExtractorGDBRemote.cpp =================================================================== --- lldb/source/Utility/StringExtractorGDBRemote.cpp +++ lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -363,6 +363,8 @@ return eServerPacketType_vCont; if (PACKET_MATCHES("vCont?")) return eServerPacketType_vCont_actions; + if (PACKET_STARTS_WITH("vRun;")) + return eServerPacketType_vRun; } break; case '_': Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -202,6 +202,8 @@ PacketResult Handle_vAttachOrWait(StringExtractorGDBRemote &packet); + PacketResult Handle_vRun(StringExtractorGDBRemote &packet); + PacketResult Handle_D(StringExtractorGDBRemote &packet); PacketResult Handle_qThreadStopInfo(StringExtractorGDBRemote &packet); 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 @@ -182,6 +182,9 @@ RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_vCont_actions, &GDBRemoteCommunicationServerLLGS::Handle_vCont_actions); + RegisterMemberFunctionHandler( + StringExtractorGDBRemote::eServerPacketType_vRun, + &GDBRemoteCommunicationServerLLGS::Handle_vRun); RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_x, &GDBRemoteCommunicationServerLLGS::Handle_memory_read); @@ -3251,6 +3254,38 @@ 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(); Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -598,7 +598,8 @@ 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; Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -66,7 +66,7 @@ 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), @@ -767,6 +767,11 @@ 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)); @@ -805,6 +810,36 @@ } } 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) { Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h =================================================================== --- lldb/include/lldb/Utility/StringExtractorGDBRemote.h +++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h @@ -136,6 +136,7 @@ eServerPacketType_vAttachName, eServerPacketType_vCont, eServerPacketType_vCont_actions, // vCont? + eServerPacketType_vRun, eServerPacketType_stop_reason, // '?'
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits