public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Marek Polacek <polacek@redhat.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>,
	Jason Merrill <jason@redhat.com>,
	       Joseph Myers <joseph@codesourcery.com>
Subject: [C/C++ PATCH] Implement -Wshift-negative-value (PR c/65179)
Date: Wed, 22 Apr 2015 18:36:00 -0000	[thread overview]
Message-ID: <20150422183633.GI28950@redhat.com> (raw)

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  <polacek@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

             reply	other threads:[~2015-04-22 18:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-22 18:36 Marek Polacek [this message]
2015-04-23 22:04 ` Jeff Law
2015-04-24  3:11 ` Martin Sebor
2015-04-24 16:27   ` Marek Polacek
2015-04-24 21:32     ` Martin Sebor
2015-04-24 21:44       ` Martin Sebor
2015-04-25 17:31       ` Marek Polacek
2015-04-25 20:13         ` Joseph Myers
2015-04-27 15:46           ` Marek Polacek
2015-04-29 23:57             ` Joseph Myers
2015-05-06 11:38               ` Marek Polacek
2015-05-07 18:00                 ` Jeff Law
2015-05-07 19:15                   ` Marek Polacek
2015-05-08 16:38                     ` Steve Ellcey
2015-05-08 16:40                       ` Markus Trippelsdorf
2015-05-08 16:59                       ` Joseph Myers
2015-05-08 21:54                         ` Paul Eggert
2015-05-12  2:58                           ` Steve Ellcey
2015-05-12 12:40                             ` [tz] " Christos Zoulas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150422183633.GI28950@redhat.com \
    --to=polacek@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=joseph@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).