Skip to content

[lldb] Fixing warnings / win32 builds in MainLoop. #146632

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

Merged
merged 3 commits into from
Jul 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions lldb/include/lldb/Host/windows/MainLoopWindows.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

namespace lldb_private {

using handle_t = void *;

// Windows-specific implementation of the MainLoopBase class. It can monitor
// socket descriptors for readability using WSAEventSelect. Non-socket file
// descriptors are not supported.
Expand All @@ -33,15 +35,15 @@ class MainLoopWindows : public MainLoopBase {

class IOEvent {
public:
IOEvent(IOObject::WaitableHandle event) : m_event(event) {}
IOEvent(handle_t event) : m_event(event) {}
virtual ~IOEvent() {}
virtual void WillPoll() {}
virtual void DidPoll() {}
virtual void Disarm() {}
IOObject::WaitableHandle GetHandle() { return m_event; }
handle_t GetHandle() { return m_event; }

protected:
IOObject::WaitableHandle m_event;
handle_t m_event;
};
using IOEventUP = std::unique_ptr<IOEvent>;

Expand Down
8 changes: 7 additions & 1 deletion lldb/include/lldb/lldb-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ typedef void *rwlock_t;
typedef void *process_t; // Process type is HANDLE
typedef void *thread_t; // Host thread type
typedef void *file_t; // Host file type
typedef unsigned int __w64 socket_t; // Host socket type
typedef uintptr_t socket_t; // Host socket type
typedef void *thread_arg_t; // Host thread argument type
typedef unsigned thread_result_t; // Host thread result type
typedef thread_result_t (*thread_func_t)(void *); // Host thread function type
typedef void *pipe_t; // Host pipe type is HANDLE

// printf macro for file_t
#define PRIuFT PRIuPTR

Comment on lines +51 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we remove this part. It's technically a part of our public API. I'd recommend using llvm::formatv and LLDB_LOG (not LLDB_LOGF) to format these. It should not have this issue as it doesn't require a type specification.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm trying to fix Windows warnings at the minute, I'll try this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks.

#else

#include <pthread.h>
Expand All @@ -63,6 +66,9 @@ typedef void *thread_result_t; // Host thread result type
typedef void *(*thread_func_t)(void *); // Host thread function type
typedef int pipe_t; // Host pipe type

// printf macro for file_t
#define PRIuFT PRIi32

#endif // _WIN32

#define LLDB_INVALID_PROCESS ((lldb::process_t)-1)
Expand Down
3 changes: 2 additions & 1 deletion lldb/source/Host/common/JSONTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ ReadFull(IOObject &descriptor, size_t length,
if (timeout && timeout_supported) {
SelectHelper sh;
sh.SetTimeout(*timeout);
sh.FDSetRead((lldb::socket_t)descriptor.GetWaitableHandle());
sh.FDSetRead(
reinterpret_cast<lldb::socket_t>(descriptor.GetWaitableHandle()));
Status status = sh.Select();
if (status.Fail()) {
// Convert timeouts into a specific error.
Expand Down
10 changes: 5 additions & 5 deletions lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,10 @@ size_t ConnectionFileDescriptor::Read(void *dst, size_t dst_len,

if (log) {
LLDB_LOGF(log,
"%p ConnectionFileDescriptor::Read() fd = %" PRIu64
"%p ConnectionFileDescriptor::Read() fd = %" PRIuFT
", dst = %p, dst_len = %" PRIu64 ") => %" PRIu64 ", error = %s",
static_cast<void *>(this),
static_cast<uint64_t>(m_io_sp->GetWaitableHandle()),
static_cast<file_t>(m_io_sp->GetWaitableHandle()),
static_cast<void *>(dst), static_cast<uint64_t>(dst_len),
static_cast<uint64_t>(bytes_read), error.AsCString());
}
Expand Down Expand Up @@ -377,10 +377,10 @@ size_t ConnectionFileDescriptor::Write(const void *src, size_t src_len,

if (log) {
LLDB_LOGF(log,
"%p ConnectionFileDescriptor::Write(fd = %" PRIu64
"%p ConnectionFileDescriptor::Write(fd = %" PRIuFT
", src = %p, src_len = %" PRIu64 ") => %" PRIu64 " (error = %s)",
static_cast<void *>(this),
static_cast<uint64_t>(m_io_sp->GetWaitableHandle()),
static_cast<file_t>(m_io_sp->GetWaitableHandle()),
static_cast<const void *>(src), static_cast<uint64_t>(src_len),
static_cast<uint64_t>(bytes_sent), error.AsCString());
}
Expand Down Expand Up @@ -452,7 +452,7 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout<std::micro> &timeout,
select_helper.SetTimeout(*timeout);

// FIXME: Migrate to MainLoop.
select_helper.FDSetRead((lldb::socket_t)handle);
select_helper.FDSetRead(reinterpret_cast<socket_t>(handle));
#if defined(_WIN32)
// select() won't accept pipes on Windows. The entire Windows codepath
// needs to be converted over to using WaitForMultipleObjects and event
Expand Down
49 changes: 28 additions & 21 deletions lldb/source/Host/windows/MainLoopWindows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "lldb/Host/windows/MainLoopWindows.h"
#include "lldb/Host/Config.h"
#include "lldb/Host/Socket.h"
#include "lldb/Host/windows/windows.h"
#include "lldb/Utility/Status.h"
#include "llvm/Config/llvm-config.h"
#include "llvm/Support/Casting.h"
Expand All @@ -18,6 +19,8 @@
#include <cerrno>
#include <csignal>
#include <ctime>
#include <io.h>
#include <thread>
#include <vector>
#include <winsock2.h>

Expand All @@ -39,9 +42,8 @@ namespace {
class PipeEvent : public MainLoopWindows::IOEvent {
public:
explicit PipeEvent(HANDLE handle)
: IOEvent((IOObject::WaitableHandle)CreateEventW(
NULL, /*bManualReset=*/FALSE,
/*bInitialState=*/FALSE, NULL)),
: IOEvent(CreateEventW(NULL, /*bManualReset=*/FALSE,
/*bInitialState=*/FALSE, NULL)),
m_handle(handle), m_ready(CreateEventW(NULL, /*bManualReset=*/FALSE,
/*bInitialState=*/FALSE, NULL)) {
assert(m_event && m_ready);
Expand All @@ -53,12 +55,12 @@ class PipeEvent : public MainLoopWindows::IOEvent {
SetEvent(m_ready);
// Keep trying to cancel ReadFile() until the thread exits.
do {
CancelIoEx((HANDLE)m_handle, /*lpOverlapped=*/NULL);
CancelIoEx(m_handle, /*lpOverlapped=*/NULL);
} while (WaitForSingleObject(m_monitor_thread.native_handle(), 1) ==
WAIT_TIMEOUT);
m_monitor_thread.join();
}
CloseHandle((HANDLE)m_event);
CloseHandle(m_event);
CloseHandle(m_ready);
}

Expand Down Expand Up @@ -107,7 +109,7 @@ class PipeEvent : public MainLoopWindows::IOEvent {
continue;
}

SetEvent((HANDLE)m_event);
SetEvent(m_event);

// Wait until the current read is consumed before doing the next read.
WaitForSingleObject(m_ready, INFINITE);
Expand All @@ -124,15 +126,15 @@ class PipeEvent : public MainLoopWindows::IOEvent {
class SocketEvent : public MainLoopWindows::IOEvent {
public:
explicit SocketEvent(SOCKET socket)
: IOEvent((IOObject::WaitableHandle)WSACreateEvent()), m_socket(socket) {
: IOEvent(WSACreateEvent()), m_socket(socket) {
assert(m_event != WSA_INVALID_EVENT);
}

~SocketEvent() override { WSACloseEvent((HANDLE)m_event); }
~SocketEvent() override { WSACloseEvent(m_event); }

void WillPoll() {
int result = WSAEventSelect(m_socket, (HANDLE)m_event,
FD_READ | FD_ACCEPT | FD_CLOSE);
int result =
WSAEventSelect(m_socket, m_event, FD_READ | FD_ACCEPT | FD_CLOSE);
assert(result == 0);
UNUSED_IF_ASSERT_DISABLED(result);
}
Expand All @@ -143,7 +145,7 @@ class SocketEvent : public MainLoopWindows::IOEvent {
UNUSED_IF_ASSERT_DISABLED(result);
}

void Disarm() override { WSAResetEvent((HANDLE)m_event); }
void Disarm() override { WSAResetEvent(m_event); }

SOCKET m_socket;
};
Expand All @@ -167,7 +169,7 @@ llvm::Expected<size_t> MainLoopWindows::Poll() {
events.reserve(m_read_fds.size() + 1);
for (auto &[_, fd_info] : m_read_fds) {
fd_info.event->WillPoll();
events.push_back((HANDLE)fd_info.event->GetHandle());
events.push_back(fd_info.event->GetHandle());
}
events.push_back(m_interrupt_event);

Expand Down Expand Up @@ -206,16 +208,21 @@ MainLoopWindows::RegisterReadObject(const IOObjectSP &object_sp,
return nullptr;
}

if (object_sp->GetFdType() == IOObject::eFDTypeSocket)
if (object_sp->GetFdType() == IOObject::eFDTypeSocket) {
m_read_fds[waitable_handle] = {
std::make_unique<SocketEvent>((SOCKET)waitable_handle), callback};
else if (GetFileType(waitable_handle) == FILE_TYPE_PIPE)
m_read_fds[waitable_handle] = {
std::make_unique<PipeEvent>((HANDLE)waitable_handle), callback};
else {
error = Status::FromErrorStringWithFormat("Unsupported file type %d",
GetFileType(waitable_handle));
return nullptr;
std::make_unique<SocketEvent>(
reinterpret_cast<SOCKET>(waitable_handle)),
callback};
} else {
DWORD file_type = GetFileType(waitable_handle);
if (file_type != FILE_TYPE_PIPE) {
error = Status::FromErrorStringWithFormat("Unsupported file type %d",
file_type);
return nullptr;
}

m_read_fds[waitable_handle] = {std::make_unique<PipeEvent>(waitable_handle),
callback};
}

return CreateReadHandle(object_sp);
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Utility/SelectHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ lldb_private::Status SelectHelper::Select() {
#endif
// Set the FD bits in the fdsets for read/write/error
for (auto &pair : m_fd_map) {
const lldb::socket_t fd = pair.first;
lldb::socket_t fd = pair.first;

if (pair.second.read_set)
FD_SET(fd, read_fdset_ptr);
Expand Down
Loading