Add a NativeProcessProtocol Factory class
Summary:
This replaces the static functions used for creating
NativeProcessProtocol instances with a factory pattern, and modernizes
the interface of the new class in the process -- I use llvm::Expected
instead of the Status+value combo. I also move some of the common code
(like the Delegate registration into the base class). The new
arrangement has multiple benefits:
- it removes the NativeProcess*** dependency from Process/gdb-remote
(which for example means that liblldb no longer pulls in this code).
- it enables unit testing of the GDBRemoteCommunicationServerLLGS class
(by providing a mock Native Process).
- serves as another example on how to use the llvm::Expected class (I
couldn't get rid of the Initialize-type functions completely here
because of the use of shared_from_this, but that's the next thing on
my list here)
Tests still pass on Linux and I've made sure NetBSD compiles after this.
Reviewers: zturner, eugene, krytarowski
Subscribers: srhines, lldb-commits, mgorny
Differential Revision: https://reviews.llvm.org/D33778
llvm-svn: 307390
diff --git a/lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt b/lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
index 3d008f4..5e51fee 100644
--- a/lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
@@ -7,14 +7,6 @@
lldbPluginPlatformMacOSX
)
-if(CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
- list(APPEND LLDB_PLUGINS lldbPluginProcessLinux)
-endif()
-
-if(CMAKE_SYSTEM_NAME MATCHES "NetBSD")
- list(APPEND LLDB_PLUGINS lldbPluginProcessNetBSD)
-endif()
-
add_lldb_library(lldbPluginProcessGDBRemote PLUGIN
GDBRemoteClientBase.cpp
GDBRemoteCommunication.cpp
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index de2400c..f53db50 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1046,14 +1046,9 @@
if (success) {
m_process_launch_error = LaunchProcess();
- if (m_process_launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID) {
+ if (m_process_launch_error.Success())
return SendOKResponse();
- } else {
- Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
- if (log)
- log->Printf("LLGSPacketHandler::%s failed to launch exe: %s",
- __FUNCTION__, m_process_launch_error.AsCString());
- }
+ LLDB_LOG(log, "failed to launch exe: {0}", m_process_launch_error);
}
return SendErrorResponse(8);
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 7523260..c2aa261 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -73,15 +73,11 @@
// GDBRemoteCommunicationServerLLGS constructor
//----------------------------------------------------------------------
GDBRemoteCommunicationServerLLGS::GDBRemoteCommunicationServerLLGS(
- MainLoop &mainloop)
+ MainLoop &mainloop, const NativeProcessProtocol::Factory &process_factory)
: GDBRemoteCommunicationServerCommon("gdb-remote.server",
"gdb-remote.server.rx_packet"),
- m_mainloop(mainloop), m_current_tid(LLDB_INVALID_THREAD_ID),
- m_continue_tid(LLDB_INVALID_THREAD_ID), m_debugged_process_mutex(),
- m_debugged_process_sp(), m_stdio_communication("process.stdio"),
- m_inferior_prev_state(StateType::eStateInvalid),
- m_saved_registers_map(), m_next_saved_registers_id(1),
- m_handshake_completed(false) {
+ m_mainloop(mainloop), m_process_factory(process_factory),
+ m_stdio_communication("process.stdio") {
RegisterPacketHandlers();
}
@@ -241,19 +237,20 @@
const bool default_to_use_pty = true;
m_process_launch_info.FinalizeFileActions(nullptr, default_to_use_pty);
- Status error;
{
std::lock_guard<std::recursive_mutex> guard(m_debugged_process_mutex);
assert(!m_debugged_process_sp && "lldb-server creating debugged "
"process but one already exists");
- error = NativeProcessProtocol::Launch(m_process_launch_info, *this,
- m_mainloop, m_debugged_process_sp);
- }
-
- if (!error.Success()) {
- fprintf(stderr, "%s: failed to launch executable %s", __FUNCTION__,
- m_process_launch_info.GetArguments().GetArgumentAtIndex(0));
- return error;
+ auto process_or =
+ m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
+ if (!process_or) {
+ Status status(process_or.takeError());
+ llvm::errs() << llvm::formatv(
+ "failed to launch executable `{0}`: {1}",
+ m_process_launch_info.GetArguments().GetArgumentAtIndex(0), status);
+ return status;
+ }
+ m_debugged_process_sp = *process_or;
}
// Handle mirroring of inferior stdout/stderr over the gdb-remote protocol
@@ -279,9 +276,9 @@
log->Printf("ProcessGDBRemoteCommunicationServerLLGS::%s setting "
"inferior STDIO fd to %d",
__FUNCTION__, terminal_fd);
- error = SetSTDIOFileDescriptor(terminal_fd);
- if (error.Fail())
- return error;
+ Status status = SetSTDIOFileDescriptor(terminal_fd);
+ if (status.Fail())
+ return status;
} else {
if (log)
log->Printf("ProcessGDBRemoteCommunicationServerLLGS::%s ignoring "
@@ -298,14 +295,12 @@
printf("Launched '%s' as process %" PRIu64 "...\n",
m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
- m_process_launch_info.GetProcessID());
+ m_debugged_process_sp->GetID());
- return error;
+ return Status();
}
Status GDBRemoteCommunicationServerLLGS::AttachToProcess(lldb::pid_t pid) {
- Status error;
-
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
if (log)
log->Printf("GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64,
@@ -321,13 +316,14 @@
pid, m_debugged_process_sp->GetID());
// Try to attach.
- error = NativeProcessProtocol::Attach(pid, *this, m_mainloop,
- m_debugged_process_sp);
- if (!error.Success()) {
- fprintf(stderr, "%s: failed to attach to process %" PRIu64 ": %s",
- __FUNCTION__, pid, error.AsCString());
- return error;
+ auto process_or = m_process_factory.Attach(pid, *this, m_mainloop);
+ if (!process_or) {
+ Status status(process_or.takeError());
+ llvm::errs() << llvm::formatv("failed to attach to process {0}: {1}", pid,
+ status);
+ return status;
}
+ m_debugged_process_sp = *process_or;
// Setup stdout/stderr mapping from inferior.
auto terminal_fd = m_debugged_process_sp->GetTerminalFileDescriptor();
@@ -336,9 +332,9 @@
log->Printf("ProcessGDBRemoteCommunicationServerLLGS::%s setting "
"inferior STDIO fd to %d",
__FUNCTION__, terminal_fd);
- error = SetSTDIOFileDescriptor(terminal_fd);
- if (error.Fail())
- return error;
+ Status status = SetSTDIOFileDescriptor(terminal_fd);
+ if (status.Fail())
+ return status;
} else {
if (log)
log->Printf("ProcessGDBRemoteCommunicationServerLLGS::%s ignoring "
@@ -347,8 +343,7 @@
}
printf("Attached to process %" PRIu64 "...\n", pid);
-
- return error;
+ return Status();
}
void GDBRemoteCommunicationServerLLGS::InitializeDelegate(
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index a7d7850..b065642 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -39,7 +39,9 @@
//------------------------------------------------------------------
// Constructors and Destructors
//------------------------------------------------------------------
- GDBRemoteCommunicationServerLLGS(MainLoop &mainloop);
+ GDBRemoteCommunicationServerLLGS(
+ MainLoop &mainloop,
+ const NativeProcessProtocol::Factory &process_factory);
//------------------------------------------------------------------
/// Specify the program to launch and its arguments.
@@ -108,20 +110,21 @@
protected:
MainLoop &m_mainloop;
MainLoop::ReadHandleUP m_network_handle_up;
- lldb::tid_t m_current_tid;
- lldb::tid_t m_continue_tid;
+ const NativeProcessProtocol::Factory &m_process_factory;
+ lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
+ lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
std::recursive_mutex m_debugged_process_mutex;
NativeProcessProtocolSP m_debugged_process_sp;
Communication m_stdio_communication;
MainLoop::ReadHandleUP m_stdio_handle_up;
- lldb::StateType m_inferior_prev_state;
+ lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
std::unique_ptr<llvm::MemoryBuffer> m_active_auxv_buffer_up;
std::mutex m_saved_registers_mutex;
std::unordered_map<uint32_t, lldb::DataBufferSP> m_saved_registers_map;
- uint32_t m_next_saved_registers_id;
- bool m_handshake_completed : 1;
+ uint32_t m_next_saved_registers_id = 1;
+ bool m_handshake_completed = false;
PacketResult SendONotification(const char *buffer, uint32_t len);