This revision was automatically updated to reflect the committed changes.
Closed by commit rG8ccfcab34ffb: [lldb/platform-gdb] Clear cached protocol
state upon disconnection (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D116539?vs=397067&id=398636#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116539/new/
https://reviews.llvm.org/D116539
Files:
lldb/packages/Python/lldbsuite/test/lldbgdbclient.py
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===================================================================
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -107,6 +107,25 @@
"vFile:close:5",
])
+ self.runCmd("platform disconnect")
+
+ # For a new onnection, we should attempt vFile:size once again.
+ server2 = MockGDBServer(self.server_socket_class())
+ server2.responder = Responder()
+ server2.start()
+ self.addTearDownHook(lambda:server2.stop())
+ self.runCmd("platform connect " + server2.get_connect_url())
+ self.match("platform get-size /other/file.txt",
+ [r"File size of /other/file\.txt \(remote\): 66051"])
+
+ self.assertPacketLogContains([
+ "vFile:size:2f6f746865722f66696c652e747874",
+ "vFile:open:2f6f746865722f66696c652e747874,00000000,00000000",
+ "vFile:fstat:5",
+ "vFile:close:5",
+ ],
+ log=server2.responder.packetLog)
+
@skipIfWindows
def test_file_permissions(self):
"""Test 'platform get-permissions'"""
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
===================================================================
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
@@ -153,7 +153,8 @@
GetPendingGdbServerList(std::vector<std::string> &connection_urls);
protected:
- process_gdb_remote::GDBRemoteCommunicationClient m_gdb_client;
+ std::unique_ptr<process_gdb_remote::GDBRemoteCommunicationClient>
+ m_gdb_client_up;
std::string m_platform_description; // After we connect we can get a more
// complete description of what we are
// connected to
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===================================================================
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -96,7 +96,8 @@
const auto module_path = module_file_spec.GetPath(false);
- if (!m_gdb_client.GetModuleInfo(module_file_spec, arch, module_spec)) {
+ if (!m_gdb_client_up ||
+ !m_gdb_client_up->GetModuleInfo(module_file_spec, arch, module_spec)) {
LLDB_LOGF(
log,
"PlatformRemoteGDBServer::%s - failed to get module info for %s:%s",
@@ -127,11 +128,7 @@
/// Default Constructor
PlatformRemoteGDBServer::PlatformRemoteGDBServer()
- : Platform(false), // This is a remote platform
- m_gdb_client() {
- m_gdb_client.SetPacketTimeout(
- process_gdb_remote::ProcessGDBRemote::GetPacketTimeout());
-}
+ : Platform(/*is_host=*/false) {}
/// Destructor.
///
@@ -147,29 +144,36 @@
}
bool PlatformRemoteGDBServer::GetRemoteOSVersion() {
- m_os_version = m_gdb_client.GetOSVersion();
+ if (m_gdb_client_up)
+ m_os_version = m_gdb_client_up->GetOSVersion();
return !m_os_version.empty();
}
llvm::Optional<std::string> PlatformRemoteGDBServer::GetRemoteOSBuildString() {
- return m_gdb_client.GetOSBuildString();
+ if (!m_gdb_client_up)
+ return llvm::None;
+ return m_gdb_client_up->GetOSBuildString();
}
llvm::Optional<std::string>
PlatformRemoteGDBServer::GetRemoteOSKernelDescription() {
- return m_gdb_client.GetOSKernelDescription();
+ if (!m_gdb_client_up)
+ return llvm::None;
+ return m_gdb_client_up->GetOSKernelDescription();
}
// Remote Platform subclasses need to override this function
ArchSpec PlatformRemoteGDBServer::GetRemoteSystemArchitecture() {
- return m_gdb_client.GetSystemArchitecture();
+ if (!m_gdb_client_up)
+ return ArchSpec();
+ return m_gdb_client_up->GetSystemArchitecture();
}
FileSpec PlatformRemoteGDBServer::GetRemoteWorkingDirectory() {
if (IsConnected()) {
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM);
FileSpec working_dir;
- if (m_gdb_client.GetWorkingDir(working_dir) && log)
+ if (m_gdb_client_up->GetWorkingDir(working_dir) && log)
LLDB_LOGF(log,
"PlatformRemoteGDBServer::GetRemoteWorkingDirectory() -> '%s'",
working_dir.GetCString());
@@ -187,13 +191,17 @@
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM);
LLDB_LOGF(log, "PlatformRemoteGDBServer::SetRemoteWorkingDirectory('%s')",
working_dir.GetCString());
- return m_gdb_client.SetWorkingDir(working_dir) == 0;
+ return m_gdb_client_up->SetWorkingDir(working_dir) == 0;
} else
return Platform::SetRemoteWorkingDirectory(working_dir);
}
bool PlatformRemoteGDBServer::IsConnected() const {
- return m_gdb_client.IsConnected();
+ if (m_gdb_client_up) {
+ assert(m_gdb_client_up->IsConnected());
+ return true;
+ }
+ return false;
}
Status PlatformRemoteGDBServer::ConnectRemote(Args &args) {
@@ -224,26 +232,31 @@
m_platform_scheme = parsed_url->scheme.str();
m_platform_hostname = parsed_url->hostname.str();
- m_gdb_client.SetConnection(std::make_unique<ConnectionFileDescriptor>());
+ auto client_up =
+ std::make_unique<process_gdb_remote::GDBRemoteCommunicationClient>();
+ client_up->SetPacketTimeout(
+ process_gdb_remote::ProcessGDBRemote::GetPacketTimeout());
+ client_up->SetConnection(std::make_unique<ConnectionFileDescriptor>());
if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) {
repro::GDBRemoteProvider &provider =
g->GetOrCreate<repro::GDBRemoteProvider>();
- m_gdb_client.SetPacketRecorder(provider.GetNewPacketRecorder());
+ client_up->SetPacketRecorder(provider.GetNewPacketRecorder());
}
- m_gdb_client.Connect(url, &error);
+ client_up->Connect(url, &error);
if (error.Fail())
return error;
- if (m_gdb_client.HandshakeWithServer(&error)) {
- m_gdb_client.GetHostInfo();
+ if (client_up->HandshakeWithServer(&error)) {
+ m_gdb_client_up = std::move(client_up);
+ m_gdb_client_up->GetHostInfo();
// If a working directory was set prior to connecting, send it down
// now.
if (m_working_dir)
- m_gdb_client.SetWorkingDir(m_working_dir);
+ m_gdb_client_up->SetWorkingDir(m_working_dir);
m_supported_architectures.clear();
- ArchSpec remote_arch = m_gdb_client.GetSystemArchitecture();
+ ArchSpec remote_arch = m_gdb_client_up->GetSystemArchitecture();
if (remote_arch) {
m_supported_architectures.push_back(remote_arch);
if (remote_arch.GetTriple().isArch64Bit())
@@ -251,7 +264,7 @@
ArchSpec(remote_arch.GetTriple().get32BitArchVariant()));
}
} else {
- m_gdb_client.Disconnect();
+ client_up->Disconnect();
if (error.Success())
error.SetErrorString("handshake failed");
}
@@ -260,13 +273,14 @@
Status PlatformRemoteGDBServer::DisconnectRemote() {
Status error;
- m_gdb_client.Disconnect(&error);
+ m_gdb_client_up.reset();
m_remote_signals_sp.reset();
return error;
}
const char *PlatformRemoteGDBServer::GetHostname() {
- m_gdb_client.GetHostname(m_name);
+ if (m_gdb_client_up)
+ m_gdb_client_up->GetHostname(m_name);
if (m_name.empty())
return nullptr;
return m_name.c_str();
@@ -275,7 +289,7 @@
llvm::Optional<std::string>
PlatformRemoteGDBServer::DoGetUserName(UserIDResolver::id_t uid) {
std::string name;
- if (m_gdb_client.GetUserName(uid, name))
+ if (m_gdb_client_up && m_gdb_client_up->GetUserName(uid, name))
return std::move(name);
return llvm::None;
}
@@ -283,7 +297,7 @@
llvm::Optional<std::string>
PlatformRemoteGDBServer::DoGetGroupName(UserIDResolver::id_t gid) {
std::string name;
- if (m_gdb_client.GetGroupName(gid, name))
+ if (m_gdb_client_up && m_gdb_client_up->GetGroupName(gid, name))
return std::move(name);
return llvm::None;
}
@@ -291,12 +305,16 @@
uint32_t PlatformRemoteGDBServer::FindProcesses(
const ProcessInstanceInfoMatch &match_info,
ProcessInstanceInfoList &process_infos) {
- return m_gdb_client.FindProcesses(match_info, process_infos);
+ if (m_gdb_client_up)
+ return m_gdb_client_up->FindProcesses(match_info, process_infos);
+ return 0;
}
bool PlatformRemoteGDBServer::GetProcessInfo(
lldb::pid_t pid, ProcessInstanceInfo &process_info) {
- return m_gdb_client.GetProcessInfo(pid, process_info);
+ if (m_gdb_client_up)
+ return m_gdb_client_up->GetProcessInfo(pid, process_info);
+ return false;
}
Status PlatformRemoteGDBServer::LaunchProcess(ProcessLaunchInfo &launch_info) {
@@ -305,6 +323,8 @@
LLDB_LOGF(log, "PlatformRemoteGDBServer::%s() called", __FUNCTION__);
+ if (!IsConnected())
+ return Status("Not connected.");
auto num_file_actions = launch_info.GetNumFileActions();
for (decltype(num_file_actions) i = 0; i < num_file_actions; ++i) {
const auto file_action = launch_info.GetFileActionAtIndex(i);
@@ -312,34 +332,34 @@
continue;
switch (file_action->GetFD()) {
case STDIN_FILENO:
- m_gdb_client.SetSTDIN(file_action->GetFileSpec());
+ m_gdb_client_up->SetSTDIN(file_action->GetFileSpec());
break;
case STDOUT_FILENO:
- m_gdb_client.SetSTDOUT(file_action->GetFileSpec());
+ m_gdb_client_up->SetSTDOUT(file_action->GetFileSpec());
break;
case STDERR_FILENO:
- m_gdb_client.SetSTDERR(file_action->GetFileSpec());
+ m_gdb_client_up->SetSTDERR(file_action->GetFileSpec());
break;
}
}
- m_gdb_client.SetDisableASLR(
+ m_gdb_client_up->SetDisableASLR(
launch_info.GetFlags().Test(eLaunchFlagDisableASLR));
- m_gdb_client.SetDetachOnError(
+ m_gdb_client_up->SetDetachOnError(
launch_info.GetFlags().Test(eLaunchFlagDetachOnError));
FileSpec working_dir = launch_info.GetWorkingDirectory();
if (working_dir) {
- m_gdb_client.SetWorkingDir(working_dir);
+ m_gdb_client_up->SetWorkingDir(working_dir);
}
// Send the environment and the program + arguments after we connect
- m_gdb_client.SendEnvironment(launch_info.GetEnvironment());
+ m_gdb_client_up->SendEnvironment(launch_info.GetEnvironment());
ArchSpec arch_spec = launch_info.GetArchitecture();
const char *arch_triple = arch_spec.GetTriple().str().c_str();
- m_gdb_client.SendLaunchArchPacket(arch_triple);
+ m_gdb_client_up->SendLaunchArchPacket(arch_triple);
LLDB_LOGF(
log,
"PlatformRemoteGDBServer::%s() set launch architecture triple to '%s'",
@@ -349,14 +369,14 @@
{
// Scope for the scoped timeout object
process_gdb_remote::GDBRemoteCommunication::ScopedTimeout timeout(
- m_gdb_client, std::chrono::seconds(5));
- arg_packet_err = m_gdb_client.SendArgumentsPacket(launch_info);
+ *m_gdb_client_up, std::chrono::seconds(5));
+ arg_packet_err = m_gdb_client_up->SendArgumentsPacket(launch_info);
}
if (arg_packet_err == 0) {
std::string error_str;
- if (m_gdb_client.GetLaunchSuccess(error_str)) {
- const auto pid = m_gdb_client.GetCurrentProcessID(false);
+ if (m_gdb_client_up->GetLaunchSuccess(error_str)) {
+ const auto pid = m_gdb_client_up->GetCurrentProcessID(false);
if (pid != LLDB_INVALID_PROCESS_ID) {
launch_info.SetProcessID(pid);
LLDB_LOGF(log,
@@ -428,6 +448,8 @@
bool PlatformRemoteGDBServer::LaunchGDBServer(lldb::pid_t &pid,
std::string &connect_url) {
+ assert(IsConnected());
+
ArchSpec remote_arch = GetRemoteSystemArchitecture();
llvm::Triple &remote_triple = remote_arch.GetTriple();
@@ -440,11 +462,11 @@
// localhost, so we will need the remote debugserver to accept connections
// only from localhost, no matter what our current hostname is
launch_result =
- m_gdb_client.LaunchGDBServer("127.0.0.1", pid, port, socket_name);
+ m_gdb_client_up->LaunchGDBServer("127.0.0.1", pid, port, socket_name);
} else {
// All other hosts should use their actual hostname
launch_result =
- m_gdb_client.LaunchGDBServer(nullptr, pid, port, socket_name);
+ m_gdb_client_up->LaunchGDBServer(nullptr, pid, port, socket_name);
}
if (!launch_result)
@@ -457,7 +479,8 @@
}
bool PlatformRemoteGDBServer::KillSpawnedProcess(lldb::pid_t pid) {
- return m_gdb_client.KillSpawnedProcess(pid);
+ assert(IsConnected());
+ return m_gdb_client_up->KillSpawnedProcess(pid);
}
lldb::ProcessSP PlatformRemoteGDBServer::Attach(
@@ -513,7 +536,9 @@
Status PlatformRemoteGDBServer::MakeDirectory(const FileSpec &file_spec,
uint32_t mode) {
- Status error = m_gdb_client.MakeDirectory(file_spec, mode);
+ if (!IsConnected())
+ return Status("Not connected.");
+ Status error = m_gdb_client_up->MakeDirectory(file_spec, mode);
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM);
LLDB_LOGF(log,
"PlatformRemoteGDBServer::MakeDirectory(path='%s', mode=%o) "
@@ -524,7 +549,10 @@
Status PlatformRemoteGDBServer::GetFilePermissions(const FileSpec &file_spec,
uint32_t &file_permissions) {
- Status error = m_gdb_client.GetFilePermissions(file_spec, file_permissions);
+ if (!IsConnected())
+ return Status("Not connected.");
+ Status error =
+ m_gdb_client_up->GetFilePermissions(file_spec, file_permissions);
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM);
LLDB_LOGF(log,
"PlatformRemoteGDBServer::GetFilePermissions(path='%s', "
@@ -536,7 +564,10 @@
Status PlatformRemoteGDBServer::SetFilePermissions(const FileSpec &file_spec,
uint32_t file_permissions) {
- Status error = m_gdb_client.SetFilePermissions(file_spec, file_permissions);
+ if (!IsConnected())
+ return Status("Not connected.");
+ Status error =
+ m_gdb_client_up->SetFilePermissions(file_spec, file_permissions);
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM);
LLDB_LOGF(log,
"PlatformRemoteGDBServer::SetFilePermissions(path='%s', "
@@ -550,33 +581,47 @@
File::OpenOptions flags,
uint32_t mode,
Status &error) {
- return m_gdb_client.OpenFile(file_spec, flags, mode, error);
+ if (IsConnected())
+ return m_gdb_client_up->OpenFile(file_spec, flags, mode, error);
+ return LLDB_INVALID_UID;
}
bool PlatformRemoteGDBServer::CloseFile(lldb::user_id_t fd, Status &error) {
- return m_gdb_client.CloseFile(fd, error);
+ if (IsConnected())
+ return m_gdb_client_up->CloseFile(fd, error);
+ error = Status("Not connected.");
+ return false;
}
lldb::user_id_t
PlatformRemoteGDBServer::GetFileSize(const FileSpec &file_spec) {
- return m_gdb_client.GetFileSize(file_spec);
+ if (IsConnected())
+ return m_gdb_client_up->GetFileSize(file_spec);
+ return LLDB_INVALID_UID;
}
void PlatformRemoteGDBServer::AutoCompleteDiskFileOrDirectory(
CompletionRequest &request, bool only_dir) {
- m_gdb_client.AutoCompleteDiskFileOrDirectory(request, only_dir);
+ if (IsConnected())
+ m_gdb_client_up->AutoCompleteDiskFileOrDirectory(request, only_dir);
}
uint64_t PlatformRemoteGDBServer::ReadFile(lldb::user_id_t fd, uint64_t offset,
void *dst, uint64_t dst_len,
Status &error) {
- return m_gdb_client.ReadFile(fd, offset, dst, dst_len, error);
+ if (IsConnected())
+ return m_gdb_client_up->ReadFile(fd, offset, dst, dst_len, error);
+ error = Status("Not connected.");
+ return 0;
}
uint64_t PlatformRemoteGDBServer::WriteFile(lldb::user_id_t fd, uint64_t offset,
const void *src, uint64_t src_len,
Status &error) {
- return m_gdb_client.WriteFile(fd, offset, src, src_len, error);
+ if (IsConnected())
+ return m_gdb_client_up->WriteFile(fd, offset, src, src_len, error);
+ error = Status("Not connected.");
+ return 0;
}
Status PlatformRemoteGDBServer::PutFile(const FileSpec &source,
@@ -589,7 +634,9 @@
const FileSpec &src, // The name of the link is in src
const FileSpec &dst) // The symlink points to dst
{
- Status error = m_gdb_client.CreateSymlink(src, dst);
+ if (!IsConnected())
+ return Status("Not connected.");
+ Status error = m_gdb_client_up->CreateSymlink(src, dst);
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM);
LLDB_LOGF(log,
"PlatformRemoteGDBServer::CreateSymlink(src='%s', dst='%s') "
@@ -600,7 +647,9 @@
}
Status PlatformRemoteGDBServer::Unlink(const FileSpec &file_spec) {
- Status error = m_gdb_client.Unlink(file_spec);
+ if (!IsConnected())
+ return Status("Not connected.");
+ Status error = m_gdb_client_up->Unlink(file_spec);
Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM);
LLDB_LOGF(log, "PlatformRemoteGDBServer::Unlink(path='%s') error = %u (%s)",
file_spec.GetCString(), error.GetError(), error.AsCString());
@@ -608,7 +657,9 @@
}
bool PlatformRemoteGDBServer::GetFileExists(const FileSpec &file_spec) {
- return m_gdb_client.GetFileExists(file_spec);
+ if (IsConnected())
+ return m_gdb_client_up->GetFileExists(file_spec);
+ return false;
}
Status PlatformRemoteGDBServer::RunShellCommand(
@@ -621,8 +672,10 @@
std::string
*command_output, // Pass NULL if you don't want the command output
const Timeout<std::micro> &timeout) {
- return m_gdb_client.RunShellCommand(command, working_dir, status_ptr,
- signo_ptr, command_output, timeout);
+ if (!IsConnected())
+ return Status("Not connected.");
+ return m_gdb_client_up->RunShellCommand(command, working_dir, status_ptr,
+ signo_ptr, command_output, timeout);
}
void PlatformRemoteGDBServer::CalculateTrapHandlerSymbolNames() {
@@ -642,7 +695,7 @@
StringExtractorGDBRemote response;
auto result =
- m_gdb_client.SendPacketAndWaitForResponse("jSignalsInfo", response);
+ m_gdb_client_up->SendPacketAndWaitForResponse("jSignalsInfo", response);
if (result != decltype(result)::Success ||
response.GetResponseType() != response.eResponse)
@@ -754,7 +807,9 @@
size_t PlatformRemoteGDBServer::GetPendingGdbServerList(
std::vector<std::string> &connection_urls) {
std::vector<std::pair<uint16_t, std::string>> remote_servers;
- m_gdb_client.QueryGDBServer(remote_servers);
+ if (!IsConnected())
+ return 0;
+ m_gdb_client_up->QueryGDBServer(remote_servers);
for (const auto &gdbserver : remote_servers) {
const char *socket_name_cstr =
gdbserver.second.empty() ? nullptr : gdbserver.second.c_str();
Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
===================================================================
--- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -82,9 +82,11 @@
bool PlatformAndroidRemoteGDBServer::LaunchGDBServer(lldb::pid_t &pid,
std::string &connect_url) {
+ assert(IsConnected());
uint16_t remote_port = 0;
std::string socket_name;
- if (!m_gdb_client.LaunchGDBServer("127.0.0.1", pid, remote_port, socket_name))
+ if (!m_gdb_client_up->LaunchGDBServer("127.0.0.1", pid, remote_port,
+ socket_name))
return false;
Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
@@ -98,8 +100,9 @@
}
bool PlatformAndroidRemoteGDBServer::KillSpawnedProcess(lldb::pid_t pid) {
+ assert(IsConnected());
DeleteForwardPort(pid);
- return m_gdb_client.KillSpawnedProcess(pid);
+ return m_gdb_client_up->KillSpawnedProcess(pid);
}
Status PlatformAndroidRemoteGDBServer::ConnectRemote(Args &args) {
Index: lldb/packages/Python/lldbsuite/test/lldbgdbclient.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbgdbclient.py
+++ lldb/packages/Python/lldbsuite/test/lldbgdbclient.py
@@ -58,7 +58,7 @@
self.assertTrue(process, PROCESS_IS_VALID)
return process
- def assertPacketLogContains(self, packets):
+ def assertPacketLogContains(self, packets, log=None):
"""
Assert that the mock server's packet log contains the given packets.
@@ -69,9 +69,10 @@
The check does not require that the packets be consecutive, but does
require that they are ordered in the log as they ordered in the arg.
"""
+ if log is None:
+ log = self.server.responder.packetLog
i = 0
j = 0
- log = self.server.responder.packetLog
while i < len(packets) and j < len(log):
if log[j] == packets[i]:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
