]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
Fix undefined left shift with signed ints 30/head
authorSean McBride <sean@rogue-research.com>
Thu, 29 Nov 2012 23:20:11 +0000 (18:20 -0500)
committerBrad King <brad.king@kitware.com>
Fri, 30 Nov 2012 13:06:58 +0000 (08:06 -0500)
Caught by Clang's -fsanitize=shift.  A small unsigned int was promoted,
according to C's regular promotion rules, to a signed int, it was then
left shifted.  This sometimes pushed a 1 into the sign bit, which is
undefined behaviour.  Fix by using unsigned temporaries.

libarchive/archive_endian.h

index 68123b0dd6a34a10a7af883f8048f1426351eacd..750e190b9df1331a181d2137225efebbf1d155d9 100644 (file)
@@ -58,7 +58,13 @@ archive_be16dec(const void *pp)
 {
        unsigned char const *p = (unsigned char const *)pp;
 
-       return ((p[0] << 8) | p[1]);
+       /* Store into unsigned temporaries before left shifting, to avoid
+       promotion to signed int and then left shifting into the sign bit,
+       which is undefined behaviour. */
+       unsigned int p1 = p[1];
+       unsigned int p0 = p[0];
+
+       return ((p0 << 8) | p1);
 }
 
 static inline uint32_t
@@ -66,7 +72,15 @@ archive_be32dec(const void *pp)
 {
        unsigned char const *p = (unsigned char const *)pp;
 
-       return ((p[0] << 24) | (p[1] << 16) | (p[2] << 8) | p[3]);
+       /* Store into unsigned temporaries before left shifting, to avoid
+       promotion to signed int and then left shifting into the sign bit,
+       which is undefined behaviour. */
+       unsigned int p3 = p[3];
+       unsigned int p2 = p[2];
+       unsigned int p1 = p[1];
+       unsigned int p0 = p[0];
+
+       return ((p0 << 24) | (p1 << 16) | (p2 << 8) | p3);
 }
 
 static inline uint64_t
@@ -82,7 +96,13 @@ archive_le16dec(const void *pp)
 {
        unsigned char const *p = (unsigned char const *)pp;
 
-       return ((p[1] << 8) | p[0]);
+       /* Store into unsigned temporaries before left shifting, to avoid
+       promotion to signed int and then left shifting into the sign bit,
+       which is undefined behaviour. */
+       unsigned int p1 = p[1];
+       unsigned int p0 = p[0];
+
+       return ((p1 << 8) | p0);
 }
 
 static inline uint32_t
@@ -90,7 +110,15 @@ archive_le32dec(const void *pp)
 {
        unsigned char const *p = (unsigned char const *)pp;
 
-       return ((p[3] << 24) | (p[2] << 16) | (p[1] << 8) | p[0]);
+       /* Store into unsigned temporaries before left shifting, to avoid
+       promotion to signed int and then left shifting into the sign bit,
+       which is undefined behaviour. */
+       unsigned int p3 = p[3];
+       unsigned int p2 = p[2];
+       unsigned int p1 = p[1];
+       unsigned int p0 = p[0];
+
+       return ((p3 << 24) | (p2 << 16) | (p1 << 8) | p0);
 }
 
 static inline uint64_t