-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[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
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
Commit 1a7b7e2 introduced a few casting warnings and a build issue in Win32 platforms. Trying to correct the casts to c++ style casts instead of C style casts.
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesCommit 1a7b7e2 introduced a few casting warnings and a build issue in Win32 platforms. Trying to correct the casts to c++ style casts instead of C style casts. Full diff: https://github.com/llvm/llvm-project/pull/146632.diff 6 Files Affected:
diff --git a/lldb/include/lldb/Host/windows/MainLoopWindows.h b/lldb/include/lldb/Host/windows/MainLoopWindows.h
index 53df815255c3d..705e7e78ba48a 100644
--- a/lldb/include/lldb/Host/windows/MainLoopWindows.h
+++ b/lldb/include/lldb/Host/windows/MainLoopWindows.h
@@ -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.
@@ -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>;
diff --git a/lldb/include/lldb/lldb-types.h b/lldb/include/lldb/lldb-types.h
index 15781ad626efb..cef43892c8efa 100644
--- a/lldb/include/lldb/lldb-types.h
+++ b/lldb/include/lldb/lldb-types.h
@@ -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
+
#else
#include <pthread.h>
@@ -63,11 +66,14 @@ 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)
+#define LLDB_INVALID_PROCESS ((lldb::process_t) - 1)
#define LLDB_INVALID_HOST_THREAD ((lldb::thread_t)NULL)
-#define LLDB_INVALID_PIPE ((lldb::pipe_t)-1)
+#define LLDB_INVALID_PIPE ((lldb::pipe_t) - 1)
#define LLDB_INVALID_CALLBACK_TOKEN ((lldb::callback_token_t) - 1)
typedef void (*LogOutputCallback)(const char *, void *baton);
diff --git a/lldb/source/Host/common/JSONTransport.cpp b/lldb/source/Host/common/JSONTransport.cpp
index bf269ffa45966..546c12c8f7114 100644
--- a/lldb/source/Host/common/JSONTransport.cpp
+++ b/lldb/source/Host/common/JSONTransport.cpp
@@ -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.
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 44a3ed2e59d5f..83eb0c56853b3 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -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());
}
@@ -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());
}
@@ -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
diff --git a/lldb/source/Host/windows/MainLoopWindows.cpp b/lldb/source/Host/windows/MainLoopWindows.cpp
index abb4345b011e4..7ce5277329d9f 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -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"
@@ -18,6 +19,7 @@
#include <cerrno>
#include <csignal>
#include <ctime>
+#include <io.h>
#include <vector>
#include <winsock2.h>
@@ -39,9 +41,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);
@@ -53,12 +54,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);
}
@@ -107,7 +108,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);
@@ -124,15 +125,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);
}
@@ -143,7 +144,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;
};
@@ -167,7 +168,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);
@@ -206,16 +207,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);
diff --git a/lldb/source/Utility/SelectHelper.cpp b/lldb/source/Utility/SelectHelper.cpp
index 34c1b62228fdc..3044cdae110ed 100644
--- a/lldb/source/Utility/SelectHelper.cpp
+++ b/lldb/source/Utility/SelectHelper.cpp
@@ -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);
|
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.
This fixes compilation on 64 bit mingw for me at least.
Please look at the following problem Upd: most probably this test has been fixed here #146562. |
We have the same one, I'll take care of it. |
// printf macro for file_t | ||
#define PRIuFT PRIuPTR | ||
|
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.
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.
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.
I'm trying to fix Windows warnings at the minute, I'll try this.
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.
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.
Thanks.
Commit 1a7b7e2 introduced a few casting warnings and a build issue in Win32 platforms.
Trying to correct the casts to c++ style casts instead of C style casts.