| Message ID | 1477463189-26971-3-git-send-email-nikunj@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/25/2016 11:26 PM, Nikunj A Dadhania wrote: > All the variants for rol/ror have a bug in case where the shift == 0. > For example rol32, would generate: > > return (word << 0) | (word >> 32); > > Which though works, would be flagged as a runtime error on clang's > sanitizer. > > Suggested-by: Richard Henderson <rth@twiddle.net> > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > include/qemu/bitops.h | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Richard Henderson <rth@twiddle.net> r~
On Wed, Oct 26, 2016 at 08:20:10AM -0700, Richard Henderson wrote: > On 10/25/2016 11:26 PM, Nikunj A Dadhania wrote: > > All the variants for rol/ror have a bug in case where the shift == 0. > > For example rol32, would generate: > > > > return (word << 0) | (word >> 32); > > > > Which though works, would be flagged as a runtime error on clang's > > sanitizer. > > > > Suggested-by: Richard Henderson <rth@twiddle.net> > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > > --- > > include/qemu/bitops.h | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > Reviewed-by: Richard Henderson <rth@twiddle.net> This looks fine to me too, but I'm not sure if it should be going via my tree or not.
David Gibson <david@gibson.dropbear.id.au> writes: > [ Unknown signature status ] > On Wed, Oct 26, 2016 at 08:20:10AM -0700, Richard Henderson wrote: >> On 10/25/2016 11:26 PM, Nikunj A Dadhania wrote: >> > All the variants for rol/ror have a bug in case where the shift == 0. >> > For example rol32, would generate: >> > >> > return (word << 0) | (word >> 32); >> > >> > Which though works, would be flagged as a runtime error on clang's >> > sanitizer. >> > >> > Suggested-by: Richard Henderson <rth@twiddle.net> >> > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> > --- >> > include/qemu/bitops.h | 16 ++++++++-------- >> > 1 file changed, 8 insertions(+), 8 deletions(-) >> >> Reviewed-by: Richard Henderson <rth@twiddle.net> > > This looks fine to me too, but I'm not sure if it should be going via > my tree or not. get_maintainer.pl does not help either. Regards Nikunj
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h index 98fb005..1881284 100644 --- a/include/qemu/bitops.h +++ b/include/qemu/bitops.h @@ -218,7 +218,7 @@ static inline unsigned long hweight_long(unsigned long w) */ static inline uint8_t rol8(uint8_t word, unsigned int shift) { - return (word << shift) | (word >> (8 - shift)); + return (word << shift) | (word >> ((8 - shift) & 7)); } /** @@ -228,7 +228,7 @@ static inline uint8_t rol8(uint8_t word, unsigned int shift) */ static inline uint8_t ror8(uint8_t word, unsigned int shift) { - return (word >> shift) | (word << (8 - shift)); + return (word >> shift) | (word << ((8 - shift) & 7)); } /** @@ -238,7 +238,7 @@ static inline uint8_t ror8(uint8_t word, unsigned int shift) */ static inline uint16_t rol16(uint16_t word, unsigned int shift) { - return (word << shift) | (word >> (16 - shift)); + return (word << shift) | (word >> ((16 - shift) & 15)); } /** @@ -248,7 +248,7 @@ static inline uint16_t rol16(uint16_t word, unsigned int shift) */ static inline uint16_t ror16(uint16_t word, unsigned int shift) { - return (word >> shift) | (word << (16 - shift)); + return (word >> shift) | (word << ((16 - shift) & 15)); } /** @@ -258,7 +258,7 @@ static inline uint16_t ror16(uint16_t word, unsigned int shift) */ static inline uint32_t rol32(uint32_t word, unsigned int shift) { - return (word << shift) | (word >> (32 - shift)); + return (word << shift) | (word >> ((32 - shift) & 31)); } /** @@ -268,7 +268,7 @@ static inline uint32_t rol32(uint32_t word, unsigned int shift) */ static inline uint32_t ror32(uint32_t word, unsigned int shift) { - return (word >> shift) | (word << (32 - shift)); + return (word >> shift) | (word << ((32 - shift) & 31)); } /** @@ -278,7 +278,7 @@ static inline uint32_t ror32(uint32_t word, unsigned int shift) */ static inline uint64_t rol64(uint64_t word, unsigned int shift) { - return (word << shift) | (word >> (64 - shift)); + return (word << shift) | (word >> ((64 - shift) & 63)); } /** @@ -288,7 +288,7 @@ static inline uint64_t rol64(uint64_t word, unsigned int shift) */ static inline uint64_t ror64(uint64_t word, unsigned int shift) { - return (word >> shift) | (word << (64 - shift)); + return (word >> shift) | (word << ((64 - shift) & 63)); } /**
All the variants for rol/ror have a bug in case where the shift == 0. For example rol32, would generate: return (word << 0) | (word >> 32); Which though works, would be flagged as a runtime error on clang's sanitizer. Suggested-by: Richard Henderson <rth@twiddle.net> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- include/qemu/bitops.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)