From bc51f6533042df622689bb90a041f2df979798f6 Mon Sep 17 00:00:00 2001 From: Walter Erquinigo Date: Mon, 9 Jun 2025 16:55:17 +0000 Subject: [PATCH] [To upstream] Add a name to server comm channels and include them in the logs This allows differenciating GPU and CPU gdb-remote logs emitted by the server. E.g. ``` 1749485814.359431028 [754988/755010] nvidia-gpu.server < 19> read packet: $QStartNoAckMode#b0 1749485814.359459400 [754988/755010] nvidia-gpu.server < 1> send packet: + 1749485813.658699989 [754988/754988] gdb-server < 29> read packet: $qXfer:auxv:read::0,131071#d7 1749485813.658788681 [754988/754988] gdb-server < 341> send packet: $l! ``` --- .../gdb-remote/GDBRemoteClientBase.cpp | 2 +- .../gdb-remote/GDBRemoteCommunication.cpp | 38 +++++++++---------- .../gdb-remote/GDBRemoteCommunication.h | 6 ++- .../GDBRemoteCommunicationServer.cpp | 4 +- .../gdb-remote/GDBRemoteCommunicationServer.h | 4 +- .../GDBRemoteCommunicationServerCommon.cpp | 5 ++- .../GDBRemoteCommunicationServerCommon.h | 4 +- .../GDBRemoteCommunicationServerLLGS.cpp | 5 ++- .../GDBRemoteCommunicationServerLLGS.h | 10 ++++- .../GDBRemoteCommunicationServerPlatform.cpp | 4 +- .../MockGPU/LLDBServerPluginMockGPU.cpp | 4 +- lldb/tools/lldb-server/lldb-gdbserver.cpp | 2 +- 12 files changed, 51 insertions(+), 37 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp index 394b62559da76..6268650242a90 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp @@ -32,7 +32,7 @@ static const seconds kWakeupInterval(5); GDBRemoteClientBase::ContinueDelegate::~ContinueDelegate() = default; GDBRemoteClientBase::GDBRemoteClientBase(const char *comm_name) - : GDBRemoteCommunication(), Broadcaster(nullptr, comm_name), + : GDBRemoteCommunication(comm_name), Broadcaster(nullptr, comm_name), m_async_count(0), m_is_running(false), m_should_stop(false) {} StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse( diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 7ed80b048c77e..68d18d6741232 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -59,7 +59,7 @@ using namespace lldb_private; using namespace lldb_private::process_gdb_remote; // GDBRemoteCommunication constructor -GDBRemoteCommunication::GDBRemoteCommunication() +GDBRemoteCommunication::GDBRemoteCommunication(llvm::StringRef name) : Communication(), #ifdef LLDB_CONFIGURATION_DEBUG m_packet_timeout(1000), @@ -68,7 +68,7 @@ GDBRemoteCommunication::GDBRemoteCommunication() #endif m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), m_history(512), m_send_acks(true), m_is_platform(false), - m_compression_type(CompressionType::None), m_listen_url() { + m_compression_type(CompressionType::None), m_listen_url(), m_name(name) { } // Destructor @@ -97,7 +97,7 @@ size_t GDBRemoteCommunication::SendAck() { ConnectionStatus status = eConnectionStatusSuccess; char ch = '+'; const size_t bytes_written = WriteAll(&ch, 1, status, nullptr); - LLDB_LOGF(log, "%p <%4" PRIu64 "> send packet: %c", static_cast(this), + LLDB_LOGF(log, "%s <%4" PRIu64 "> send packet: %c", m_name.c_str(), (uint64_t)bytes_written, ch); m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written); return bytes_written; @@ -108,7 +108,7 @@ size_t GDBRemoteCommunication::SendNack() { ConnectionStatus status = eConnectionStatusSuccess; char ch = '-'; const size_t bytes_written = WriteAll(&ch, 1, status, nullptr); - LLDB_LOGF(log, "%p <%4" PRIu64 "> send packet: %c", static_cast(this), + LLDB_LOGF(log, "%s <%4" PRIu64 "> send packet: %c", m_name.c_str(), (uint64_t)bytes_written, ch); m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written); return bytes_written; @@ -180,9 +180,9 @@ GDBRemoteCommunication::SendRawPacketNoLock(llvm::StringRef packet, if (binary_start_offset) { StreamString strm; // Print non binary data header - strm.Printf("%p <%4" PRIu64 "> send packet: %.*s", - static_cast(this), (uint64_t)bytes_written, - (int)binary_start_offset, packet_data); + strm.Printf("%s <%4" PRIu64 "> send packet: %.*s", m_name.c_str(), + (uint64_t)bytes_written, (int)binary_start_offset, + packet_data); const uint8_t *p; // Print binary data exactly as sent for (p = (const uint8_t *)packet_data + binary_start_offset; *p != '#'; @@ -192,9 +192,8 @@ GDBRemoteCommunication::SendRawPacketNoLock(llvm::StringRef packet, strm.Printf("%*s", (int)3, p); log->PutString(strm.GetString()); } else - LLDB_LOGF(log, "%p <%4" PRIu64 "> send packet: %.*s", - static_cast(this), (uint64_t)bytes_written, - (int)packet_length, packet_data); + LLDB_LOGF(log, "%s <%4" PRIu64 "> send packet: %.*s", m_name.c_str(), + (uint64_t)bytes_written, (int)packet_length, packet_data); } m_history.AddPacket(packet.str(), packet_length, @@ -753,13 +752,12 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len, StreamString strm; // Packet header... if (CompressionIsEnabled()) - strm.Printf("%p <%4" PRIu64 ":%" PRIu64 "> read packet: %c", - static_cast(this), (uint64_t)original_packet_size, + strm.Printf("%s <%4" PRIu64 ":%" PRIu64 "> read packet: %c", + m_name.c_str(), (uint64_t)original_packet_size, (uint64_t)total_length, m_bytes[0]); else - strm.Printf("%p <%4" PRIu64 "> read packet: %c", - static_cast(this), (uint64_t)total_length, - m_bytes[0]); + strm.Printf("%s <%4" PRIu64 "> read packet: %c", m_name.c_str(), + (uint64_t)total_length, m_bytes[0]); for (size_t i = content_start; i < content_end; ++i) { // Remove binary escaped bytes when displaying the packet... const char ch = m_bytes[i]; @@ -778,13 +776,13 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len, log->PutString(strm.GetString()); } else { if (CompressionIsEnabled()) - LLDB_LOGF(log, "%p <%4" PRIu64 ":%" PRIu64 "> read packet: %.*s", - static_cast(this), (uint64_t)original_packet_size, - (uint64_t)total_length, (int)(total_length), + LLDB_LOGF(log, "%s <%4" PRIu64 ":%" PRIu64 "> read packet: %.*s", + m_name.c_str(), (uint64_t)original_packet_size, + (uint64_t)total_length, (int)(total_length), m_bytes.c_str()); else - LLDB_LOGF(log, "%p <%4" PRIu64 "> read packet: %.*s", - static_cast(this), (uint64_t)total_length, + LLDB_LOGF(log, "%s <%4" PRIu64 "> read packet: %.*s", + m_name.c_str(), (uint64_t)total_length, (int)(total_length), m_bytes.c_str()); } } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index 107c0896c4e61..834e9ea2002f3 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -117,7 +117,9 @@ class GDBRemoteCommunication : public Communication { bool m_timeout_modified; }; - GDBRemoteCommunication(); + /// \param[in] name + /// The name of the communication channel. + GDBRemoteCommunication(llvm::StringRef name); ~GDBRemoteCommunication() override; @@ -227,6 +229,8 @@ class GDBRemoteCommunication : public Communication { HostThread m_listen_thread; std::string m_listen_url; + std::string m_name; + #if defined(HAVE_LIBCOMPRESSION) CompressionType m_decompression_scratch_type = CompressionType::None; void *m_decompression_scratch = nullptr; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp index 5bd29ae40aa9e..4c0c1e9348b6d 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp @@ -24,8 +24,8 @@ using namespace lldb_private; using namespace lldb_private::process_gdb_remote; using namespace llvm; -GDBRemoteCommunicationServer::GDBRemoteCommunicationServer() - : GDBRemoteCommunication(), m_exit_now(false) { +GDBRemoteCommunicationServer::GDBRemoteCommunicationServer(llvm::StringRef name) + : GDBRemoteCommunication(name), m_exit_now(false) { RegisterPacketHandler( StringExtractorGDBRemote::eServerPacketType_QEnableErrorStrings, [this](StringExtractorGDBRemote packet, Status &error, bool &interrupt, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h index ccbcd0ac5bf58..b40800ed7c70e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h @@ -31,7 +31,9 @@ class GDBRemoteCommunicationServer : public GDBRemoteCommunication { std::function; - GDBRemoteCommunicationServer(); + /// \param[in] name + /// The name of the communication channel. + GDBRemoteCommunicationServer(llvm::StringRef name); ~GDBRemoteCommunicationServer() override; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index 67ba42f33d1dd..0846ca5241c76 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -58,8 +58,9 @@ const static uint32_t g_default_packet_timeout_sec = 0; // not specified #endif // GDBRemoteCommunicationServerCommon constructor -GDBRemoteCommunicationServerCommon::GDBRemoteCommunicationServerCommon() - : GDBRemoteCommunicationServer(), m_process_launch_info(), +GDBRemoteCommunicationServerCommon::GDBRemoteCommunicationServerCommon( + llvm::StringRef name) + : GDBRemoteCommunicationServer(name), m_process_launch_info(), m_process_launch_error(), m_proc_infos(), m_proc_infos_index(0) { RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_A, &GDBRemoteCommunicationServerCommon::Handle_A); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h index b4f1eb3e61c41..36a39efcaef60 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h @@ -25,7 +25,9 @@ class ProcessGDBRemote; class GDBRemoteCommunicationServerCommon : public GDBRemoteCommunicationServer { public: - GDBRemoteCommunicationServerCommon(); + /// \param[in] name + /// The name of the communication channel. + GDBRemoteCommunicationServerCommon(llvm::StringRef name); ~GDBRemoteCommunicationServerCommon() override; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index b0e0e14f2a4b2..401ba691493f8 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -72,8 +72,9 @@ enum GDBRemoteServerError { // GDBRemoteCommunicationServerLLGS constructor GDBRemoteCommunicationServerLLGS::GDBRemoteCommunicationServerLLGS( - MainLoop &mainloop, NativeProcessProtocol::Manager &process_manager) - : GDBRemoteCommunicationServerCommon(), m_mainloop(mainloop), + MainLoop &mainloop, NativeProcessProtocol::Manager &process_manager, + llvm::StringRef name) + : GDBRemoteCommunicationServerCommon(name), m_mainloop(mainloop), m_process_manager(process_manager), m_current_process(nullptr), m_continue_process(nullptr), m_stdio_communication() { RegisterPacketHandlers(); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h index a2ac100ccdde0..4fe653e232ec5 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -37,9 +37,15 @@ class GDBRemoteCommunicationServerLLGS : public GDBRemoteCommunicationServerCommon, public NativeProcessProtocol::NativeDelegate { public: - // Constructors and Destructors + /// \param[in] mainloop + /// The main loop. + /// \param[in] process_manager + /// The process manager. + /// \param[in] name + /// The name of the communication channel. GDBRemoteCommunicationServerLLGS( - MainLoop &mainloop, NativeProcessProtocol::Manager &process_manager); + MainLoop &mainloop, NativeProcessProtocol::Manager &process_manager, + llvm::StringRef name); void SetLaunchInfo(const ProcessLaunchInfo &info); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index 89fdfa74bc025..10b99d7695abb 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -47,8 +47,8 @@ using namespace lldb_private; // GDBRemoteCommunicationServerPlatform constructor GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform( const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port) - : GDBRemoteCommunicationServerCommon(), m_socket_protocol(socket_protocol), - m_gdbserver_port(gdbserver_port) { + : GDBRemoteCommunicationServerCommon("gdb-server-platform"), + m_socket_protocol(socket_protocol), m_gdbserver_port(gdbserver_port) { RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_qC, diff --git a/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp b/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp index 03a50f4220870..15c91c0f67964 100644 --- a/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp +++ b/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp @@ -29,8 +29,8 @@ LLDBServerPluginMockGPU::LLDBServerPluginMockGPU( LLDBServerPlugin::GDBServer &native_process) : LLDBServerPlugin(native_process) { m_process_manager_up.reset(new ProcessMockGPU::Manager(m_main_loop)); - m_gdb_server.reset( - new GDBRemoteCommunicationServerLLGS(m_main_loop, *m_process_manager_up)); + m_gdb_server.reset(new GDBRemoteCommunicationServerLLGS( + m_main_loop, *m_process_manager_up, "mock-gpu.server")); Log *log = GetLog(GDBRLog::Plugin); LLDB_LOGF(log, "LLDBServerPluginMockGPU::LLDBServerPluginMockGPU() faking launch..."); diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp index caebc98b43928..07a0a18818fa3 100644 --- a/lldb/tools/lldb-server/lldb-gdbserver.cpp +++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp @@ -454,7 +454,7 @@ int main_gdbserver(int argc, char *argv[]) { } NativeProcessManager manager(mainloop); - GDBRemoteCommunicationServerLLGS gdb_server(mainloop, manager); + GDBRemoteCommunicationServerLLGS gdb_server(mainloop, manager, "gdb-server"); // Install the mock GPU plugin. gdb_server.InstallPlugin(std::make_unique(gdb_server));