-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lldb][windows] use Windows APIs to print to the console #156469
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][windows] use Windows APIs to print to the console #156469
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Windows 11, x86) Rendering in the terminal works as expected and the previously failing tests now pass for me.
Updating the branch to hopefully get the CI job to pass. |
This is a relanding of llvm#149493. The tests were failing because we were interpreting a proper file descriptor as a console file descriptor. This patch uses the Windows APIs to print to the Windows Console, through `llvm::raw_fd_ostream`. This fixes a rendering issue where the characters defined in `DiagnosticsRendering.cpp` ("╰" for instance) are not rendered properly on Windows out of the box, because the default codepage is not `utf-8`. This solution is based on [this patch downstream](https://github.com/swiftlang/swift/pull/40632/files#diff-e948e4bd7a601e3ca82d596058ccb39326459a4751470eec4d393adeaf516977R37-R38). rdar://156064500
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-platform-windows Author: Charles Zablit (charles-zablit) ChangesThis is a relanding of #149493. The tests were failing because we were interpreting a proper file descriptor as a console file descriptor. This patch uses the Windows APIs to print to the Windows Console, through This fixes a rendering issue where the characters defined in This solution is based on this patch downstream. rdar://156064500 Full diff: https://github.com/llvm/llvm-project/pull/156469.diff 2 Files Affected:
diff --git a/lldb/include/lldb/Host/File.h b/lldb/include/lldb/Host/File.h
index 9e2d0abe0b1af..7402a2231735a 100644
--- a/lldb/include/lldb/Host/File.h
+++ b/lldb/include/lldb/Host/File.h
@@ -382,15 +382,11 @@ class NativeFile : public File {
Unowned = false,
};
- NativeFile() : m_descriptor(kInvalidDescriptor), m_stream(kInvalidStream) {}
+ NativeFile();
- NativeFile(FILE *fh, bool transfer_ownership)
- : m_descriptor(kInvalidDescriptor), m_own_descriptor(false), m_stream(fh),
- m_options(), m_own_stream(transfer_ownership) {}
+ NativeFile(FILE *fh, bool transfer_ownership);
- NativeFile(int fd, OpenOptions options, bool transfer_ownership)
- : m_descriptor(fd), m_own_descriptor(transfer_ownership),
- m_stream(kInvalidStream), m_options(options), m_own_stream(false) {}
+ NativeFile(int fd, OpenOptions options, bool transfer_ownership);
~NativeFile() override { Close(); }
@@ -444,17 +440,19 @@ class NativeFile : public File {
return ValueGuard(m_stream_mutex, StreamIsValidUnlocked());
}
- int m_descriptor;
+ int m_descriptor = kInvalidDescriptor;
bool m_own_descriptor = false;
mutable std::mutex m_descriptor_mutex;
- FILE *m_stream;
+ FILE *m_stream = kInvalidStream;
mutable std::mutex m_stream_mutex;
OpenOptions m_options{};
bool m_own_stream = false;
std::mutex offset_access_mutex;
+ bool is_windows_console = false;
+
private:
NativeFile(const NativeFile &) = delete;
const NativeFile &operator=(const NativeFile &) = delete;
diff --git a/lldb/source/Host/common/File.cpp b/lldb/source/Host/common/File.cpp
index 23b6dc9fe850d..8fd1ca069dc01 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -36,6 +36,7 @@
#include "llvm/Support/Errno.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Process.h"
+#include "llvm/Support/raw_ostream.h"
using namespace lldb;
using namespace lldb_private;
@@ -247,6 +248,32 @@ uint32_t File::GetPermissions(Status &error) const {
return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
}
+NativeFile::NativeFile() = default;
+
+NativeFile::NativeFile(FILE *fh, bool transfer_ownership)
+ : m_stream(fh), m_own_stream(transfer_ownership) {
+#ifdef _WIN32
+ // In order to properly display non ASCII characters in Windows, we need to
+ // use Windows APIs to print to the console. This is only required if the
+ // stream outputs to a console.
+ int fd = _fileno(fh);
+ is_windows_console =
+ ::GetFileType((HANDLE)::_get_osfhandle(fd)) == FILE_TYPE_CHAR;
+#endif
+}
+
+NativeFile::NativeFile(int fd, OpenOptions options, bool transfer_ownership)
+ : m_descriptor(fd), m_own_descriptor(transfer_ownership),
+ m_options(options) {
+#ifdef _WIN32
+ // In order to properly display non ASCII characters in Windows, we need to
+ // use Windows APIs to print to the console. This is only required if the
+ // file outputs to a console.
+ is_windows_console =
+ ::GetFileType((HANDLE)::_get_osfhandle(fd)) == FILE_TYPE_CHAR;
+#endif
+}
+
bool NativeFile::IsValid() const {
std::scoped_lock<std::mutex, std::mutex> lock(m_descriptor_mutex, m_stream_mutex);
return DescriptorIsValidUnlocked() || StreamIsValidUnlocked();
@@ -629,6 +656,13 @@ Status NativeFile::Write(const void *buf, size_t &num_bytes) {
}
if (ValueGuard stream_guard = StreamIsValid()) {
+#ifdef _WIN32
+ if (is_windows_console) {
+ llvm::raw_fd_ostream(_fileno(m_stream), false)
+ .write((char *)buf, num_bytes);
+ return error;
+ }
+#endif
bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);
if (bytes_written == 0) {
|
This is a relanding of llvm#149493. The tests were failing because we were interpreting a proper file descriptor as a console file descriptor. This patch uses the Windows APIs to print to the Windows Console, through `llvm::raw_fd_ostream`. This fixes a rendering issue where the characters defined in `DiagnosticsRendering.cpp` ("╰" for instance) are not rendered properly on Windows out of the box, because the default codepage is not `utf-8`. This solution is based on [this patch downstream](https://github.com/swiftlang/swift/pull/40632/files#diff-e948e4bd7a601e3ca82d596058ccb39326459a4751470eec4d393adeaf516977R37-R38). rdar://156064500
This is a relanding of #149493. The tests were failing because we were interpreting a proper file descriptor as a console file descriptor.
This patch uses the Windows APIs to print to the Windows Console, through
llvm::raw_fd_ostream
.This fixes a rendering issue where the characters defined in
DiagnosticsRendering.cpp
("╰" for instance) are not rendered properly on Windows out of the box, because the default codepage is notutf-8
.This solution is based on this patch downstream.
rdar://156064500