Both C11 and C++14 standards specify that integral promotions are
performed on both operands of a shift-expression. This we do just
fine. But then we convert the right operand to integer_type_node.
Not only is this unnecessary, it can also be harfmul, because for
e.g.
void
foo (unsigned int x)
{
if (-1 >> x != -1)
bar ();
}
with (int) x we lose info that x is nonnegative, which means that
tree_expr_nonnegative_p cannot fold this expr. Another problem
with the conversion is that we weren't able to detect e.g. shift
by 0x100000000ULL, since after the conversion this is 0.
This patch does away with the conversion to integer type for both
C and C++ FEs. It exposed an ubsan bug where I was using incorrect
type for a MINUS_EXPR. With this patch, the XFAIL in shiftopt-1.c
is not needed anymore.
Joseph, is that C FE part ok?
Jason, is that C++ FE part ok?
Jakub, is that ubsan part ok?
Bootstrapped/regtested on ppc64-linux; ubsan.exp tested even on x86_64.
2014-11-26 Marek Polacek <pola...@redhat.com>
PR c/63862
c-family/
* c-ubsan.c (ubsan_instrument_shift): Change the type of a MINUS_EXPR
to op1_utype.
c/
* c-typeck.c (build_binary_op) <RSHIFT_EXPR, LSHIFT_EXPR>: Don't
convert the right operand to integer type.
cp/
* typeck.c (cp_build_binary_op) <RSHIFT_EXPR, LSHIFT_EXPR>: Don't
convert the right operand to integer type.
testsuite/
* gcc.c-torture/execute/shiftopt-1.c: Don't XFAIL anymore.
* c-c++-common/ubsan/shift-7.c: New test.
diff --git gcc/c-family/c-ubsan.c gcc/c-family/c-ubsan.c
index 90b03f2..96afc67 100644
--- gcc/c-family/c-ubsan.c
+++ gcc/c-family/c-ubsan.c
@@ -151,7 +151,7 @@ ubsan_instrument_shift (location_t loc, enum tree_code code,
&& !TYPE_UNSIGNED (type0)
&& flag_isoc99)
{
- tree x = fold_build2 (MINUS_EXPR, unsigned_type_node, uprecm1,
+ tree x = fold_build2 (MINUS_EXPR, op1_utype, 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);
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 67efb46..bf0f306 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -10513,11 +10513,6 @@ build_binary_op (location_t location, enum tree_code
code,
/* Use the type of the value to be shifted. */
result_type = type0;
- /* Convert the non vector shift-count to an integer, regardless
- of size of value being shifted. */
- if (TREE_CODE (TREE_TYPE (op1)) != VECTOR_TYPE
- && TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = convert (integer_type_node, op1);
/* Avoid converting op1 to result_type later. */
converted = 1;
}
@@ -10563,11 +10558,6 @@ build_binary_op (location_t location, enum tree_code
code,
/* Use the type of the value to be shifted. */
result_type = type0;
- /* Convert the non vector shift-count to an integer, regardless
- of size of value being shifted. */
- if (TREE_CODE (TREE_TYPE (op1)) != VECTOR_TYPE
- && TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = convert (integer_type_node, op1);
/* Avoid converting op1 to result_type later. */
converted = 1;
}
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 8b66acc..6ca346b 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -4295,10 +4295,6 @@ cp_build_binary_op (location_t location,
"right shift count >= width of type");
}
}
- /* Convert the shift-count to an integer, regardless of
- size of value being shifted. */
- if (TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = cp_convert (integer_type_node, op1, complain);
/* Avoid converting op1 to result_type later. */
converted = 1;
}
@@ -4344,10 +4340,6 @@ cp_build_binary_op (location_t location,
"left shift count >= width of type");
}
}
- /* Convert the shift-count to an integer, regardless of
- size of value being shifted. */
- if (TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = cp_convert (integer_type_node, op1, complain);
/* Avoid converting op1 to result_type later. */
converted = 1;
}
diff --git gcc/testsuite/c-c++-common/ubsan/shift-7.c
gcc/testsuite/c-c++-common/ubsan/shift-7.c
index e69de29..1e33273 100644
--- gcc/testsuite/c-c++-common/ubsan/shift-7.c
+++ gcc/testsuite/c-c++-common/ubsan/shift-7.c
@@ -0,0 +1,27 @@
+/* PR c/63862 */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined" } */
+
+unsigned long long int __attribute__ ((noinline, noclone))
+foo (unsigned long long int i, unsigned long long int j)
+{
+ asm ("");
+ return i >> j;
+}
+
+unsigned long long int __attribute__ ((noinline, noclone))
+bar (unsigned long long int i, unsigned long long int j)
+{
+ asm ("");
+ return i << j;
+}
+
+int
+main ()
+{
+ foo (1ULL, 0x100000000ULL);
+ bar (1ULL, 0x100000000ULL);
+}
+
+/* { dg-output "shift exponent 4294967296 is too large for \[^\n\r]*-bit type
'long long unsigned int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*shift exponent 4294967296 is too large for
\[^\n\r]*-bit type 'long long unsigned int'\[^\n\r]*(\n|\r\n|\r)" } */
diff --git gcc/testsuite/gcc.c-torture/execute/shiftopt-1.c
gcc/testsuite/gcc.c-torture/execute/shiftopt-1.c
index 3ff714d..8c855b8 100644
--- gcc/testsuite/gcc.c-torture/execute/shiftopt-1.c
+++ gcc/testsuite/gcc.c-torture/execute/shiftopt-1.c
@@ -22,16 +22,11 @@ utest (unsigned int x)
if (0 >> x != 0)
link_error ();
- /* XFAIL: the C frontend converts the shift amount to 'int'
- thus we get -1 >> (int)x which means the shift amount may
- be negative. See PR63862. */
-#if 0
if (-1 >> x != -1)
link_error ();
if (~0 >> x != ~0)
link_error ();
-#endif
}
void
Marek
