diff mbox

Fix ubsan shift instrumentation

Message ID 20141023123404.GK10501@redhat.com
State New
Headers show

Commit Message

Marek Polacek Oct. 23, 2014, 12:34 p.m. UTC 
The issue here was that we were diagnosing an artificial check
that we created within the scope of shift instrumentation.  In
other words, for shifts we create something like
(unsigned) A >> (B - C) and signed-integer-overflow triggered
on that subtraction.  Fixed by making the subtraction work on
unsigned types.  This only happened in C99/C++11 mode.
Middle end seems to cope well with RSHIFT_EXPR whose second op
has an unsigned type.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2014-10-23  Marek Polacek  <polacek@redhat.com>

	* c-ubsan.c (ubsan_instrument_shift): Perform the MINUS_EXPR
	in unsigned type.

	* c-c++-common/ubsan/undefined-2.c: New test.


	Marek

Comments

Jakub Jelinek Oct. 23, 2014, 12:46 p.m. UTC | #1 
On Thu, Oct 23, 2014 at 02:34:04PM +0200, Marek Polacek wrote:
> The issue here was that we were diagnosing an artificial check
> that we created within the scope of shift instrumentation.  In
> other words, for shifts we create something like
> (unsigned) A >> (B - C) and signed-integer-overflow triggered
> on that subtraction.  Fixed by making the subtraction work on
> unsigned types.  This only happened in C99/C++11 mode.
> Middle end seems to cope well with RSHIFT_EXPR whose second op
> has an unsigned type.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2014-10-23  Marek Polacek  <polacek@redhat.com>
> 
> 	* c-ubsan.c (ubsan_instrument_shift): Perform the MINUS_EXPR
> 	in unsigned type.
> 
> 	* c-c++-common/ubsan/undefined-2.c: New test.

Ok.  Can you please queue it for 4.9 branch too, after 4.9.2 is released?
There is no -f*sanitize-recover* support, but it can be supposedly left out
from dg-options for the branch.

	Jakub
Marek Polacek Oct. 23, 2014, 12:51 p.m. UTC | #2 
On Thu, Oct 23, 2014 at 02:46:52PM +0200, Jakub Jelinek wrote:
> Ok.  Can you please queue it for 4.9 branch too, after 4.9.2 is released?
> There is no -f*sanitize-recover* support, but it can be supposedly left out
> from dg-options for the branch.

Sure.

	Marek
diff mbox

Patch

diff --git gcc/c-family/c-ubsan.c gcc/c-family/c-ubsan.c
index 5a42303..7f4dc25 100644
--- gcc/c-family/c-ubsan.c
+++ gcc/c-family/c-ubsan.c
@@ -128,19 +128,19 @@  ubsan_instrument_shift (location_t loc, enum tree_code code,
   tree op1_utype = unsigned_type_for (type1);
   HOST_WIDE_INT op0_prec = TYPE_PRECISION (type0);
   tree uprecm1 = build_int_cst (op1_utype, op0_prec - 1);
-  tree precm1 = build_int_cst (type1, op0_prec - 1);
 
   t = fold_convert_loc (loc, op1_utype, op1);
   t = fold_build2 (GT_EXPR, boolean_type_node, t, uprecm1);
 
   /* For signed x << y, in C99/C11, the following:
-     (unsigned) x >> (precm1 - y)
+     (unsigned) x >> (uprecm1 - y)
      if non-zero, is undefined.  */
   if (code == LSHIFT_EXPR
       && !TYPE_UNSIGNED (type0)
       && flag_isoc99)
     {
-      tree x = fold_build2 (MINUS_EXPR, integer_type_node, precm1, op1);
+      tree x = fold_build2 (MINUS_EXPR, unsigned_type_node, uprecm1,
+			    fold_convert (op1_utype, op1));
       tt = fold_convert_loc (loc, unsigned_type_for (type0), op0);
       tt = fold_build2 (RSHIFT_EXPR, TREE_TYPE (tt), tt, x);
       tt = fold_build2 (NE_EXPR, boolean_type_node, tt,
@@ -148,13 +148,14 @@  ubsan_instrument_shift (location_t loc, enum tree_code code,
     }
 
   /* For signed x << y, in C++11 and later, the following:
-     x < 0 || ((unsigned) x >> (precm1 - y))
+     x < 0 || ((unsigned) x >> (uprecm1 - y))
      if > 1, is undefined.  */
   if (code == LSHIFT_EXPR
       && !TYPE_UNSIGNED (TREE_TYPE (op0))
       && (cxx_dialect >= cxx11))
     {
-      tree x = fold_build2 (MINUS_EXPR, integer_type_node, precm1, op1);
+      tree x = fold_build2 (MINUS_EXPR, unsigned_type_node, uprecm1,
+			    fold_convert (op1_utype, op1));
       tt = fold_convert_loc (loc, unsigned_type_for (type0), op0);
       tt = fold_build2 (RSHIFT_EXPR, TREE_TYPE (tt), tt, x);
       tt = fold_build2 (GT_EXPR, boolean_type_node, tt,
diff --git gcc/testsuite/c-c++-common/ubsan/undefined-2.c gcc/testsuite/c-c++-common/ubsan/undefined-2.c
index e69de29..7b06709 100644
--- gcc/testsuite/c-c++-common/ubsan/undefined-2.c
+++ gcc/testsuite/c-c++-common/ubsan/undefined-2.c
@@ -0,0 +1,22 @@ 
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=signed-integer-overflow" } */
+/* { dg-additional-options "-std=gnu11" { target c } } */
+/* { dg-additional-options "-std=c++11" { target c++ } } */
+
+volatile int w, z;
+
+__attribute__ ((noinline, noclone)) int
+foo (int x, int y)
+{
+  z++;
+  return x << y;
+}
+
+int
+main ()
+{
+  w = foo (0, -__INT_MAX__);
+  return 0;
+}
+
+/* { dg-output "shift exponent -\[^\n\r]* is negative\[^\n\r]*(\n|\r\n|\r)" } */