[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

via lldb-commits lldb-commits at lists.llvm.org
Thu Sep 12 22:37:41 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

Add an "always on" log category and channel. Unlike other, existing log channels, it is not exposed to users. The channel is meant to be used sparsely and deliberately for logging high-value information to the system log.

We have a similar concept in the downstream Swift fork and this has proven to be extremely valuable. This is especially true on macOS where system log messages are automatically captured as part of a sysdiagnose.

This patch revives https://reviews.llvm.org/D135621 although the purpose is slightly different (logging to the system log rather than to a ring buffer dumped as part of the diagnostics). This patch addresses all the remaining oustanding comments.

---
Full diff: https://github.com/llvm/llvm-project/pull/108495.diff


7 Files Affected:

- (modified) lldb/include/lldb/Host/Host.h (+16) 
- (modified) lldb/include/lldb/Utility/LLDBLog.h (+1) 
- (modified) lldb/include/lldb/Utility/Log.h (+3-2) 
- (modified) lldb/source/Host/common/Host.cpp (+19) 
- (modified) lldb/source/Host/common/HostInfoBase.cpp (+2) 
- (modified) lldb/source/Utility/Log.cpp (+13-4) 
- (added) lldb/test/Shell/Host/TestSytemLogChannel.test (+3) 


``````````diff
diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index 9d0994978402f7..1ca89fa056aa93 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -31,6 +31,22 @@ class ProcessInstanceInfo;
 class ProcessInstanceInfoMatch;
 typedef std::vector<ProcessInstanceInfo> ProcessInstanceInfoList;
 
+// Always on system log category and channel.
+enum class SystemLog : Log::MaskType {
+  System = Log::ChannelFlag<0>,
+  LLVM_MARK_AS_BITMASK_ENUM(System)
+};
+
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+class LogChannelSystem {
+public:
+  static void Initialize();
+  static void Terminate();
+};
+
+template <> Log::Channel &LogChannelFor<SystemLog>();
+
 // Exit Type for inferior processes
 struct WaitStatus {
   enum Type : uint8_t {
diff --git a/lldb/include/lldb/Utility/LLDBLog.h b/lldb/include/lldb/Utility/LLDBLog.h
index c7de41e74e85bd..b3e0e77514b8be 100644
--- a/lldb/include/lldb/Utility/LLDBLog.h
+++ b/lldb/include/lldb/Utility/LLDBLog.h
@@ -49,6 +49,7 @@ enum class LLDBLog : Log::MaskType {
   Watchpoints = Log::ChannelFlag<30>,
   OnDemand = Log::ChannelFlag<31>,
   Source = Log::ChannelFlag<32>,
+  AlwaysOn = Log::ChannelFlag<32>,
   LLVM_MARK_AS_BITMASK_ENUM(OnDemand),
 };
 
diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 27707c17f9b824..bcb022093c9001 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -165,13 +165,14 @@ class Log final {
     friend class Log;
 
   public:
+    const bool internal = false;
     const llvm::ArrayRef<Category> categories;
     const MaskType default_flags;
 
     template <typename Cat>
     constexpr Channel(llvm::ArrayRef<Log::Category> categories,
-                      Cat default_flags)
-        : log_ptr(nullptr), categories(categories),
+                      Cat default_flags, bool internal = false)
+        : log_ptr(nullptr), internal(internal), categories(categories),
           default_flags(MaskType(default_flags)) {
       static_assert(
           std::is_same<Log::MaskType, std::underlying_type_t<Cat>>::value);
diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index f08adea6546ae1..3baf40a4700ffb 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -125,6 +125,25 @@ void Host::SystemLog(Severity severity, llvm::StringRef message) {
 #endif
 #endif
 
+static constexpr Log::Category g_categories[] = {
+    {{"system"}, {"system log"}, SystemLog::System}};
+
+static Log::Channel g_channel(g_categories, SystemLog::System,
+                              /*internal=*/true);
+
+template <> Log::Channel &lldb_private::LogChannelFor<SystemLog>() {
+  return g_channel;
+}
+
+void LogChannelSystem::Initialize() {
+  Log::Register("system", g_channel);
+  const uint32_t log_options = LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
+  Log::EnableLogChannel(std::make_shared<SystemLogHandler>(), log_options,
+                        "system", {"all"}, llvm::nulls());
+}
+
+void LogChannelSystem::Terminate() { Log::Unregister("system"); }
+
 #if !defined(__APPLE__) && !defined(_WIN32)
 static thread_result_t
 MonitorChildProcessThreadFunction(::pid_t pid,
diff --git a/lldb/source/Host/common/HostInfoBase.cpp b/lldb/source/Host/common/HostInfoBase.cpp
index 5c44c2f38b2879..89dfe4a9e9baa3 100644
--- a/lldb/source/Host/common/HostInfoBase.cpp
+++ b/lldb/source/Host/common/HostInfoBase.cpp
@@ -76,9 +76,11 @@ static HostInfoBase::SharedLibraryDirectoryHelper *g_shlib_dir_helper = nullptr;
 void HostInfoBase::Initialize(SharedLibraryDirectoryHelper *helper) {
   g_shlib_dir_helper = helper;
   g_fields = new HostInfoBaseFields();
+  LogChannelSystem::Initialize();
 }
 
 void HostInfoBase::Terminate() {
+  LogChannelSystem::Terminate();
   g_shlib_dir_helper = nullptr;
   delete g_fields;
   g_fields = nullptr;
diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index 6713a5bd758204..040bd47606ce18 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -245,6 +245,11 @@ bool Log::DisableLogChannel(llvm::StringRef channel,
     error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
     return false;
   }
+  if (iter->second.m_channel.internal) {
+    error_stream << llvm::formatv(
+        "Cannot disable internal log channel '{0}'.\n", channel);
+    return false;
+  }
   MaskType flags = categories.empty()
                        ? std::numeric_limits<MaskType>::max()
                        : GetFlags(error_stream, *iter, categories);
@@ -296,8 +301,10 @@ void Log::ForEachChannelCategory(
 
 std::vector<llvm::StringRef> Log::ListChannels() {
   std::vector<llvm::StringRef> result;
-  for (const auto &channel : *g_channel_map)
-    result.push_back(channel.first());
+  for (const auto &entry : *g_channel_map) {
+    if (!entry.second.m_channel.internal)
+      result.push_back(entry.first());
+  }
   return result;
 }
 
@@ -307,8 +314,10 @@ void Log::ListAllLogChannels(llvm::raw_ostream &stream) {
     return;
   }
 
-  for (const auto &channel : *g_channel_map)
-    ListCategories(stream, channel);
+  for (const auto &entry : *g_channel_map) {
+    if (!entry.second.m_channel.internal)
+      ListCategories(stream, entry);
+  }
 }
 
 bool Log::GetVerbose() const {
diff --git a/lldb/test/Shell/Host/TestSytemLogChannel.test b/lldb/test/Shell/Host/TestSytemLogChannel.test
new file mode 100644
index 00000000000000..9ac4985da6164e
--- /dev/null
+++ b/lldb/test/Shell/Host/TestSytemLogChannel.test
@@ -0,0 +1,3 @@
+RUN: %lldb -o 'log list' -o 'log disable system' 2>&1 | FileCheck %s
+CHECK-NOT: Logging categories for 'system'
+CHECK: Cannot disable internal log channel 'system'.

``````````

</details>


https://github.com/llvm/llvm-project/pull/108495


More information about the lldb-commits mailing list