[Lldb-commits] [lldb] 6ba3f72 - [lldb] [gdb-remote] Implement the vRun packet
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 10 05:08:47 PDT 2021
Author: Michał Górny
Date: 2021-09-10T14:08:36+02:00
New Revision: 6ba3f7237dc750aad2ce1d6a7a15e3b78370221a
URL: https://github.com/llvm/llvm-project/commit/6ba3f7237dc750aad2ce1d6a7a15e3b78370221a
DIFF: https://github.com/llvm/llvm-project/commit/6ba3f7237dc750aad2ce1d6a7a15e3b78370221a.diff
LOG: [lldb] [gdb-remote] Implement the vRun packet
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. This also improves GDB compatibility since modern
versions of gdbserver do not implement the A packet at all.
Make qLaunchSuccess not obligatory when using vRun. It is not
implemented by gdbserver, and since vRun returns the stop reason,
we can assume it to be successful.
Differential Revision: https://reviews.llvm.org/D107931
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 0c4ae32691b71..1712c113d396b 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -136,6 +136,7 @@ class StringExtractorGDBRemote : public StringExtractor {
eServerPacketType_vAttachName,
eServerPacketType_vCont,
eServerPacketType_vCont_actions, // vCont?
+ eServerPacketType_vRun,
eServerPacketType_stop_reason, // '?'
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 588e3faf01874..3696c7096e30e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -66,7 +66,7 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
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),
@@ -794,6 +794,11 @@ bool GDBRemoteCommunicationClient::GetLaunchSuccess(std::string &error_str) {
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));
@@ -832,6 +837,36 @@ int GDBRemoteCommunicationClient::SendArgumentsPacket(
}
}
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) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index fde5897f84136..376adfeb95d0f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -588,7 +588,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
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;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 3b8ed86d9f372..1ad838b51c26b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -182,6 +182,9 @@ void GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() {
RegisterMemberFunctionHandler(
StringExtractorGDBRemote::eServerPacketType_vCont_actions,
&GDBRemoteCommunicationServerLLGS::Handle_vCont_actions);
+ RegisterMemberFunctionHandler(
+ StringExtractorGDBRemote::eServerPacketType_vRun,
+ &GDBRemoteCommunicationServerLLGS::Handle_vRun);
RegisterMemberFunctionHandler(
StringExtractorGDBRemote::eServerPacketType_x,
&GDBRemoteCommunicationServerLLGS::Handle_memory_read);
@@ -3255,6 +3258,38 @@ GDBRemoteCommunicationServerLLGS::Handle_vAttachOrWait(
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();
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index f054e37eae948..89c0fd8d7b7f3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -202,6 +202,8 @@ class GDBRemoteCommunicationServerLLGS
PacketResult Handle_vAttachOrWait(StringExtractorGDBRemote &packet);
+ PacketResult Handle_vRun(StringExtractorGDBRemote &packet);
+
PacketResult Handle_D(StringExtractorGDBRemote &packet);
PacketResult Handle_qThreadStopInfo(StringExtractorGDBRemote &packet);
diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp
index 64509931223e1..d6bbf7171916a 100644
--- a/lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -365,6 +365,8 @@ StringExtractorGDBRemote::GetServerPacketType() const {
return eServerPacketType_vCont;
if (PACKET_MATCHES("vCont?"))
return eServerPacketType_vCont_actions;
+ if (PACKET_STARTS_WITH("vRun;"))
+ return eServerPacketType_vRun;
}
break;
case '_':
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
index c95e4a41e8813..f5b59c6890202 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -57,7 +57,6 @@ def qfThreadInfo(self):
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 @@ def for_each_gpr(self, process, operation):
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,)
+ ])
diff --git a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
index bfbd48dd5167e..bfdefc8c37852 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -190,6 +190,10 @@ def respond(self, packet):
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)
@@ -303,6 +307,12 @@ def qPathComplete(self):
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
diff --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
index 96337e46aaace..6f68a8b038ee4 100644
--- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ b/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 @@ def test_P_and_p_thread_suffix_work(self):
# 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()
More information about the lldb-commits
mailing list