* [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; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread
end of thread, other threads:[~2019-05-17 20:45 UTC | newest]
Thread overview: 4+ 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
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).