diff mbox

libiberty/md5: fix strict alias warnings

Message ID 201207271436.09109.vapier@gentoo.org
State New
Headers show

Commit Message

Mike Frysinger July 27, 2012, 6:36 p.m. UTC 
Current libiberty md5 code triggers these warnings with gcc-4.7.1 for me:

libiberty/md5.c: In function ‘md5_finish_ctx’:
libiberty/md5.c:117:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
libiberty/md5.c:118:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]

The change below fixes things for me.  The optimized output (-O2) is the same
before/after my change on x86_64-linux.  I imagine it'll be the same for most
targets.  It seems simpler than using a union on the md5_ctx buffer since these
are the only two locations in the code where this occurs.

2012-07-27  Mike Frysinger  <vapier@gentoo.org>

	* md5.c (md5_finish_ctx): Declare swap_bytes.  Assign SWAP() output
	to swap_bytes, and then call memcpy to move it to ctx->buffer.

Comments

Ian Lance Taylor July 27, 2012, 6:45 p.m. UTC | #1 
On Fri, Jul 27, 2012 at 11:36 AM, Mike Frysinger <vapier@gentoo.org> wrote:
>
> 2012-07-27  Mike Frysinger  <vapier@gentoo.org>
>
>         * md5.c (md5_finish_ctx): Declare swap_bytes.  Assign SWAP() output
>         to swap_bytes, and then call memcpy to move it to ctx->buffer.
>
>    /* Take yet unprocessed bytes into account.  */
> -  md5_uint32 bytes = ctx->buflen;
> +  md5_uint32 swap_bytes, bytes = ctx->buflen;

Please add a new line, rather than declaring one uninitialized and one
initialized variable on the same line.

This patch is OK with that change.

Thanks.

Ian
diff mbox

Patch

--- a/libiberty/md5.c
+++ b/libiberty/md5.c
@@ -102,7 +102,7 @@ 
 md5_finish_ctx (struct md5_ctx *ctx, void *resbuf)
 {
   /* Take yet unprocessed bytes into account.  */
-  md5_uint32 bytes = ctx->buflen;
+  md5_uint32 swap_bytes, bytes = ctx->buflen;
   size_t pad;
 
   /* Now count remaining bytes.  */
@@ -113,10 +113,13 @@ 
   pad = bytes >= 56 ? 64 + 56 - bytes : 56 - bytes;
   memcpy (&ctx->buffer[bytes], fillbuf, pad);
 
-  /* Put the 64-bit file length in *bits* at the end of the buffer.  */
-  *(md5_uint32 *) &ctx->buffer[bytes + pad] = SWAP (ctx->total[0] << 3);
-  *(md5_uint32 *) &ctx->buffer[bytes + pad + 4] = SWAP ((ctx->total[1] << 3) |
-							(ctx->total[0] >> 29));
+  /* Put the 64-bit file length in *bits* at the end of the buffer.
+     Use memcpy to avoid aliasing problems.  On most systems, this
+     will be optimized away to the same code.  */
+  swap_bytes = SWAP (ctx->total[0] << 3);
+  memcpy (&ctx->buffer[bytes + pad], &swap_bytes, sizeof (swap_bytes));
+  swap_bytes = SWAP ((ctx->total[1] << 3) | (ctx->total[0] >> 29));
+  memcpy (&ctx->buffer[bytes + pad + 4], &swap_bytes, sizeof (swap_bytes));
 
   /* Process last bytes.  */
   md5_process_block (ctx->buffer, bytes + pad + 8, ctx);