[RFC, C] add warning for unpromoted bit-field uses

[Sandra Loosemore <sandra at codesourcery dot com>]

We had a request from a customer to add a warning to the C front end to 
diagnose cases where bit-fields larger than an int are used in shift 
expressions; confusingly, the operation is done in the precision of the 
bit-field size rather than its declared type.  This is a symptom of a 
larger problem, of course, as it affects bit-field uses in other 
expressions besides shifts, too.

The C standard specifies that bit-fields have an integral type of the 
specified number of bits.  On the other hand, in C++, bit-fields do have 
their declared type, so it seems appropriate to tie the new proposed 
warning to -Wc++-compat.

I thought that the point at which integral promotions are applied would 
be a good place to catch this, as it excludes places where bit-fields 
are already being converted by assignment or explicit cast.  I think we 
also want to exclude warning about bit-fields whose promoted type is 
identical to the type that would result from promoting the field's 
declared type, so it makes sense to emit the warning at the place where 
we're dealing with such promotions.

I've put together the attached patch as a first cut.  I haven't done the 
requisite bootstrap and full testing on it yet, but I thought I'd put it 
out there for comment first to see if there is consensus that this is a 
reasonable thing to do.  Maybe details like the warning option name and 
message could be improved, etc, too.  Any other feedback?

-Sandra


2014-11-08  Sandra Loosemore  <sandra@codesourcery.com>

	gcc/c-family/
	* c.opt (-Wbitfield-conversion): New option.

	gcc/
	* doc/invoke.texi (Option Summary): Add -Wbitfield-conversion.
	(Warning Options): Document it.

	gcc/c/
	* c-typeck.c (perform_integral_promotions): Handle
	-Wbitfield-conversion.

	gcc/testsuite/
	* gcc.dg/Wbitfield-conversion.c: New test case.

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 217259)
+++ gcc/c-family/c.opt	(working copy)
@@ -287,6 +287,10 @@ Wbad-function-cast
 C ObjC Var(warn_bad_function_cast) Warning
 Warn about casting functions to incompatible types
 
+Wbitfield-conversion
+C ObjC Var(warn_bitfield_conversion) Warning LangEnabledBy(C ObjC,Wc++-compat)
+Warn about bit-fields not promoted to their declared types
+
 Wbool-compare
 C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about boolean expression compared with an integer value different from true/false
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 217259)
+++ gcc/doc/invoke.texi	(working copy)
@@ -291,7 +291,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wmissing-parameter-type  -Wmissing-prototypes  -Wnested-externs @gol
 -Wold-style-declaration  -Wold-style-definition @gol
 -Wstrict-prototypes  -Wtraditional  -Wtraditional-conversion @gol
--Wdeclaration-after-statement -Wpointer-sign}
+-Wdeclaration-after-statement -Wpointer-sign -Wbitfield-conversion}
 
 @item Debugging Options
 @xref{Debugging Options,,Options for Debugging Your Program or GCC}.
@@ -5265,6 +5265,18 @@ Suppress warnings when a positional init
 a structure that has been marked with the @code{designated_init}
 attribute.
 
+@item -Wbitfield-conversion @r{(C and Objective-C only)}
+@opindex Wbitfield-conversion
+In C, bit-fields have an integral type of the specified bit size,
+rather than the declared type of the bit-field as they do in C++.
+This may lead to unexpected results when uncasted accesses to bit-fields
+of types or sizes larger than @code{int} are used in other expressions.
+This option causes GCC to emit a warning suggesting
+an explicit cast in situations where the integral promotions do not give
+a bit-field value its promoted declared type.
+
+@option{-Wbitfield-conversion} is enabled by @option{-Wc++-compat}.
+
 @end table
 
 @node Debugging Options
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 217259)
+++ gcc/c/c-typeck.c	(working copy)
@@ -2057,6 +2057,27 @@ perform_integral_promotions (tree exp)
       return convert (type, exp);
     }
 
+  /* In C, bit-fields have an integral type of the specified number of
+     bits, while in C++ they have their declared types.  If both the
+     declared type and bit-field size are int-sized or smaller, the
+     integral promotions will smooth over the difference by promoting
+     to (possibly unsigned) int.  Otherwise, we may emit a warning
+     suggesting an explicit cast.  */
+  if (warn_bitfield_conversion
+      && TREE_CODE (exp) == COMPONENT_REF
+      && DECL_C_BIT_FIELD (TREE_OPERAND (exp, 1)))
+    {
+      tree field = TREE_OPERAND (exp, 1);
+      tree fieldtype = DECL_BIT_FIELD_TYPE (field);
+
+      if (0 < compare_tree_int (TYPE_SIZE (fieldtype),
+				TYPE_PRECISION (integer_type_node))
+	  || 0 < compare_tree_int (DECL_SIZE (field),
+				   TYPE_PRECISION (integer_type_node)))
+	warning_at (EXPR_LOCATION (exp), OPT_Wbitfield_conversion,
+		    "suggest casting bit-field to its declared type");
+    }
+
   /* ??? This should no longer be needed now bit-fields have their
      proper types.  */
   if (TREE_CODE (exp) == COMPONENT_REF
Index: gcc/testsuite/gcc.dg/Wbitfield-conversion.c
===================================================================
--- gcc/testsuite/gcc.dg/Wbitfield-conversion.c	(revision 0)
+++ gcc/testsuite/gcc.dg/Wbitfield-conversion.c	(revision 0)
@@ -0,0 +1,57 @@
+/* { dg-do compile} */
+/* { dg-options "-Wbitfield-conversion" } */
+
+/* Test for warnings when the promoted types of bit-fields don't match
+   their promoted declared types.  */
+
+struct s {
+  unsigned char a : 3;
+  unsigned long long b : 40;
+  unsigned long long c : 5;
+};
+
+static void
+frob (struct s *sp)
+{
+  unsigned long long temp;
+
+  /* Promoted type of bitfield is unsigned int, which is the same as the
+     promoted type of unsigned char.*/
+  temp = sp->a << 1;
+  temp = sp->a + 1;
+  temp = sp->a * 2;
+
+  /* Promoted type of bit-field is 40-bit unsigned integer rather than
+     unsigned long long.  */
+  temp = sp->b << 1;  /* { dg-warning "suggest casting bit-field to its declared type" } */
+  temp = sp->b + 1;  /* { dg-warning "suggest casting bit-field to its declared type" } */
+  temp = sp->b * 2;  /* { dg-warning "suggest casting bit-field to its declared type" } */
+
+  /* Promoted type of bit-field is int rather than unsigned long long.  */
+  temp = sp->c << 1;  /* { dg-warning "suggest casting bit-field to its declared type" } */
+  temp = sp->c + 1;  /* { dg-warning "suggest casting bit-field to its declared type" } */
+  temp = sp->c * 2;  /* { dg-warning "suggest casting bit-field to its declared type" } */
+
+  /* No warning should be produced due to conversion on assignment.  */
+  temp = sp->b;
+  temp = sp->c;
+
+  /* No warning should be produced due to explicit casts.  */
+  temp = ((unsigned long long) sp->b) << 1;
+  temp = ((unsigned long long) sp->b) + 1;
+  temp = ((unsigned long long) sp->b) * 2;
+  temp = ((unsigned long long) sp->c) << 1;
+  temp = ((unsigned long long) sp->c) + 1;
+  temp = ((unsigned long long) sp->c) * 2;
+}
+
+
+int
+main (void)
+{
+  struct s foo;
+  foo.a = 0;
+  foo.b = 0xffffffffffLL;
+  frob (&foo);
+  return 0;
+}