From 6995a2e1bac6812ec4f48b98cfaf321f75c320d8 Mon Sep 17 00:00:00 2001 From: AbigailBuccaneer Date: Mon, 24 Aug 2020 18:04:37 +0100 Subject: [PATCH] Avoid undefined behaviour when byteswapping `a << b` is undefined when `a` is negative, and `a >> b` is implementation-defined. The correct thing to do here is to cast to unsigned, swap the bytes there and then swap back. This also improves performance on some compilers: Clang is smart enough to recognise that we're byteswapping here and reduce it to a single `bswap` instruction on x86_64, but only for the unsigned versions. --- libraries/classparser/src/javaendian.h | 27 +++++--------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/libraries/classparser/src/javaendian.h b/libraries/classparser/src/javaendian.h index 6eb4de63..5a6e107b 100644 --- a/libraries/classparser/src/javaendian.h +++ b/libraries/classparser/src/javaendian.h @@ -22,21 +22,6 @@ inline uint16_t bigswap(uint16_t x) return x; } -inline int64_t bigswap(int64_t x) -{ - return x; -} - -inline int32_t bigswap(int32_t x) -{ - return x; -} - -inline int16_t bigswap(int16_t x) -{ - return x; -} - #else inline uint64_t bigswap(uint64_t x) { @@ -55,22 +40,20 @@ inline uint16_t bigswap(uint16_t x) return (x >> 8) | (x << 8); } +#endif + inline int64_t bigswap(int64_t x) { - return (x >> 56) | ((x << 40) & 0x00FF000000000000) | ((x << 24) & 0x0000FF0000000000) | - ((x << 8) & 0x000000FF00000000) | ((x >> 8) & 0x00000000FF000000) | - ((x >> 24) & 0x0000000000FF0000) | ((x >> 40) & 0x000000000000FF00) | (x << 56); + return static_cast(bigswap(static_cast(x))); } inline int32_t bigswap(int32_t x) { - return (x >> 24) | ((x << 8) & 0x00FF0000) | ((x >> 8) & 0x0000FF00) | (x << 24); + return static_cast(bigswap(static_cast(x))); } inline int16_t bigswap(int16_t x) { - return (x >> 8) | (x << 8); + return static_cast(bigswap(static_cast(x))); } - -#endif }