[RFC PATCH] Do not sanitize left shifts for -fwrapv

[RFC PATCH] Do not sanitize left shifts for -fwrapv

[Paolo Bonzini]

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,

Re: [RFC PATCH] Do not sanitize left shifts for -fwrapv

[Marek Polacek]

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