Fix: bitfield: shift undefined/implementation defined behaviors
authorMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Thu, 16 May 2019 20:58:45 +0000 (16:58 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Fri, 17 May 2019 20:41:21 +0000 (16:41 -0400)
commit7f140ae8b9d8fe9a9df9451ef86eebdbbd62a1f1
treec727d9fd76ae7acd12e60d1be678c3137973bfda
parent75a151671b0e3d2a90eeff2ccd1ab32d4a1729de
Fix: bitfield: shift undefined/implementation defined behaviors

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
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1311
Tested-by: jenkins
include/babeltrace/bitfield-internal.h
plugins/text/pretty/print.c
This page took 0.024621 seconds and 4 git commands to generate.