From a2d16120177a03f101a350a9b2531e4d2ac39233 Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Tue, 30 May 2017 20:13:37 -0400 Subject: [PATCH] bitfield-internal.h: fix negative value shifting warnings MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This patch makes _bt_unsigned_cast() shift the complement of 0 casted to uint64_t instead of the complement of 0 casted to a possibly signed type, which results in shifting a negative value, an undefined operation in C. Because we're imposing uint64_t now, the size of the type and value's type cannot be greater than the size of uint64_t... not a considerable worry with the current state of commercial computing. Having said that, the new _bt_check_max_64bit() macro, a static assertion, checks that those type sizes are not greater than the size of uint64_t at build time. Signed-off-by: Philippe Proulx Signed-off-by: Jérémie Galarneau --- include/babeltrace/bitfield-internal.h | 28 +++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/include/babeltrace/bitfield-internal.h b/include/babeltrace/bitfield-internal.h index d3609e3c..122ccda6 100644 --- a/include/babeltrace/bitfield-internal.h +++ b/include/babeltrace/bitfield-internal.h @@ -59,13 +59,23 @@ #define _bt_is_signed_type(type) ((type) -1 < (type) 0) +/* + * NOTE: The cast to (uint64_t) below ensures that we're not casting a + * negative value, which is undefined in C. However, this limits the + * maximum type size of `type` and `v` to 64-bit. The + * _bt_check_max_64bit() is used to check that the users of this header + * do not use types with a size greater than 64-bit. + */ #define _bt_unsigned_cast(type, v) \ ({ \ (sizeof(v) < sizeof(type)) ? \ - ((type) (v)) & (~(~(type) 0 << (sizeof(v) * CHAR_BIT))) : \ + ((type) (v)) & ((type) (~(~(uint64_t) 0 << (sizeof(v) * CHAR_BIT)))) : \ (type) (v); \ }) +#define _bt_check_max_64bit(type) \ + char _max_64bit_assertion[sizeof(type) <= sizeof(uint64_t) ? 1 : -1] __attribute__((unused)) + /* * bt_bitfield_write - write integer to a bitfield in native endianness * @@ -215,7 +225,7 @@ do { \ #define bt_bitfield_write_le(ptr, type, _start, _length, _v) \ _bt_bitfield_write_le(ptr, type, _start, _length, _v) - + #define bt_bitfield_write_be(ptr, type, _start, _length, _v) \ _bt_bitfield_write_be(ptr, unsigned char, _start, _length, _v) @@ -226,7 +236,7 @@ do { \ #define bt_bitfield_write_le(ptr, type, _start, _length, _v) \ _bt_bitfield_write_le(ptr, unsigned char, _start, _length, _v) - + #define bt_bitfield_write_be(ptr, type, _start, _length, _v) \ _bt_bitfield_write_be(ptr, type, _start, _length, _v) @@ -247,6 +257,10 @@ do { \ unsigned long start_unit, end_unit, this_unit; \ unsigned long end, cshift; /* cshift is "complement shift" */ \ \ + { _bt_check_max_64bit(type); } \ + { _bt_check_max_64bit(typeof(*_vptr)); } \ + { _bt_check_max_64bit(typeof(*_ptr)); } \ + \ if (!__length) { \ *__vptr = 0; \ break; \ @@ -314,6 +328,10 @@ do { \ unsigned long start_unit, end_unit, this_unit; \ unsigned long end, cshift; /* cshift is "complement shift" */ \ \ + { _bt_check_max_64bit(type); } \ + { _bt_check_max_64bit(typeof(*_vptr)); } \ + { _bt_check_max_64bit(typeof(*_ptr)); } \ + \ if (!__length) { \ *__vptr = 0; \ break; \ @@ -383,7 +401,7 @@ do { \ #define bt_bitfield_read_le(_ptr, type, _start, _length, _vptr) \ _bt_bitfield_read_le(_ptr, type, _start, _length, _vptr) - + #define bt_bitfield_read_be(_ptr, type, _start, _length, _vptr) \ _bt_bitfield_read_be(_ptr, unsigned char, _start, _length, _vptr) @@ -394,7 +412,7 @@ do { \ #define bt_bitfield_read_le(_ptr, type, _start, _length, _vptr) \ _bt_bitfield_read_le(_ptr, unsigned char, _start, _length, _vptr) - + #define bt_bitfield_read_be(_ptr, type, _start, _length, _vptr) \ _bt_bitfield_read_be(_ptr, type, _start, _length, _vptr) -- 2.34.1