Author: Pavel Labath Date: 2021-11-09T15:31:07+01:00 New Revision: a40929dcd295997736caf066758dd359216443c2
URL: https://github.com/llvm/llvm-project/commit/a40929dcd295997736caf066758dd359216443c2 DIFF: https://github.com/llvm/llvm-project/commit/a40929dcd295997736caf066758dd359216443c2.diff LOG: [lldb] Fix cross-platform kills This patch fixes an amusing bug where a Platform::Kill operation would happily terminate a proces on a completely different platform, as long as they have the same process ID. This was due to the fact that the implementation was iterating through all known (debugged) processes in order terminate them directly. This patch just deletes that logic, and makes everything go through the OS process termination APIs. While it would be possible to fix the logic to check for a platform match, it seemed to me that the implementation was being too smart for its own good -- accessing random Process objects without knowing anything about their state is risky at best. Going through the os ensures we avoid any races. I also "upgrade" the termination signal to a SIGKILL to ensure the process really dies after this operation. Differential Revision: https://reviews.llvm.org/D113184 Added: lldb/test/API/functionalities/gdb_remote_client/Makefile lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py lldb/test/API/functionalities/gdb_remote_client/sleep.cpp Modified: lldb/packages/Python/lldbsuite/test/lldbtest.py lldb/source/Target/Platform.cpp lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py Removed: ################################################################################ diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index 7477905a131d0..924e1fed4f38c 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -422,6 +422,9 @@ def terminate(self): def poll(self): return self._proc.poll() + def wait(self, timeout=None): + return self._proc.wait(timeout) + class _RemoteProcess(_BaseProcess): diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp index b7c691b058be5..be91a90158e8c 100644 --- a/lldb/source/Target/Platform.cpp +++ b/lldb/source/Target/Platform.cpp @@ -1044,25 +1044,11 @@ Status Platform::KillProcess(const lldb::pid_t pid) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM)); LLDB_LOGF(log, "Platform::%s, pid %" PRIu64, __FUNCTION__, pid); - // Try to find a process plugin to handle this Kill request. If we can't, - // fall back to the default OS implementation. - size_t num_debuggers = Debugger::GetNumDebuggers(); - for (size_t didx = 0; didx < num_debuggers; ++didx) { - DebuggerSP debugger = Debugger::GetDebuggerAtIndex(didx); - lldb_private::TargetList &targets = debugger->GetTargetList(); - for (int tidx = 0; tidx < targets.GetNumTargets(); ++tidx) { - ProcessSP process = targets.GetTargetAtIndex(tidx)->GetProcessSP(); - if (process->GetID() == pid) - return process->Destroy(true); - } - } - if (!IsHost()) { return Status( - "base lldb_private::Platform class can't kill remote processes unless " - "they are controlled by a process plugin"); + "base lldb_private::Platform class can't kill remote processes"); } - Host::Kill(pid, SIGTERM); + Host::Kill(pid, SIGKILL); return Status(); } diff --git a/lldb/test/API/functionalities/gdb_remote_client/Makefile b/lldb/test/API/functionalities/gdb_remote_client/Makefile new file mode 100644 index 0000000000000..22f1051530f87 --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/Makefile @@ -0,0 +1 @@ +include Makefile.rules diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py new file mode 100644 index 0000000000000..2ae9f8ac6f5f2 --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py @@ -0,0 +1,47 @@ +import lldb +import time +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * +from gdbclientutils import * + +class TestPlatformKill(GDBRemoteTestBase): + + @skipIfRemote + def test_kill_ diff erent_platform(self): + """Test connecting to a remote linux platform""" + + self.build(dictionary={"CXX_SOURCES":"sleep.cpp"}) + host_process = self.spawnSubprocess(self.getBuildArtifact()) + + # Create a fake remote process with the same PID as host_process + class MyResponder(MockGDBServerResponder): + def __init__(self): + MockGDBServerResponder.__init__(self) + self.got_kill = False + + def qC(self): + return "QC%x"%host_process.pid + + def k(self): + self.got_kill = True + return "X09" + + self.server.responder = MyResponder() + + error = lldb.SBError() + target = self.dbg.CreateTarget("", "x86_64-pc-linux", "remote-linux", + False, error) + self.assertSuccess(error) + process = self.connect(target) + self.assertEqual(process.GetProcessID(), host_process.pid) + + host_platform = lldb.SBPlatform("host") + self.assertSuccess(host_platform.Kill(host_process.pid)) + + # Host dies, remote process lives. + self.assertFalse(self.server.responder.got_kill) + self.assertIsNotNone(host_process.wait(timeout=10)) + + # Now kill the remote one as well + self.assertSuccess(process.Kill()) + self.assertTrue(self.server.responder.got_kill) diff --git a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py index 64330db86c09d..a1ab7ab052e28 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py +++ b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py @@ -203,6 +203,8 @@ def respond(self, packet): if packet.startswith("qRegisterInfo"): regnum = int(packet[len("qRegisterInfo"):], 16) return self.qRegisterInfo(regnum) + if packet == "k": + return self.k() return self.other(packet) @@ -331,6 +333,9 @@ def QEnvironmentHexEncoded(self, packet): def qRegisterInfo(self, num): return "" + def k(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/functionalities/gdb_remote_client/sleep.cpp b/lldb/test/API/functionalities/gdb_remote_client/sleep.cpp new file mode 100644 index 0000000000000..3118d8f6ce9b5 --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/sleep.cpp @@ -0,0 +1,6 @@ +#include <thread> + +int main() { + std::this_thread::sleep_for(std::chrono::minutes(1)); + return 0; +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits