diff --git a/lldb/packages/Python/lldbsuite/test/lldbgdbclient.py b/lldb/packages/Python/lldbsuite/test/lldbgdbclient.py --- a/lldb/packages/Python/lldbsuite/test/lldbgdbclient.py +++ b/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]: diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp --- a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp +++ b/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) { diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h @@ -153,7 +153,8 @@ GetPendingGdbServerList(std::vector &connection_urls); protected: - process_gdb_remote::GDBRemoteCommunicationClient m_gdb_client; + std::unique_ptr + 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 diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ b/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 PlatformRemoteGDBServer::GetRemoteOSBuildString() { - return m_gdb_client.GetOSBuildString(); + if (!m_gdb_client_up) + return llvm::None; + return m_gdb_client_up->GetOSBuildString(); } llvm::Optional 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()); + auto client_up = + std::make_unique(); + client_up->SetPacketTimeout( + process_gdb_remote::ProcessGDBRemote::GetPacketTimeout()); + client_up->SetConnection(std::make_unique()); if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) { repro::GDBRemoteProvider &provider = g->GetOrCreate(); - 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 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 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 &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 &connection_urls) { std::vector> 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(); diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/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'"""