[PATCH 0/3] MR7231: rpcrt4, winedbg, winevulkan: Various small fixes for Clang 21 + -Werror.
I don't think these are serious problems, but Clang 21's defaults (from llvm-mingw nightly) were resulting in build failures with `--enable-werror` due to `-Wtautological-compare` and `-Wunused-function`. I hope that the buffer wrap checking is somewhat idiomatic/standard. If necessary, I can split off the winevulkan change, but it seems minor enough to be grouped in this MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7231
From: William Horvath <william(a)horvath.blog> --- dlls/rpcrt4/ndr_marshall.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/dlls/rpcrt4/ndr_marshall.c b/dlls/rpcrt4/ndr_marshall.c index c68eb0ee57e..f0249cba953 100644 --- a/dlls/rpcrt4/ndr_marshall.c +++ b/dlls/rpcrt4/ndr_marshall.c @@ -711,9 +711,8 @@ static inline ULONG safe_multiply(ULONG a, ULONG b) static inline void safe_buffer_increment(MIDL_STUB_MESSAGE *pStubMsg, ULONG size) { - if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */ - (pStubMsg->Buffer + size > (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength)) - RpcRaiseException(RPC_X_BAD_STUB_DATA); + if (size > (SIZE_T)(((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) - pStubMsg->Buffer)) + RpcRaiseException(RPC_X_BAD_STUB_DATA); /* integer overflow of pStubMsg->BufferSize */ pStubMsg->Buffer += size; } @@ -732,8 +731,7 @@ static inline void safe_buffer_length_increment(MIDL_STUB_MESSAGE *pStubMsg, ULO * to do so */ static inline void safe_copy_from_buffer(MIDL_STUB_MESSAGE *pStubMsg, void *p, ULONG size) { - if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */ - (pStubMsg->Buffer + size > pStubMsg->BufferEnd)) + if (size > (SIZE_T)(pStubMsg->BufferEnd - pStubMsg->Buffer)) /* integer overflow of pStubMsg->Buffer */ { ERR("buffer overflow - Buffer = %p, BufferEnd = %p, size = %lu\n", pStubMsg->Buffer, pStubMsg->BufferEnd, size); @@ -748,8 +746,7 @@ static inline void safe_copy_from_buffer(MIDL_STUB_MESSAGE *pStubMsg, void *p, U /* copies data to the buffer, checking that there is enough space to do so */ static inline void safe_copy_to_buffer(MIDL_STUB_MESSAGE *pStubMsg, const void *p, ULONG size) { - if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */ - (pStubMsg->Buffer + size > (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength)) + if (size > (SIZE_T)(((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) - pStubMsg->Buffer)) /* integer overflow of pStubMsg->Buffer */ { ERR("buffer overflow - Buffer = %p, BufferEnd = %p, size = %lu\n", pStubMsg->Buffer, (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength, @@ -767,8 +764,7 @@ static void validate_string_data(MIDL_STUB_MESSAGE *pStubMsg, ULONG bufsize, ULO ULONG i; /* verify the buffer is safe to access */ - if ((pStubMsg->Buffer + bufsize < pStubMsg->Buffer) || - (pStubMsg->Buffer + bufsize > pStubMsg->BufferEnd)) + if (bufsize > (SIZE_T)(pStubMsg->BufferEnd - pStubMsg->Buffer)) { ERR("bufsize 0x%lx exceeded buffer end %p of buffer %p\n", bufsize, pStubMsg->BufferEnd, pStubMsg->Buffer); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7231
From: William Horvath <william(a)horvath.blog> --- programs/winedbg/gdbproxy.c | 2 +- programs/winedbg/info.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 2d20c6a4f5c..ccbee243447 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -1632,7 +1632,7 @@ static void packet_query_monitor_mem(struct gdb_context* gdbctx, int len, const packet_reply_hex_to_str(gdbctx, buffer); packet_reply_close(gdbctx); - if (addr + mbi.RegionSize < addr) /* wrap around ? */ + if (mbi.RegionSize > (SIZE_T)-1 - (SIZE_T)addr) /* wrap around ? */ break; addr += mbi.RegionSize; } diff --git a/programs/winedbg/info.c b/programs/winedbg/info.c index 5c1cd100903..14c07f13174 100644 --- a/programs/winedbg/info.c +++ b/programs/winedbg/info.c @@ -913,7 +913,7 @@ void info_win32_virtual(DWORD pid) } dbg_printf("%0*Ix %0*Ix %s %s %s\n", ADDRWIDTH, (DWORD_PTR)addr, ADDRWIDTH, (DWORD_PTR)addr + mbi.RegionSize - 1, state, type, prot); - if (addr + mbi.RegionSize < addr) /* wrap around ? */ + if (mbi.RegionSize > (SIZE_T)-1 - (SIZE_T)addr) /* wrap around ? */ break; addr += mbi.RegionSize; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7231
From: William Horvath <william(a)horvath.blog> When generating conversion functions, we were creating unnecessary input conversions for unions that are members of returnedonly structs. These conversions are never used since the parent struct is only used for output. This prevents creating the vestigial "convert_VkPerformanceValueDataINTEL_win32_to_host" converter, which clang complains about with -Wunused-function set. No functional change intended. --- dlls/winevulkan/make_vulkan | 2 +- dlls/winevulkan/vulkan_thunks.c | 16 ---------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 72a8d548b5d..6fd7debb5c3 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -1402,7 +1402,7 @@ class VkVariable(object): conversions.extend(struct.get_conversions(unwrap, is_const)) for conv in [False, True]: - if struct.needs_conversion(conv, unwrap, Direction.INPUT, is_const): + if not (self.is_union() and self.returnedonly) and struct.needs_conversion(conv, unwrap, Direction.INPUT, is_const): conversions.append(StructConversionFunction(struct, Direction.INPUT, conv, unwrap, is_const)) if struct.needs_conversion(conv, unwrap, Direction.OUTPUT, is_const): conversions.append(StructConversionFunction(struct, Direction.OUTPUT, conv, unwrap, is_const)) diff --git a/dlls/winevulkan/vulkan_thunks.c b/dlls/winevulkan/vulkan_thunks.c index 98b0abda785..7659281c960 100644 --- a/dlls/winevulkan/vulkan_thunks.c +++ b/dlls/winevulkan/vulkan_thunks.c @@ -26084,22 +26084,6 @@ static inline void convert_VkMicromapBuildSizesInfoEXT_host_to_win32(const VkMic out->discardable = in->discardable; } -static inline void convert_VkPerformanceValueDataINTEL_win32_to_host(const VkPerformanceValueDataINTEL32 *in, VkPerformanceValueDataINTEL *out, VkFlags selector) -{ - if (!in) return; - - if (selector == VK_PERFORMANCE_VALUE_TYPE_UINT32_INTEL) - out->value32 = in->value32; - if (selector == VK_PERFORMANCE_VALUE_TYPE_UINT64_INTEL) - out->value64 = in->value64; - if (selector == VK_PERFORMANCE_VALUE_TYPE_FLOAT_INTEL) - out->valueFloat = in->valueFloat; - if (selector == VK_PERFORMANCE_VALUE_TYPE_BOOL_INTEL) - out->valueBool = in->valueBool; - if (selector == VK_PERFORMANCE_VALUE_TYPE_STRING_INTEL) - out->valueString = UlongToPtr(in->valueString); -} - static inline void convert_VkPerformanceValueDataINTEL_host_to_win32(const VkPerformanceValueDataINTEL *in, VkPerformanceValueDataINTEL32 *out, VkFlags selector) { if (!in) return; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7231
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=151163 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4306: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000015B00FA, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
if it's really from clang 21, and as clang 20 isn't even released yet, I think it's better to wait until it's released so that we settle on something stable -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7231#note_93319
On Fri Jan 31 08:09:01 2025 +0000, eric pouech wrote:
if it's really from clang 21, and as clang 20 isn't even released yet, I think it's better to wait until it's released so that we settle on something stable The major version was bumped in the last week:
clang --version
clang version 21.0.0git (/mnt/ssd2/PKGBUILDs/llvm-mingw-toolchain-git/llvm-project 1fdf3340fbc8b6f2dcb3f63b9d6763933b3d3335)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/llvm-mingw/bin
Given that there's only these few extra warnings the compiler emits, it might be worth just updating the code than needing to silence the warnings (assuming these changes are correct). In any case, it's unlikely that they (especially `-Wunused-function`) pointed to anything that caused a problem in practice. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7231#note_93320
participants (4)
-
eric pouech (＠epo) -
Marvin -
William Horvath
-
William Horvath (＠whrvth)