c, c++: Implement -Wsizeof-array-div [PR91741]

This patch implements a new warning, -Wsizeof-array-div.  It warns about
code like

  int arr[10];
  sizeof (arr) / sizeof (short);

where we have a division of two sizeof expressions, where the first
argument is an array, and the second sizeof does not equal the size
of the array element.  See e.g. <https://www.viva64.com/en/examples/v706/>.

Clang makes it possible to suppress the warning by parenthesizing the
second sizeof like this:

  sizeof (arr) / (sizeof (short));

so I followed suit.  In the C++ FE this was rather easy, because
finish_parenthesized_expr already set TREE_NO_WARNING.  In the C FE
I've added a new tree code, PAREN_SIZEOF_EXPR, to discern between the
non-() and () versions.

This warning is enabled by -Wall.  An example of the output:

x.c:5:23: warning: expression does not compute the number of elements in this array; element type is ‘int’, not ‘short int’ [-Wsizeof-array-div]
    5 |   return sizeof (arr) / sizeof (short);
      |          ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
x.c:5:25: note: add parentheses around ‘sizeof (short int)’ to silence this warning
    5 |   return sizeof (arr) / sizeof (short);
      |                         ^~~~~~~~~~~~~~
      |                         (             )
x.c:4:7: note: array ‘arr’ declared here
    4 |   int arr[10];
      |       ^~~

gcc/c-family/ChangeLog:

	PR c++/91741
	* c-common.c (verify_tree): Handle PAREN_SIZEOF_EXPR.
	(c_common_init_ts): Likewise.
	* c-common.def (PAREN_SIZEOF_EXPR): New tree code.
	* c-common.h (maybe_warn_sizeof_array_div): Declare.
	* c-warn.c (sizeof_pointer_memaccess_warning): Unwrap NOP_EXPRs.
	(maybe_warn_sizeof_array_div): New function.
	* c.opt (Wsizeof-array-div): New option.

gcc/c/ChangeLog:

	PR c++/91741
	* c-parser.c (c_parser_binary_expression): Implement -Wsizeof-array-div.
	(c_parser_postfix_expression): Set PAREN_SIZEOF_EXPR.
	(c_parser_expr_list): Handle PAREN_SIZEOF_EXPR like SIZEOF_EXPR.
	* c-tree.h (char_type_p): Declare.
	* c-typeck.c (char_type_p): No longer static.

gcc/cp/ChangeLog:

	PR c++/91741
	* typeck.c (cp_build_binary_op): Implement -Wsizeof-array-div.

gcc/ChangeLog:

	PR c++/91741
	* doc/invoke.texi: Document -Wsizeof-array-div.

gcc/testsuite/ChangeLog:

	PR c++/91741
	* c-c++-common/Wsizeof-pointer-div.c: Add dg-warning.
	* c-c++-common/Wsizeof-array-div1.c: New test.
	* g++.dg/warn/Wsizeof-array-div1.C: New test.
	* g++.dg/warn/Wsizeof-array-div2.C: New test.

gcc/c-family/c-common.c

@@ -1854,6 +1854,7 @@ verify_tree (tree x, struct tlist **pbefore_sp, struct tlist **pno_sp,
     {
     case CONSTRUCTOR:
     case SIZEOF_EXPR:
+    case PAREN_SIZEOF_EXPR:
       return;
 
     case COMPOUND_EXPR:
@@ -8142,6 +8143,7 @@ void
 c_common_init_ts (void)
 {
   MARK_TS_EXP (SIZEOF_EXPR);
+  MARK_TS_EXP (PAREN_SIZEOF_EXPR);
   MARK_TS_EXP (C_MAYBE_CONST_EXPR);
   MARK_TS_EXP (EXCESS_PRECISION_EXPR);
   MARK_TS_EXP (BREAK_STMT);

gcc/c-family/c-common.def

@@ -55,6 +55,9 @@ DEFTREECODE (USERDEF_LITERAL, "userdef_literal", tcc_exceptional, 3)
    or for the purpose of -Wsizeof-pointer-memaccess warning.  */
 DEFTREECODE (SIZEOF_EXPR, "sizeof_expr", tcc_expression, 1)
 
+/* Like above, but enclosed in parentheses.  Used to suppress warnings.  */
+DEFTREECODE (PAREN_SIZEOF_EXPR, "paren_sizeof_expr", tcc_expression, 1)
+
 /* Used to represent a `for' statement. The operands are
    FOR_INIT_STMT, FOR_COND, FOR_EXPR, FOR_BODY, and FOR_SCOPE,
    respectively.  */

gcc/c-family/c-common.h

@@ -1373,6 +1373,7 @@ extern void warn_for_omitted_condop (location_t, tree);
 extern bool warn_for_restrict (unsigned, tree *, unsigned);
 extern void warn_for_address_or_pointer_of_packed_member (tree, tree);
 extern void warn_parm_array_mismatch (location_t, tree, tree);
+extern void maybe_warn_sizeof_array_div (location_t, tree, tree, tree, tree);
 
 /* Places where an lvalue, or modifiable lvalue, may be required.
    Used to select diagnostic messages in lvalue_error and

gcc/c-family/c-warn.c

@@ -3665,3 +3665,50 @@ warn_parm_array_mismatch (location_t origloc, tree fndecl, tree newparms)
 	inform (origloc, "previously declared as %s", curparmstr.c_str ());
     }
 }
+
+/* Warn about divisions of two sizeof operators when the first one is applied
+   to an array and the divisor does not equal the size of the array element.
+   For instance:
+
+     sizeof (ARR) / sizeof (OP)
+
+   ARR is the array argument of the first sizeof, ARR_TYPE is its ARRAY_TYPE.
+   OP1 is the whole second SIZEOF_EXPR, or its argument; TYPE1 is the type
+   of the second argument.  */
+
+void
+maybe_warn_sizeof_array_div (location_t loc, tree arr, tree arr_type,
+			     tree op1, tree type1)
+{
+  tree elt_type = TREE_TYPE (arr_type);
+
+  if (!warn_sizeof_array_div
+      /* Don't warn on multidimensional arrays.  */
+      || TREE_CODE (elt_type) == ARRAY_TYPE)
+    return;
+
+  if (!tree_int_cst_equal (TYPE_SIZE (elt_type), TYPE_SIZE (type1)))
+    {
+      auto_diagnostic_group d;
+      if (warning_at (loc, OPT_Wsizeof_array_div,
+		      "expression does not compute the number of "
+		      "elements in this array; element type is "
+		      "%qT, not %qT", elt_type, type1))
+	{
+	  if (EXPR_HAS_LOCATION (op1))
+	    {
+	      location_t op1_loc = EXPR_LOCATION (op1);
+	      gcc_rich_location richloc (op1_loc);
+	      richloc.add_fixit_insert_before (op1_loc, "(");
+	      richloc.add_fixit_insert_after (op1_loc, ")");
+	      inform (&richloc, "add parentheses around %qE to "
+		      "silence this warning", op1);
+	    }
+	  else
+	    inform (loc, "add parentheses around the second %<sizeof%> "
+		    "to silence this warning");
+	  if (DECL_P (arr))
+	    inform (DECL_SOURCE_LOCATION (arr), "array %qD declared here", arr);
+	}
+    }
+}

gcc/c-family/c.opt

@@ -816,6 +816,11 @@ Wsizeof-pointer-div
 C ObjC C++ ObjC++ Var(warn_sizeof_pointer_div) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about suspicious divisions of two sizeof expressions that don't work correctly with pointers.
 
+Wsizeof-array-div
+C ObjC C++ ObjC++ Var(warn_sizeof_array_div) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about divisions of two sizeof operators when the first one is applied
+to an array and the divisor does not equal the size of the array element.
+
 Wsizeof-pointer-memaccess
 C ObjC C++ ObjC++ Var(warn_sizeof_pointer_memaccess) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about suspicious length parameters to certain string functions if the argument uses sizeof.

gcc/c/c-parser.c

@@ -7876,7 +7876,7 @@ c_parser_binary_expression (c_parser *parser, struct c_expr *after,
     enum tree_code op;
     /* The source location of this operation.  */
     location_t loc;
-    /* The sizeof argument if expr.original_code == SIZEOF_EXPR.  */
+    /* The sizeof argument if expr.original_code == {PAREN_,}SIZEOF_EXPR.  */
     tree sizeof_arg;
   } stack[NUM_PRECS];
   int sp;
@@ -7894,9 +7894,11 @@ c_parser_binary_expression (c_parser *parser, struct c_expr *after,
 	c_inhibit_evaluation_warnings -= (stack[sp - 1].expr.value	      \
 					  == truthvalue_true_node);	      \
 	break;								      \
-      case TRUNC_DIV_EXPR: 						      \
-	if (stack[sp - 1].expr.original_code == SIZEOF_EXPR		      \
-	    && stack[sp].expr.original_code == SIZEOF_EXPR)		      \
+      case TRUNC_DIV_EXPR:						      \
+	if ((stack[sp - 1].expr.original_code == SIZEOF_EXPR		      \
+	     || stack[sp - 1].expr.original_code == PAREN_SIZEOF_EXPR)	      \
+	    && (stack[sp].expr.original_code == SIZEOF_EXPR		      \
+		|| stack[sp].expr.original_code == PAREN_SIZEOF_EXPR))	      \
 	  {								      \
 	    tree type0 = stack[sp - 1].sizeof_arg;			      \
 	    tree type1 = stack[sp].sizeof_arg;				      \
@@ -7910,18 +7912,23 @@ c_parser_binary_expression (c_parser *parser, struct c_expr *after,
 		&& !(TREE_CODE (first_arg) == PARM_DECL			      \
 		     && C_ARRAY_PARAMETER (first_arg)			      \
 		     && warn_sizeof_array_argument))			      \
-	      {								\
-		auto_diagnostic_group d;					\
-		if (warning_at (stack[sp].loc, OPT_Wsizeof_pointer_div, \
-				  "division %<sizeof (%T) / sizeof (%T)%> " \
-				  "does not compute the number of array " \
-				  "elements",				\
-				  type0, type1))			\
-		  if (DECL_P (first_arg))				\
-		    inform (DECL_SOURCE_LOCATION (first_arg),		\
-			      "first %<sizeof%> operand was declared here"); \
-	      }								\
-	  }								\
+	      {								      \
+		auto_diagnostic_group d;				      \
+		if (warning_at (stack[sp].loc, OPT_Wsizeof_pointer_div,	      \
+				  "division %<sizeof (%T) / sizeof (%T)%> "   \
+				  "does not compute the number of array "     \
+				  "elements",				      \
+				  type0, type1))			      \
+		  if (DECL_P (first_arg))				      \
+		    inform (DECL_SOURCE_LOCATION (first_arg),		      \
+			      "first %<sizeof%> operand was declared here");  \
+	      }								      \
+	    else if (TREE_CODE (type0) == ARRAY_TYPE			      \
+		     && !char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type0)))  \
+		     && stack[sp].expr.original_code != PAREN_SIZEOF_EXPR)    \
+	      maybe_warn_sizeof_array_div (stack[sp].loc, first_arg, type0,   \
+					   stack[sp].sizeof_arg, type1);      \
+	  }								      \
 	break;								      \
       default:								      \
 	break;								      \
@@ -9177,6 +9184,9 @@ c_parser_postfix_expression (c_parser *parser)
 	  if (expr.original_code != C_MAYBE_CONST_EXPR
 	      && expr.original_code != SIZEOF_EXPR)
 	    expr.original_code = ERROR_MARK;
+	  /* Remember that we saw ( ) around the sizeof.  */
+	  if (expr.original_code == SIZEOF_EXPR)
+	    expr.original_code = PAREN_SIZEOF_EXPR;
 	  /* Don't change EXPR.ORIGINAL_TYPE.  */
 	  location_t loc_close_paren = c_parser_peek_token (parser)->location;
 	  set_c_expr_source_range (&expr, loc_open_paren, loc_close_paren);
@@ -10792,7 +10802,8 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
   if (locations)
     locations->safe_push (expr.get_location ());
   if (sizeof_arg != NULL
-      && expr.original_code == SIZEOF_EXPR)
+      && (expr.original_code == SIZEOF_EXPR
+	  || expr.original_code == PAREN_SIZEOF_EXPR))
     {
       sizeof_arg[0] = c_last_sizeof_arg;
       sizeof_arg_loc[0] = c_last_sizeof_loc;
@@ -10815,7 +10826,8 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
 	locations->safe_push (expr.get_location ());
       if (++idx < 3
 	  && sizeof_arg != NULL
-	  && expr.original_code == SIZEOF_EXPR)
+	  && (expr.original_code == SIZEOF_EXPR
+	      || expr.original_code == PAREN_SIZEOF_EXPR))
 	{
 	  sizeof_arg[idx] = c_last_sizeof_arg;
 	  sizeof_arg_loc[idx] = c_last_sizeof_loc;

gcc/c/c-tree.h

@@ -675,6 +675,7 @@ extern location_t c_last_sizeof_loc;
 
 extern struct c_switch *c_switch_stack;
 
+extern bool char_type_p (tree);
 extern tree c_objc_common_truthvalue_conversion (location_t, tree);
 extern tree require_complete_type (location_t, tree);
 extern bool same_translation_unit_p (const_tree, const_tree);

gcc/c/c-typeck.c

@@ -3719,7 +3719,7 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
 
 /* Returns true if TYPE is a character type, *not* including wchar_t.  */
 
-static bool
+bool
 char_type_p (tree type)
 {
   return (type == char_type_node

gcc/cp/typeck.c

@@ -4706,14 +4706,13 @@ cp_build_binary_op (const op_location_t &location,
 	{
 	  tree type0 = TREE_OPERAND (op0, 0);
 	  tree type1 = TREE_OPERAND (op1, 0);
-	  tree first_arg = type0;
+	  tree first_arg = tree_strip_any_location_wrapper (type0);
 	  if (!TYPE_P (type0))
 	    type0 = TREE_TYPE (type0);
 	  if (!TYPE_P (type1))
 	    type1 = TREE_TYPE (type1);
 	  if (INDIRECT_TYPE_P (type0) && same_type_p (TREE_TYPE (type0), type1))
 	    {
-	      STRIP_ANY_LOCATION_WRAPPER (first_arg);
 	      if (!(TREE_CODE (first_arg) == PARM_DECL
 		    && DECL_ARRAY_PARAMETER_P (first_arg)
 		    && warn_sizeof_array_argument)
@@ -4729,6 +4728,13 @@ cp_build_binary_op (const op_location_t &location,
 			      "first %<sizeof%> operand was declared here");
 		}
 	    }
+	  else if (TREE_CODE (type0) == ARRAY_TYPE
+		   && !char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type0)))
+		   /* Set by finish_parenthesized_expr.  */
+		   && !TREE_NO_WARNING (op1)
+		   && (complain & tf_warning))
+	    maybe_warn_sizeof_array_div (location, first_arg, type0,
+					 op1, non_reference (type1));
 	}
 
       if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE

gcc/doc/invoke.texi

@@ -363,6 +363,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-shift-overflow  -Wshift-overflow=@var{n} @gol
 -Wsign-compare  -Wsign-conversion @gol
 -Wno-sizeof-array-argument @gol
+-Wsizeof-array-div @gol
 -Wsizeof-pointer-div  -Wsizeof-pointer-memaccess @gol
 -Wstack-protector  -Wstack-usage=@var{byte-size}  -Wstrict-aliasing @gol
 -Wstrict-aliasing=n  -Wstrict-overflow  -Wstrict-overflow=@var{n} @gol
@@ -5301,6 +5302,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 -Wreturn-type  @gol
 -Wsequence-point  @gol
 -Wsign-compare @r{(only in C++)}  @gol
+-Wsizeof-array-div @gol
 -Wsizeof-pointer-div @gol
 -Wsizeof-pointer-memaccess @gol
 -Wstrict-aliasing  @gol
@@ -8055,6 +8057,23 @@ real to lower precision real values.  This option is also enabled by
 @opindex Wscalar-storage-order
 Do not warn on suspicious constructs involving reverse scalar storage order.
 
+@item -Wsizeof-array-div
+@opindex Wsizeof-array-div
+@opindex Wno-sizeof-array-div
+Warn about divisions of two sizeof operators when the first one is applied
+to an array and the divisor does not equal the size of the array element.
+In such a case, the computation will not yield the number of elements in the
+array, which is likely what the user intended.  This warning warns e.g. about
+@smallexample
+int fn ()
+@{
+  int arr[10];
+  return sizeof (arr) / sizeof (short);
+@}
+@end smallexample
+
+This warning is enabled by @option{-Wall}.
+
 @item -Wsizeof-pointer-div
 @opindex Wsizeof-pointer-div
 @opindex Wno-sizeof-pointer-div

gcc/testsuite/c-c++-common/Wsizeof-array-div1.c

@@ -0,0 +1,56 @@
+/* PR c++/91741 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+typedef int T;
+
+int
+fn (int ap[])
+{
+  int arr[10];
+  int *arr2[10];
+  int *p = &arr[0];
+  int r = 0;
+
+  r += sizeof (arr) / sizeof (*arr);
+  r += sizeof (arr) / sizeof (p); /* { dg-warning "expression does not compute" } */
+  r += sizeof (arr) / sizeof p; /* { dg-warning "expression does not compute" } */
+  r += sizeof (arr) / (sizeof p);
+  r += sizeof (arr) / (sizeof (p));
+  r += sizeof (arr2) / sizeof p;
+  r += sizeof (arr2) / sizeof (int); /* { dg-warning "expression does not compute" } */
+  r += sizeof (arr2) / sizeof (int *);
+  r += sizeof (arr2) / sizeof (short *);
+  r += sizeof (arr) / sizeof (int);
+  r += sizeof (arr) / sizeof (unsigned int);
+  r += sizeof (arr) / sizeof (T);
+  r += sizeof (arr) / sizeof (short); /* { dg-warning "expression does not compute" } */
+  r += sizeof (arr) / (sizeof (short));
+
+  r += sizeof (ap) / sizeof (char); /* { dg-warning ".sizeof. on array function parameter" } */
+
+  const char arr3[] = "foo";
+  r += sizeof (arr3) / sizeof(char);
+  r += sizeof (arr3) / sizeof(int);
+  r += sizeof (arr3) / sizeof (*arr3);
+
+  int arr4[5][5];
+  r += sizeof (arr4) / sizeof (arr4[0]);
+  r += sizeof (arr4) / sizeof (*arr4);
+  r += sizeof (arr4) / sizeof (**arr4);
+  r += sizeof (arr4) / sizeof (int *);
+  r += sizeof (arr4) / sizeof (int);
+  r += sizeof (arr4) / sizeof (short int);
+
+  T arr5[10];
+  r += sizeof (arr5) / sizeof (T);
+  r += sizeof (arr5) / sizeof (int);
+  r += sizeof (arr5) / sizeof (short); /* { dg-warning "expression does not compute" } */
+
+  double arr6[10];
+  r += sizeof (arr6) / sizeof (double);
+  r += sizeof (arr6) / sizeof (float); /* { dg-warning "expression does not compute" } */
+  r += sizeof (arr6) / sizeof (*arr6);
+
+  return r;
+}

gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c

@@ -29,7 +29,7 @@ f2 (void)
   i += sizeof(array) / sizeof(array[0]);
   i += (sizeof(array)) / (sizeof(array[0]));
   i += sizeof(array) / sizeof(int);
-  i += sizeof(array) / sizeof(char);
+  i += sizeof(array) / sizeof(char);		/* { dg-warning "expression does not compute" } */
   i += sizeof(*array) / sizeof(char);
   i += sizeof(array[0]) / sizeof(char);
   return i;

gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C

@@ -0,0 +1,37 @@
+// PR c++/91741
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wall" }
+
+int
+fn1 ()
+{
+  int arr[10];
+  return sizeof (arr) / sizeof (decltype(arr[0]));
+}
+
+template<typename T, int N>
+int fn2 (T (&arr)[N])
+{
+  return sizeof (arr) / sizeof (T);
+}
+
+template<typename T, int N>
+int fn3 (T (&arr)[N])
+{
+  return sizeof (arr) / sizeof (bool); // { dg-warning "expression does not compute" }
+}
+
+template<typename U, int N, typename T>
+int fn4 (T (&arr)[N])
+{
+  return sizeof (arr) / sizeof (U); // { dg-warning "expression does not compute" }
+}
+
+void
+fn ()
+{
+  int arr[10];
+  fn2 (arr);
+  fn3 (arr);
+  fn4<short> (arr);
+}

gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C

@@ -0,0 +1,15 @@
+// PR c++/91741
+// { dg-do compile }
+// { dg-options "-Wall" }
+// From <https://www.viva64.com/en/examples/v706/>.
+
+const int kBaudrates[] = { 50, 75, 110 };
+
+void
+foo ()
+{
+  for(int i = sizeof(kBaudrates) / sizeof(char*); // { dg-warning "expression does not compute" }
+      --i >= 0;)
+    {
+    }
+}