extend shift count warnings to vector types

gcc/c/
2015-12-10  Jan Beulich  <jbeulich@suse.com>

	* c-fold.c (c_fully_fold_internal): Also emit shift count
	warnings for vector types.
	* c-typeck.c (build_binary_op): Likewise.
extend shift count warnings to vector types

gcc/c/
2015-12-10  Jan Beulich  <jbeulich@suse.com>

	* c-fold.c (c_fully_fold_internal): Also emit shift count
	warnings for vector types.
	* c-typeck.c (build_binary_op): Likewise.

--- 2015-12-09/gcc/c/c-fold.c
+++ 2015-12-09/gcc/c/c-fold.c
@@ -320,8 +320,6 @@ c_fully_fold_internal (tree expr, bool i
       if ((code == LSHIFT_EXPR || code == RSHIFT_EXPR)
 	  && TREE_CODE (orig_op1) != INTEGER_CST
 	  && TREE_CODE (op1) == INTEGER_CST
-	  && (TREE_CODE (TREE_TYPE (orig_op0)) == INTEGER_TYPE
-	      || TREE_CODE (TREE_TYPE (orig_op0)) == FIXED_POINT_TYPE)
 	  && TREE_CODE (TREE_TYPE (orig_op1)) == INTEGER_TYPE
 	  && c_inhibit_evaluation_warnings == 0)
 	{
@@ -330,13 +328,23 @@ c_fully_fold_internal (tree expr, bool i
 			(code == LSHIFT_EXPR
 			 ? G_("left shift count is negative")
 			 : G_("right shift count is negative")));
-	  else if (compare_tree_int (op1,
-				     TYPE_PRECISION (TREE_TYPE (orig_op0)))
-		   >= 0)
+	  else if ((TREE_CODE (TREE_TYPE (orig_op0)) == INTEGER_TYPE
+		    || TREE_CODE (TREE_TYPE (orig_op0)) == FIXED_POINT_TYPE)
+		   && compare_tree_int (op1,
+					TYPE_PRECISION (TREE_TYPE (orig_op0)))
+		      >= 0)
 	    warning_at (loc, OPT_Wshift_count_overflow,
 			(code == LSHIFT_EXPR
 			 ? G_("left shift count >= width of type")
 			 : G_("right shift count >= width of type")));
+	  else if (TREE_CODE (TREE_TYPE (orig_op0)) == VECTOR_TYPE
+		   && compare_tree_int (op1,
+					TYPE_PRECISION (TREE_TYPE (TREE_TYPE (orig_op0))))
+		      >= 0)
+	    warning_at (loc, OPT_Wshift_count_overflow,
+			code == LSHIFT_EXPR
+			? G_("left shift count >= width of vector element")
+			: G_("right shift count >= width of vector element"));
 	}
       if (code == LSHIFT_EXPR
 	  /* If either OP0 has been folded to INTEGER_CST...  */
--- 2015-12-09/gcc/c/c-typeck.c
+++ 2015-12-09/gcc/c/c-typeck.c
@@ -10809,21 +10809,16 @@ build_binary_op (location_t location, en
 	 Also set SHORT_SHIFT if shifting rightward.  */
 
     case RSHIFT_EXPR:
-      if (code0 == VECTOR_TYPE && code1 == INTEGER_TYPE
-          && TREE_CODE (TREE_TYPE (type0)) == INTEGER_TYPE)
-        {
-          result_type = type0;
-          converted = 1;
-        }
-      else if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE
-	       && TREE_CODE (TREE_TYPE (type0)) == INTEGER_TYPE
-	       && TREE_CODE (TREE_TYPE (type1)) == INTEGER_TYPE
-	       && TYPE_VECTOR_SUBPARTS (type0) == TYPE_VECTOR_SUBPARTS (type1))
+      if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE
+	  && TREE_CODE (TREE_TYPE (type0)) == INTEGER_TYPE
+	  && TREE_CODE (TREE_TYPE (type1)) == INTEGER_TYPE
+	  && TYPE_VECTOR_SUBPARTS (type0) == TYPE_VECTOR_SUBPARTS (type1))
 	{
 	  result_type = type0;
 	  converted = 1;
 	}
-      else if ((code0 == INTEGER_TYPE || code0 == FIXED_POINT_TYPE)
+      else if ((code0 == INTEGER_TYPE || code0 == FIXED_POINT_TYPE
+		|| code0 == VECTOR_TYPE)
 	       && code1 == INTEGER_TYPE)
 	{
 	  doing_shift = true;
@@ -10836,6 +10831,18 @@ build_binary_op (location_t location, en
 		    warning_at (location, OPT_Wshift_count_negative,
 				"right shift count is negative");
 		}
+	      else if (code0 == VECTOR_TYPE)
+		{
+		  if (compare_tree_int (op1,
+					TYPE_PRECISION (TREE_TYPE (type0)))
+		      >= 0)
+		    {
+		      int_const = false;
+		      if (c_inhibit_evaluation_warnings == 0)
+			warning_at (location, OPT_Wshift_count_overflow,
+				    "right shift count >= width of vector element");
+		    }
+		}
 	      else
 		{
 		  if (!integer_zerop (op1))
@@ -10859,21 +10866,16 @@ build_binary_op (location_t location, en
       break;
 
     case LSHIFT_EXPR:
-      if (code0 == VECTOR_TYPE && code1 == INTEGER_TYPE
-          && TREE_CODE (TREE_TYPE (type0)) == INTEGER_TYPE)
-        {
-          result_type = type0;
-          converted = 1;
-        }
-      else if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE
-	       && TREE_CODE (TREE_TYPE (type0)) == INTEGER_TYPE
-	       && TREE_CODE (TREE_TYPE (type1)) == INTEGER_TYPE
-	       && TYPE_VECTOR_SUBPARTS (type0) == TYPE_VECTOR_SUBPARTS (type1))
+      if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE
+	  && TREE_CODE (TREE_TYPE (type0)) == INTEGER_TYPE
+	  && TREE_CODE (TREE_TYPE (type1)) == INTEGER_TYPE
+	  && TYPE_VECTOR_SUBPARTS (type0) == TYPE_VECTOR_SUBPARTS (type1))
 	{
 	  result_type = type0;
 	  converted = 1;
 	}
-      else if ((code0 == INTEGER_TYPE || code0 == FIXED_POINT_TYPE)
+      else if ((code0 == INTEGER_TYPE || code0 == FIXED_POINT_TYPE
+		|| code0 == VECTOR_TYPE)
 	       && code1 == INTEGER_TYPE)
 	{
 	  doing_shift = true;
@@ -10897,6 +10899,18 @@ build_binary_op (location_t location, en
 		    warning_at (location, OPT_Wshift_count_negative,
 				"left shift count is negative");
 		}
+	      else if (code0 == VECTOR_TYPE)
+		{
+		  if (compare_tree_int (op1,
+					TYPE_PRECISION (TREE_TYPE (type0)))
+		      >= 0)
+		    {
+		      int_const = false;
+		      if (c_inhibit_evaluation_warnings == 0)
+			warning_at (location, OPT_Wshift_count_overflow,
+				    "left shift count >= width of vector element");
+		    }
+		}
 	      else if (compare_tree_int (op1, TYPE_PRECISION (type0)) >= 0)
 		{
 		  int_const = false;

On 12/11/2015 12:28 AM, Jan Beulich wrote:
> gcc/c/
> 2015-12-10  Jan Beulich  <jbeulich@suse.com>
>
> 	* c-fold.c (c_fully_fold_internal): Also emit shift count
> 	warnings for vector types.
> 	* c-typeck.c (build_binary_op): Likewise.
Needs testcases for the added warnings.

My additional concern here would be that in build_binary_op, after your 
change, we'll be setting doing_shift to true.  That in turn will enable 
ubsan instrumentation of the shift.  Does ubsan work properly for vector 
shifts?


Jeff

>>> On 11.12.15 at 21:40, <law@redhat.com> wrote:
> On 12/11/2015 12:28 AM, Jan Beulich wrote:
>> gcc/c/
>> 2015-12-10  Jan Beulich  <jbeulich@suse.com>
>>
>> 	* c-fold.c (c_fully_fold_internal): Also emit shift count
>> 	warnings for vector types.
>> 	* c-typeck.c (build_binary_op): Likewise.
> Needs testcases for the added warnings.
> 
> My additional concern here would be that in build_binary_op, after your 
> change, we'll be setting doing_shift to true.  That in turn will enable 
> ubsan instrumentation of the shift.  Does ubsan work properly for vector 
> shifts?

You say that it may be safe with that other patch you replied to a
little later. I have no idea myself.

Jan

On 12/14/2015 01:29 AM, Jan Beulich wrote:
>>>> On 11.12.15 at 21:40, <law@redhat.com> wrote:
>> On 12/11/2015 12:28 AM, Jan Beulich wrote:
>>> gcc/c/
>>> 2015-12-10  Jan Beulich  <jbeulich@suse.com>
>>>
>>> 	* c-fold.c (c_fully_fold_internal): Also emit shift count
>>> 	warnings for vector types.
>>> 	* c-typeck.c (build_binary_op): Likewise.
>> Needs testcases for the added warnings.
>>
>> My additional concern here would be that in build_binary_op, after your
>> change, we'll be setting doing_shift to true.  That in turn will enable
>> ubsan instrumentation of the shift.  Does ubsan work properly for vector
>> shifts?
>
> You say that it may be safe with that other patch you replied to a
> little later. I have no idea myself.
I think Paolo's change makes yours safe.

Essentially Paolo's change avoids the overflow sanitization if the type 
is not an integral type.  Specifically in ubsan_instrument_shift tt will 
always be NULL, which in turn causes ubsan_instrument_shift to return 
NULL without generating any instrumentation.

So I think you just need to build some testcases for your change.
jeff