[Lldb-commits] [lldb] a40929d - [lldb] Fix cross-platform kills
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 9 06:31:32 PST 2021
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;
+}
More information about the lldb-commits
mailing list