Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h =================================================================== --- lldb/include/lldb/Host/common/NativeProcessProtocol.h +++ lldb/include/lldb/Host/common/NativeProcessProtocol.h @@ -220,6 +220,10 @@ lldb::StateType state) = 0; virtual void DidExec(NativeProcessProtocol *process) = 0; + + virtual void + NewSubprocess(NativeProcessProtocol *parent_process, + std::unique_ptr &child_process) = 0; }; virtual Status GetLoadedModuleFileSpec(const char *module_path, Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -78,6 +78,10 @@ void DidExec(NativeProcessProtocol *process) override; + void + NewSubprocess(NativeProcessProtocol *parent_process, + std::unique_ptr &child_process) override; + Status InitializeConnection(std::unique_ptr connection); protected: @@ -89,7 +93,8 @@ NativeProcessProtocol *m_current_process; NativeProcessProtocol *m_continue_process; std::recursive_mutex m_debugged_process_mutex; - std::unique_ptr m_debugged_process_up; + std::unordered_map> + m_debugged_processes; Communication m_stdio_communication; MainLoop::ReadHandleUP m_stdio_handle_up; Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -249,14 +249,14 @@ { std::lock_guard 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); } // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as @@ -271,10 +271,10 @@ 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 " @@ -293,12 +293,12 @@ 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(); } @@ -310,12 +310,11 @@ // 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); @@ -325,11 +324,11 @@ 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); // 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 " @@ -1035,6 +1034,15 @@ ClearProcessSpecificData(); } +void GDBRemoteCommunicationServerLLGS::NewSubprocess( + NativeProcessProtocol *parent_process, + std::unique_ptr &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)); @@ -3203,20 +3211,8 @@ 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. @@ -3231,19 +3227,32 @@ 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(); } Index: lldb/source/Utility/StringExtractorGDBRemote.cpp =================================================================== --- lldb/source/Utility/StringExtractorGDBRemote.cpp +++ lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -378,9 +378,7 @@ return eServerPacketType_C; case 'D': - if (packet_size == 1) - return eServerPacketType_D; - break; + return eServerPacketType_D; case 'g': return eServerPacketType_g; Index: lldb/unittests/Process/gdb-remote/CMakeLists.txt =================================================================== --- lldb/unittests/Process/gdb-remote/CMakeLists.txt +++ lldb/unittests/Process/gdb-remote/CMakeLists.txt @@ -1,6 +1,7 @@ add_lldb_unittest(ProcessGdbRemoteTests GDBRemoteClientBaseTest.cpp GDBRemoteCommunicationClientTest.cpp + GDBRemoteCommunicationServerLLGSTest.cpp GDBRemoteCommunicationServerTest.cpp GDBRemoteCommunicationTest.cpp GDBRemoteTestUtils.cpp Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp @@ -0,0 +1,139 @@ +//===-- GDBRemoteCommunicationServerLLGSTest.cpp --------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "lldb/Host/common/NativeProcessProtocol.h" + +#include "GDBRemoteTestUtils.h" +#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h" +#include "TestingSupport/Host/NativeProcessTestUtils.h" + +namespace lldb_private { +namespace process_gdb_remote { + +class MockFactory : public NativeProcessProtocol::Factory { +public: + llvm::Expected> + Launch(ProcessLaunchInfo &launch_info, + NativeProcessProtocol::NativeDelegate &native_delegate, + MainLoop &mainloop) const override { + return llvm::createStringError(llvm::inconvertibleErrorCode(), "Not supported"); + } + + virtual llvm::Expected> + Attach(lldb::pid_t pid, + NativeProcessProtocol::NativeDelegate &native_delegate, + MainLoop &mainloop) const override { + NativeProcessProtocol *process = new MockProcess(native_delegate, ArchSpec("x86_64-pc-linux"), pid); + return std::unique_ptr(process); + } +}; + +class TestServer : public GDBRemoteCommunicationServerLLGS { + MainLoop m_main_loop; + MockFactory m_process_factory; + +public: + TestServer() : GDBRemoteCommunicationServerLLGS(m_main_loop, m_process_factory) {} + + using GDBRemoteCommunicationServerLLGS::m_debugged_processes; + using GDBRemoteCommunicationServerLLGS::m_current_process; + using GDBRemoteCommunicationServerLLGS::m_continue_process; + + using GDBRemoteCommunicationServerLLGS::Handle_D; +}; + +TEST(GDBRemoteCommunicationServerLLGSTest, NewSubprocess) { + TestServer server; + Status error; + + // Initial state: no process. + EXPECT_EQ(server.m_debugged_processes.size(), 0u); + EXPECT_EQ(server.m_current_process, nullptr); + EXPECT_EQ(server.m_continue_process, nullptr); + + // Attach the main process. + error = server.AttachToProcess(1234); + ASSERT_TRUE(error.Success()); + EXPECT_EQ(server.m_debugged_processes.size(), 1u); + auto it = server.m_debugged_processes.find(1234); + ASSERT_NE(it, server.m_debugged_processes.end()); + NativeProcessProtocol *parent = it->second.get(); + EXPECT_EQ(parent->GetID(), 1234u); + EXPECT_EQ(server.m_current_process, parent); + EXPECT_EQ(server.m_continue_process, parent); + + // Simulate a fork. + std::unique_ptr child_up{new MockProcess(server, ArchSpec("x86_64-pc-linux"), 1235)}; + server.NewSubprocess(parent, child_up); + EXPECT_EQ(child_up, nullptr); + EXPECT_EQ(server.m_debugged_processes.size(), 2u); + it = server.m_debugged_processes.find(1235); + ASSERT_NE(it, server.m_debugged_processes.end()); + NativeProcessProtocol *child = it->second.get(); + EXPECT_EQ(child->GetID(), 1235u); + EXPECT_EQ(server.m_current_process, parent); + EXPECT_EQ(server.m_continue_process, parent); + + // Detach the child. + StringExtractorGDBRemote child_detach{"D;04D3"}; + EXPECT_CALL(*static_cast *>(child), Detach()); + server.Handle_D(child_detach); + EXPECT_EQ(server.m_debugged_processes.size(), 1u); + EXPECT_EQ(server.m_debugged_processes.find(1235), server.m_debugged_processes.end()); + EXPECT_EQ(server.m_current_process, parent); + EXPECT_EQ(server.m_continue_process, parent); + + // Simulate another fork. + child_up.reset(new MockProcess(server, ArchSpec("x86_64-pc-linux"), 1236)); + server.NewSubprocess(parent, child_up); + EXPECT_EQ(child_up, nullptr); + EXPECT_EQ(server.m_debugged_processes.size(), 2u); + it = server.m_debugged_processes.find(1236); + ASSERT_NE(it, server.m_debugged_processes.end()); + child = it->second.get(); + EXPECT_EQ(child->GetID(), 1236u); + EXPECT_EQ(server.m_current_process, parent); + EXPECT_EQ(server.m_continue_process, parent); + + // Detach the parent. + StringExtractorGDBRemote parent_detach{"D;04D2"}; + EXPECT_CALL(*static_cast *>(parent), Detach()); + server.Handle_D(parent_detach); + EXPECT_EQ(server.m_debugged_processes.size(), 1u); + EXPECT_EQ(server.m_debugged_processes.find(1234), server.m_debugged_processes.end()); + EXPECT_EQ(server.m_current_process, nullptr); + EXPECT_EQ(server.m_continue_process, nullptr); + ASSERT_NE(server.m_debugged_processes.find(1236), server.m_debugged_processes.end()); + + // Simulate one more fork. + parent = child; + child_up.reset(new MockProcess(server, ArchSpec("x86_64-pc-linux"), 1237)); + server.NewSubprocess(parent, child_up); + EXPECT_EQ(child_up, nullptr); + EXPECT_EQ(server.m_debugged_processes.size(), 2u); + it = server.m_debugged_processes.find(1237); + ASSERT_NE(it, server.m_debugged_processes.end()); + child = it->second.get(); + EXPECT_EQ(child->GetID(), 1237u); + EXPECT_EQ(server.m_current_process, nullptr); + EXPECT_EQ(server.m_continue_process, nullptr); + + // Detach all. + StringExtractorGDBRemote all_detach{"D"}; + EXPECT_CALL(*static_cast *>(parent), Detach()); + EXPECT_CALL(*static_cast *>(child), Detach()); + server.Handle_D(all_detach); + EXPECT_EQ(server.m_debugged_processes.size(), 0u); + EXPECT_EQ(server.m_current_process, nullptr); + EXPECT_EQ(server.m_continue_process, nullptr); +} + +} // namespace process_gdb_remote +} // namespace lldb_private Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h =================================================================== --- lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h +++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h @@ -25,6 +25,9 @@ MOCK_METHOD2(ProcessStateChanged, void(NativeProcessProtocol *Process, StateType State)); MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process)); + MOCK_METHOD2(NewSubprocess, + void(NativeProcessProtocol *parent_process, + std::unique_ptr &child_process)); }; // NB: This class doesn't use the override keyword to avoid