[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
This commit is contained in:
Michał Górny 2021-08-11 22:58:11 +02:00
parent 501eaf8877
commit 6ba3f7237d
9 changed files with 260 additions and 3 deletions

View file

@ -136,6 +136,7 @@ public:
eServerPacketType_vAttachName,
eServerPacketType_vCont,
eServerPacketType_vCont_actions, // vCont?
eServerPacketType_vRun,
eServerPacketType_stop_reason, // '?'

View file

@ -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) {

View file

@ -588,7 +588,8 @@ protected:
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;

View file

@ -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();

View file

@ -202,6 +202,8 @@ protected:
PacketResult Handle_vAttachOrWait(StringExtractorGDBRemote &packet);
PacketResult Handle_vRun(StringExtractorGDBRemote &packet);
PacketResult Handle_D(StringExtractorGDBRemote &packet);
PacketResult Handle_qThreadStopInfo(StringExtractorGDBRemote &packet);

View file

@ -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 '_':

View file

@ -57,7 +57,6 @@ class TestGDBRemoteClient(GDBRemoteTestBase):
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 @@ class TestGDBRemoteClient(GDBRemoteTestBase):
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,)
])

View file

@ -190,6 +190,10 @@ class MockGDBServerResponder:
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 @@ class MockGDBServerResponder:
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

View file

@ -10,6 +10,9 @@ 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
@ -1246,3 +1249,61 @@ class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase, DwarfOpcod
# 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()