Skip to content

🍒 [windows][lldb] implement system logging on Windows (#150213) #11096

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

🍒 [windows][lldb] implement system logging on Windows (#150213) #11096

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
51648ef
[windows][lldb] implement system logging on Windows (#150213)
charles-zablit Jul 31, 2025
c94e72c
[lldb][windows] remove duplicate implementation of UTF8ToUTF16 (#154424)
charles-zablit Aug 20, 2025
ff458ab
[lldb][windows] use OutputDebugStringA instead of c to log events (#1…
charles-zablit Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 1 addition & 15 deletions lldb/source/Host/common/Host.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ int __pthread_fchdir(int fildes);
using namespace lldb;
using namespace lldb_private;

#if !defined(__APPLE__)
#if !defined(_WIN32)
#if !defined(__APPLE__) && !defined(_WIN32)
#include <syslog.h>
void Host::SystemLog(Severity severity, llvm::StringRef message) {
static llvm::once_flag g_openlog_once;
Expand All @@ -110,19 +109,6 @@ void Host::SystemLog(Severity severity, llvm::StringRef message) {
}
syslog(level, "%s", message.data());
}
#else
void Host::SystemLog(Severity severity, llvm::StringRef message) {
switch (severity) {
case lldb::eSeverityInfo:
case lldb::eSeverityWarning:
llvm::outs() << message;
break;
case lldb::eSeverityError:
llvm::errs() << message;
break;
}
}
#endif
#endif

#if !defined(__APPLE__) && !defined(_WIN32)
Expand Down
63 changes: 63 additions & 0 deletions lldb/source/Host/windows/Host.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/StructuredData.h"

#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/ManagedStatic.h"

// Windows includes
#include <tlhelp32.h>
Expand Down Expand Up @@ -302,3 +304,64 @@ Environment Host::GetEnvironment() {
}
return env;
}

/// Manages the lifecycle of a Windows Event's Source.
/// The destructor will call DeregisterEventSource.
/// This class is meant to be used with \ref llvm::ManagedStatic.
class WindowsEventLog {
public:
WindowsEventLog() : handle(RegisterEventSource(nullptr, L"lldb")) {}

~WindowsEventLog() {
if (handle)
DeregisterEventSource(handle);
}

HANDLE GetHandle() const { return handle; }

private:
HANDLE handle;
};

static llvm::ManagedStatic<WindowsEventLog> event_log;

static std::wstring AnsiToUtf16(const std::string &ansi) {
if (ansi.empty())
return {};

const int unicode_length =
MultiByteToWideChar(CP_ACP, 0, ansi.c_str(), -1, nullptr, 0);
if (unicode_length == 0)
return {};

std::wstring unicode(unicode_length, L'\0');
MultiByteToWideChar(CP_ACP, 0, ansi.c_str(), -1, &unicode[0], unicode_length);
return unicode;
}
Copy link
Member

@compnerd compnerd Aug 7, 2025

Choose a reason for hiding this comment

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

Why re-implement UTF8ToUTF16 and UTF16ToUTF8 from LLVM?

Copy link
Author

@charles-zablit charles-zablit Aug 19, 2025

Choose a reason for hiding this comment

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

Opened a PR upstream to address this. Thanks!


void Host::SystemLog(Severity severity, llvm::StringRef message) {
HANDLE h = event_log->GetHandle();
if (!h)
return;

std::wstring wide_message = AnsiToUtf16(message.str());
if (wide_message.empty())
return;

LPCWSTR msg_ptr = wide_message.c_str();

WORD event_type;
switch (severity) {
case lldb::eSeverityWarning:
event_type = EVENTLOG_WARNING_TYPE;
break;
case lldb::eSeverityError:
event_type = EVENTLOG_ERROR_TYPE;
break;
case lldb::eSeverityInfo:
default:
event_type = EVENTLOG_INFORMATION_TYPE;
}

ReportEventW(h, event_type, 0, 0, nullptr, 1, 0, &msg_ptr, nullptr);
Copy link
Member

@compnerd compnerd Aug 7, 2025

Choose a reason for hiding this comment

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

Is this really the best place to put the messages? The Event Log is for system events, this feels more developer oriented and feels like the kernel debug buffer might be better (OutputDebugStringA).

Copy link
Author

@charles-zablit charles-zablit Aug 19, 2025

Choose a reason for hiding this comment

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

From my understanding, the messages outputted through OutputDebugStringA are only accessible from an attached debugger or something like DebugView (which still requires attaching). The goal of this PR is to mimic what lldb does on Darwin with system logging, which does not require a debugger.

this feels more developer oriented

I think the health logs are useful for users as well to understand if their install is broken. I would say that they are also useful for lldb users.

Copy link
Member

@compnerd compnerd Aug 19, 2025

Choose a reason for hiding this comment

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

Sure, but I don't think that the event viewer is the right location for the logging. This log tends to be very very very very verbose. If this only was triggered in the case that lldb is dying, that would be one thing. But this is effectively triggered on each run of lldb.

Copy link
Author

@charles-zablit charles-zablit Aug 20, 2025

Choose a reason for hiding this comment

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

This log tends to be very very very very verbose.

Are you talking about the overall system events or just the lldb health logs? I agree that the overall system logs (the contents of the event viewer) has a lot of entries. However the UI and the search bar allow to filter them easily. The Console on macOS also has a lot of entries and we can filter messages fine. I assume it's the same on Linux.

My reasoning for using the event viewer is:

  1. Even though there are a lot of entries, the UI allows to quickly find lldb related entries.
  2. This is the closest match to how we log on other platforms. We could switch to using OutputDebugStringA however we should also do so on other platforms. I don't think it's a good UX for users to need to use a debugger on Windows to get the health logs, but to only have to open the Console to get them on macOS.

I would be in favor of taking this change in 6.2 and to propose a migration to another destination for health logs on all platforms on the forums further down the line. What do you think?

Copy link
Member

@compnerd compnerd Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I am referring to the lldb side. I don't think that Linux is the same. syslog is just a flat file though these days you would find the logs somewhere in systemd I imagine which isn't really easy to filter.

This is too late for 6.2 IMO. So we are better off just changing this now before merging.

}