Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lldb] Add a log level to Host::SystemLog #90904

Merged
merged 1 commit into from
May 2, 2024

Conversation

JDevlieghere
Copy link
Member

Add the ability to specify a log level to Host::SystemLog.

@llvmbot
Copy link
Collaborator

llvmbot commented May 2, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Add the ability to specify a log level to Host::SystemLog.


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

3 Files Affected:

  • (modified) lldb/include/lldb/Host/Host.h (+8-1)
  • (modified) lldb/source/Host/common/Host.cpp (+26-4)
  • (modified) lldb/source/Host/macosx/objcxx/Host.mm (+10-2)
diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index 30549cd7891497..b0cb477d82fa14 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -87,8 +87,15 @@ class Host {
   StartMonitoringChildProcess(const MonitorChildProcessCallback &callback,
                               lldb::pid_t pid);
 
+  /// System log level.
+  enum SystemLogLevel {
+    eSystemLogInfo,
+    eSystemLogWarning,
+    eSystemLogError,
+  };
+
   /// Emit the given message to the operating system log.
-  static void SystemLog(llvm::StringRef message);
+  static void SystemLog(SystemLogLevel log_level, llvm::StringRef message);
 
   /// Get the process ID for the calling process.
   ///
diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index 565138ba17031f..a0aa6a62755f92 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -91,15 +91,37 @@ using namespace lldb_private;
 #if !defined(__APPLE__)
 #if !defined(_WIN32)
 #include <syslog.h>
-void Host::SystemLog(llvm::StringRef message) {
+void Host::SystemLog(SystemLogLevel log_level, llvm::StringRef message) {
   static llvm::once_flag g_openlog_once;
   llvm::call_once(g_openlog_once, [] {
     openlog("lldb", LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER);
   });
-  syslog(LOG_INFO, "%s", message.data());
+  int level = LOG_DEBUG;
+  switch (log_level) {
+  case eSystemLogInfo:
+    level = LOG_INFO;
+    break;
+  case eSystemLogWarning:
+    level = LOG_WARNING;
+    break;
+  case eSystemLogError:
+    level = LOG_WARNING;
+    break;
+  }
+  syslog(level, "%s", message.data());
 }
 #else
-void Host::SystemLog(llvm::StringRef message) { llvm::errs() << message; }
+void Host::SystemLog(SystemLogLevel log_level, llvm::StringRef message) {
+  switch (log_level) {
+  case eSystemLogInfo:
+  case eSystemLogWarning:
+    llvm::outs() << message;
+    break;
+  case eSystemLogError:
+    llvm::errs() << message;
+    break;
+  }
+}
 #endif
 #endif
 
@@ -629,5 +651,5 @@ char SystemLogHandler::ID;
 SystemLogHandler::SystemLogHandler() {}
 
 void SystemLogHandler::Emit(llvm::StringRef message) {
-  Host::SystemLog(message);
+  Host::SystemLog(Host::eSystemLogInfo, message);
 }
diff --git a/lldb/source/Host/macosx/objcxx/Host.mm b/lldb/source/Host/macosx/objcxx/Host.mm
index 070a49208639a2..1f67141de55d70 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -102,12 +102,20 @@
 static os_log_t g_os_log;
 static std::once_flag g_os_log_once;
 
-void Host::SystemLog(llvm::StringRef message) {
+void Host::SystemLog(SystemLogLevel log_level, llvm::StringRef message) {
   if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
     std::call_once(g_os_log_once, []() {
       g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
     });
-    os_log(g_os_log, "%{public}s", message.str().c_str());
+    switch (log_level) {
+    case eSystemLogInfo:
+    case eSystemLogWarning:
+      os_log(g_os_log, "%{public}s", message.str().c_str());
+      break;
+    case eSystemLogError:
+      os_log_error(g_os_log, "%{public}s", message.str().c_str());
+      break;
+    }
   } else {
     llvm::errs() << message;
   }

Add the ability to specify a log level to Host::SystemLog.
Copy link
Member

@medismailben medismailben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@JDevlieghere JDevlieghere merged commit a7e9e3e into llvm:main May 2, 2024
4 checks passed
@JDevlieghere JDevlieghere deleted the system-log-level branch May 2, 2024 23:07
/// System log level.
enum SystemLogLevel {
eSystemLogInfo,
eSystemLogWarning,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a good reason to keep this separate from DiagnosticSeverity?
https://lldb.llvm.org/cpp_reference/namespacelldb__private.html#a3b3fff978827f40d27f288b4f1f29369
Maybe we could factor this into private enums?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! #90917

JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request May 3, 2024
Add the ability to specify a log level to Host::SystemLog.

(cherry picked from commit a7e9e3e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants