On Thu, Nov 27, 2014 at 10:56:06AM +0100, Richard Biener wrote:
> But then rather than using integer_type_node I'd convert to
> unsigned_type_node (if that doesn't work you know we have some nasty
> dependence on negative shift counts "working")
Yeah, I think adding the conversion into c_gimplify_expr is the best
thing we can do until we have type demotion/promotion pass. Seems
that unsigned_type_node there works well.
(What happened to Kai's type elevation pass?)
Bootstrapped/regtested on ppc64-linux, ok for trunk?
2014-11-27 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-gimplify.c (c_gimplify_expr): Convert right operand of a shift
expression to unsigned_type_node.
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-gimplify.c gcc/c-family/c-gimplify.c
index 85b4223..8e0eb4c 100644
--- gcc/c-family/c-gimplify.c
+++ gcc/c-family/c-gimplify.c
@@ -242,6 +242,24 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p
ATTRIBUTE_UNUSED,
switch (code)
{
+ case LSHIFT_EXPR:
+ case RSHIFT_EXPR:
+ {
+ /* We used to convert the right operand of a shift-expression
+ to an integer_type_node in the FEs. But it is unnecessary
+ and not desirable for diagnostics and sanitizers. We keep
+ this here to not pessimize the code, but we convert to an
+ unsigned type, because negative shift counts are undefined
+ anyway.
+ We should get rid of this conversion when we have a proper
+ type demotion/promotion pass. */
+ tree *op1_p = &TREE_OPERAND (*expr_p, 1);
+ if (TREE_CODE (TREE_TYPE (*op1_p)) != VECTOR_TYPE
+ && TYPE_MAIN_VARIANT (TREE_TYPE (*op1_p)) != unsigned_type_node)
+ *op1_p = convert (unsigned_type_node, *op1_p);
+ break;
+ }
+
case DECL_EXPR:
/* This is handled mostly by gimplify.c, but we have to deal with
not warning about int x = x; as it is a GCC extension to turn off
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
