On Tue, Nov 17, 2015 at 02:32:50PM +0100, Paolo Bonzini wrote:
> Left shifts into the sign bit is a kind of overflow, and the
> standard chooses to treat left shifts of negative values the
> same way.
>
> However, the -fwrapv option modifies the language to one where
> integers are defined as two's complement---which also defines
> entirely the behavior of shifts. Disable sanitization of left
> shifts when -fwrapv is in effect.
>
> This needs test cases of course, but I wanted to be sure in advance
> whether this is an acceptable change and whether it is considered
> a bug (thus acceptable for stage 3). The same change was proposed
> for LLVM at https://llvm.org/bugs/show_bug.cgi?id=25552.
>
> Paolo
>
> * c-family/c-ubsan.c (ubsan_instrument_shift): Disable sanitization
> of left shifts for wrapping signed types as well.
>
>
> Index: c-family/c-ubsan.c
> ===================================================================
> --- c-family/c-ubsan.c (revision 227511)
> +++ c-family/c-ubsan.c (working copy)
> @@ -150,7 +150,7 @@
> (unsigned) x >> (uprecm1 - y)
> if non-zero, is undefined. */
> if (code == LSHIFT_EXPR
> - && !TYPE_UNSIGNED (type0)
> + && !TYPE_OVERFLOW_WRAPS (type0)
> && flag_isoc99)
> {
> tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
> @@ -165,7 +165,7 @@
> x < 0 || ((unsigned) x >> (uprecm1 - y))
> if > 1, is undefined. */
> if (code == LSHIFT_EXPR
> - && !TYPE_UNSIGNED (type0)
> + && !TYPE_OVERFLOW_WRAPS (type0)
> && (cxx_dialect >= cxx11))
> {
> tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
I think this would be ok provided you add some testcases (unless I'm missing
something). Note that this suppresses instrumenting not only left-shifting
into the sign bit, but also shift overflows, so e.g. 10 << 30.
And I think this might be viewed on as a bug, thus should be ok even at this
stage if you open a PR.
Marek
