[Lldb-commits] [lldb] bbae0c1 - [lldb] [llgs] Support owning and detaching extra processes
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Sat Apr 24 02:26:38 PDT 2021
Author: Michał Górny
Date: 2021-04-24T11:08:33+02:00
New Revision: bbae0c1f7b4f60e9b8ac2be24e35bec79d9b7b01
URL: https://github.com/llvm/llvm-project/commit/bbae0c1f7b4f60e9b8ac2be24e35bec79d9b7b01
DIFF: https://github.com/llvm/llvm-project/commit/bbae0c1f7b4f60e9b8ac2be24e35bec79d9b7b01.diff
LOG: [lldb] [llgs] Support owning and detaching extra processes
Add a NativeDelegate API to pass new processes (forks) to LLGS,
and support detaching them via the 'D' packet. A 'D' packet without
a specific PID detaches all processes, otherwise it detaches either
the specified subprocess or the main process, depending on the passed
PID.
Differential Revision: https://reviews.llvm.org/D100191
Added:
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
Modified:
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/main.cpp
lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 2f0ac6d740d97..8054bcba8a720 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -222,6 +222,10 @@ class NativeProcessProtocol {
lldb::StateType state) = 0;
virtual void DidExec(NativeProcessProtocol *process) = 0;
+
+ virtual void
+ NewSubprocess(NativeProcessProtocol *parent_process,
+ std::unique_ptr<NativeProcessProtocol> child_process) = 0;
};
virtual Status GetLoadedModuleFileSpec(const char *module_path,
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 49f560122e514..15908c785ad63 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -249,14 +249,14 @@ Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
{
std::lock_guard<std::recursive_mutex> guard(m_debugged_process_mutex);
- assert(!m_debugged_process_up && "lldb-server creating debugged "
- "process but one already exists");
+ assert(m_debugged_processes.empty() && "lldb-server creating debugged "
+ "process but one already exists");
auto process_or =
m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
if (!process_or)
return Status(process_or.takeError());
- m_debugged_process_up = std::move(*process_or);
- m_continue_process = m_current_process = m_debugged_process_up.get();
+ m_continue_process = m_current_process = process_or->get();
+ m_debugged_processes[m_current_process->GetID()] = std::move(*process_or);
}
SetEnabledExtensions(*m_current_process);
@@ -273,10 +273,10 @@ Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
LLDB_LOG(log,
"pid = {0}: setting up stdout/stderr redirection via $O "
"gdb-remote commands",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
// Setup stdout/stderr mapping from inferior to $O
- auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
+ auto terminal_fd = m_current_process->GetTerminalFileDescriptor();
if (terminal_fd >= 0) {
LLDB_LOGF(log,
"ProcessGDBRemoteCommunicationServerLLGS::%s setting "
@@ -295,12 +295,12 @@ Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
LLDB_LOG(log,
"pid = {0} skipping stdout/stderr redirection via $O: inferior "
"will communicate over client-provided file descriptors",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
}
printf("Launched '%s' as process %" PRIu64 "...\n",
m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
return Status();
}
@@ -312,12 +312,11 @@ Status GDBRemoteCommunicationServerLLGS::AttachToProcess(lldb::pid_t pid) {
// Before we try to attach, make sure we aren't already monitoring something
// else.
- if (m_debugged_process_up &&
- m_debugged_process_up->GetID() != LLDB_INVALID_PROCESS_ID)
+ if (!m_debugged_processes.empty())
return Status("cannot attach to process %" PRIu64
" when another process with pid %" PRIu64
" is being debugged.",
- pid, m_debugged_process_up->GetID());
+ pid, m_current_process->GetID());
// Try to attach.
auto process_or = m_process_factory.Attach(pid, *this, m_mainloop);
@@ -327,12 +326,12 @@ Status GDBRemoteCommunicationServerLLGS::AttachToProcess(lldb::pid_t pid) {
status);
return status;
}
- m_debugged_process_up = std::move(*process_or);
- m_continue_process = m_current_process = m_debugged_process_up.get();
+ m_continue_process = m_current_process = process_or->get();
+ m_debugged_processes[m_current_process->GetID()] = std::move(*process_or);
SetEnabledExtensions(*m_current_process);
// Setup stdout/stderr mapping from inferior.
- auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
+ auto terminal_fd = m_current_process->GetTerminalFileDescriptor();
if (terminal_fd >= 0) {
LLDB_LOGF(log,
"ProcessGDBRemoteCommunicationServerLLGS::%s setting "
@@ -1060,6 +1059,15 @@ void GDBRemoteCommunicationServerLLGS::DidExec(NativeProcessProtocol *process) {
ClearProcessSpecificData();
}
+void GDBRemoteCommunicationServerLLGS::NewSubprocess(
+ NativeProcessProtocol *parent_process,
+ std::unique_ptr<NativeProcessProtocol> child_process) {
+ lldb::pid_t child_pid = child_process->GetID();
+ assert(child_pid != LLDB_INVALID_PROCESS_ID);
+ assert(m_debugged_processes.find(child_pid) == m_debugged_processes.end());
+ m_debugged_processes[child_pid] = std::move(child_process);
+}
+
void GDBRemoteCommunicationServerLLGS::DataAvailableCallback() {
Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_COMM));
@@ -3228,20 +3236,8 @@ GDBRemoteCommunicationServerLLGS::Handle_vAttachOrWait(
GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) {
- Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
-
StopSTDIOForwarding();
- // Fail if we don't have a current process.
- if (!m_current_process ||
- (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
- LLDB_LOGF(
- log,
- "GDBRemoteCommunicationServerLLGS::%s failed, no process available",
- __FUNCTION__);
- return SendErrorResponse(0x15);
- }
-
lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
// Consume the ';' after D.
@@ -3256,19 +3252,32 @@ GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) {
return SendIllFormedResponse(packet, "D failed to parse the process id");
}
- if (pid != LLDB_INVALID_PROCESS_ID && m_current_process->GetID() != pid) {
- return SendIllFormedResponse(packet, "Invalid pid");
- }
-
- const Status error = m_current_process->Detach();
- if (error.Fail()) {
- LLDB_LOGF(log,
- "GDBRemoteCommunicationServerLLGS::%s failed to detach from "
- "pid %" PRIu64 ": %s\n",
- __FUNCTION__, m_current_process->GetID(), error.AsCString());
- return SendErrorResponse(0x01);
+ // Detach forked children if their PID was specified *or* no PID was requested
+ // (i.e. detach-all packet).
+ llvm::Error detach_error = llvm::Error::success();
+ bool detached = false;
+ for (auto it = m_debugged_processes.begin();
+ it != m_debugged_processes.end();) {
+ if (pid == LLDB_INVALID_PROCESS_ID || pid == it->first) {
+ if (llvm::Error e = it->second->Detach().ToError())
+ detach_error = llvm::joinErrors(std::move(detach_error), std::move(e));
+ else {
+ if (it->second.get() == m_current_process)
+ m_current_process = nullptr;
+ if (it->second.get() == m_continue_process)
+ m_continue_process = nullptr;
+ it = m_debugged_processes.erase(it);
+ detached = true;
+ continue;
+ }
+ }
+ ++it;
}
+ if (detach_error)
+ return SendErrorResponse(std::move(detach_error));
+ if (!detached)
+ return SendErrorResponse(Status("PID %" PRIu64 " not traced", pid));
return SendOKResponse();
}
@@ -3616,8 +3625,8 @@ std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
if (bool(m_extensions_supported & Extension::vfork))
ret.push_back("vfork-events+");
- if (m_debugged_process_up)
- SetEnabledExtensions(*m_debugged_process_up);
+ for (auto &x : m_debugged_processes)
+ SetEnabledExtensions(*x.second);
return ret;
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index 32970ae1a025d..bfc4f79398936 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -78,6 +78,10 @@ class GDBRemoteCommunicationServerLLGS
void DidExec(NativeProcessProtocol *process) override;
+ void
+ NewSubprocess(NativeProcessProtocol *parent_process,
+ std::unique_ptr<NativeProcessProtocol> child_process) override;
+
Status InitializeConnection(std::unique_ptr<Connection> connection);
protected:
@@ -89,7 +93,8 @@ class GDBRemoteCommunicationServerLLGS
NativeProcessProtocol *m_current_process;
NativeProcessProtocol *m_continue_process;
std::recursive_mutex m_debugged_process_mutex;
- std::unique_ptr<NativeProcessProtocol> m_debugged_process_up;
+ std::unordered_map<lldb::pid_t, std::unique_ptr<NativeProcessProtocol>>
+ m_debugged_processes;
Communication m_stdio_communication;
MainLoop::ReadHandleUP m_stdio_handle_up;
diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp
index 6815ddf989840..dd8064ffc5900 100644
--- a/lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -378,9 +378,7 @@ StringExtractorGDBRemote::GetServerPacketType() const {
return eServerPacketType_C;
case 'D':
- if (packet_size == 1)
- return eServerPacketType_D;
- break;
+ return eServerPacketType_D;
case 'g':
return eServerPacketType_g;
diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
new file mode 100644
index 0000000000000..af39dbbb188b2
--- /dev/null
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -0,0 +1,59 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
+ mydir = TestBase.compute_mydir(__file__)
+
+ def fork_and_detach_test(self, variant):
+ self.build()
+ self.prep_debug_monitor_and_inferior(inferior_args=[variant])
+ self.add_qSupported_packets(["multiprocess+",
+ "{}-events+".format(variant)])
+ ret = self.expect_gdbremote_sequence()
+ self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
+ self.reset_test_sequence()
+
+ # continue and expect fork
+ fork_regex = "[$]T.*;{}:p([0-9a-f]*)[.]([0-9a-f]*).*".format(variant)
+ self.test_sequence.add_log_lines([
+ "read packet: $c#00",
+ {"direction": "send", "regex": fork_regex,
+ "capture": {1: "pid", 2: "tid"}},
+ ], True)
+ ret = self.expect_gdbremote_sequence()
+ pid = int(ret["pid"], 16)
+ self.reset_test_sequence()
+
+ # detach the forked child
+ self.test_sequence.add_log_lines([
+ "read packet: $D;{:x}#00".format(pid),
+ {"direction": "send", "regex": r"[$]OK#.*"},
+ ], True)
+ ret = self.expect_gdbremote_sequence()
+ self.reset_test_sequence()
+
+ @add_test_categories(["fork"])
+ def test_fork(self):
+ self.fork_and_detach_test("fork")
+
+ # resume the parent
+ self.test_sequence.add_log_lines([
+ "read packet: $c#00",
+ {"direction": "send", "regex": r"[$]W00#.*"},
+ ], True)
+ self.expect_gdbremote_sequence()
+
+ @add_test_categories(["fork"])
+ def test_vfork(self):
+ self.fork_and_detach_test("vfork")
+
+ # resume the parent
+ self.test_sequence.add_log_lines([
+ "read packet: $c#00",
+ {"direction": "send", "regex": r"[$]T.*vforkdone.*"},
+ "read packet: $c#00",
+ {"direction": "send", "regex": r"[$]W00#.*"},
+ ], True)
+ self.expect_gdbremote_sequence()
diff --git a/lldb/test/API/tools/lldb-server/main.cpp b/lldb/test/API/tools/lldb-server/main.cpp
index f1f576ab09bfc..a161a95708ee4 100644
--- a/lldb/test/API/tools/lldb-server/main.cpp
+++ b/lldb/test/API/tools/lldb-server/main.cpp
@@ -216,7 +216,13 @@ int main(int argc, char **argv) {
sig_result = signal(SIGSEGV, signal_handler);
if (sig_result == SIG_ERR) {
- fprintf(stderr, "failed to set SIGUSR1 handler: errno=%d\n", errno);
+ fprintf(stderr, "failed to set SIGSEGV handler: errno=%d\n", errno);
+ exit(1);
+ }
+
+ sig_result = signal(SIGCHLD, SIG_IGN);
+ if (sig_result == SIG_ERR) {
+ fprintf(stderr, "failed to set SIGCHLD handler: errno=%d\n", errno);
exit(1);
}
#endif
@@ -293,6 +299,14 @@ int main(int argc, char **argv) {
else if (arg == "swap_chars")
func_p = swap_chars;
func_p();
+#if !defined(_WIN32)
+ } else if (arg == "fork") {
+ if (fork() == 0)
+ _exit(0);
+ } else if (arg == "vfork") {
+ if (vfork() == 0)
+ _exit(0);
+#endif
} else if (consume_front(arg, "thread:new")) {
threads.push_back(std::thread(thread_func, nullptr));
} else if (consume_front(arg, "thread:print-ids")) {
diff --git a/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h b/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
index cdb16589dc27f..4efb18397d9d3 100644
--- a/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
+++ b/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -25,6 +25,15 @@ class MockDelegate : public NativeProcessProtocol::NativeDelegate {
MOCK_METHOD2(ProcessStateChanged,
void(NativeProcessProtocol *Process, StateType State));
MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+ MOCK_METHOD2(NewSubprocessImpl,
+ void(NativeProcessProtocol *parent_process,
+ std::unique_ptr<NativeProcessProtocol> &child_process));
+ // This is a hack to avoid MOCK_METHOD2 incompatibility with std::unique_ptr
+ // passed as value.
+ void NewSubprocess(NativeProcessProtocol *parent_process,
+ std::unique_ptr<NativeProcessProtocol> child_process) {
+ NewSubprocessImpl(parent_process, child_process);
+ }
};
// NB: This class doesn't use the override keyword to avoid
More information about the lldb-commits
mailing list