Currently, we warn if the right operand of a shift expression is negative, or greater than or equal to the length in bits of the promoted left operand.
But we don't warn when we see a left shift of a negative value. That is undefined behavior since C99 and I believe C++11, so this patch implements a new warning, -Wshift-negative-value, only active in C99/C++11. A bunch of tests needed tweaking; I find it scary that some vect tests are invoking UB. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2015-04-22 Marek Polacek <pola...@redhat.com> PR c/65179 * c-common.c (c_fully_fold_internal): Warn when left shifting a negative value. * c.opt (Wshift-negative-value): New option. * c-typeck.c (build_binary_op): Warn when left shifting a negative value. * typeck.c (cp_build_binary_op): Warn when left shifting a negative value. * doc/invoke.texi: Document -Wshift-negative-value. * c-c++-common/Wshift-negative-value-1.c: New test. * c-c++-common/Wshift-negative-value-2.c: New test. * c-c++-common/torture/vector-shift2.c: Use -Wno-shift-negative-value. * gcc.dg/torture/vector-shift2.c: Likewise. * gcc.c-torture/execute/pr40386.c: Likewise. * gcc.dg/tree-ssa/vrp65.c: Likewise. * gcc.dg/tree-ssa/vrp66.c: Likewise. * gcc.dg/vect/vect-sdivmod-1.c: Likewise. * gcc.dg/vect/vect-shift-2-big-array.c: Likewise. * gcc.dg/vect/vect-shift-2.c: Likewise. * gcc.target/i386/pr31167.c: Likewise. * g++.dg/init/array11.C: Add dg-warning. * gcc.dg/c99-const-expr-7.c: Add dg-warning and dg-error. diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c index 7fe7fa6..e944f11 100644 --- gcc/c-family/c-common.c +++ gcc/c-family/c-common.c @@ -1361,6 +1361,15 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, && !TREE_OVERFLOW_P (op0) && !TREE_OVERFLOW_P (op1)) overflow_warning (EXPR_LOCATION (expr), ret); + if (code == LSHIFT_EXPR + && TREE_CODE (orig_op0) != INTEGER_CST + && TREE_CODE (TREE_TYPE (orig_op0)) == INTEGER_TYPE + && TREE_CODE (op0) == INTEGER_CST + && c_inhibit_evaluation_warnings == 0 + && tree_int_cst_sgn (op0) < 0 + && flag_isoc99) + warning_at (loc, OPT_Wshift_negative_value, + "left shift of negative value"); if ((code == LSHIFT_EXPR || code == RSHIFT_EXPR) && TREE_CODE (orig_op1) != INTEGER_CST && TREE_CODE (op1) == INTEGER_CST diff --git gcc/c-family/c.opt gcc/c-family/c.opt index 983f4a8..47e0913 100644 --- gcc/c-family/c.opt +++ gcc/c-family/c.opt @@ -781,6 +781,10 @@ Wshift-count-overflow C ObjC C++ ObjC++ Var(warn_shift_count_overflow) Init(1) Warning Warn if shift count >= width of type +Wshift-negative-value +C ObjC C++ ObjC++ Var(warn_shift_negative_value) Init(1) Warning +Warn if left shifting a negative value + Wsign-compare C ObjC C++ ObjC++ Var(warn_sign_compare) Warning LangEnabledBy(C++ ObjC++,Wall) Warn about signed-unsigned comparisons diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index ebe4c73..17d2cac 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -10701,6 +10701,15 @@ build_binary_op (location_t location, enum tree_code code, && code1 == INTEGER_TYPE) { doing_shift = true; + if (TREE_CODE (op0) == INTEGER_CST + && tree_int_cst_sgn (op0) < 0 + && flag_isoc99) + { + int_const = false; + if (c_inhibit_evaluation_warnings == 0) + warning_at (location, OPT_Wshift_negative_value, + "left shift of negative value"); + } if (TREE_CODE (op1) == INTEGER_CST) { if (tree_int_cst_sgn (op1) < 0) diff --git gcc/cp/typeck.c gcc/cp/typeck.c index 250b5d6..d5d36bf 100644 --- gcc/cp/typeck.c +++ gcc/cp/typeck.c @@ -4327,11 +4327,21 @@ cp_build_binary_op (location_t location, } else if (code0 == INTEGER_TYPE && code1 == INTEGER_TYPE) { + tree const_op0 = fold_non_dependent_expr (op0); + if (TREE_CODE (const_op0) != INTEGER_CST) + const_op0 = op0; tree const_op1 = fold_non_dependent_expr (op1); if (TREE_CODE (const_op1) != INTEGER_CST) const_op1 = op1; result_type = type0; doing_shift = true; + if (TREE_CODE (const_op0) == INTEGER_CST + && tree_int_cst_sgn (const_op0) < 0 + && (complain & tf_warning) + && c_inhibit_evaluation_warnings == 0 + && cxx_dialect >= cxx11) + warning (OPT_Wshift_negative_value, + "left shift of negative value"); if (TREE_CODE (const_op1) == INTEGER_CST) { if (tree_int_cst_lt (const_op1, integer_zero_node)) diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi index a939ff7..2e14921 100644 --- gcc/doc/invoke.texi +++ gcc/doc/invoke.texi @@ -271,7 +271,7 @@ Objective-C and Objective-C++ Dialects}. -Wpointer-arith -Wno-pointer-to-int-cast @gol -Wredundant-decls -Wno-return-local-addr @gol -Wreturn-type -Wsequence-point -Wshadow -Wno-shadow-ivar @gol --Wshift-count-negative -Wshift-count-overflow @gol +-Wshift-count-negative -Wshift-count-overflow -Wshift-negative-value @gol -Wsign-compare -Wsign-conversion -Wfloat-conversion @gol -Wsizeof-pointer-memaccess -Wsizeof-array-argument @gol -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol @@ -3914,6 +3914,12 @@ Warn if shift count is negative. This warning is enabled by default. @opindex Wno-shift-count-overflow Warn if shift count >= width of type. This warning is enabled by default. +@item -Wshift-negative-value +@opindex Wshift-negative-value +@opindex Wno-shift-negative-value +Warn if left shifting a negative value. This warning only occurs in C99 and +C++11 modes (and newer). This warning is enabled by default. + @item -Wswitch @opindex Wswitch @opindex Wno-switch diff --git gcc/testsuite/c-c++-common/Wshift-negative-value-1.c gcc/testsuite/c-c++-common/Wshift-negative-value-1.c index e69de29..49104e8 100644 --- gcc/testsuite/c-c++-common/Wshift-negative-value-1.c +++ gcc/testsuite/c-c++-common/Wshift-negative-value-1.c @@ -0,0 +1,49 @@ +/* PR c/65179 */ +/* { dg-do compile } */ +/* { dg-options "-O" } */ +/* { dg-additional-options "-std=c++11" { target c++ } } */ + +enum E { + A = 0 << 1, + B = 1 << 1, + C = -1 << 1, /* { dg-warning "left shift of negative value" } */ + D = 0 >> 1, + E = 1 >> 1, + F = -1 >> 1 +}; + +int +left (int x) +{ + /* Warn for LSHIFT_EXPR. */ + const int z = 0; + const int o = 1; + const int m = -1; + int r = 0; + r += z << x; + r += o << x; + r += m << x; /* { dg-warning "left shift of negative value" } */ + r += 0 << x; + r += 1 << x; + r += -1 << x; /* { dg-warning "left shift of negative value" } */ + r += -1U << x; + return r; +} + +int +right (int x) +{ + /* Shouldn't warn for RSHIFT_EXPR. */ + const int z = 0; + const int o = 1; + const int m = -1; + int r = 0; + r += z >> x; + r += o >> x; + r += m >> x; + r += 0 >> x; + r += 1 >> x; + r += -1 >> x; + r += -1U >> x; + return r; +} diff --git gcc/testsuite/c-c++-common/Wshift-negative-value-2.c gcc/testsuite/c-c++-common/Wshift-negative-value-2.c index e69de29..0aea931 100644 --- gcc/testsuite/c-c++-common/Wshift-negative-value-2.c +++ gcc/testsuite/c-c++-common/Wshift-negative-value-2.c @@ -0,0 +1,49 @@ +/* PR c/65179 */ +/* { dg-do compile } */ +/* { dg-options "-O -Wno-shift-negative-value" } */ +/* { dg-additional-options "-std=c++11" { target c++ } } */ + +enum E { + A = 0 << 1, + B = 1 << 1, + C = -1 << 1, /* { dg-bogus "left shift of negative value" } */ + D = 0 >> 1, + E = 1 >> 1, + F = -1 >> 1 +}; + +int +left (int x) +{ + /* Warn for LSHIFT_EXPR. */ + const int z = 0; + const int o = 1; + const int m = -1; + int r = 0; + r += z << x; + r += o << x; + r += m << x; /* { dg-bogus "left shift of negative value" } */ + r += 0 << x; + r += 1 << x; + r += -1 << x; /* { dg-bogus "left shift of negative value" } */ + r += -1U << x; + return r; +} + +int +right (int x) +{ + /* Shouldn't warn for RSHIFT_EXPR. */ + const int z = 0; + const int o = 1; + const int m = -1; + int r = 0; + r += z >> x; + r += o >> x; + r += m >> x; + r += 0 >> x; + r += 1 >> x; + r += -1 >> x; + r += -1U >> x; + return r; +} diff --git gcc/testsuite/c-c++-common/torture/vector-shift2.c gcc/testsuite/c-c++-common/torture/vector-shift2.c index d3a2ef8..6d280d8 100644 --- gcc/testsuite/c-c++-common/torture/vector-shift2.c +++ gcc/testsuite/c-c++-common/torture/vector-shift2.c @@ -1,4 +1,6 @@ /* { dg-do run } */ +/* { dg-additional-options "-Wno-shift-negative-value" } */ + #define vector(elcount, type) \ __attribute__((vector_size((elcount)*sizeof(type)))) type diff --git gcc/testsuite/g++.dg/init/array11.C gcc/testsuite/g++.dg/init/array11.C index e52effe..096b145 100644 --- gcc/testsuite/g++.dg/init/array11.C +++ gcc/testsuite/g++.dg/init/array11.C @@ -21,7 +21,7 @@ struct gdt gdt_table[2]= { { 0, - ( (((size_t)(&x))<<(24))&(-1<<(8)) ), + ( (((size_t)(&x))<<(24))&(-1<<(8)) ), // { dg-warning "left shift of negative value" "" { target c++11 } } }, }; } diff --git gcc/testsuite/gcc.c-torture/execute/pr40386.c gcc/testsuite/gcc.c-torture/execute/pr40386.c index f39f1de..15966f6 100644 --- gcc/testsuite/gcc.c-torture/execute/pr40386.c +++ gcc/testsuite/gcc.c-torture/execute/pr40386.c @@ -1,4 +1,4 @@ -/* { dg-options "-fno-ira-share-spill-slots" } */ +/* { dg-options "-fno-ira-share-spill-slots -Wno-shift-negative-value" } */ extern void abort (void); extern void exit (int); diff --git gcc/testsuite/gcc.dg/c99-const-expr-7.c gcc/testsuite/gcc.dg/c99-const-expr-7.c index b872077..ef87722 100644 --- gcc/testsuite/gcc.dg/c99-const-expr-7.c +++ gcc/testsuite/gcc.dg/c99-const-expr-7.c @@ -30,8 +30,8 @@ int f1 = (0 ? 0 << -1 : 0); int g1 = (0 ? 0 >> 1000 : 0); int h1 = (0 ? 0 >> -1: 0); -/* Allowed for now, but actually undefined behavior in C99. */ -int i = -1 << 0; +int i = -1 << 0; /* { dg-warning "left shift of negative value" } */ +/* { dg-error "constant" "constant" { target *-*-* } 33 } */ int j[1] = { DBL_MAX }; /* { dg-warning "overflow in implicit constant conversion" } */ /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 36 } */ diff --git gcc/testsuite/gcc.dg/torture/vector-shift2.c gcc/testsuite/gcc.dg/torture/vector-shift2.c index 0e8a0eb..2cb3bdc 100644 --- gcc/testsuite/gcc.dg/torture/vector-shift2.c +++ gcc/testsuite/gcc.dg/torture/vector-shift2.c @@ -1,4 +1,5 @@ /* { dg-do run } */ +/* { dg-additional-options "-Wno-shift-negative-value" } */ #define vector(elcount, type) \ __attribute__((vector_size((elcount)*sizeof(type)))) type diff --git gcc/testsuite/gcc.dg/tree-ssa/vrp65.c gcc/testsuite/gcc.dg/tree-ssa/vrp65.c index d109068..a118c71 100644 --- gcc/testsuite/gcc.dg/tree-ssa/vrp65.c +++ gcc/testsuite/gcc.dg/tree-ssa/vrp65.c @@ -1,6 +1,6 @@ /* PR tree-optimization/52267 */ /* { dg-do link } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -Wno-shift-negative-value" } */ extern void link_error (void); diff --git gcc/testsuite/gcc.dg/tree-ssa/vrp66.c gcc/testsuite/gcc.dg/tree-ssa/vrp66.c index 6a6ab34..76cbeed 100644 --- gcc/testsuite/gcc.dg/tree-ssa/vrp66.c +++ gcc/testsuite/gcc.dg/tree-ssa/vrp66.c @@ -1,6 +1,6 @@ /* PR tree-optimization/52267 */ /* { dg-do run { target { ! int16 } } } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -Wno-shift-negative-value" } */ extern void abort (void); diff --git gcc/testsuite/gcc.dg/vect/vect-sdivmod-1.c gcc/testsuite/gcc.dg/vect/vect-sdivmod-1.c index c18204e..2f5e86d 100644 --- gcc/testsuite/gcc.dg/vect/vect-sdivmod-1.c +++ gcc/testsuite/gcc.dg/vect/vect-sdivmod-1.c @@ -1,3 +1,5 @@ +/* { dg-additional-options "-Wno-shift-negative-value" } */ + #include "tree-vect.h" extern void abort (void); diff --git gcc/testsuite/gcc.dg/vect/vect-shift-2-big-array.c gcc/testsuite/gcc.dg/vect/vect-shift-2-big-array.c index 0e1acfb..a36dc5e 100644 --- gcc/testsuite/gcc.dg/vect/vect-shift-2-big-array.c +++ gcc/testsuite/gcc.dg/vect/vect-shift-2-big-array.c @@ -1,3 +1,4 @@ +/* { dg-additional-options "-Wno-shift-negative-value" } */ /* { dg-require-effective-target vect_shift } */ /* { dg-require-effective-target vect_int } */ /* Check the standard integer types for left and right shifts to see if the diff --git gcc/testsuite/gcc.dg/vect/vect-shift-2.c gcc/testsuite/gcc.dg/vect/vect-shift-2.c index 83211eb..4388b78 100644 --- gcc/testsuite/gcc.dg/vect/vect-shift-2.c +++ gcc/testsuite/gcc.dg/vect/vect-shift-2.c @@ -1,3 +1,4 @@ +/* { dg-additional-options "-Wno-shift-negative-value" } */ /* { dg-require-effective-target vect_shift } */ /* { dg-require-effective-target vect_int } */ /* Check the standard integer types for left and right shifts to see if the diff --git gcc/testsuite/gcc.target/i386/pr31167.c gcc/testsuite/gcc.target/i386/pr31167.c index 43d9f84..5470d36 100644 --- gcc/testsuite/gcc.target/i386/pr31167.c +++ gcc/testsuite/gcc.target/i386/pr31167.c @@ -1,6 +1,6 @@ /* { dg-do compile } */ /* { dg-require-effective-target int128 } */ -/* { dg-options "-O" } */ +/* { dg-options "-O -Wno-shift-negative-value" } */ typedef int int32_t; Marek