On 2019-05-08 6:23 p.m., Mathieu Desnoyers 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] > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoy...@efficios.com> > --- > 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().
I might have more cases for you to investigate :) Can you try building with -fsanitize=undefined and run test_bitfield? I get some: /home/smarchi/src/babeltrace/tests/lib/test_bitfield.c:222:4: runtime error: left shift of negative value -1 Which would hint that we are missing a cast before a shift. Simon _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
