Initialize ProcessGDBRemoteLog for LLGS to fix remote platform logging

[flackr]

Summary

This was previously initialized by ProcessGDBRemote::Initialize but lldb-server does not contain ProcessGDBRemote anymore so this needs to be initialized directly.

[ovyalov]

Please see my comments.

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

142 ↗

(On Diff #21525)

Maybe introduce ProcessGDBRemoteLog::Initialize() instead of this method?

We can put

Log::Callbacks log_callbacks = {
            ProcessGDBRemoteLog::DisableLog,
            ProcessGDBRemoteLog::EnableLog,
            ProcessGDBRemoteLog::ListLogCategories
        };

        Log::RegisterLogChannel (g_name, log_callbacks);

inside of ProcessGDBRemoteLog::Initialize and call it from ProcessGDBRemote::Initialize and lldb-server.cpp.

147 ↗

(On Diff #21525)

Could you use std::call_once here?

tools/lldb-server/lldb-server.cpp

11 ↗

(On Diff #21525)

GDBRemoteCommunication.h ?

[tberghammer]

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

142 ↗

(On Diff #21525)

I like your idea about creating ProcessGDBRemoteLog::Initialize, but I would prefer to call it from lldb.cpp:initializeLLGS().

[flackr]

Move Log initialization to Log class Initialize functions and use std::call_once to guard against multiple calls.

[flackr]

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

142 ↗

(On Diff #21525)

Done, for ProcessPOSIXLog as well.

147 ↗

(On Diff #21525)

Done.

tools/lldb-server/lldb-server.cpp

11 ↗

(On Diff #21525)

Oops, done.

[tberghammer]

Please update ProcessFreeBSD::Initialize() to use the newly created ProcessPOSIXLog::Initialize() function.

source/Plugins/Process/Linux/ProcessLinux.cpp

64

Please move this line into ProcessPOSIXLog::Initialize()

source/Plugins/Process/POSIX/ProcessPOSIXLog.cpp

41

Please make it an argument, instead of a hard coded string.

[flackr]

Added argument to ProcessPOSIXLog to specify log channel name, called from ProcessFreeBSD::Initialize as well.

source/Plugins/Process/Linux/ProcessLinux.cpp

64

Done.

source/Plugins/Process/POSIX/ProcessPOSIXLog.cpp

41

Done.

[ovyalov]

Looks good.

tools/lldb-server/lldb-gdbserver.cpp

490–494

We'll need to apply such conditional compilation in lldb-platform.cpp as well https://github.com/llvm-mirror/lldb/blob/master/tools/lldb-server/lldb-platform.cpp#L108 - but not necessary for this CL.

[tberghammer]

tools/lldb-server/lldb-gdbserver.cpp

490–494

lldb-platform not used on Windows (at least not yet)

source/Plugins/Process/Linux/NativeProcessLinux.h

namespace lldb_private
     /// Upon construction, this class prepares and launches an inferior process for
     /// debugging.
     ///
     /// Changes in the inferior process state are broadcasted.
     class NativeProcessLinux: public NativeProcessProtocol
     {
     public:
 
-        // ---------------------------------------------------------------------
-        // Public Static Methods
-        // ---------------------------------------------------------------------
-        static void
-        Initialize();
-
         static lldb_private::Error
         LaunchProcess (
             Module *exe_module,
             ProcessLaunchInfo &launch_info,
             lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
             NativeProcessProtocolSP &native_process_sp);
 
         static lldb_private::Error

source/Plugins/Process/Linux/NativeProcessLinux.cpp

 
 NativeProcessLinux::AttachArgs::~AttachArgs()
 { }
 
 // -----------------------------------------------------------------------------
 // Public Static Methods
 // -----------------------------------------------------------------------------
 
-void
-NativeProcessLinux::Initialize()
-{
-    static ConstString g_name("linux");
-    static bool g_initialized = false;
-
-    if (!g_initialized)
-    {
-        g_initialized = true;
-
-        Log::Callbacks log_callbacks = {
-            ProcessPOSIXLog::DisableLog,
-            ProcessPOSIXLog::EnableLog,
-            ProcessPOSIXLog::ListLogCategories
-        };
-
-        Log::RegisterLogChannel (g_name, log_callbacks);
-        ProcessPOSIXLog::RegisterPluginName (g_name);
-    }
-}
-
 lldb_private::Error
 NativeProcessLinux::LaunchProcess (
     lldb_private::Module *exe_module,
     lldb_private::ProcessLaunchInfo &launch_info,
     lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
     NativeProcessProtocolSP &native_process_sp)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));

source/Plugins/Process/Linux/ProcessLinux.cpp

ProcessLinux::Initialize()
     static bool g_initialized = false;
 
     if (!g_initialized)
     {
         g_initialized = true;
         PluginManager::RegisterPlugin(GetPluginNameStatic(),
                                       GetPluginDescriptionStatic(),
                                       CreateInstance);
-
-        Log::Callbacks log_callbacks = {
-            ProcessPOSIXLog::DisableLog,
-            ProcessPOSIXLog::EnableLog,
-            ProcessPOSIXLog::ListLogCategories
-        };
-        
-        Log::RegisterLogChannel (ProcessLinux::GetPluginNameStatic(), log_callbacks);
+        ProcessPOSIXLog::Initialize();
         ProcessPOSIXLog::RegisterPluginName(GetPluginNameStatic());
     }
 }
 
 //------------------------------------------------------------------------------
 // Constructors and destructors.
 
 ProcessLinux::ProcessLinux(Target& target, Listener &listener, FileSpec *core_file)
     : ProcessPOSIX(target, listener, GetStaticLinuxSignalsSP ()), m_core_file(core_file), m_stopping_threads(false)

source/Plugins/Process/POSIX/ProcessPOSIXLog.h

 #define POSIX_LOG_MEMORY_SHORT_BYTES       (4 * sizeof(ptrdiff_t))
 
 class ProcessPOSIXLog
 {
     static int m_nestinglevel;
     static const char *m_pluginname;
 
 public:
+    // ---------------------------------------------------------------------
+    // Public Static Methods
+    // ---------------------------------------------------------------------
+    static void
+    Initialize();
+
     static void
     RegisterPluginName(const char *pluginName)
     {
         m_pluginname = pluginName;
     }
 
     static void
     RegisterPluginName(lldb_private::ConstString pluginName)

source/Plugins/Process/POSIX/ProcessPOSIXLog.cpp

 //===-- ProcessPOSIXLog.cpp ---------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ProcessPOSIXLog.h"
 
+#include <mutex>
+
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Core/StreamFile.h"
 
 #include "ProcessPOSIX.h"
 #include "ProcessPOSIXLog.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 
 // We want to avoid global constructors where code needs to be run so here we
 // control access to our static g_log_sp by hiding it in a singleton function
 // that will construct the static g_log_sp the first time this function is 
 // called.
 static bool g_log_enabled = false;
 static Log * g_log = NULL;
 static Log *
 GetLog ()
 {
     if (!g_log_enabled)
         return NULL;
     return g_log;
 }
 
+void
+ProcessPOSIXLog::Initialize()
+{
+    static ConstString g_name("linux");
+    static std::once_flag g_once_flag;
+
+    std::call_once(g_once_flag, [](){
+        Log::Callbacks log_callbacks = {
+            DisableLog,
+            EnableLog,
+            ListLogCategories
+        };
+
+        Log::RegisterLogChannel (g_name, log_callbacks);
+    });
+}
 
 Log *
 ProcessPOSIXLog::GetLogIfAllCategoriesSet (uint32_t mask)
 {
     Log *log(GetLog ());
     if (log && mask)
     {
         uint32_t log_mask = log->GetMask().Get();

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

ProcessGDBRemote::Initialize()
 
     if (g_initialized == false)
     {
         g_initialized = true;
         PluginManager::RegisterPlugin (GetPluginNameStatic(),
                                        GetPluginDescriptionStatic(),
                                        CreateInstance,
                                        DebuggerInitialize);
-
-        Log::Callbacks log_callbacks = {
-            ProcessGDBRemoteLog::DisableLog,
-            ProcessGDBRemoteLog::EnableLog,
-            ProcessGDBRemoteLog::ListLogCategories
-        };
-
-        Log::RegisterLogChannel (ProcessGDBRemote::GetPluginNameStatic(), log_callbacks);
     }
 }
 
 void
 ProcessGDBRemote::DebuggerInitialize (lldb_private::Debugger &debugger)
 {
     if (!PluginManager::GetSettingForProcessPlugin(debugger, PluginProperties::GetSettingName()))
     {

source/Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h

 #define GDBR_LOG_COMM                     (1u << 10)
 #define GDBR_LOG_ASYNC                    (1u << 11)
 #define GDBR_LOG_ALL                      (UINT32_MAX)
 #define GDBR_LOG_DEFAULT                  GDBR_LOG_PACKETS
 
 class ProcessGDBRemoteLog
 {
 public:
+    static void
+    Initialize();
+
     static lldb_private::Log *
     GetLogIfAllCategoriesSet(uint32_t mask = 0);
     
     static lldb_private::Log *
     GetLogIfAnyCategoryIsSet (uint32_t mask);
 
     static void
     DisableLog (const char **categories, lldb_private::Stream *feedback_strm);

source/Plugins/Process/gdb-remote/ProcessGDBRemoteLog.cpp

 //===-- ProcessGDBRemoteLog.cpp ---------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ProcessGDBRemoteLog.h"
 
+#include <mutex>
+
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Core/StreamFile.h"
 
 #include "ProcessGDBRemote.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 
 // We want to avoid global constructors where code needs to be run so here we
 // control access to our static g_log_sp by hiding it in a singleton function
 // that will construct the static g_lob_sp the first time this function is 
 // called.
 static bool g_log_enabled = false;
 static Log * g_log = NULL;
 static Log *
 GetLog ()
 {
     if (!g_log_enabled)
         return NULL;
     return g_log;
 }
 
+void
+ProcessGDBRemoteLog::Initialize()
+{
+    static ConstString g_name("gdb-remote");
+    static std::once_flag g_once_flag;
+
+    std::call_once(g_once_flag, [](){
+        Log::Callbacks log_callbacks = {
+            DisableLog,
+            EnableLog,
+            ListLogCategories
+        };
+
+        Log::RegisterLogChannel (g_name, log_callbacks);
+    });
+}
 
 Log *
 ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (uint32_t mask)
 {
     Log *log(GetLog ());
     if (log && mask)
     {
         uint32_t log_mask = log->GetMask().Get();

source/lldb.cpp

 #include "Plugins/Platform/MacOSX/PlatformDarwinKernel.h"
 #include "Plugins/Process/mach-core/ProcessMachCore.h"
 #include "Plugins/Process/MacOSX-Kernel/ProcessKDP.h"
 #include "Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.h"
 #endif
 
 #if defined (__linux__)
 #include "Plugins/Process/Linux/ProcessLinux.h"
+#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #endif
 
 #if defined (_WIN32)
 #include "lldb/Host/windows/windows.h"
 #include "Plugins/Process/Windows/DynamicLoaderWindows.h"
 #include "Plugins/Process/Windows/ProcessWindows.h"
 #endif
 
 #if defined (__FreeBSD__)
 #include "Plugins/Process/POSIX/ProcessPOSIX.h"
 #include "Plugins/Process/FreeBSD/ProcessFreeBSD.h"
 #endif
 
 #include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
 #include "Plugins/Process/gdb-remote/ProcessGDBRemote.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/MemoryHistory/asan/MemoryHistoryASan.h"
 #include "Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
#endif
 
         Log::Initialize();
         HostInfo::Initialize();
         Timer::Initialize();
         Timer scoped_timer(__PRETTY_FUNCTION__, __PRETTY_FUNCTION__);
 
         llvm::install_fatal_error_handler(fatal_error_handler, 0);
 
-        // Initialize plug-ins
+        ProcessGDBRemoteLog::Initialize();
 
+        // Initialize plug-ins
         ObjectContainerBSDArchive::Initialize();
         ObjectFileELF::Initialize();
         SymbolVendorELF::Initialize();
         SymbolFileDWARF::Initialize();
         SymbolFileSymtab::Initialize();
         UnwindAssemblyInstEmulation::Initialize();
         UnwindAssembly_x86::Initialize();
         EmulateInstructionARM::Initialize();
#endif
         SystemRuntimeMacOSX::Initialize();
 
 #if defined (__APPLE__)
         SymbolVendorMacOSX::Initialize();
         DynamicLoaderDarwinKernel::Initialize();
         PlatformDarwinKernel::Initialize();
         ObjectFileMachO::Initialize();
 #endif
+#if defined (__linux__)
+        ProcessPOSIXLog::Initialize();
+#endif
 #ifndef LLDB_DISABLE_PYTHON
         ScriptInterpreterPython::InitializePrivate();
         OperatingSystemPython::Initialize();
 #endif
     }
 }
 
 static bool g_inited = false;

tools/lldb-server/lldb-gdbserver.cpp

 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Host/StringConvert.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h"
 #include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
-#include "Plugins/Process/Linux/NativeProcessLinux.h"
 
 #ifndef LLGS_PROGRAM_NAME
 #define LLGS_PROGRAM_NAME "lldb-server"
 #endif
 
 #ifndef LLGS_VERSION_STR
 #define LLGS_VERSION_STR "local_build"
 #endif
ConnectToRemote (GDBRemoteCommunicationServerLLGS &gdb_server, bool reverse_connect, const char *const host_and_port, const char *const progname, const char *const subcommand, const char *const named_pipe_path)
     else
     {
         fprintf (stderr, "no connection information provided, unable to run\n");
         display_usage (progname, subcommand);
         exit (1);
     }
 }
 
-static void
-initialize ()
-{
-#ifndef _WIN32
-    // Setup signal handlers first thing.
-    signal (SIGPIPE, signal_handler);
-    signal (SIGHUP, signal_handler);
-#endif
-
-#if defined (__linux__)
-    //----------------------------------------------------------------------
-    // Linux hosted plugins
-    //----------------------------------------------------------------------
-    NativeProcessLinux::Initialize();
-#endif
-}
-
 //----------------------------------------------------------------------
 // main
 //----------------------------------------------------------------------
 int
 main_gdbserver (int argc, char *argv[])
 {
-    initialize ();
+#ifndef _WIN32
+    // Setup signal handlers first thing.
+    signal (SIGPIPE, signal_handler);
+    signal (SIGHUP, signal_handler);
+#endif
 
     const char *progname = argv[0];
     const char *subcommand = argv[1];
     argc--;
     argv++;
     int long_option_index = 0;
     StreamSP log_stream_sp;
     Args log_args;