[PATCH v3 0/1] MR7231: rpcrt4: Change some buffer wrap checks to satisfy Clang 20.

[William Horvath (＠whrvt)]

Soon-to-be-released Clang 20 introduced a [`-Wtautological-compare`](https://github.com/llvm/llvm-project/commit/6d34cfac53b993a6cdf3d6669e017eac...) warning which these checks trigger. Update them in a similar fashion to the example shown in the [Clang doc](https://github.com/llvm/llvm-project/blob/release/20.x/clang/docs/ReleaseNot...) (see "To avoid pointer addition overflow...").

--
  v3: rpcrt4: Pointer arithmetic fixes (-Wtautological-compare).

https://gitlab.winehq.org/wine/wine/-/merge_requests/7231

[PATCH v3 1/1] rpcrt4: Pointer arithmetic fixes (-Wtautological-compare).

[William Horvath]

From: William Horvath

---
 dlls/rpcrt4/ndr_marshall.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/dlls/rpcrt4/ndr_marshall.c b/dlls/rpcrt4/ndr_marshall.c
index c68eb0ee57e..ddb69001911 100644
--- a/dlls/rpcrt4/ndr_marshall.c
+++ b/dlls/rpcrt4/ndr_marshall.c
@@ -711,8 +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))
+    if (((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) < pStubMsg->Buffer || /* integer overflow of pStubMsg->BufferSize */
+        size > ((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) - pStubMsg->Buffer)
         RpcRaiseException(RPC_X_BAD_STUB_DATA);
     pStubMsg->Buffer += size;
 }
@@ -732,8 +732,8 @@ 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 (pStubMsg->BufferEnd < pStubMsg->Buffer || /* integer overflow of pStubMsg->Buffer */
+        size > (pStubMsg->BufferEnd - pStubMsg->Buffer))
     {
         ERR("buffer overflow - Buffer = %p, BufferEnd = %p, size = %lu\n",
             pStubMsg->Buffer, pStubMsg->BufferEnd, size);
@@ -748,8 +748,8 @@ 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 (((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) < pStubMsg->Buffer || /* integer overflow of pStubMsg->Buffer */
+        size > ((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) - pStubMsg->Buffer)
     {
         ERR("buffer overflow - Buffer = %p, BufferEnd = %p, size = %lu\n",
             pStubMsg->Buffer, (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength,
@@ -767,8 +767,8 @@ 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 (pStubMsg->BufferEnd < pStubMsg->Buffer ||
+        bufsize > (pStubMsg->BufferEnd - pStubMsg->Buffer))
     {
         ERR("bufsize 0x%lx exceeded buffer end %p of buffer %p\n", bufsize,
             pStubMsg->BufferEnd, pStubMsg->Buffer);

[William Horvath (＠whrvt)]

On Mon Feb 17 18:00:51 2025 +0000, William Horvath wrote:
> changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/7231/diffs?diff_id=158136&start_sha=294e2338a222d73247412b989272c08d3e01a6d9#3149510afd8a6b07ec8f8b4b4a493ffae70d2105_734_735)

That's a good point. I've updated each case to validate the pointer order before subtraction, without casting to `SIZE_T`.