lttng-dev.lists.lttng.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 babeltrace 2/4] Silence compiler "always false comparison" warning
       [not found] <20190516205848.20409-1-mathieu.desnoyers@efficios.com>
@ 2019-05-16 20:58 ` Mathieu Desnoyers
  2019-05-16 20:58 ` [PATCH babeltrace 3/4] Cleanup: bitfields: streamline use of underscores Mathieu Desnoyers
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-16 20:58 UTC (permalink / raw)
  To: jgalar, simon.marchi; +Cc: lttng-dev

Compiling the bitfield test with gcc -Wextra generates those warnings:

 ../../include/babeltrace/bitfield-internal.h:38:45: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
 #define _bt_is_signed_type(type) ((type) -1 < (type) 0)

This is the intent of the macro. Disable compiler warnings around use of
that macro.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I2dd980e11ebd6cd37ae71b013925a6fca5d7b483
---
Changes since v1:
- Use stdbool.h.
---
 include/babeltrace/bitfield-internal.h | 35 ++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/include/babeltrace/bitfield-internal.h b/include/babeltrace/bitfield-internal.h
index 5ecde05e..bb0283a3 100644
--- a/include/babeltrace/bitfield-internal.h
+++ b/include/babeltrace/bitfield-internal.h
@@ -24,6 +24,7 @@
  */
 
 #include <stdint.h>	/* C99 5.2.4.2 Numerical limits */
+#include <stdbool.h>	/* C99 7.16 bool type */
 #include <babeltrace/compat/limits-internal.h>	/* C99 5.2.4.2 Numerical limits */
 #include <babeltrace/endian-internal.h>	/* Non-standard BIG_ENDIAN, LITTLE_ENDIAN, BYTE_ORDER */
 
@@ -40,6 +41,26 @@
 #error "bitfield.h requires the compiler representation of signed integers to be two's complement."
 #endif
 
+/*
+ * _bt_is_signed_type() willingly generates comparison of unsigned
+ * expression < 0, which is always false. Silence compiler warnings.
+ */
+#ifdef __GNUC__
+# define _BT_DIAG_PUSH			_Pragma("GCC diagnostic push")
+# define _BT_DIAG_POP			_Pragma("GCC diagnostic pop")
+
+# define _BT_DIAG_STRINGIFY_1(x)	#x
+# define _BT_DIAG_STRINGIFY(x)		_BT_DIAG_STRINGIFY_1(x)
+
+# define _BT_DIAG_IGNORE(option)	\
+	_Pragma(_BT_DIAG_STRINGIFY(GCC diagnostic ignored option))
+# define _BT_DIAG_IGNORE_TYPE_LIMITS	_BT_DIAG_IGNORE("-Wtype-limits")
+#else
+# define _BT_DIAG_PUSH
+# define _BT_DIAG_POP
+# define _BT_DIAG_IGNORE
+#endif
+
 #define _bt_is_signed_type(type)	((type) -1 < (type) 0)
 
 /*
@@ -359,6 +380,7 @@ do {									\
 	unsigned long ts = sizeof(type) * CHAR_BIT; /* type size */	\
 	unsigned long start_unit, end_unit, this_unit;			\
 	unsigned long end, cshift; /* cshift is "complement shift" */	\
+	bool is_signed_type;						\
 									\
 	if (!__length) {						\
 		*__vptr = 0;						\
@@ -370,7 +392,11 @@ do {									\
 	end_unit = (end + (ts - 1)) / ts;				\
 									\
 	this_unit = end_unit - 1;					\
-	if (_bt_is_signed_type(__typeof__(__v))				\
+	_BT_DIAG_PUSH							\
+	_BT_DIAG_IGNORE_TYPE_LIMITS					\
+	is_signed_type = _bt_is_signed_type(__typeof__(__v));		\
+	_BT_DIAG_POP							\
+	if (is_signed_type						\
 	    && (__ptr[this_unit] & _bt_lshift((type) 1, (end % ts ? end % ts : ts) - 1))) \
 		__v = ~(__typeof__(__v)) 0;				\
 	else								\
@@ -426,6 +452,7 @@ do {									\
 	unsigned long ts = sizeof(type) * CHAR_BIT; /* type size */	\
 	unsigned long start_unit, end_unit, this_unit;			\
 	unsigned long end, cshift; /* cshift is "complement shift" */	\
+	bool is_signed_type;						\
 									\
 	if (!__length) {						\
 		*__vptr = 0;						\
@@ -437,7 +464,11 @@ do {									\
 	end_unit = (end + (ts - 1)) / ts;				\
 									\
 	this_unit = start_unit;						\
-	if (_bt_is_signed_type(__typeof__(__v))				\
+	_BT_DIAG_PUSH							\
+	_BT_DIAG_IGNORE_TYPE_LIMITS					\
+	is_signed_type = _bt_is_signed_type(__typeof__(__v));		\
+	_BT_DIAG_POP							\
+	if (is_signed_type						\
 	    && (__ptr[this_unit] & _bt_lshift((type) 1, ts - (__start % ts) - 1))) \
 		__v = ~(__typeof__(__v)) 0;				\
 	else								\
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH babeltrace 3/4] Cleanup: bitfields: streamline use of underscores
       [not found] <20190516205848.20409-1-mathieu.desnoyers@efficios.com>
  2019-05-16 20:58 ` [PATCH v2 babeltrace 2/4] Silence compiler "always false comparison" warning Mathieu Desnoyers
@ 2019-05-16 20:58 ` Mathieu Desnoyers
  2019-05-16 20:58 ` [PATCH v2 babeltrace 4/4] Extend test_bitfield coverage Mathieu Desnoyers
  2019-05-17 20:44 ` [PATCH v8 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors Jérémie Galarneau
  3 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-16 20:58 UTC (permalink / raw)
  To: jgalar, simon.marchi; +Cc: lttng-dev

Do not prefix macro arguments with underscores. Use one leading
underscore as prefix for local variables defined within macros.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ie1c2f7f59f605ac62d483aba67b3f70cef27bf21
---
 include/babeltrace/bitfield-internal.h | 436 ++++++++++++++++-----------------
 1 file changed, 218 insertions(+), 218 deletions(-)

diff --git a/include/babeltrace/bitfield-internal.h b/include/babeltrace/bitfield-internal.h
index bb0283a3..6969179c 100644
--- a/include/babeltrace/bitfield-internal.h
+++ b/include/babeltrace/bitfield-internal.h
@@ -218,122 +218,122 @@ do {									\
  * Also, consecutive bitfields are placed from higher to lower bits.
  */
 
-#define _bt_bitfield_write_le(_ptr, type, _start, _length, _v)		\
+#define _bt_bitfield_write_le(ptr, type, start, length, v)		\
 do {									\
-	__typeof__(_v) __v = (_v);					\
-	type *__ptr = (void *) (_ptr);					\
-	unsigned long __start = (_start), __length = (_length);		\
-	type mask, cmask;						\
-	unsigned long ts = sizeof(type) * CHAR_BIT; /* type size */	\
-	unsigned long start_unit, end_unit, this_unit;			\
-	unsigned long end, cshift; /* cshift is "complement shift" */	\
+	__typeof__(v) _v = (v);					\
+	type *_ptr = (void *) (ptr);					\
+	unsigned long _start = (start), _length = (length);		\
+	type _mask, _cmask;						\
+	unsigned long _ts = sizeof(type) * CHAR_BIT; /* type size */	\
+	unsigned long _start_unit, _end_unit, _this_unit;		\
+	unsigned long _end, _cshift; /* _cshift is "complement shift" */ \
 									\
-	if (!__length)							\
+	if (!_length)							\
 		break;							\
 									\
-	end = __start + __length;					\
-	start_unit = __start / ts;					\
-	end_unit = (end + (ts - 1)) / ts;				\
+	_end = _start + _length;					\
+	_start_unit = _start / _ts;					\
+	_end_unit = (_end + (_ts - 1)) / _ts;				\
 									\
 	/* Trim v high bits */						\
-	if (__length < sizeof(__v) * CHAR_BIT)				\
-		__v &= _bt_make_mask(__typeof__(__v), __length);	\
+	if (_length < sizeof(_v) * CHAR_BIT)				\
+		_v &= _bt_make_mask(__typeof__(_v), _length);		\
 									\
 	/* We can now append v with a simple "or", shift it piece-wise */ \
-	this_unit = start_unit;						\
-	if (start_unit == end_unit - 1) {				\
-		mask = _bt_make_mask(type, __start % ts);		\
-		if (end % ts)						\
-			mask |= _bt_make_mask_complement(type, end % ts); \
-		cmask = _bt_lshift((type) (__v), __start % ts);		\
-		cmask &= ~mask;						\
-		__ptr[this_unit] &= mask;				\
-		__ptr[this_unit] |= cmask;				\
+	_this_unit = _start_unit;					\
+	if (_start_unit == _end_unit - 1) {				\
+		_mask = _bt_make_mask(type, _start % _ts);		\
+		if (_end % _ts)						\
+			_mask |= _bt_make_mask_complement(type, _end % _ts); \
+		_cmask = _bt_lshift((type) (_v), _start % _ts);		\
+		_cmask &= ~_mask;					\
+		_ptr[_this_unit] &= _mask;				\
+		_ptr[_this_unit] |= _cmask;				\
 		break;							\
 	}								\
-	if (__start % ts) {						\
-		cshift = __start % ts;					\
-		mask = _bt_make_mask(type, cshift);			\
-		cmask = _bt_lshift((type) (__v), cshift);		\
-		cmask &= ~mask;						\
-		__ptr[this_unit] &= mask;				\
-		__ptr[this_unit] |= cmask;				\
-		_bt_safe_rshift(__v, ts - cshift);			\
-		__start += ts - cshift;					\
-		this_unit++;						\
+	if (_start % _ts) {						\
+		_cshift = _start % _ts;					\
+		_mask = _bt_make_mask(type, _cshift);			\
+		_cmask = _bt_lshift((type) (_v), _cshift);		\
+		_cmask &= ~_mask;					\
+		_ptr[_this_unit] &= _mask;				\
+		_ptr[_this_unit] |= _cmask;				\
+		_bt_safe_rshift(_v, _ts - _cshift);			\
+		_start += _ts - _cshift;				\
+		_this_unit++;						\
 	}								\
-	for (; this_unit < end_unit - 1; this_unit++) {			\
-		__ptr[this_unit] = (type) __v;				\
-		_bt_safe_rshift(__v, ts);				\
-		__start += ts;						\
+	for (; _this_unit < _end_unit - 1; _this_unit++) {		\
+		_ptr[_this_unit] = (type) _v;				\
+		_bt_safe_rshift(_v, _ts);				\
+		_start += _ts;						\
 	}								\
-	if (end % ts) {							\
-		mask = _bt_make_mask_complement(type, end % ts);	\
-		cmask = (type) __v;					\
-		cmask &= ~mask;						\
-		__ptr[this_unit] &= mask;				\
-		__ptr[this_unit] |= cmask;				\
+	if (_end % _ts) {						\
+		_mask = _bt_make_mask_complement(type, _end % _ts);	\
+		_cmask = (type) _v;					\
+		_cmask &= ~_mask;					\
+		_ptr[_this_unit] &= _mask;				\
+		_ptr[_this_unit] |= _cmask;				\
 	} else								\
-		__ptr[this_unit] = (type) __v;				\
+		_ptr[_this_unit] = (type) _v;				\
 } while (0)
 
-#define _bt_bitfield_write_be(_ptr, type, _start, _length, _v)		\
+#define _bt_bitfield_write_be(ptr, type, start, length, v)		\
 do {									\
-	__typeof__(_v) __v = (_v);					\
-	type *__ptr = (void *) (_ptr);					\
-	unsigned long __start = (_start), __length = (_length);		\
-	type mask, cmask;						\
-	unsigned long ts = sizeof(type) * CHAR_BIT; /* type size */	\
-	unsigned long start_unit, end_unit, this_unit;			\
-	unsigned long end, cshift; /* cshift is "complement shift" */	\
+	__typeof__(v) _v = (v);						\
+	type *_ptr = (void *) (ptr);					\
+	unsigned long _start = (start), _length = (length);		\
+	type _mask, _cmask;						\
+	unsigned long _ts = sizeof(type) * CHAR_BIT; /* type size */	\
+	unsigned long _start_unit, _end_unit, _this_unit;		\
+	unsigned long _end, _cshift; /* _cshift is "complement shift" */ \
 									\
-	if (!__length)							\
+	if (!_length)							\
 		break;							\
 									\
-	end = __start + __length;					\
-	start_unit = __start / ts;					\
-	end_unit = (end + (ts - 1)) / ts;				\
+	_end = _start + _length;					\
+	_start_unit = _start / _ts;					\
+	_end_unit = (_end + (_ts - 1)) / _ts;				\
 									\
 	/* Trim v high bits */						\
-	if (__length < sizeof(__v) * CHAR_BIT)				\
-		__v &= _bt_make_mask(__typeof__(__v), __length);	\
+	if (_length < sizeof(_v) * CHAR_BIT)				\
+		_v &= _bt_make_mask(__typeof__(_v), _length);		\
 									\
 	/* We can now append v with a simple "or", shift it piece-wise */ \
-	this_unit = end_unit - 1;					\
-	if (start_unit == end_unit - 1) {				\
-		mask = _bt_make_mask(type, (ts - (end % ts)) % ts);	\
-		if (__start % ts)					\
-			mask |= _bt_make_mask_complement(type, ts - (__start % ts)); \
-		cmask = _bt_lshift((type) (__v), (ts - (end % ts)) % ts); \
-		cmask &= ~mask;						\
-		__ptr[this_unit] &= mask;				\
-		__ptr[this_unit] |= cmask;				\
+	_this_unit = _end_unit - 1;					\
+	if (_start_unit == _end_unit - 1) {				\
+		_mask = _bt_make_mask(type, (_ts - (_end % _ts)) % _ts); \
+		if (_start % _ts)					\
+			_mask |= _bt_make_mask_complement(type, _ts - (_start % _ts)); \
+		_cmask = _bt_lshift((type) (_v), (_ts - (_end % _ts)) % _ts); \
+		_cmask &= ~_mask;					\
+		_ptr[_this_unit] &= _mask;				\
+		_ptr[_this_unit] |= _cmask;				\
 		break;							\
 	}								\
-	if (end % ts) {							\
-		cshift = end % ts;					\
-		mask = _bt_make_mask(type, ts - cshift);		\
-		cmask = _bt_lshift((type) (__v), ts - cshift);		\
-		cmask &= ~mask;						\
-		__ptr[this_unit] &= mask;				\
-		__ptr[this_unit] |= cmask;				\
-		_bt_safe_rshift(__v, cshift);				\
-		end -= cshift;						\
-		this_unit--;						\
+	if (_end % _ts) {						\
+		_cshift = _end % _ts;					\
+		_mask = _bt_make_mask(type, _ts - _cshift);		\
+		_cmask = _bt_lshift((type) (_v), _ts - _cshift);	\
+		_cmask &= ~_mask;					\
+		_ptr[_this_unit] &= _mask;				\
+		_ptr[_this_unit] |= _cmask;				\
+		_bt_safe_rshift(_v, _cshift);				\
+		_end -= _cshift;					\
+		_this_unit--;						\
 	}								\
-	for (; (long) this_unit >= (long) start_unit + 1; this_unit--) { \
-		__ptr[this_unit] = (type) __v;				\
-		_bt_safe_rshift(__v, ts);				\
-		end -= ts;						\
+	for (; (long) _this_unit >= (long) _start_unit + 1; _this_unit--) { \
+		_ptr[_this_unit] = (type) _v;				\
+		_bt_safe_rshift(_v, _ts);				\
+		_end -= _ts;						\
 	}								\
-	if (__start % ts) {						\
-		mask = _bt_make_mask_complement(type, ts - (__start % ts)); \
-		cmask = (type) __v;					\
-		cmask &= ~mask;						\
-		__ptr[this_unit] &= mask;				\
-		__ptr[this_unit] |= cmask;				\
+	if (_start % _ts) {						\
+		_mask = _bt_make_mask_complement(type, _ts - (_start % _ts)); \
+		_cmask = (type) _v;					\
+		_cmask &= ~_mask;					\
+		_ptr[_this_unit] &= _mask;				\
+		_ptr[_this_unit] |= _cmask;				\
 	} else								\
-		__ptr[this_unit] = (type) __v;				\
+		_ptr[_this_unit] = (type) _v;				\
 } while (0)
 
 /*
@@ -344,25 +344,25 @@ do {									\
 
 #if (BYTE_ORDER == LITTLE_ENDIAN)
 
-#define bt_bitfield_write(ptr, type, _start, _length, _v)		\
-	_bt_bitfield_write_le(ptr, type, _start, _length, _v)
+#define bt_bitfield_write(ptr, type, start, length, v)			\
+	_bt_bitfield_write_le(ptr, type, start, length, v)
 
-#define bt_bitfield_write_le(ptr, type, _start, _length, _v)		\
-	_bt_bitfield_write_le(ptr, type, _start, _length, _v)
+#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)
+#define bt_bitfield_write_be(ptr, type, start, length, v)		\
+	_bt_bitfield_write_be(ptr, unsigned char, start, length, v)
 
 #elif (BYTE_ORDER == BIG_ENDIAN)
 
-#define bt_bitfield_write(ptr, type, _start, _length, _v)		\
-	_bt_bitfield_write_be(ptr, type, _start, _length, _v)
+#define bt_bitfield_write(ptr, type, start, length, v)			\
+	_bt_bitfield_write_be(ptr, type, start, length, v)
 
-#define bt_bitfield_write_le(ptr, type, _start, _length, _v)		\
-	_bt_bitfield_write_le(ptr, unsigned char, _start, _length, _v)
+#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)
+#define bt_bitfield_write_be(ptr, type, start, length, v)		\
+	_bt_bitfield_write_be(ptr, type, start, length, v)
 
 #else /* (BYTE_ORDER == PDP_ENDIAN) */
 
@@ -370,148 +370,148 @@ do {									\
 
 #endif
 
-#define _bt_bitfield_read_le(_ptr, type, _start, _length, _vptr)	\
+#define _bt_bitfield_read_le(ptr, type, start, length, vptr)		\
 do {									\
-	__typeof__(*(_vptr)) *__vptr = (_vptr);				\
-	__typeof__(*__vptr) __v;					\
-	type *__ptr = (void *) (_ptr);					\
-	unsigned long __start = (_start), __length = (_length);		\
-	type mask, cmask;						\
-	unsigned long ts = sizeof(type) * CHAR_BIT; /* type size */	\
-	unsigned long start_unit, end_unit, this_unit;			\
-	unsigned long end, cshift; /* cshift is "complement shift" */	\
-	bool is_signed_type;						\
+	__typeof__(*(vptr)) *_vptr = (vptr);				\
+	__typeof__(*_vptr) _v;						\
+	type *_ptr = (void *) (ptr);					\
+	unsigned long _start = (start), _length = (length);		\
+	type _mask, _cmask;						\
+	unsigned long _ts = sizeof(type) * CHAR_BIT; /* type size */	\
+	unsigned long _start_unit, _end_unit, _this_unit;		\
+	unsigned long _end, _cshift; /* _cshift is "complement shift" */ \
+	bool _is_signed_type;						\
 									\
-	if (!__length) {						\
-		*__vptr = 0;						\
+	if (!_length) {							\
+		*_vptr = 0;						\
 		break;							\
 	}								\
 									\
-	end = __start + __length;					\
-	start_unit = __start / ts;					\
-	end_unit = (end + (ts - 1)) / ts;				\
+	_end = _start + _length;					\
+	_start_unit = _start / _ts;					\
+	_end_unit = (_end + (_ts - 1)) / _ts;				\
 									\
-	this_unit = end_unit - 1;					\
+	_this_unit = _end_unit - 1;					\
 	_BT_DIAG_PUSH							\
 	_BT_DIAG_IGNORE_TYPE_LIMITS					\
-	is_signed_type = _bt_is_signed_type(__typeof__(__v));		\
+	_is_signed_type = _bt_is_signed_type(__typeof__(_v));		\
 	_BT_DIAG_POP							\
-	if (is_signed_type						\
-	    && (__ptr[this_unit] & _bt_lshift((type) 1, (end % ts ? end % ts : ts) - 1))) \
-		__v = ~(__typeof__(__v)) 0;				\
+	if (_is_signed_type						\
+	    && (_ptr[_this_unit] & _bt_lshift((type) 1, (_end % _ts ? _end % _ts : _ts) - 1))) \
+		_v = ~(__typeof__(_v)) 0;				\
 	else								\
-		__v = 0;						\
-	if (start_unit == end_unit - 1) {				\
-		cmask = __ptr[this_unit];				\
-		cmask = _bt_rshift(cmask, __start % ts);		\
-		if ((end - __start) % ts) {				\
-			mask = _bt_make_mask(type, end - __start);	\
-			cmask &= mask;					\
+		_v = 0;							\
+	if (_start_unit == _end_unit - 1) {				\
+		_cmask = _ptr[_this_unit];				\
+		_cmask = _bt_rshift(_cmask, _start % _ts);		\
+		if ((_end - _start) % _ts) {				\
+			_mask = _bt_make_mask(type, _end - _start);	\
+			_cmask &= _mask;				\
 		}							\
-		_bt_safe_lshift(__v, end - __start);			\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
-		*__vptr = __v;						\
+		_bt_safe_lshift(_v, _end - _start);			\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _cmask); \
+		*_vptr = _v;						\
 		break;							\
 	}								\
-	if (end % ts) {							\
-		cshift = end % ts;					\
-		mask = _bt_make_mask(type, cshift);			\
-		cmask = __ptr[this_unit];				\
-		cmask &= mask;						\
-		_bt_safe_lshift(__v, cshift);				\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
-		end -= cshift;						\
-		this_unit--;						\
+	if (_end % _ts) {						\
+		_cshift = _end % _ts;					\
+		_mask = _bt_make_mask(type, _cshift);			\
+		_cmask = _ptr[_this_unit];				\
+		_cmask &= _mask;					\
+		_bt_safe_lshift(_v, _cshift);				\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _cmask); \
+		_end -= _cshift;					\
+		_this_unit--;						\
 	}								\
-	for (; (long) this_unit >= (long) start_unit + 1; this_unit--) { \
-		_bt_safe_lshift(__v, ts);				\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
-		end -= ts;						\
+	for (; (long) _this_unit >= (long) _start_unit + 1; _this_unit--) { \
+		_bt_safe_lshift(_v, _ts);				\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _ptr[_this_unit]); \
+		_end -= _ts;						\
 	}								\
-	if (__start % ts) {						\
-		mask = _bt_make_mask(type, ts - (__start % ts));	\
-		cmask = __ptr[this_unit];				\
-		cmask = _bt_rshift(cmask, __start % ts);		\
-		cmask &= mask;						\
-		_bt_safe_lshift(__v, ts - (__start % ts));		\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
+	if (_start % _ts) {						\
+		_mask = _bt_make_mask(type, _ts - (_start % _ts));	\
+		_cmask = _ptr[_this_unit];				\
+		_cmask = _bt_rshift(_cmask, _start % _ts);		\
+		_cmask &= _mask;					\
+		_bt_safe_lshift(_v, _ts - (_start % _ts));		\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _cmask); \
 	} else {							\
-		_bt_safe_lshift(__v, ts);				\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
+		_bt_safe_lshift(_v, _ts);				\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _ptr[_this_unit]); \
 	}								\
-	*__vptr = __v;							\
+	*_vptr = _v;							\
 } while (0)
 
-#define _bt_bitfield_read_be(_ptr, type, _start, _length, _vptr)	\
+#define _bt_bitfield_read_be(ptr, type, start, length, vptr)		\
 do {									\
-	__typeof__(*(_vptr)) *__vptr = (_vptr);				\
-	__typeof__(*__vptr) __v;					\
-	type *__ptr = (void *) (_ptr);					\
-	unsigned long __start = (_start), __length = (_length);		\
-	type mask, cmask;						\
-	unsigned long ts = sizeof(type) * CHAR_BIT; /* type size */	\
-	unsigned long start_unit, end_unit, this_unit;			\
-	unsigned long end, cshift; /* cshift is "complement shift" */	\
-	bool is_signed_type;						\
+	__typeof__(*(vptr)) *_vptr = (vptr);				\
+	__typeof__(*_vptr) _v;						\
+	type *_ptr = (void *) (ptr);					\
+	unsigned long _start = (start), _length = (length);		\
+	type _mask, _cmask;						\
+	unsigned long _ts = sizeof(type) * CHAR_BIT; /* type size */	\
+	unsigned long _start_unit, _end_unit, _this_unit;		\
+	unsigned long _end, _cshift; /* _cshift is "complement shift" */ \
+	bool _is_signed_type;						\
 									\
-	if (!__length) {						\
-		*__vptr = 0;						\
+	if (!_length) {							\
+		*_vptr = 0;						\
 		break;							\
 	}								\
 									\
-	end = __start + __length;					\
-	start_unit = __start / ts;					\
-	end_unit = (end + (ts - 1)) / ts;				\
+	_end = _start + _length;					\
+	_start_unit = _start / _ts;					\
+	_end_unit = (_end + (_ts - 1)) / _ts;				\
 									\
-	this_unit = start_unit;						\
+	_this_unit = _start_unit;					\
 	_BT_DIAG_PUSH							\
 	_BT_DIAG_IGNORE_TYPE_LIMITS					\
-	is_signed_type = _bt_is_signed_type(__typeof__(__v));		\
+	_is_signed_type = _bt_is_signed_type(__typeof__(_v));		\
 	_BT_DIAG_POP							\
-	if (is_signed_type						\
-	    && (__ptr[this_unit] & _bt_lshift((type) 1, ts - (__start % ts) - 1))) \
-		__v = ~(__typeof__(__v)) 0;				\
+	if (_is_signed_type						\
+	    && (_ptr[_this_unit] & _bt_lshift((type) 1, _ts - (_start % _ts) - 1))) \
+		_v = ~(__typeof__(_v)) 0;				\
 	else								\
-		__v = 0;						\
-	if (start_unit == end_unit - 1) {				\
-		cmask = __ptr[this_unit];				\
-		cmask = _bt_rshift(cmask, (ts - (end % ts)) % ts);	\
-		if ((end - __start) % ts) {				\
-			mask = _bt_make_mask(type, end - __start);	\
-			cmask &= mask;					\
+		_v = 0;							\
+	if (_start_unit == _end_unit - 1) {				\
+		_cmask = _ptr[_this_unit];				\
+		_cmask = _bt_rshift(_cmask, (_ts - (_end % _ts)) % _ts); \
+		if ((_end - _start) % _ts) {				\
+			_mask = _bt_make_mask(type, _end - _start);	\
+			_cmask &= _mask;				\
 		}							\
-		_bt_safe_lshift(__v, end - __start);		\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
-		*__vptr = __v;						\
+		_bt_safe_lshift(_v, _end - _start);			\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _cmask); \
+		*_vptr = _v;						\
 		break;							\
 	}								\
-	if (__start % ts) {						\
-		cshift = __start % ts;					\
-		mask = _bt_make_mask(type, ts - cshift);		\
-		cmask = __ptr[this_unit];				\
-		cmask &= mask;						\
-		_bt_safe_lshift(__v, ts - cshift);			\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
-		__start += ts - cshift;					\
-		this_unit++;						\
+	if (_start % _ts) {						\
+		_cshift = _start % _ts;					\
+		_mask = _bt_make_mask(type, _ts - _cshift);		\
+		_cmask = _ptr[_this_unit];				\
+		_cmask &= _mask;					\
+		_bt_safe_lshift(_v, _ts - _cshift);			\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _cmask); \
+		_start += _ts - _cshift;				\
+		_this_unit++;						\
 	}								\
-	for (; this_unit < end_unit - 1; this_unit++) {			\
-		_bt_safe_lshift(__v, ts);				\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
-		__start += ts;						\
+	for (; _this_unit < _end_unit - 1; _this_unit++) {		\
+		_bt_safe_lshift(_v, _ts);				\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _ptr[_this_unit]); \
+		_start += _ts;						\
 	}								\
-	if (end % ts) {							\
-		mask = _bt_make_mask(type, end % ts);			\
-		cmask = __ptr[this_unit];				\
-		cmask = _bt_rshift(cmask, ts - (end % ts));		\
-		cmask &= mask;						\
-		_bt_safe_lshift(__v, end % ts);				\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
+	if (_end % _ts) {						\
+		_mask = _bt_make_mask(type, _end % _ts);		\
+		_cmask = _ptr[_this_unit];				\
+		_cmask = _bt_rshift(_cmask, _ts - (_end % _ts));	\
+		_cmask &= _mask;					\
+		_bt_safe_lshift(_v, _end % _ts);			\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _cmask); \
 	} else {							\
-		_bt_safe_lshift(__v, ts);				\
-		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
+		_bt_safe_lshift(_v, _ts);				\
+		_v |= _bt_cast_value_to_unsigned_type(__typeof__(_v), _ptr[_this_unit]); \
 	}								\
-	*__vptr = __v;							\
+	*_vptr = _v;							\
 } while (0)
 
 /*
@@ -522,25 +522,25 @@ do {									\
 
 #if (BYTE_ORDER == LITTLE_ENDIAN)
 
-#define bt_bitfield_read(_ptr, type, _start, _length, _vptr)		\
-	_bt_bitfield_read_le(_ptr, type, _start, _length, _vptr)
+#define bt_bitfield_read(ptr, type, start, length, vptr)		\
+	_bt_bitfield_read_le(ptr, type, start, length, vptr)
 
-#define bt_bitfield_read_le(_ptr, type, _start, _length, _vptr)		\
-	_bt_bitfield_read_le(_ptr, type, _start, _length, _vptr)
+#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)
+#define bt_bitfield_read_be(ptr, type, start, length, vptr)		\
+	_bt_bitfield_read_be(ptr, unsigned char, start, length, vptr)
 
 #elif (BYTE_ORDER == BIG_ENDIAN)
 
-#define bt_bitfield_read(_ptr, type, _start, _length, _vptr)		\
-	_bt_bitfield_read_be(_ptr, type, _start, _length, _vptr)
+#define bt_bitfield_read(ptr, type, start, length, vptr)		\
+	_bt_bitfield_read_be(ptr, type, start, length, vptr)
 
-#define bt_bitfield_read_le(_ptr, type, _start, _length, _vptr)		\
-	_bt_bitfield_read_le(_ptr, unsigned char, _start, _length, _vptr)
+#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)
+#define bt_bitfield_read_be(ptr, type, start, length, vptr)		\
+	_bt_bitfield_read_be(ptr, type, start, length, vptr)
 
 #else /* (BYTE_ORDER == PDP_ENDIAN) */
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 babeltrace 4/4] Extend test_bitfield coverage
       [not found] <20190516205848.20409-1-mathieu.desnoyers@efficios.com>
  2019-05-16 20:58 ` [PATCH v2 babeltrace 2/4] Silence compiler "always false comparison" warning Mathieu Desnoyers
  2019-05-16 20:58 ` [PATCH babeltrace 3/4] Cleanup: bitfields: streamline use of underscores Mathieu Desnoyers
@ 2019-05-16 20:58 ` Mathieu Desnoyers
  2019-05-17 20:44 ` [PATCH v8 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors Jérémie Galarneau
  3 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-16 20:58 UTC (permalink / raw)
  To: jgalar, simon.marchi; +Cc: lttng-dev

test_bitfield was mainly testing various write unit size. Add
variations of read unit size as well.

Previously, the test was only covering input from a 32-bit integer.
Additionally test source and destination of 64-bit.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I71d2f908bf532f6b183dfee7fdc8d4ef74beba4b
---
Changes since v1:
- Fold "Cleanup: test_bitfield: pass source integers as parameters",
- Coding style cleanups.
- Remove arch-specific "fls" implementations, only use a generic one.
---
 tests/lib/test_bitfield.c | 526 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 435 insertions(+), 91 deletions(-)

diff --git a/tests/lib/test_bitfield.c b/tests/lib/test_bitfield.c
index efbbc30e..5ba566f6 100644
--- a/tests/lib/test_bitfield.c
+++ b/tests/lib/test_bitfield.c
@@ -3,7 +3,7 @@
  *
  * BabelTrace - bitfield test program
  *
- * Copyright 2010 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+ * Copyright 2010-2019 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -40,39 +40,56 @@ void fct(void)
 /* Test array size, in bytes */
 #define TEST_LEN 128
 #define NR_TESTS 10
-#define SIGNED_TEST_DESC_FMT_STR "Writing and reading back 0x%X, signed"
-#define UNSIGNED_TEST_DESC_FMT_STR "Writing and reading back 0x%X, unsigned"
+#define SIGNED_INT_READ_TEST_DESC_FMT_STR "Writing and reading back 0x%X, signed int dest, varying read unit size"
+#define SIGNED_INT_WRITE_TEST_DESC_FMT_STR "Writing and reading back 0x%X, signed int source, varying write unit size"
+#define SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR "Writing and reading back 0x%llX, signed long long dest, varying read unit size"
+#define SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR "Writing and reading back 0x%llX, signed long long source, varying write unit size"
+#define UNSIGNED_INT_READ_TEST_DESC_FMT_STR "Writing and reading back 0x%X, unsigned int dest, varying read unit size"
+#define UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR "Writing and reading back 0x%X, unsigned int source, varying write unit size"
+#define UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR "Writing and reading back 0x%llX, unsigned long long dest, varying read unit size"
+#define UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR "Writing and reading back 0x%llX, unsigned long long source, varying write unit size"
 #define DIAG_FMT_STR "Failed reading value written \"%s\"-wise, with start=%i" \
 	" and length=%i. Read %llX"
 
-unsigned int srcrand;
-
-#if defined(__i386) || defined(__x86_64)
-
-static inline int fls(int x)
+static
+unsigned int fls_u64(uint64_t x)
 {
-	int r;
-	asm("bsrl %1,%0\n\t"
-	    "cmovzl %2,%0"
-	    : "=&r" (r) : "rm" (x), "rm" (-1));
-	return r + 1;
-}
+	unsigned int r = 64;
 
-#elif defined(__PPC__)
-
-static __inline__ int fls(unsigned int x)
-{
-	int lz;
+	if (!x)
+		return 0;
 
-	asm ("cntlzw %0,%1" : "=r" (lz) : "r" (x));
-	return 32 - lz;
+	if (!(x & 0xFFFFFFFF00000000ULL)) {
+		x <<= 32;
+		r -= 32;
+	}
+	if (!(x & 0xFFFF000000000000ULL)) {
+		x <<= 16;
+		r -= 16;
+	}
+	if (!(x & 0xFF00000000000000ULL)) {
+		x <<= 8;
+		r -= 8;
+	}
+	if (!(x & 0xF000000000000000ULL)) {
+		x <<= 4;
+		r -= 4;
+	}
+	if (!(x & 0xC000000000000000ULL)) {
+		x <<= 2;
+		r -= 2;
+	}
+	if (!(x & 0x8000000000000000ULL)) {
+		x <<= 1;
+		r -= 1;
+	}
+	return r;
 }
 
-#else
-
-static int fls(unsigned int x)
+static
+unsigned int fls_u32(uint32_t x)
 {
-	int r = 32;
+	unsigned int r = 32;
 
 	if (!x)
 		return 0;
@@ -99,8 +116,6 @@ static int fls(unsigned int x)
 	return r;
 }
 
-#endif
-
 #define print_byte_array(c, len)	\
 do {					\
 	unsigned long i;		\
@@ -122,7 +137,7 @@ do {					\
 } while (0)
 
 #define check_result(ref, val, buffer, typename, start, len,		\
-		     desc_fmt_str)					\
+		desc_fmt_str)						\
 ({									\
 	if ((val) != (ref)) {						\
 		fail(desc_fmt_str, ref);				\
@@ -133,9 +148,9 @@ do {					\
 	(val) != (ref);							\
 })
 
-void run_test_unsigned(void)
+void run_test_unsigned_write(unsigned int src_ui, unsigned long long src_ull)
 {
-	unsigned int src, nrbits;
+	unsigned int nrbits_ui, nrbits_ull;
 	union {
 		unsigned char c[TEST_LEN];
 		unsigned short s[TEST_LEN/sizeof(unsigned short)];
@@ -146,59 +161,222 @@ void run_test_unsigned(void)
 	unsigned long long readval;
 	unsigned int s, l;
 
-	src = srcrand;
-	nrbits = fls(src);
+	nrbits_ui = fls_u32(src_ui);
 
+	/* Write from unsigned integer src input. */
 	for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
-		for (l = nrbits; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+		for (l = nrbits_ui; l < (CHAR_BIT * TEST_LEN) - s; l++) {
 			init_byte_array(target.c, TEST_LEN, 0xFF);
-			bt_bitfield_write(target.c, unsigned char, s, l, src);
+			bt_bitfield_write(target.c, unsigned char, s, l, src_ui);
 			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
-			if (check_result(src, readval, target.c, unsigned char,
-					  s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_ui, readval, target.c, unsigned char,
+					s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 
 			init_byte_array(target.c, TEST_LEN, 0xFF);
-			bt_bitfield_write(target.s, unsigned short, s, l, src);
+			bt_bitfield_write(target.s, unsigned short, s, l, src_ui);
 			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
-			if (check_result(src, readval, target.c, unsigned short,
-					  s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_ui, readval, target.c, unsigned short,
+					s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 
 			init_byte_array(target.c, TEST_LEN, 0xFF);
-			bt_bitfield_write(target.i, unsigned int, s, l, src);
+			bt_bitfield_write(target.i, unsigned int, s, l, src_ui);
 			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
-			if (check_result(src, readval, target.c, unsigned int,
-					   s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_ui, readval, target.c, unsigned int,
+					s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 
 			init_byte_array(target.c, TEST_LEN, 0xFF);
-			bt_bitfield_write(target.l, unsigned long, s, l, src);
+			bt_bitfield_write(target.l, unsigned long, s, l, src_ui);
 			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
-			if (check_result(src, readval, target.c, unsigned long,
-					  s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_ui, readval, target.c, unsigned long,
+					s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 
 			init_byte_array(target.c, TEST_LEN, 0xFF);
-			bt_bitfield_write(target.ll, unsigned long long, s, l, src);
+			bt_bitfield_write(target.ll, unsigned long long, s, l, src_ui);
 			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
-			if (check_result(src, readval, target.c, unsigned long long,
-				     s, l, UNSIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_ui, readval, target.c, unsigned long long,
+					s, l, UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 		}
 	}
+	pass(UNSIGNED_INT_WRITE_TEST_DESC_FMT_STR, src_ui);
+
+	nrbits_ull = fls_u64(src_ull);
+
+	/* Write from unsigned long long src input. */
+	for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+		for (l = nrbits_ull; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.c, unsigned char, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+			if (check_result(src_ull, readval, target.c, unsigned char,
+					s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
 
-	pass(UNSIGNED_TEST_DESC_FMT_STR, src);
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.s, unsigned short, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+			if (check_result(src_ull, readval, target.c, unsigned short,
+					s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.i, unsigned int, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+			if (check_result(src_ull, readval, target.c, unsigned int,
+					s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.l, unsigned long, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+			if (check_result(src_ull, readval, target.c, unsigned long,
+					s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.ll, unsigned long long, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned char, s, l, &readval);
+			if (check_result(src_ull, readval, target.c, unsigned long long,
+					s, l, UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
+		}
+	}
+	pass(UNSIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR, src_ull);
 }
 
-void run_test_signed(void)
+void run_test_unsigned_read(unsigned int src_ui, unsigned long long src_ull)
 {
-	int src, nrbits;
+	unsigned int nrbits_ui, nrbits_ull, readval_ui;
+	union {
+		unsigned char c[TEST_LEN];
+		unsigned short s[TEST_LEN/sizeof(unsigned short)];
+		unsigned int i[TEST_LEN/sizeof(unsigned int)];
+		unsigned long l[TEST_LEN/sizeof(unsigned long)];
+		unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
+	} target;
+	unsigned long long readval_ull;
+	unsigned int s, l;
+
+	nrbits_ui = fls_u32(src_ui);
+
+	/* Read to unsigned integer readval output. */
+	for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+		for (l = nrbits_ui; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.c, unsigned char, s, l, src_ui);
+			bt_bitfield_read(target.c, unsigned char, s, l, &readval_ui);
+			if (check_result(src_ui, readval_ui, target.c, unsigned char,
+					s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.s, unsigned char, s, l, src_ui);
+			bt_bitfield_read(target.c, unsigned short, s, l, &readval_ui);
+			if (check_result(src_ui, readval_ui, target.c, unsigned short,
+					s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.i, unsigned char, s, l, src_ui);
+			bt_bitfield_read(target.c, unsigned int, s, l, &readval_ui);
+			if (check_result(src_ui, readval_ui, target.c, unsigned int,
+					s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.l, unsigned char, s, l, src_ui);
+			bt_bitfield_read(target.c, unsigned long, s, l, &readval_ui);
+			if (check_result(src_ui, readval_ui, target.c, unsigned long,
+					s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.ll, unsigned char, s, l, src_ui);
+			bt_bitfield_read(target.c, unsigned long long, s, l, &readval_ui);
+			if (check_result(src_ui, readval_ui, target.c, unsigned long long,
+					s, l, UNSIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+		}
+	}
+	pass(UNSIGNED_INT_READ_TEST_DESC_FMT_STR, src_ui);
+
+	nrbits_ull = fls_u64(src_ull);
+
+	/* Read to unsigned long long readval output. */
+	for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+		for (l = nrbits_ull; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.c, unsigned char, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned char, s, l, &readval_ull);
+			if (check_result(src_ull, readval_ull, target.c, unsigned char,
+					s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.s, unsigned char, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned short, s, l, &readval_ull);
+			if (check_result(src_ull, readval_ull, target.c, unsigned short,
+					s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.i, unsigned char, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned int, s, l, &readval_ull);
+			if (check_result(src_ull, readval_ull, target.c, unsigned int,
+					s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.l, unsigned char, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned long, s, l, &readval_ull);
+			if (check_result(src_ull, readval_ull, target.c, unsigned long,
+					s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.ll, unsigned char, s, l, src_ull);
+			bt_bitfield_read(target.c, unsigned long long, s, l, &readval_ull);
+			if (check_result(src_ull, readval_ull, target.c, unsigned long long,
+					s, l, UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+		}
+	}
+	pass(UNSIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR, src_ull);
+}
+
+void run_test_unsigned(unsigned int src_ui, unsigned long long src_ull)
+{
+	run_test_unsigned_write(src_ui, src_ull);
+	run_test_unsigned_read(src_ui, src_ull);
+}
+
+void run_test_signed_write(int src_i, long long src_ll)
+{
+	int nrbits_i, nrbits_ll;
 	union {
 		signed char c[TEST_LEN];
 		short s[TEST_LEN/sizeof(short)];
@@ -209,87 +387,253 @@ void run_test_signed(void)
 	long long readval;
 	unsigned int s, l;
 
-	src = srcrand;
-	if (src & 0x80000000U)
-		nrbits = fls(~src) + 1;	/* Find least significant bit conveying sign */
+	if (src_i & 0x80000000U)
+		nrbits_i = fls_u32(~src_i) + 1;	/* Find least significant bit conveying sign */
 	else
-		nrbits = fls(src) + 1;	/* Keep sign at 0 */
+		nrbits_i = fls_u32(src_i) + 1;	/* Keep sign at 0 */
 
+	/* Write from signed integer src input. */
 	for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
-		for (l = nrbits; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+		for (l = nrbits_i; l < (CHAR_BIT * TEST_LEN) - s; l++) {
 			init_byte_array(target.c, TEST_LEN, 0x0);
-			bt_bitfield_write(target.c, signed char, s, l, src);
+			bt_bitfield_write(target.c, signed char, s, l, src_i);
 			bt_bitfield_read(target.c, signed char, s, l, &readval);
-			if (check_result(src, readval, target.c, signed char,
-					  s, l, SIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_i, readval, target.c, signed char,
+					s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 
 			init_byte_array(target.c, TEST_LEN, 0x0);
-			bt_bitfield_write(target.s, short, s, l, src);
+			bt_bitfield_write(target.s, short, s, l, src_i);
 			bt_bitfield_read(target.c, signed char, s, l, &readval);
-			if (check_result(src, readval, target.c, short,
-					  s, l, SIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_i, readval, target.c, short,
+					s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 
 			init_byte_array(target.c, TEST_LEN, 0x0);
-			bt_bitfield_write(target.i, int, s, l, src);
+			bt_bitfield_write(target.i, int, s, l, src_i);
 			bt_bitfield_read(target.c, signed char, s, l, &readval);
-			if (check_result(src, readval, target.c, int,
-					  s, l, SIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_i, readval, target.c, int,
+					s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 
 			init_byte_array(target.c, TEST_LEN, 0x0);
-			bt_bitfield_write(target.l, long, s, l, src);
+			bt_bitfield_write(target.l, long, s, l, src_i);
 			bt_bitfield_read(target.c, signed char, s, l, &readval);
-			if (check_result(src, readval, target.c, long,
-					  s, l, SIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_i, readval, target.c, long,
+					s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 
 			init_byte_array(target.c, TEST_LEN, 0x0);
-			bt_bitfield_write(target.ll, long long, s, l, src);
+			bt_bitfield_write(target.ll, long long, s, l, src_i);
 			bt_bitfield_read(target.c, signed char, s, l, &readval);
-			if (check_result(src, readval, target.c, long long,
-					  s, l, SIGNED_TEST_DESC_FMT_STR)) {
+			if (check_result(src_i, readval, target.c, long long,
+					s, l, SIGNED_INT_WRITE_TEST_DESC_FMT_STR)) {
 				return;
 			}
 		}
 	}
+	pass(SIGNED_INT_WRITE_TEST_DESC_FMT_STR, src_i);
+
+	if (src_ll & 0x8000000000000000ULL)
+		nrbits_ll = fls_u64(~src_ll) + 1;	/* Find least significant bit conveying sign */
+	else
+		nrbits_ll = fls_u64(src_ll) + 1;	/* Keep sign at 0 */
+
+	/* Write from signed long long src input. */
+	for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+		for (l = nrbits_ll; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+			init_byte_array(target.c, TEST_LEN, 0x0);
+			bt_bitfield_write(target.c, signed char, s, l, src_ll);
+			bt_bitfield_read(target.c, signed char, s, l, &readval);
+			if (check_result(src_ll, readval, target.c, signed char,
+					s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0x0);
+			bt_bitfield_write(target.s, short, s, l, src_ll);
+			bt_bitfield_read(target.c, signed char, s, l, &readval);
+			if (check_result(src_ll, readval, target.c, short,
+					s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0x0);
+			bt_bitfield_write(target.i, int, s, l, src_ll);
+			bt_bitfield_read(target.c, signed char, s, l, &readval);
+			if (check_result(src_ll, readval, target.c, int,
+					s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
 
-	pass(SIGNED_TEST_DESC_FMT_STR, src);
+			init_byte_array(target.c, TEST_LEN, 0x0);
+			bt_bitfield_write(target.l, long, s, l, src_ll);
+			bt_bitfield_read(target.c, signed char, s, l, &readval);
+			if (check_result(src_ll, readval, target.c, long,
+					s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0x0);
+			bt_bitfield_write(target.ll, long long, s, l, src_ll);
+			bt_bitfield_read(target.c, signed char, s, l, &readval);
+			if (check_result(src_ll, readval, target.c, long long,
+					s, l, SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR)) {
+				return;
+			}
+		}
+	}
+	pass(SIGNED_LONG_LONG_WRITE_TEST_DESC_FMT_STR, src_ll);
 }
 
-void run_test(void)
+void run_test_signed_read(int src_i, long long src_ll)
 {
-	int i;
-	plan_tests(NR_TESTS * 2 + 6);
+	int nrbits_i, nrbits_ll, readval_i;
+	union {
+		unsigned char c[TEST_LEN];
+		unsigned short s[TEST_LEN/sizeof(unsigned short)];
+		unsigned int i[TEST_LEN/sizeof(unsigned int)];
+		unsigned long l[TEST_LEN/sizeof(unsigned long)];
+		unsigned long long ll[TEST_LEN/sizeof(unsigned long long)];
+	} target;
+	long long readval_ll;
+	unsigned int s, l;
 
-	srand(time(NULL));
+	if (src_i & 0x80000000U)
+		nrbits_i = fls_u32(~src_i) + 1;	/* Find least significant bit conveying sign */
+	else
+		nrbits_i = fls_u32(src_i) + 1;	/* Keep sign at 0 */
 
-	srcrand = 0;
-	run_test_unsigned();
-	srcrand = 0;
-	run_test_signed();
+	/* Read to signed integer readval output. */
+	for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+		for (l = nrbits_i; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.c, signed char, s, l, src_i);
+			bt_bitfield_read(target.c, signed char, s, l, &readval_i);
+			if (check_result(src_i, readval_i, target.c, signed char,
+					s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
 
-	srcrand = 1;
-	run_test_unsigned();
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.s, signed char, s, l, src_i);
+			bt_bitfield_read(target.c, short, s, l, &readval_i);
+			if (check_result(src_i, readval_i, target.c, short,
+					s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
 
-	srcrand = ~0U;
-	run_test_unsigned();
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.i, signed char, s, l, src_i);
+			bt_bitfield_read(target.c, int, s, l, &readval_i);
+			if (check_result(src_i, readval_i, target.c, int,
+					s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
 
-	srcrand = -1;
-	run_test_signed();
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.l, signed char, s, l, src_i);
+			bt_bitfield_read(target.c, long, s, l, &readval_i);
+			if (check_result(src_i, readval_i, target.c, long,
+					s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
 
-	srcrand = (int)0x80000000U;
-	run_test_signed();
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.ll, signed char, s, l, src_i);
+			bt_bitfield_read(target.c, long long, s, l, &readval_i);
+			if (check_result(src_i, readval_i, target.c, long long,
+					s, l, SIGNED_INT_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+		}
+	}
+	pass(SIGNED_INT_READ_TEST_DESC_FMT_STR, src_i);
+
+	if (src_ll & 0x8000000000000000ULL)
+		nrbits_ll = fls_u64(~src_ll) + 1;	/* Find least significant bit conveying sign */
+	else
+		nrbits_ll = fls_u64(src_ll) + 1;	/* Keep sign at 0 */
+
+	/* Read to signed long long readval output. */
+	for (s = 0; s < CHAR_BIT * TEST_LEN; s++) {
+		for (l = nrbits_ll; l < (CHAR_BIT * TEST_LEN) - s; l++) {
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.c, signed char, s, l, src_ll);
+			bt_bitfield_read(target.c, signed char, s, l, &readval_ll);
+			if (check_result(src_ll, readval_ll, target.c, signed char,
+					s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.s, signed char, s, l, src_ll);
+			bt_bitfield_read(target.c, short, s, l, &readval_ll);
+			if (check_result(src_ll, readval_ll, target.c, short,
+					s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.i, signed char, s, l, src_ll);
+			bt_bitfield_read(target.c, int, s, l, &readval_ll);
+			if (check_result(src_ll, readval_ll, target.c, int,
+					s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.l, signed char, s, l, src_ll);
+			bt_bitfield_read(target.c, long, s, l, &readval_ll);
+			if (check_result(src_ll, readval_ll, target.c, long,
+					s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+
+			init_byte_array(target.c, TEST_LEN, 0xFF);
+			bt_bitfield_write(target.ll, signed char, s, l, src_ll);
+			bt_bitfield_read(target.c, long long, s, l, &readval_ll);
+			if (check_result(src_ll, readval_ll, target.c, long long,
+					s, l, SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR)) {
+				return;
+			}
+		}
+	}
+	pass(SIGNED_LONG_LONG_READ_TEST_DESC_FMT_STR, src_ll);
+}
+
+void run_test_signed(int src_i, long long src_ll)
+{
+	run_test_signed_write(src_i, src_ll);
+	run_test_signed_read(src_i, src_ll);
+}
+
+void run_test(void)
+{
+	int i;
+
+	plan_tests(NR_TESTS * 8 + 24);
+
+	srand(time(NULL));
+
+	run_test_unsigned(0, 0);
+	run_test_signed(0, 0);
+	run_test_unsigned(1, 1);
+	run_test_unsigned(~0U, ~0ULL);
+	run_test_signed(-1U, -1ULL);
+	run_test_signed(0x80000000U, 0x8000000000000000ULL);
 
 	for (i = 0; i < NR_TESTS; i++) {
-		srcrand = rand();
-		run_test_unsigned();
-		run_test_signed();
+		unsigned int src_ui = rand();
+		unsigned long long src_ull = ((unsigned long long) (unsigned int) rand() << 32) |
+				(unsigned long long) (unsigned int) rand();
+
+		run_test_unsigned(src_ui, src_ull);
+		run_test_signed((int) src_ui, (long long) src_ull);
 	}
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v8 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors
       [not found] <20190516205848.20409-1-mathieu.desnoyers@efficios.com>
                   ` (2 preceding siblings ...)
  2019-05-16 20:58 ` [PATCH v2 babeltrace 4/4] Extend test_bitfield coverage Mathieu Desnoyers
@ 2019-05-17 20:44 ` Jérémie Galarneau
  3 siblings, 0 replies; 5+ messages in thread
From: Jérémie Galarneau @ 2019-05-17 20:44 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: Simon Marchi, Jeremie Galarneau, lttng-dev


[-- Attachment #1.1: Type: text/plain, Size: 39356 bytes --]

Merged in master and stable-2.0.
The first patch (the fix) was back-ported to stable-1.5.

Anyone interested in the discussion that lead to v8, see the following
gerrit changes:
https://review.lttng.org/c/babeltrace/+/1311
https://review.lttng.org/c/babeltrace/+/1305/4
https://review.lttng.org/c/babeltrace/+/1307/4
https://review.lttng.org/c/babeltrace/+/1318/1

Thanks!
Jérémie




On Thu, 16 May 2019 at 16:58, Mathieu Desnoyers <
mathieu.desnoyers@efficios.com> wrote:

> bitfield.h uses the left shift operator with a left operand which
> may be negative. The C99 standard states that shifting a negative
> value is undefined.
>
> When building with -Wshift-negative-value, we get this gcc warning:
>
> In file included from
> /home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:44:0,
>                  from /home/smarchi/src/babeltrace/ctfser/ctfser.c:42:
> /home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h: In
> function ‘bt_ctfser_write_unsigned_int’:
> /home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:116:24:
> error: left shift of negative value [-Werror=shift-negative-value]
>    mask = ~((~(type) 0) << (__start % ts));  \
>                         ^
> /home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:222:2:
> note: in expansion of macro ‘_bt_bitfield_write_le’
>   _bt_bitfield_write_le(ptr, type, _start, _length, _v)
>   ^~~~~~~~~~~~~~~~~~~~~
> /home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:418:3:
> note: in expansion of macro ‘bt_bitfield_write_le’
>    bt_bitfield_write_le(mmap_align_addr(ctfser->base_mma) +
>    ^~~~~~~~~~~~~~~~~~~~
>
> This boils down to the fact that the expression ~((uint8_t)0) has type
> "signed int", which is used as an operand of the left shift.  This is due
> to the integer promotion rules of C99 (6.3.3.1):
>
>     If an int can represent all values of the original type, the value is
>     converted to an int; otherwise, it is converted to an unsigned int.
>     These are called the integer promotions. All other types are unchanged
>     by the integer promotions.
>
> We also need to cast the result explicitly into the left hand
> side type to deal with:
>
> warning: large integer implicitly truncated to unsigned type [-Woverflow]
>
> The C99 standard states that a right shift has implementation-defined
> behavior when shifting a signed negative value. Add a preprocessor check
> that the compiler provides the expected behavior, else provide an
> alternative implementation which guarantees the intended behavior.
>
> A preprocessor check is also added to ensure that the compiler
> representation for signed values is two's complement, which is expected
> by this header.
>
> Document that this header strictly respects the C99 standard, with
> the exception of its use of __typeof__.
>
> Adapt use of _bt_piecewise_lshift in plugins/text/pretty/print.c
> to the new API.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Acked-by: Simon Marchi <simon.marchi@efficios.com>
> Change-Id: I47d2655cfe671baf0fc18813883adf7ce11dfd6a
> ---
> Changes since v1:
> - Generate compile-time error if the type argument passed to
>   _bt_unsigned_cast() is larger than sizeof(uint64_t), this
>   allows removing _bt_check_max_64bit,
> - Introduce _br_fill_mask, which replaces _bt_bitwise_not,
> - Clarify _bt_unsigned_cast comment,
> - Expand explanation of the issue within the patch commit message.
>
> Changes since v2:
> - Fix unwanted sign extension when generating masks,
> - Introduce macro helpers to clarify code:
>   - _bt_cast_value_to_unsigned()
>   - _bt_cast_value_to_unsigned_type(),
>   - _bt_make_mask_complement(),
>   - _bt_make_mask().
>
> Changes since v3:
> - Fix additional left shift undefined behavior. Identified with
>   -fsanitize=undefined.
> - Create fallback for right shift implementation-defined behavior
>   if the behavior is found not to be as intended. Detect the behavior
>   with a preprocessor check.
> - Ensure that the compiler represents signed types with two's complement
>   with a preprocessor check.
> - Add _bt_rshift() and _bt_lshift() shift helpers to take care of
>   C99's undefined behaviors related to shifting signed types.
> - Remove statement-expressions to allow building with -std=c99.
> - Use __typeof__ instead of typeof to allow building with -std=c99.
>   Strictly speaking, the use of __typeof__ is specific to the compiler.
>   Therefore, porting to a strict ansi C compiler would require removing
>   those __typeof__.
> - Remove use of gnu extension: replace (a ? : b) expressions by (a ? a :
>   b) to please compiling with -std=c99 -Wpedantic.
>
> Changes since v4:
> - Document strict C89 compliance, except for use of __typeof__
>   compiler-specific extension.
> - Adapt use of _bt_piecewise_lshift in plugins/text/pretty/print.c
>   to the new API.
>
> Changes since v5:
> - Bump standard compliance from C89 to C99, because we use
>   C99 types.h.
>
> Changes since v6:
> - Fix wrong use of _bt_make_mask() in _bt_bitfield_write_be. Should
>   be _bt_make_mask_complement. Caught by testing on powerpc.
>
> Changes since v7:
> - Style fixes.
> ---
>  include/babeltrace/bitfield-internal.h | 312
> ++++++++++++++++++++++-----------
>  plugins/text/pretty/print.c            |   4 +-
>  2 files changed, 208 insertions(+), 108 deletions(-)
>
> diff --git a/include/babeltrace/bitfield-internal.h
> b/include/babeltrace/bitfield-internal.h
> index c5d5eccd..5ecde05e 100644
> --- a/include/babeltrace/bitfield-internal.h
> +++ b/include/babeltrace/bitfield-internal.h
> @@ -2,7 +2,7 @@
>  #define _BABELTRACE_BITFIELD_H
>
>  /*
> - * Copyright 2010 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> + * Copyright 2010-2019 - Mathieu Desnoyers <
> mathieu.desnoyers@efficios.com>
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining
> a copy
>   * of this software and associated documentation files (the "Software"),
> to deal
> @@ -27,49 +27,157 @@
>  #include <babeltrace/compat/limits-internal.h> /* C99 5.2.4.2 Numerical
> limits */
>  #include <babeltrace/endian-internal.h>        /* Non-standard
> BIG_ENDIAN, LITTLE_ENDIAN, BYTE_ORDER */
>
> -/* We can't shift a int from 32 bit, >> 32 and << 32 on int is undefined
> */
> -#define _bt_piecewise_rshift(_v, _shift)                               \
> -({                                                                     \
> -       typeof(_v) ___v = (_v);                                         \
> -       typeof(_shift) ___shift = (_shift);                             \
> -       unsigned long sb = (___shift) / (sizeof(___v) * CHAR_BIT - 1);  \
> -       unsigned long final = (___shift) % (sizeof(___v) * CHAR_BIT - 1); \
> -                                                                       \
> -       for (; sb; sb--)                                                \
> -               ___v >>= sizeof(___v) * CHAR_BIT - 1;                   \
> -       ___v >>= final;                                                 \
> -})
> -
> -#define _bt_piecewise_lshift(_v, _shift)                               \
> -({                                                                     \
> -       typeof(_v) ___v = (_v);                                         \
> -       typeof(_shift) ___shift = (_shift);                             \
> -       unsigned long sb = (___shift) / (sizeof(___v) * CHAR_BIT - 1);  \
> -       unsigned long final = (___shift) % (sizeof(___v) * CHAR_BIT - 1); \
> -                                                                       \
> -       for (; sb; sb--)                                                \
> -               ___v <<= sizeof(___v) * CHAR_BIT - 1;                   \
> -       ___v <<= final;                                                 \
> -})
> +/*
> + * This header strictly follows the C99 standard, except for use of the
> + * compiler-specific __typeof__.
> + */
> +
> +/*
> + * This bitfield header requires the compiler representation of signed
> + * integers to be two's complement.
> + */
> +#if (-1 != ~0)
> +#error "bitfield.h requires the compiler representation of signed
> integers to be two's complement."
> +#endif
>
>  #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.
> + * Produce a build-time error if the condition `cond` is non-zero.
> + * Evaluates as a size_t expression.
> + */
> +#define _BT_BUILD_ASSERT(cond)                                 \
> +       sizeof(struct { int f:(2 * !!(cond) - 1); })
> +
> +/*
> + * Cast value `v` to an unsigned integer of the same size as `v`.
> + */
> +#define _bt_cast_value_to_unsigned(v)                                  \
> +       (sizeof(v) == sizeof(uint8_t) ? (uint8_t) (v) :                 \
> +       sizeof(v) == sizeof(uint16_t) ? (uint16_t) (v) :                \
> +       sizeof(v) == sizeof(uint32_t) ? (uint32_t) (v) :                \
> +       sizeof(v) == sizeof(uint64_t) ? (uint64_t) (v) :                \
> +       _BT_BUILD_ASSERT(sizeof(v) <= sizeof(uint64_t)))
> +
> +/*
> + * Cast value `v` to an unsigned integer type of the size of type `type`
> + * *without* sign-extension.
> + *
> + * The unsigned cast ensures that we're not shifting a negative value,
> + * which is undefined in C. However, this limits the maximum type size
> + * of `type` to 64-bit. Generate a compile-time error if the size of
> + * `type` is larger than 64-bit.
> + */
> +#define _bt_cast_value_to_unsigned_type(type, v)                       \
> +       (sizeof(type) == sizeof(uint8_t) ?                              \
> +               (uint8_t) _bt_cast_value_to_unsigned(v) :               \
> +       sizeof(type) == sizeof(uint16_t) ?                              \
> +               (uint16_t) _bt_cast_value_to_unsigned(v) :              \
> +       sizeof(type) == sizeof(uint32_t) ?                              \
> +               (uint32_t) _bt_cast_value_to_unsigned(v) :              \
> +       sizeof(type) == sizeof(uint64_t) ?                              \
> +               (uint64_t) _bt_cast_value_to_unsigned(v) :              \
> +       _BT_BUILD_ASSERT(sizeof(v) <= sizeof(uint64_t)))
> +
> +/*
> + * _bt_fill_mask evaluates to a "type" integer with all bits set.
> + */
> +#define _bt_fill_mask(type)    ((type) ~(type) 0)
> +
> +/*
> + * Left shift a value `v` of `shift` bits.
> + *
> + * The type of `v` can be signed or unsigned integer.
> + * The value of `shift` must be less than the size of `v` (in bits),
> + * otherwise the behavior is undefined.
> + * Evaluates to the result of the shift operation.
> + *
> + * According to the C99 standard, left shift of a left hand-side signed
> + * type is undefined if it has a negative value or if the result cannot
> + * be represented in the result type. This bitfield header discards the
> + * bits that are left-shifted beyond the result type representation,
> + * which is the behavior of an unsigned type left shift operation.
> + * Therefore, always perform left shift on an unsigned type.
> + *
> + * This macro should not be used if `shift` can be greater or equal than
> + * the bitwidth of `v`. See `_bt_safe_lshift`.
> + */
> +#define _bt_lshift(v, shift)                                           \
> +       ((__typeof__(v)) (_bt_cast_value_to_unsigned(v) << (shift)))
> +
> +/*
> + * Generate a mask of type `type` with the `length` least significant bits
> + * cleared, and the most significant bits set.
>   */
> -#define _bt_unsigned_cast(type, v)                                     \
> -({                                                                     \
> -       (sizeof(v) < sizeof(type)) ?                                    \
> -               ((type) (v)) & ((type) (~(~(uint64_t) 0 << (sizeof(v) *
> CHAR_BIT)))) : \
> -               (type) (v);                                             \
> -})
> +#define _bt_make_mask_complement(type, length)                         \
> +       _bt_lshift(_bt_fill_mask(type), length)
>
> -#define _bt_check_max_64bit(type)                                      \
> -       char _max_64bit_assertion[sizeof(type) <= sizeof(uint64_t) ? 1 :
> -1] __attribute__((unused))
> +/*
> + * Generate a mask of type `type` with the `length` least significant bits
> + * set, and the most significant bits cleared.
> + */
> +#define _bt_make_mask(type, length)                                    \
> +       ((type) ~_bt_make_mask_complement(type, length))
> +
> +/*
> + * Right shift a value `v` of `shift` bits.
> + *
> + * The type of `v` can be signed or unsigned integer.
> + * The value of `shift` must be less than the size of `v` (in bits),
> + * otherwise the behavior is undefined.
> + * Evaluates to the result of the shift operation.
> + *
> + * According to the C99 standard, right shift of a left hand-side signed
> + * type which has a negative value is implementation defined. This
> + * bitfield header relies on the right shift implementation carrying the
> + * sign bit. If the compiler implementation has a different behavior,
> + * emulate carrying the sign bit.
> + *
> + * This macro should not be used if `shift` can be greater or equal than
> + * the bitwidth of `v`. See `_bt_safe_rshift`.
> + */
> +#if ((-1 >> 1) == -1)
> +#define _bt_rshift(v, shift)   ((v) >> (shift))
> +#else
> +#define _bt_rshift(v, shift)                                           \
> +       ((__typeof__(v)) ((_bt_cast_value_to_unsigned(v) >> (shift)) |  \
> +               ((v) < 0 ? _bt_make_mask_complement(__typeof__(v),      \
> +                       sizeof(v) * CHAR_BIT - (shift)) : 0)))
> +#endif
> +
> +/*
> + * Right shift a signed or unsigned integer with `shift` value being an
> + * arbitrary number of bits. `v` is modified by this macro. The shift
> + * is transformed into a sequence of `_nr_partial_shifts` consecutive
> + * shift operations, each of a number of bits smaller than the bitwidth
> + * of `v`, ending with a shift of the number of left over bits.
> + */
> +#define _bt_safe_rshift(v, shift)                                      \
> +do {                                                                   \
> +       unsigned long _nr_partial_shifts = (shift) / (sizeof(v) * CHAR_BIT
> - 1); \
> +       unsigned long _leftover_bits = (shift) % (sizeof(v) * CHAR_BIT -
> 1); \
> +                                                                       \
> +       for (; _nr_partial_shifts; _nr_partial_shifts--)                \
> +               (v) = _bt_rshift(v, sizeof(v) * CHAR_BIT - 1);          \
> +       (v) = _bt_rshift(v, _leftover_bits);                            \
> +} while (0)
> +
> +/*
> + * Left shift a signed or unsigned integer with `shift` value being an
> + * arbitrary number of bits. `v` is modified by this macro. The shift
> + * is transformed into a sequence of `_nr_partial_shifts` consecutive
> + * shift operations, each of a number of bits smaller than the bitwidth
> + * of `v`, ending with a shift of the number of left over bits.
> + */
> +#define _bt_safe_lshift(v, shift)                                      \
> +do {                                                                   \
> +       unsigned long _nr_partial_shifts = (shift) / (sizeof(v) * CHAR_BIT
> - 1); \
> +       unsigned long _leftover_bits = (shift) % (sizeof(v) * CHAR_BIT -
> 1); \
> +                                                                       \
> +       for (; _nr_partial_shifts; _nr_partial_shifts--)                \
> +               (v) = _bt_lshift(v, sizeof(v) * CHAR_BIT - 1);          \
> +       (v) = _bt_lshift(v, _leftover_bits);                            \
> +} while (0)
>
>  /*
>   * bt_bitfield_write - write integer to a bitfield in native endianness
> @@ -91,7 +199,7 @@
>
>  #define _bt_bitfield_write_le(_ptr, type, _start, _length, _v)         \
>  do {                                                                   \
> -       typeof(_v) __v = (_v);                                          \
> +       __typeof__(_v) __v = (_v);                                      \
>         type *__ptr = (void *) (_ptr);                                  \
>         unsigned long __start = (_start), __length = (_length);         \
>         type mask, cmask;                                               \
> @@ -108,15 +216,15 @@ do {
>                       \
>                                                                         \
>         /* Trim v high bits */                                          \
>         if (__length < sizeof(__v) * CHAR_BIT)                          \
> -               __v &= ~((~(typeof(__v)) 0) << __length);               \
> +               __v &= _bt_make_mask(__typeof__(__v), __length);        \
>                                                                         \
>         /* We can now append v with a simple "or", shift it piece-wise */ \
>         this_unit = start_unit;                                         \
>         if (start_unit == end_unit - 1) {                               \
> -               mask = ~((~(type) 0) << (__start % ts));                \
> +               mask = _bt_make_mask(type, __start % ts);               \
>                 if (end % ts)                                           \
> -                       mask |= (~(type) 0) << (end % ts);              \
> -               cmask = (type) __v << (__start % ts);                   \
> +                       mask |= _bt_make_mask_complement(type, end % ts); \
> +               cmask = _bt_lshift((type) (__v), __start % ts);         \
>                 cmask &= ~mask;                                         \
>                 __ptr[this_unit] &= mask;                               \
>                 __ptr[this_unit] |= cmask;                              \
> @@ -124,22 +232,22 @@ do {
>                       \
>         }                                                               \
>         if (__start % ts) {                                             \
>                 cshift = __start % ts;                                  \
> -               mask = ~((~(type) 0) << cshift);                        \
> -               cmask = (type) __v << cshift;                           \
> +               mask = _bt_make_mask(type, cshift);                     \
> +               cmask = _bt_lshift((type) (__v), cshift);               \
>                 cmask &= ~mask;                                         \
>                 __ptr[this_unit] &= mask;                               \
>                 __ptr[this_unit] |= cmask;                              \
> -               __v = _bt_piecewise_rshift(__v, ts - cshift);           \
> +               _bt_safe_rshift(__v, ts - cshift);                      \
>                 __start += ts - cshift;                                 \
>                 this_unit++;                                            \
>         }                                                               \
>         for (; this_unit < end_unit - 1; this_unit++) {                 \
>                 __ptr[this_unit] = (type) __v;                          \
> -               __v = _bt_piecewise_rshift(__v, ts);                    \
> +               _bt_safe_rshift(__v, ts);                               \
>                 __start += ts;                                          \
>         }                                                               \
>         if (end % ts) {                                                 \
> -               mask = (~(type) 0) << (end % ts);                       \
> +               mask = _bt_make_mask_complement(type, end % ts);        \
>                 cmask = (type) __v;                                     \
>                 cmask &= ~mask;                                         \
>                 __ptr[this_unit] &= mask;                               \
> @@ -150,7 +258,7 @@ do {
>                       \
>
>  #define _bt_bitfield_write_be(_ptr, type, _start, _length, _v)         \
>  do {                                                                   \
> -       typeof(_v) __v = (_v);                                          \
> +       __typeof__(_v) __v = (_v);                                      \
>         type *__ptr = (void *) (_ptr);                                  \
>         unsigned long __start = (_start), __length = (_length);         \
>         type mask, cmask;                                               \
> @@ -167,15 +275,15 @@ do {
>                       \
>                                                                         \
>         /* Trim v high bits */                                          \
>         if (__length < sizeof(__v) * CHAR_BIT)                          \
> -               __v &= ~((~(typeof(__v)) 0) << __length);               \
> +               __v &= _bt_make_mask(__typeof__(__v), __length);        \
>                                                                         \
>         /* We can now append v with a simple "or", shift it piece-wise */ \
>         this_unit = end_unit - 1;                                       \
>         if (start_unit == end_unit - 1) {                               \
> -               mask = ~((~(type) 0) << ((ts - (end % ts)) % ts));      \
> +               mask = _bt_make_mask(type, (ts - (end % ts)) % ts);     \
>                 if (__start % ts)                                       \
> -                       mask |= (~((type) 0)) << (ts - (__start % ts)); \
> -               cmask = (type) __v << ((ts - (end % ts)) % ts);         \
> +                       mask |= _bt_make_mask_complement(type, ts -
> (__start % ts)); \
> +               cmask = _bt_lshift((type) (__v), (ts - (end % ts)) % ts); \
>                 cmask &= ~mask;                                         \
>                 __ptr[this_unit] &= mask;                               \
>                 __ptr[this_unit] |= cmask;                              \
> @@ -183,22 +291,22 @@ do {
>                       \
>         }                                                               \
>         if (end % ts) {                                                 \
>                 cshift = end % ts;                                      \
> -               mask = ~((~(type) 0) << (ts - cshift));                 \
> -               cmask = (type) __v << (ts - cshift);                    \
> +               mask = _bt_make_mask(type, ts - cshift);                \
> +               cmask = _bt_lshift((type) (__v), ts - cshift);          \
>                 cmask &= ~mask;                                         \
>                 __ptr[this_unit] &= mask;                               \
>                 __ptr[this_unit] |= cmask;                              \
> -               __v = _bt_piecewise_rshift(__v, cshift);                \
> +               _bt_safe_rshift(__v, cshift);                           \
>                 end -= cshift;                                          \
>                 this_unit--;                                            \
>         }                                                               \
>         for (; (long) this_unit >= (long) start_unit + 1; this_unit--) { \
>                 __ptr[this_unit] = (type) __v;                          \
> -               __v = _bt_piecewise_rshift(__v, ts);                    \
> +               _bt_safe_rshift(__v, ts);                               \
>                 end -= ts;                                              \
>         }                                                               \
>         if (__start % ts) {                                             \
> -               mask = (~(type) 0) << (ts - (__start % ts));            \
> +               mask = _bt_make_mask_complement(type, ts - (__start %
> ts)); \
>                 cmask = (type) __v;                                     \
>                 cmask &= ~mask;                                         \
>                 __ptr[this_unit] &= mask;                               \
> @@ -243,8 +351,8 @@ do {
>                       \
>
>  #define _bt_bitfield_read_le(_ptr, type, _start, _length, _vptr)       \
>  do {                                                                   \
> -       typeof(*(_vptr)) *__vptr = (_vptr);                             \
> -       typeof(*__vptr) __v;                                            \
> +       __typeof__(*(_vptr)) *__vptr = (_vptr);                         \
> +       __typeof__(*__vptr) __v;                                        \
>         type *__ptr = (void *) (_ptr);                                  \
>         unsigned long __start = (_start), __length = (_length);         \
>         type mask, cmask;                                               \
> @@ -252,10 +360,6 @@ 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;                                                  \
> @@ -266,56 +370,56 @@ do {
>                       \
>         end_unit = (end + (ts - 1)) / ts;                               \
>                                                                         \
>         this_unit = end_unit - 1;                                       \
> -       if (_bt_is_signed_type(typeof(__v))                             \
> -           && (__ptr[this_unit] & ((type) 1 << ((end % ts ? : ts) - 1))))
> \
> -               __v = ~(typeof(__v)) 0;                                 \
> +       if (_bt_is_signed_type(__typeof__(__v))                         \
> +           && (__ptr[this_unit] & _bt_lshift((type) 1, (end % ts ? end %
> ts : ts) - 1))) \
> +               __v = ~(__typeof__(__v)) 0;                             \
>         else                                                            \
>                 __v = 0;                                                \
>         if (start_unit == end_unit - 1) {                               \
>                 cmask = __ptr[this_unit];                               \
> -               cmask >>= (__start % ts);                               \
> +               cmask = _bt_rshift(cmask, __start % ts);                \
>                 if ((end - __start) % ts) {                             \
> -                       mask = ~((~(type) 0) << (end - __start));       \
> +                       mask = _bt_make_mask(type, end - __start);      \
>                         cmask &= mask;                                  \
>                 }                                                       \
> -               __v = _bt_piecewise_lshift(__v, end - __start);         \
> -               __v |= _bt_unsigned_cast(typeof(__v), cmask);           \
> +               _bt_safe_lshift(__v, end - __start);                    \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> cmask); \
>                 *__vptr = __v;                                          \
>                 break;                                                  \
>         }                                                               \
>         if (end % ts) {                                                 \
>                 cshift = end % ts;                                      \
> -               mask = ~((~(type) 0) << cshift);                        \
> +               mask = _bt_make_mask(type, cshift);                     \
>                 cmask = __ptr[this_unit];                               \
>                 cmask &= mask;                                          \
> -               __v = _bt_piecewise_lshift(__v, cshift);                \
> -               __v |= _bt_unsigned_cast(typeof(__v), cmask);           \
> +               _bt_safe_lshift(__v, cshift);                           \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> cmask); \
>                 end -= cshift;                                          \
>                 this_unit--;                                            \
>         }                                                               \
>         for (; (long) this_unit >= (long) start_unit + 1; this_unit--) { \
> -               __v = _bt_piecewise_lshift(__v, ts);                    \
> -               __v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
> +               _bt_safe_lshift(__v, ts);                               \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> __ptr[this_unit]); \
>                 end -= ts;                                              \
>         }                                                               \
>         if (__start % ts) {                                             \
> -               mask = ~((~(type) 0) << (ts - (__start % ts)));         \
> +               mask = _bt_make_mask(type, ts - (__start % ts));        \
>                 cmask = __ptr[this_unit];                               \
> -               cmask >>= (__start % ts);                               \
> +               cmask = _bt_rshift(cmask, __start % ts);                \
>                 cmask &= mask;                                          \
> -               __v = _bt_piecewise_lshift(__v, ts - (__start % ts));   \
> -               __v |= _bt_unsigned_cast(typeof(__v), cmask);           \
> +               _bt_safe_lshift(__v, ts - (__start % ts));              \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> cmask); \
>         } else {                                                        \
> -               __v = _bt_piecewise_lshift(__v, ts);                    \
> -               __v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
> +               _bt_safe_lshift(__v, ts);                               \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> __ptr[this_unit]); \
>         }                                                               \
>         *__vptr = __v;                                                  \
>  } while (0)
>
>  #define _bt_bitfield_read_be(_ptr, type, _start, _length, _vptr)       \
>  do {                                                                   \
> -       typeof(*(_vptr)) *__vptr = (_vptr);                             \
> -       typeof(*__vptr) __v;                                            \
> +       __typeof__(*(_vptr)) *__vptr = (_vptr);                         \
> +       __typeof__(*__vptr) __v;                                        \
>         type *__ptr = (void *) (_ptr);                                  \
>         unsigned long __start = (_start), __length = (_length);         \
>         type mask, cmask;                                               \
> @@ -323,10 +427,6 @@ 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;                                                  \
> @@ -337,48 +437,48 @@ do {
>                       \
>         end_unit = (end + (ts - 1)) / ts;                               \
>                                                                         \
>         this_unit = start_unit;                                         \
> -       if (_bt_is_signed_type(typeof(__v))                             \
> -           && (__ptr[this_unit] & ((type) 1 << (ts - (__start % ts) -
> 1)))) \
> -               __v = ~(typeof(__v)) 0;                                 \
> +       if (_bt_is_signed_type(__typeof__(__v))                         \
> +           && (__ptr[this_unit] & _bt_lshift((type) 1, ts - (__start %
> ts) - 1))) \
> +               __v = ~(__typeof__(__v)) 0;                             \
>         else                                                            \
>                 __v = 0;                                                \
>         if (start_unit == end_unit - 1) {                               \
>                 cmask = __ptr[this_unit];                               \
> -               cmask >>= (ts - (end % ts)) % ts;                       \
> +               cmask = _bt_rshift(cmask, (ts - (end % ts)) % ts);      \
>                 if ((end - __start) % ts) {                             \
> -                       mask = ~((~(type) 0) << (end - __start));       \
> +                       mask = _bt_make_mask(type, end - __start);      \
>                         cmask &= mask;                                  \
>                 }                                                       \
> -               __v = _bt_piecewise_lshift(__v, end - __start);         \
> -               __v |= _bt_unsigned_cast(typeof(__v), cmask);           \
> +               _bt_safe_lshift(__v, end - __start);            \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> cmask); \
>                 *__vptr = __v;                                          \
>                 break;                                                  \
>         }                                                               \
>         if (__start % ts) {                                             \
>                 cshift = __start % ts;                                  \
> -               mask = ~((~(type) 0) << (ts - cshift));                 \
> +               mask = _bt_make_mask(type, ts - cshift);                \
>                 cmask = __ptr[this_unit];                               \
>                 cmask &= mask;                                          \
> -               __v = _bt_piecewise_lshift(__v, ts - cshift);           \
> -               __v |= _bt_unsigned_cast(typeof(__v), cmask);           \
> +               _bt_safe_lshift(__v, ts - cshift);                      \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> cmask); \
>                 __start += ts - cshift;                                 \
>                 this_unit++;                                            \
>         }                                                               \
>         for (; this_unit < end_unit - 1; this_unit++) {                 \
> -               __v = _bt_piecewise_lshift(__v, ts);                    \
> -               __v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
> +               _bt_safe_lshift(__v, ts);                               \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> __ptr[this_unit]); \
>                 __start += ts;                                          \
>         }                                                               \
>         if (end % ts) {                                                 \
> -               mask = ~((~(type) 0) << (end % ts));                    \
> +               mask = _bt_make_mask(type, end % ts);                   \
>                 cmask = __ptr[this_unit];                               \
> -               cmask >>= ts - (end % ts);                              \
> +               cmask = _bt_rshift(cmask, ts - (end % ts));             \
>                 cmask &= mask;                                          \
> -               __v = _bt_piecewise_lshift(__v, end % ts);              \
> -               __v |= _bt_unsigned_cast(typeof(__v), cmask);           \
> +               _bt_safe_lshift(__v, end % ts);                         \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> cmask); \
>         } else {                                                        \
> -               __v = _bt_piecewise_lshift(__v, ts);                    \
> -               __v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
> +               _bt_safe_lshift(__v, ts);                               \
> +               __v |= _bt_cast_value_to_unsigned_type(__typeof__(__v),
> __ptr[this_unit]); \
>         }                                                               \
>         *__vptr = __v;                                                  \
>  } while (0)
> diff --git a/plugins/text/pretty/print.c b/plugins/text/pretty/print.c
> index 95189d5e..c36a6e81 100644
> --- a/plugins/text/pretty/print.c
> +++ b/plugins/text/pretty/print.c
> @@ -546,10 +546,10 @@ int print_integer(struct pretty_component *pretty,
>
>                 len = bt_field_class_integer_get_field_value_range(int_fc);
>                 g_string_append(pretty->string, "0b");
> -               v.u = _bt_piecewise_lshift(v.u, 64 - len);
> +               _bt_safe_lshift(v.u, 64 - len);
>                 for (bitnr = 0; bitnr < len; bitnr++) {
>                         g_string_append_printf(pretty->string, "%u", (v.u
> & (1ULL << 63)) ? 1 : 0);
> -                       v.u = _bt_piecewise_lshift(v.u, 1);
> +                       _bt_safe_lshift(v.u, 1);
>                 }
>                 break;
>         }
> --
> 2.11.0
>
>

-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com

[-- Attachment #1.2: Type: text/html, Size: 50520 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v8 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors
@ 2019-05-16 20:58 Mathieu Desnoyers
  0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2019-05-16 20:58 UTC (permalink / raw)
  To: jgalar, simon.marchi; +Cc: lttng-dev

bitfield.h uses the left shift operator with a left operand which
may be negative. The C99 standard states that shifting a negative
value is undefined.

When building with -Wshift-negative-value, we get this gcc warning:

In file included from /home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:44:0,
                 from /home/smarchi/src/babeltrace/ctfser/ctfser.c:42:
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h: In function ‘bt_ctfser_write_unsigned_int’:
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:116:24: error: left shift of negative value [-Werror=shift-negative-value]
   mask = ~((~(type) 0) << (__start % ts));  \
                        ^
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:222:2: note: in expansion of macro ‘_bt_bitfield_write_le’
  _bt_bitfield_write_le(ptr, type, _start, _length, _v)
  ^~~~~~~~~~~~~~~~~~~~~
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:418:3: note: in expansion of macro ‘bt_bitfield_write_le’
   bt_bitfield_write_le(mmap_align_addr(ctfser->base_mma) +
   ^~~~~~~~~~~~~~~~~~~~

This boils down to the fact that the expression ~((uint8_t)0) has type
"signed int", which is used as an operand of the left shift.  This is due
to the integer promotion rules of C99 (6.3.3.1):

    If an int can represent all values of the original type, the value is
    converted to an int; otherwise, it is converted to an unsigned int.
    These are called the integer promotions. All other types are unchanged
    by the integer promotions.

We also need to cast the result explicitly into the left hand
side type to deal with:

warning: large integer implicitly truncated to unsigned type [-Woverflow]

The C99 standard states that a right shift has implementation-defined
behavior when shifting a signed negative value. Add a preprocessor check
that the compiler provides the expected behavior, else provide an
alternative implementation which guarantees the intended behavior.

A preprocessor check is also added to ensure that the compiler
representation for signed values is two's complement, which is expected
by this header.

Document that this header strictly respects the C99 standard, with
the exception of its use of __typeof__.

Adapt use of _bt_piecewise_lshift in plugins/text/pretty/print.c
to the new API.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Acked-by: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I47d2655cfe671baf0fc18813883adf7ce11dfd6a
---
Changes since v1:
- Generate compile-time error if the type argument passed to
  _bt_unsigned_cast() is larger than sizeof(uint64_t), this
  allows removing _bt_check_max_64bit,
- Introduce _br_fill_mask, which replaces _bt_bitwise_not,
- Clarify _bt_unsigned_cast comment,
- Expand explanation of the issue within the patch commit message.

Changes since v2:
- Fix unwanted sign extension when generating masks,
- Introduce macro helpers to clarify code:
  - _bt_cast_value_to_unsigned()
  - _bt_cast_value_to_unsigned_type(),
  - _bt_make_mask_complement(),
  - _bt_make_mask().

Changes since v3:
- Fix additional left shift undefined behavior. Identified with
  -fsanitize=undefined.
- Create fallback for right shift implementation-defined behavior
  if the behavior is found not to be as intended. Detect the behavior
  with a preprocessor check.
- Ensure that the compiler represents signed types with two's complement
  with a preprocessor check.
- Add _bt_rshift() and _bt_lshift() shift helpers to take care of
  C99's undefined behaviors related to shifting signed types.
- Remove statement-expressions to allow building with -std=c99.
- Use __typeof__ instead of typeof to allow building with -std=c99.
  Strictly speaking, the use of __typeof__ is specific to the compiler.
  Therefore, porting to a strict ansi C compiler would require removing
  those __typeof__.
- Remove use of gnu extension: replace (a ? : b) expressions by (a ? a :
  b) to please compiling with -std=c99 -Wpedantic.

Changes since v4:
- Document strict C89 compliance, except for use of __typeof__
  compiler-specific extension.
- Adapt use of _bt_piecewise_lshift in plugins/text/pretty/print.c
  to the new API.

Changes since v5:
- Bump standard compliance from C89 to C99, because we use
  C99 types.h.

Changes since v6:
- Fix wrong use of _bt_make_mask() in _bt_bitfield_write_be. Should
  be _bt_make_mask_complement. Caught by testing on powerpc.

Changes since v7:
- Style fixes.
---
 include/babeltrace/bitfield-internal.h | 312 ++++++++++++++++++++++-----------
 plugins/text/pretty/print.c            |   4 +-
 2 files changed, 208 insertions(+), 108 deletions(-)

diff --git a/include/babeltrace/bitfield-internal.h b/include/babeltrace/bitfield-internal.h
index c5d5eccd..5ecde05e 100644
--- a/include/babeltrace/bitfield-internal.h
+++ b/include/babeltrace/bitfield-internal.h
@@ -2,7 +2,7 @@
 #define _BABELTRACE_BITFIELD_H
 
 /*
- * Copyright 2010 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+ * Copyright 2010-2019 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -27,49 +27,157 @@
 #include <babeltrace/compat/limits-internal.h>	/* C99 5.2.4.2 Numerical limits */
 #include <babeltrace/endian-internal.h>	/* Non-standard BIG_ENDIAN, LITTLE_ENDIAN, BYTE_ORDER */
 
-/* We can't shift a int from 32 bit, >> 32 and << 32 on int is undefined */
-#define _bt_piecewise_rshift(_v, _shift)				\
-({									\
-	typeof(_v) ___v = (_v);						\
-	typeof(_shift) ___shift = (_shift);				\
-	unsigned long sb = (___shift) / (sizeof(___v) * CHAR_BIT - 1);	\
-	unsigned long final = (___shift) % (sizeof(___v) * CHAR_BIT - 1); \
-									\
-	for (; sb; sb--)						\
-		___v >>= sizeof(___v) * CHAR_BIT - 1;			\
-	___v >>= final;							\
-})
-
-#define _bt_piecewise_lshift(_v, _shift)				\
-({									\
-	typeof(_v) ___v = (_v);						\
-	typeof(_shift) ___shift = (_shift);				\
-	unsigned long sb = (___shift) / (sizeof(___v) * CHAR_BIT - 1);	\
-	unsigned long final = (___shift) % (sizeof(___v) * CHAR_BIT - 1); \
-									\
-	for (; sb; sb--)						\
-		___v <<= sizeof(___v) * CHAR_BIT - 1;			\
-	___v <<= final;							\
-})
+/*
+ * This header strictly follows the C99 standard, except for use of the
+ * compiler-specific __typeof__.
+ */
+
+/*
+ * This bitfield header requires the compiler representation of signed
+ * integers to be two's complement.
+ */
+#if (-1 != ~0)
+#error "bitfield.h requires the compiler representation of signed integers to be two's complement."
+#endif
 
 #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.
+ * Produce a build-time error if the condition `cond` is non-zero.
+ * Evaluates as a size_t expression.
+ */
+#define _BT_BUILD_ASSERT(cond)					\
+	sizeof(struct { int f:(2 * !!(cond) - 1); })
+
+/*
+ * Cast value `v` to an unsigned integer of the same size as `v`.
+ */
+#define _bt_cast_value_to_unsigned(v)					\
+	(sizeof(v) == sizeof(uint8_t) ? (uint8_t) (v) :			\
+	sizeof(v) == sizeof(uint16_t) ? (uint16_t) (v) :		\
+	sizeof(v) == sizeof(uint32_t) ? (uint32_t) (v) :		\
+	sizeof(v) == sizeof(uint64_t) ? (uint64_t) (v) :		\
+	_BT_BUILD_ASSERT(sizeof(v) <= sizeof(uint64_t)))
+
+/*
+ * Cast value `v` to an unsigned integer type of the size of type `type`
+ * *without* sign-extension.
+ *
+ * The unsigned cast ensures that we're not shifting a negative value,
+ * which is undefined in C. However, this limits the maximum type size
+ * of `type` to 64-bit. Generate a compile-time error if the size of
+ * `type` is larger than 64-bit.
+ */
+#define _bt_cast_value_to_unsigned_type(type, v)			\
+	(sizeof(type) == sizeof(uint8_t) ?				\
+		(uint8_t) _bt_cast_value_to_unsigned(v) :		\
+	sizeof(type) == sizeof(uint16_t) ?				\
+		(uint16_t) _bt_cast_value_to_unsigned(v) :		\
+	sizeof(type) == sizeof(uint32_t) ?				\
+		(uint32_t) _bt_cast_value_to_unsigned(v) :		\
+	sizeof(type) == sizeof(uint64_t) ?				\
+		(uint64_t) _bt_cast_value_to_unsigned(v) :		\
+	_BT_BUILD_ASSERT(sizeof(v) <= sizeof(uint64_t)))
+
+/*
+ * _bt_fill_mask evaluates to a "type" integer with all bits set.
+ */
+#define _bt_fill_mask(type)	((type) ~(type) 0)
+
+/*
+ * Left shift a value `v` of `shift` bits.
+ *
+ * The type of `v` can be signed or unsigned integer.
+ * The value of `shift` must be less than the size of `v` (in bits),
+ * otherwise the behavior is undefined.
+ * Evaluates to the result of the shift operation.
+ *
+ * According to the C99 standard, left shift of a left hand-side signed
+ * type is undefined if it has a negative value or if the result cannot
+ * be represented in the result type. This bitfield header discards the
+ * bits that are left-shifted beyond the result type representation,
+ * which is the behavior of an unsigned type left shift operation.
+ * Therefore, always perform left shift on an unsigned type.
+ *
+ * This macro should not be used if `shift` can be greater or equal than
+ * the bitwidth of `v`. See `_bt_safe_lshift`.
+ */
+#define _bt_lshift(v, shift)						\
+	((__typeof__(v)) (_bt_cast_value_to_unsigned(v) << (shift)))
+
+/*
+ * Generate a mask of type `type` with the `length` least significant bits
+ * cleared, and the most significant bits set.
  */
-#define _bt_unsigned_cast(type, v)					\
-({									\
-	(sizeof(v) < sizeof(type)) ?					\
-		((type) (v)) & ((type) (~(~(uint64_t) 0 << (sizeof(v) * CHAR_BIT)))) : \
-		(type) (v);						\
-})
+#define _bt_make_mask_complement(type, length)				\
+	_bt_lshift(_bt_fill_mask(type), length)
 
-#define _bt_check_max_64bit(type)					\
-	char _max_64bit_assertion[sizeof(type) <= sizeof(uint64_t) ? 1 : -1] __attribute__((unused))
+/*
+ * Generate a mask of type `type` with the `length` least significant bits
+ * set, and the most significant bits cleared.
+ */
+#define _bt_make_mask(type, length)					\
+	((type) ~_bt_make_mask_complement(type, length))
+
+/*
+ * Right shift a value `v` of `shift` bits.
+ *
+ * The type of `v` can be signed or unsigned integer.
+ * The value of `shift` must be less than the size of `v` (in bits),
+ * otherwise the behavior is undefined.
+ * Evaluates to the result of the shift operation.
+ *
+ * According to the C99 standard, right shift of a left hand-side signed
+ * type which has a negative value is implementation defined. This
+ * bitfield header relies on the right shift implementation carrying the
+ * sign bit. If the compiler implementation has a different behavior,
+ * emulate carrying the sign bit.
+ *
+ * This macro should not be used if `shift` can be greater or equal than
+ * the bitwidth of `v`. See `_bt_safe_rshift`.
+ */
+#if ((-1 >> 1) == -1)
+#define _bt_rshift(v, shift)	((v) >> (shift))
+#else
+#define _bt_rshift(v, shift)						\
+	((__typeof__(v)) ((_bt_cast_value_to_unsigned(v) >> (shift)) |	\
+		((v) < 0 ? _bt_make_mask_complement(__typeof__(v),	\
+			sizeof(v) * CHAR_BIT - (shift)) : 0)))
+#endif
+
+/*
+ * Right shift a signed or unsigned integer with `shift` value being an
+ * arbitrary number of bits. `v` is modified by this macro. The shift
+ * is transformed into a sequence of `_nr_partial_shifts` consecutive
+ * shift operations, each of a number of bits smaller than the bitwidth
+ * of `v`, ending with a shift of the number of left over bits.
+ */
+#define _bt_safe_rshift(v, shift)					\
+do {									\
+	unsigned long _nr_partial_shifts = (shift) / (sizeof(v) * CHAR_BIT - 1); \
+	unsigned long _leftover_bits = (shift) % (sizeof(v) * CHAR_BIT - 1); \
+									\
+	for (; _nr_partial_shifts; _nr_partial_shifts--)		\
+		(v) = _bt_rshift(v, sizeof(v) * CHAR_BIT - 1);		\
+	(v) = _bt_rshift(v, _leftover_bits);				\
+} while (0)
+
+/*
+ * Left shift a signed or unsigned integer with `shift` value being an
+ * arbitrary number of bits. `v` is modified by this macro. The shift
+ * is transformed into a sequence of `_nr_partial_shifts` consecutive
+ * shift operations, each of a number of bits smaller than the bitwidth
+ * of `v`, ending with a shift of the number of left over bits.
+ */
+#define _bt_safe_lshift(v, shift)					\
+do {									\
+	unsigned long _nr_partial_shifts = (shift) / (sizeof(v) * CHAR_BIT - 1); \
+	unsigned long _leftover_bits = (shift) % (sizeof(v) * CHAR_BIT - 1); \
+									\
+	for (; _nr_partial_shifts; _nr_partial_shifts--)		\
+		(v) = _bt_lshift(v, sizeof(v) * CHAR_BIT - 1);		\
+	(v) = _bt_lshift(v, _leftover_bits);				\
+} while (0)
 
 /*
  * bt_bitfield_write - write integer to a bitfield in native endianness
@@ -91,7 +199,7 @@
 
 #define _bt_bitfield_write_le(_ptr, type, _start, _length, _v)		\
 do {									\
-	typeof(_v) __v = (_v);						\
+	__typeof__(_v) __v = (_v);					\
 	type *__ptr = (void *) (_ptr);					\
 	unsigned long __start = (_start), __length = (_length);		\
 	type mask, cmask;						\
@@ -108,15 +216,15 @@ do {									\
 									\
 	/* Trim v high bits */						\
 	if (__length < sizeof(__v) * CHAR_BIT)				\
-		__v &= ~((~(typeof(__v)) 0) << __length);		\
+		__v &= _bt_make_mask(__typeof__(__v), __length);	\
 									\
 	/* We can now append v with a simple "or", shift it piece-wise */ \
 	this_unit = start_unit;						\
 	if (start_unit == end_unit - 1) {				\
-		mask = ~((~(type) 0) << (__start % ts));		\
+		mask = _bt_make_mask(type, __start % ts);		\
 		if (end % ts)						\
-			mask |= (~(type) 0) << (end % ts);		\
-		cmask = (type) __v << (__start % ts);			\
+			mask |= _bt_make_mask_complement(type, end % ts); \
+		cmask = _bt_lshift((type) (__v), __start % ts);		\
 		cmask &= ~mask;						\
 		__ptr[this_unit] &= mask;				\
 		__ptr[this_unit] |= cmask;				\
@@ -124,22 +232,22 @@ do {									\
 	}								\
 	if (__start % ts) {						\
 		cshift = __start % ts;					\
-		mask = ~((~(type) 0) << cshift);			\
-		cmask = (type) __v << cshift;				\
+		mask = _bt_make_mask(type, cshift);			\
+		cmask = _bt_lshift((type) (__v), cshift);		\
 		cmask &= ~mask;						\
 		__ptr[this_unit] &= mask;				\
 		__ptr[this_unit] |= cmask;				\
-		__v = _bt_piecewise_rshift(__v, ts - cshift);		\
+		_bt_safe_rshift(__v, ts - cshift);			\
 		__start += ts - cshift;					\
 		this_unit++;						\
 	}								\
 	for (; this_unit < end_unit - 1; this_unit++) {			\
 		__ptr[this_unit] = (type) __v;				\
-		__v = _bt_piecewise_rshift(__v, ts);			\
+		_bt_safe_rshift(__v, ts);				\
 		__start += ts;						\
 	}								\
 	if (end % ts) {							\
-		mask = (~(type) 0) << (end % ts);			\
+		mask = _bt_make_mask_complement(type, end % ts);	\
 		cmask = (type) __v;					\
 		cmask &= ~mask;						\
 		__ptr[this_unit] &= mask;				\
@@ -150,7 +258,7 @@ do {									\
 
 #define _bt_bitfield_write_be(_ptr, type, _start, _length, _v)		\
 do {									\
-	typeof(_v) __v = (_v);						\
+	__typeof__(_v) __v = (_v);					\
 	type *__ptr = (void *) (_ptr);					\
 	unsigned long __start = (_start), __length = (_length);		\
 	type mask, cmask;						\
@@ -167,15 +275,15 @@ do {									\
 									\
 	/* Trim v high bits */						\
 	if (__length < sizeof(__v) * CHAR_BIT)				\
-		__v &= ~((~(typeof(__v)) 0) << __length);		\
+		__v &= _bt_make_mask(__typeof__(__v), __length);	\
 									\
 	/* We can now append v with a simple "or", shift it piece-wise */ \
 	this_unit = end_unit - 1;					\
 	if (start_unit == end_unit - 1) {				\
-		mask = ~((~(type) 0) << ((ts - (end % ts)) % ts));	\
+		mask = _bt_make_mask(type, (ts - (end % ts)) % ts);	\
 		if (__start % ts)					\
-			mask |= (~((type) 0)) << (ts - (__start % ts));	\
-		cmask = (type) __v << ((ts - (end % ts)) % ts);		\
+			mask |= _bt_make_mask_complement(type, ts - (__start % ts)); \
+		cmask = _bt_lshift((type) (__v), (ts - (end % ts)) % ts); \
 		cmask &= ~mask;						\
 		__ptr[this_unit] &= mask;				\
 		__ptr[this_unit] |= cmask;				\
@@ -183,22 +291,22 @@ do {									\
 	}								\
 	if (end % ts) {							\
 		cshift = end % ts;					\
-		mask = ~((~(type) 0) << (ts - cshift));			\
-		cmask = (type) __v << (ts - cshift);			\
+		mask = _bt_make_mask(type, ts - cshift);		\
+		cmask = _bt_lshift((type) (__v), ts - cshift);		\
 		cmask &= ~mask;						\
 		__ptr[this_unit] &= mask;				\
 		__ptr[this_unit] |= cmask;				\
-		__v = _bt_piecewise_rshift(__v, cshift);		\
+		_bt_safe_rshift(__v, cshift);				\
 		end -= cshift;						\
 		this_unit--;						\
 	}								\
 	for (; (long) this_unit >= (long) start_unit + 1; this_unit--) { \
 		__ptr[this_unit] = (type) __v;				\
-		__v = _bt_piecewise_rshift(__v, ts);			\
+		_bt_safe_rshift(__v, ts);				\
 		end -= ts;						\
 	}								\
 	if (__start % ts) {						\
-		mask = (~(type) 0) << (ts - (__start % ts));		\
+		mask = _bt_make_mask_complement(type, ts - (__start % ts)); \
 		cmask = (type) __v;					\
 		cmask &= ~mask;						\
 		__ptr[this_unit] &= mask;				\
@@ -243,8 +351,8 @@ do {									\
 
 #define _bt_bitfield_read_le(_ptr, type, _start, _length, _vptr)	\
 do {									\
-	typeof(*(_vptr)) *__vptr = (_vptr);				\
-	typeof(*__vptr) __v;						\
+	__typeof__(*(_vptr)) *__vptr = (_vptr);				\
+	__typeof__(*__vptr) __v;					\
 	type *__ptr = (void *) (_ptr);					\
 	unsigned long __start = (_start), __length = (_length);		\
 	type mask, cmask;						\
@@ -252,10 +360,6 @@ 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;							\
@@ -266,56 +370,56 @@ do {									\
 	end_unit = (end + (ts - 1)) / ts;				\
 									\
 	this_unit = end_unit - 1;					\
-	if (_bt_is_signed_type(typeof(__v))				\
-	    && (__ptr[this_unit] & ((type) 1 << ((end % ts ? : ts) - 1)))) \
-		__v = ~(typeof(__v)) 0;					\
+	if (_bt_is_signed_type(__typeof__(__v))				\
+	    && (__ptr[this_unit] & _bt_lshift((type) 1, (end % ts ? end % ts : ts) - 1))) \
+		__v = ~(__typeof__(__v)) 0;				\
 	else								\
 		__v = 0;						\
 	if (start_unit == end_unit - 1) {				\
 		cmask = __ptr[this_unit];				\
-		cmask >>= (__start % ts);				\
+		cmask = _bt_rshift(cmask, __start % ts);		\
 		if ((end - __start) % ts) {				\
-			mask = ~((~(type) 0) << (end - __start));	\
+			mask = _bt_make_mask(type, end - __start);	\
 			cmask &= mask;					\
 		}							\
-		__v = _bt_piecewise_lshift(__v, end - __start);		\
-		__v |= _bt_unsigned_cast(typeof(__v), cmask);		\
+		_bt_safe_lshift(__v, end - __start);			\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
 		*__vptr = __v;						\
 		break;							\
 	}								\
 	if (end % ts) {							\
 		cshift = end % ts;					\
-		mask = ~((~(type) 0) << cshift);			\
+		mask = _bt_make_mask(type, cshift);			\
 		cmask = __ptr[this_unit];				\
 		cmask &= mask;						\
-		__v = _bt_piecewise_lshift(__v, cshift);		\
-		__v |= _bt_unsigned_cast(typeof(__v), cmask);		\
+		_bt_safe_lshift(__v, cshift);				\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
 		end -= cshift;						\
 		this_unit--;						\
 	}								\
 	for (; (long) this_unit >= (long) start_unit + 1; this_unit--) { \
-		__v = _bt_piecewise_lshift(__v, ts);			\
-		__v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
+		_bt_safe_lshift(__v, ts);				\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
 		end -= ts;						\
 	}								\
 	if (__start % ts) {						\
-		mask = ~((~(type) 0) << (ts - (__start % ts)));		\
+		mask = _bt_make_mask(type, ts - (__start % ts));	\
 		cmask = __ptr[this_unit];				\
-		cmask >>= (__start % ts);				\
+		cmask = _bt_rshift(cmask, __start % ts);		\
 		cmask &= mask;						\
-		__v = _bt_piecewise_lshift(__v, ts - (__start % ts));	\
-		__v |= _bt_unsigned_cast(typeof(__v), cmask);		\
+		_bt_safe_lshift(__v, ts - (__start % ts));		\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
 	} else {							\
-		__v = _bt_piecewise_lshift(__v, ts);			\
-		__v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
+		_bt_safe_lshift(__v, ts);				\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
 	}								\
 	*__vptr = __v;							\
 } while (0)
 
 #define _bt_bitfield_read_be(_ptr, type, _start, _length, _vptr)	\
 do {									\
-	typeof(*(_vptr)) *__vptr = (_vptr);				\
-	typeof(*__vptr) __v;						\
+	__typeof__(*(_vptr)) *__vptr = (_vptr);				\
+	__typeof__(*__vptr) __v;					\
 	type *__ptr = (void *) (_ptr);					\
 	unsigned long __start = (_start), __length = (_length);		\
 	type mask, cmask;						\
@@ -323,10 +427,6 @@ 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;							\
@@ -337,48 +437,48 @@ do {									\
 	end_unit = (end + (ts - 1)) / ts;				\
 									\
 	this_unit = start_unit;						\
-	if (_bt_is_signed_type(typeof(__v))				\
-	    && (__ptr[this_unit] & ((type) 1 << (ts - (__start % ts) - 1)))) \
-		__v = ~(typeof(__v)) 0;					\
+	if (_bt_is_signed_type(__typeof__(__v))				\
+	    && (__ptr[this_unit] & _bt_lshift((type) 1, ts - (__start % ts) - 1))) \
+		__v = ~(__typeof__(__v)) 0;				\
 	else								\
 		__v = 0;						\
 	if (start_unit == end_unit - 1) {				\
 		cmask = __ptr[this_unit];				\
-		cmask >>= (ts - (end % ts)) % ts;			\
+		cmask = _bt_rshift(cmask, (ts - (end % ts)) % ts);	\
 		if ((end - __start) % ts) {				\
-			mask = ~((~(type) 0) << (end - __start));	\
+			mask = _bt_make_mask(type, end - __start);	\
 			cmask &= mask;					\
 		}							\
-		__v = _bt_piecewise_lshift(__v, end - __start);		\
-		__v |= _bt_unsigned_cast(typeof(__v), cmask);		\
+		_bt_safe_lshift(__v, end - __start);		\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
 		*__vptr = __v;						\
 		break;							\
 	}								\
 	if (__start % ts) {						\
 		cshift = __start % ts;					\
-		mask = ~((~(type) 0) << (ts - cshift));			\
+		mask = _bt_make_mask(type, ts - cshift);		\
 		cmask = __ptr[this_unit];				\
 		cmask &= mask;						\
-		__v = _bt_piecewise_lshift(__v, ts - cshift);		\
-		__v |= _bt_unsigned_cast(typeof(__v), cmask);		\
+		_bt_safe_lshift(__v, ts - cshift);			\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
 		__start += ts - cshift;					\
 		this_unit++;						\
 	}								\
 	for (; this_unit < end_unit - 1; this_unit++) {			\
-		__v = _bt_piecewise_lshift(__v, ts);			\
-		__v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
+		_bt_safe_lshift(__v, ts);				\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
 		__start += ts;						\
 	}								\
 	if (end % ts) {							\
-		mask = ~((~(type) 0) << (end % ts));			\
+		mask = _bt_make_mask(type, end % ts);			\
 		cmask = __ptr[this_unit];				\
-		cmask >>= ts - (end % ts);				\
+		cmask = _bt_rshift(cmask, ts - (end % ts));		\
 		cmask &= mask;						\
-		__v = _bt_piecewise_lshift(__v, end % ts);		\
-		__v |= _bt_unsigned_cast(typeof(__v), cmask);		\
+		_bt_safe_lshift(__v, end % ts);				\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), cmask); \
 	} else {							\
-		__v = _bt_piecewise_lshift(__v, ts);			\
-		__v |= _bt_unsigned_cast(typeof(__v), __ptr[this_unit]);\
+		_bt_safe_lshift(__v, ts);				\
+		__v |= _bt_cast_value_to_unsigned_type(__typeof__(__v), __ptr[this_unit]); \
 	}								\
 	*__vptr = __v;							\
 } while (0)
diff --git a/plugins/text/pretty/print.c b/plugins/text/pretty/print.c
index 95189d5e..c36a6e81 100644
--- a/plugins/text/pretty/print.c
+++ b/plugins/text/pretty/print.c
@@ -546,10 +546,10 @@ int print_integer(struct pretty_component *pretty,
 
 		len = bt_field_class_integer_get_field_value_range(int_fc);
 		g_string_append(pretty->string, "0b");
-		v.u = _bt_piecewise_lshift(v.u, 64 - len);
+		_bt_safe_lshift(v.u, 64 - len);
 		for (bitnr = 0; bitnr < len; bitnr++) {
 			g_string_append_printf(pretty->string, "%u", (v.u & (1ULL << 63)) ? 1 : 0);
-			v.u = _bt_piecewise_lshift(v.u, 1);
+			_bt_safe_lshift(v.u, 1);
 		}
 		break;
 	}
-- 
2.11.0

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-05-17 20:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190516205848.20409-1-mathieu.desnoyers@efficios.com>
2019-05-16 20:58 ` [PATCH v2 babeltrace 2/4] Silence compiler "always false comparison" warning Mathieu Desnoyers
2019-05-16 20:58 ` [PATCH babeltrace 3/4] Cleanup: bitfields: streamline use of underscores Mathieu Desnoyers
2019-05-16 20:58 ` [PATCH v2 babeltrace 4/4] Extend test_bitfield coverage Mathieu Desnoyers
2019-05-17 20:44 ` [PATCH v8 babeltrace 1/4] Fix: bitfield: shift undefined/implementation defined behaviors Jérémie Galarneau
2019-05-16 20:58 Mathieu Desnoyers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).