GCC12.2 warns about dereferencing a pointer to RpcPktHdr while it has been allocated to the size of one of the packet (hence smaller in some cases).
/home/eric/work/wine/dlls/rpcrt4/rpc_message.c:111:26: warning: array subscript 'RpcPktHdr[0]' is partly outside array bounds of 'unsigned char[24]' [-Warray-bounds] 111 | Header->common.rpc_ver = RPC_VER_MAJOR;
This patch fixes the warnings by accessing the created object through a pointer to their type (and not through the union).
Notes: - the 'max(sizeof(...), FIELD_OFFSET(...))' thingie in RPCRT4_BuildBindNackHeader avoids also a warning as the FIELD_OFFSET() can be smaller than the size of the structure. This could be avoided by using a flexible array member for the 'protocols' field instead of 'protocols[ANYSIZE_ARRAY]'. - I only changed the allocation routines when the allocated size is smaller than the union. If the strategy is validated, one could consider applying the same allocation strategy to all helpers for symmetry reasons.
Hence the draft status for now, waiting for feedback.
Signed-off-by: Eric Pouech eric.pouech@gmail.com
-- v2: rpcrt4: Fix GCC12.2 warnings. rpcrt4: Pass a common header to RPCRT4_BuildCommonHeader.
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/rpcrt4/rpc_message.c | 40 +++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/dlls/rpcrt4/rpc_message.c b/dlls/rpcrt4/rpc_message.c index a799064e8e9..77aebf44ea1 100644 --- a/dlls/rpcrt4/rpc_message.c +++ b/dlls/rpcrt4/rpc_message.c @@ -106,21 +106,21 @@ static BOOL packet_does_auth_negotiation(const RpcPktHdr *Header) } }
-static VOID RPCRT4_BuildCommonHeader(RpcPktHdr *Header, unsigned char PacketType, +static VOID RPCRT4_BuildCommonHeader(RpcPktCommonHdr *Header, unsigned char PacketType, ULONG DataRepresentation) { - Header->common.rpc_ver = RPC_VER_MAJOR; - Header->common.rpc_ver_minor = RPC_VER_MINOR; - Header->common.ptype = PacketType; - Header->common.drep[0] = LOBYTE(LOWORD(DataRepresentation)); - Header->common.drep[1] = HIBYTE(LOWORD(DataRepresentation)); - Header->common.drep[2] = LOBYTE(HIWORD(DataRepresentation)); - Header->common.drep[3] = HIBYTE(HIWORD(DataRepresentation)); - Header->common.auth_len = 0; - Header->common.call_id = 1; - Header->common.flags = 0; + Header->rpc_ver = RPC_VER_MAJOR; + Header->rpc_ver_minor = RPC_VER_MINOR; + Header->ptype = PacketType; + Header->drep[0] = LOBYTE(LOWORD(DataRepresentation)); + Header->drep[1] = HIBYTE(LOWORD(DataRepresentation)); + Header->drep[2] = LOBYTE(HIWORD(DataRepresentation)); + Header->drep[3] = HIBYTE(HIWORD(DataRepresentation)); + Header->auth_len = 0; + Header->call_id = 1; + Header->flags = 0; /* Flags and fragment length are computed in RPCRT4_Send. */ -} +}
static RpcPktHdr *RPCRT4_BuildRequestHeader(ULONG DataRepresentation, ULONG BufferLength, @@ -137,7 +137,7 @@ static RpcPktHdr *RPCRT4_BuildRequestHeader(ULONG DataRepresentation, return NULL; }
- RPCRT4_BuildCommonHeader(header, PKT_REQUEST, DataRepresentation); + RPCRT4_BuildCommonHeader(&header->common, PKT_REQUEST, DataRepresentation); header->common.frag_len = sizeof(header->request); header->request.alloc_hint = BufferLength; header->request.context_id = 0; @@ -160,7 +160,7 @@ RpcPktHdr *RPCRT4_BuildResponseHeader(ULONG DataRepresentation, ULONG BufferLeng return NULL; }
- RPCRT4_BuildCommonHeader(header, PKT_RESPONSE, DataRepresentation); + RPCRT4_BuildCommonHeader(&header->common, PKT_RESPONSE, DataRepresentation); header->common.frag_len = sizeof(header->response); header->response.alloc_hint = BufferLength;
@@ -176,7 +176,7 @@ RpcPktHdr *RPCRT4_BuildFaultHeader(ULONG DataRepresentation, RPC_STATUS Status) return NULL; }
- RPCRT4_BuildCommonHeader(header, PKT_FAULT, DataRepresentation); + RPCRT4_BuildCommonHeader(&header->common, PKT_FAULT, DataRepresentation); header->common.frag_len = sizeof(header->fault); header->fault.status = Status;
@@ -199,7 +199,7 @@ RpcPktHdr *RPCRT4_BuildBindHeader(ULONG DataRepresentation, } ctxt_elem = (RpcContextElement *)(&header->bind + 1);
- RPCRT4_BuildCommonHeader(header, PKT_BIND, DataRepresentation); + RPCRT4_BuildCommonHeader(&header->common, PKT_BIND, DataRepresentation); header->common.frag_len = sizeof(header->bind) + FIELD_OFFSET(RpcContextElement, transfer_syntaxes[1]); header->bind.max_tsize = MaxTransmissionSize; header->bind.max_rsize = MaxReceiveSize; @@ -220,7 +220,7 @@ static RpcPktHdr *RPCRT4_BuildAuthHeader(ULONG DataRepresentation) if (header == NULL) return NULL;
- RPCRT4_BuildCommonHeader(header, PKT_AUTH3, DataRepresentation); + RPCRT4_BuildCommonHeader(&header->common, PKT_AUTH3, DataRepresentation); header->common.frag_len = sizeof(header->auth3);
return header; @@ -238,7 +238,7 @@ RpcPktHdr *RPCRT4_BuildBindNackHeader(ULONG DataRepresentation, return NULL; }
- RPCRT4_BuildCommonHeader(header, PKT_BIND_NACK, DataRepresentation); + RPCRT4_BuildCommonHeader(&header->common, PKT_BIND_NACK, DataRepresentation); header->common.frag_len = FIELD_OFFSET(RpcPktHdr, bind_nack.protocols[1]); header->bind_nack.reject_reason = RejectReason; header->bind_nack.protocols_count = 1; @@ -270,7 +270,7 @@ RpcPktHdr *RPCRT4_BuildBindAckHeader(ULONG DataRepresentation, return NULL; }
- RPCRT4_BuildCommonHeader(header, PKT_BIND_ACK, DataRepresentation); + RPCRT4_BuildCommonHeader(&header->common, PKT_BIND_ACK, DataRepresentation); header->common.frag_len = header_size; header->bind_ack.max_tsize = MaxTransmissionSize; header->bind_ack.max_rsize = MaxReceiveSize; @@ -299,7 +299,7 @@ RpcPktHdr *RPCRT4_BuildHttpHeader(ULONG DataRepresentation, return NULL; }
- RPCRT4_BuildCommonHeader(header, PKT_HTTP, DataRepresentation); + RPCRT4_BuildCommonHeader(&header->common, PKT_HTTP, DataRepresentation); /* since the packet isn't current sent using RPCRT4_Send, set the flags * manually here */ header->common.flags = RPC_FLG_FIRST|RPC_FLG_LAST;
From: Eric Pouech eric.pouech@gmail.com
GCC12.2 warns about dereferencing a pointer to RpcPktHdr while it has been allocated to the size of one of the packet (hence smaller in some cases).
/home/eric/work/wine/dlls/rpcrt4/rpc_message.c:111:26: warning: array subscript 'RpcPktHdr[0]' is partly outside array bounds of 'unsigned char[24]' [-Warray-bounds] 111 | Header->common.rpc_ver = RPC_VER_MAJOR;
This patch fixes the warnings by accessing the created object through a pointer to their type (and not through the union).
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/rpcrt4/rpc_message.c | 45 ++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/dlls/rpcrt4/rpc_message.c b/dlls/rpcrt4/rpc_message.c index 77aebf44ea1..46736c21fc1 100644 --- a/dlls/rpcrt4/rpc_message.c +++ b/dlls/rpcrt4/rpc_message.c @@ -153,34 +153,34 @@ static RpcPktHdr *RPCRT4_BuildRequestHeader(ULONG DataRepresentation,
RpcPktHdr *RPCRT4_BuildResponseHeader(ULONG DataRepresentation, ULONG BufferLength) { - RpcPktHdr *header; + RpcPktResponseHdr *header;
- header = calloc(1, sizeof(header->response)); + header = calloc(1, sizeof(*header)); if (header == NULL) { return NULL; }
RPCRT4_BuildCommonHeader(&header->common, PKT_RESPONSE, DataRepresentation); - header->common.frag_len = sizeof(header->response); - header->response.alloc_hint = BufferLength; + header->common.frag_len = sizeof(*header); + header->alloc_hint = BufferLength;
- return header; + return (RpcPktHdr *)header; }
RpcPktHdr *RPCRT4_BuildFaultHeader(ULONG DataRepresentation, RPC_STATUS Status) { - RpcPktHdr *header; + RpcPktFaultHdr *header;
- header = calloc(1, sizeof(header->fault)); + header = calloc(1, sizeof(*header)); if (header == NULL) { return NULL; }
RPCRT4_BuildCommonHeader(&header->common, PKT_FAULT, DataRepresentation); - header->common.frag_len = sizeof(header->fault); - header->fault.status = Status; + header->common.frag_len = sizeof(*header); + header->status = Status;
- return header; + return (RpcPktHdr *)header; }
RpcPktHdr *RPCRT4_BuildBindHeader(ULONG DataRepresentation, @@ -214,16 +214,16 @@ RpcPktHdr *RPCRT4_BuildBindHeader(ULONG DataRepresentation,
static RpcPktHdr *RPCRT4_BuildAuthHeader(ULONG DataRepresentation) { - RpcPktHdr *header; + RpcPktAuth3Hdr *header;
- header = calloc(1, sizeof(header->auth3)); + header = calloc(1, sizeof(*header)); if (header == NULL) return NULL;
RPCRT4_BuildCommonHeader(&header->common, PKT_AUTH3, DataRepresentation); - header->common.frag_len = sizeof(header->auth3); + header->common.frag_len = sizeof(*header);
- return header; + return (RpcPktHdr*)header; }
RpcPktHdr *RPCRT4_BuildBindNackHeader(ULONG DataRepresentation, @@ -231,21 +231,22 @@ RpcPktHdr *RPCRT4_BuildBindNackHeader(ULONG DataRepresentation, unsigned char RpcVersionMinor, unsigned short RejectReason) { - RpcPktHdr *header; + RpcPktBindNAckHdr *header; + C_ASSERT(sizeof(*header) >= FIELD_OFFSET(RpcPktBindNAckHdr, protocols[1]));
- header = calloc(1, FIELD_OFFSET(RpcPktHdr, bind_nack.protocols[1])); + header = calloc(1, sizeof(*header)); if (header == NULL) { return NULL; }
RPCRT4_BuildCommonHeader(&header->common, PKT_BIND_NACK, DataRepresentation); - header->common.frag_len = FIELD_OFFSET(RpcPktHdr, bind_nack.protocols[1]); - header->bind_nack.reject_reason = RejectReason; - header->bind_nack.protocols_count = 1; - header->bind_nack.protocols[0].rpc_ver = RpcVersion; - header->bind_nack.protocols[0].rpc_ver_minor = RpcVersionMinor; + header->common.frag_len = FIELD_OFFSET(RpcPktBindNAckHdr, protocols[1]); + header->reject_reason = RejectReason; + header->protocols_count = 1; + header->protocols[0].rpc_ver = RpcVersion; + header->protocols[0].rpc_ver_minor = RpcVersionMinor;
- return header; + return (RpcPktHdr *)header; }
RpcPktHdr *RPCRT4_BuildBindAckHeader(ULONG DataRepresentation,
V2 pushed: - split the changes for RPCRT4_BuildCommonHeader() in a separate commit - no longer bother on using max() for the BindNAck header. Given the alignement, there's always enough room to store 1 protocol (enforcing it with C_ASSERT)
This merge request was approved by Huw Davies.