Patchwork [libiberty] correct md5_process_bytes with unaligned pointers

login
register
mail settings
Submitter Pierre Vittet
Date Sept. 16, 2011, 12:46 p.m.
Message ID <4E734541.7030204@pvittet.com>
Download mbox | patch
Permalink /patch/114924/
State New
Headers show

Comments

Pierre Vittet - Sept. 16, 2011, 12:46 p.m.
Hello,

The patch is the result of the following threads:

Here is a patch correcting md5_process_bytes when we are in the case of
unaligned pointers.A pair of brace was missing, leading the buffer to be
shift 2 times losing a part of its content.


The patch also remove a preprocessor #if testing if
_STRING_ARCH_unaligned is defined. This symbol is never defined in gcc
and could be only used in CFLAGS. Looking at the code, it does not looks
usefull to define it (and it is only tested on libiberty/md5.c and
libiberty/sha1.c), as we already check the pointer alignement, so
removing it clean the code. I searched on google, and it does not looks
to be used. Does anyone want it or thing that it should not be removed?

Ok for trunk ?

Thanks!

Pierre Vittet

PS: I also write a small gcc plugin, allowing to easily test
md5_process_bytes, if can change your environment in a way where the
pointer buffer is not aligned, you should get the bug.
2011-09-16  Pierre Vittet  <piervit@pvittet.com>

	* md5.c (md5_process_bytes): Remove unused _STRING_ARCH_unaligned, add
	missing braces.
Basile Starynkevitch - Sept. 16, 2011, 4:54 p.m.
On Fri, 16 Sep 2011 14:46:57 +0200
Pierre Vittet <piervit@pvittet.com> wrote:

> Hello,
[...]

> The patch also remove a preprocessor #if testing if
> _STRING_ARCH_unaligned is defined. This symbol is never defined in gcc
> and could be only used in CFLAGS. Looking at the code, it does not looks
> usefull to define it (and it is only tested on libiberty/md5.c and
> libiberty/sha1.c), as we already check the pointer alignement, so
> removing it clean the code. I searched on google, and it does not looks
> to be used. Does anyone want it or thing that it should not be removed?
> 
> Ok for trunk ?


I can't formally approve this patch, but I do hope it will be reviewed and approved soon. 

See http://gcc.gnu.org/ml/gcc-help/2011-09/msg00126.html
and http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00963.html
and http://groups.google.com/group/gcc-melt/browse_thread/thread/292c394fea5089c7

Regards.
Pierre Vittet - Sept. 19, 2011, 2:45 p.m.
Hello,

Ping!
I would like to get a return on this patch. I don't know quite well the
status of libiberty in GNU, please if I must this patch on another
mailing list, please say me on which.

Thanks!

Pierre Vittet

Patch

Index: libiberty/md5.c
===================================================================
--- libiberty/md5.c	(révision 178905)
+++ libiberty/md5.c	(copie de travail)
@@ -227,7 +227,6 @@  md5_process_bytes (const void *buffer, size_t len,
   /* Process available complete blocks.  */
   if (len > 64)
     {
-#if !_STRING_ARCH_unaligned
 /* To check alignment gcc has an appropriate operator.  Other
    compilers don't.  */
 # if __GNUC__ >= 2
@@ -244,10 +243,11 @@  md5_process_bytes (const void *buffer, size_t len,
             len -= 64;
           }
       else
-#endif
-      md5_process_block (buffer, len & ~63, ctx);
-      buffer = (const void *) ((const char *) buffer + (len & ~63));
-      len &= 63;
+	{
+	  md5_process_block (buffer, len & ~63, ctx);
+	  buffer = (const void *) ((const char *) buffer + (len & ~63));
+	  len &= 63;
+	}
     }
 
   /* Move remaining bytes in internal buffer.  */