From cf0f28ad3c13c7dc964b120e336cc802b20b442b Mon Sep 17 00:00:00 2001 From: Sean McBride Date: Thu, 29 Nov 2012 18:20:11 -0500 Subject: [PATCH] Fix undefined left shift with signed ints 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 | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/libarchive/archive_endian.h b/libarchive/archive_endian.h index 68123b0dd..750e190b9 100644 --- a/libarchive/archive_endian.h +++ b/libarchive/archive_endian.h @@ -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 -- 2.47.2