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

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

[Marek Polacek]

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
it was trickier; I've added a NOP_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];
      |       ^~~

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

gcc/c-family/ChangeLog:

	PR c++/91741
	* 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): Wrap a SIZEOF_EXPR in a NOP_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.h                       |  1 +
 gcc/c-family/c-warn.c                         | 51 +++++++++++++++++
 gcc/c-family/c.opt                            |  5 ++
 gcc/c/c-parser.c                              | 46 ++++++++++-----
 gcc/c/c-tree.h                                |  1 +
 gcc/c/c-typeck.c                              |  2 +-
 gcc/cp/typeck.c                               | 10 +++-
 gcc/doc/invoke.texi                           | 19 +++++++
 .../c-c++-common/Wsizeof-array-div1.c         | 56 +++++++++++++++++++
 .../c-c++-common/Wsizeof-pointer-div.c        |  2 +-
 .../g++.dg/warn/Wsizeof-array-div1.C          | 37 ++++++++++++
 .../g++.dg/warn/Wsizeof-array-div2.C          | 15 +++++
 12 files changed, 228 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 4fc64bc4aa6..443aaa2ca9c 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
 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 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
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index c32d8228b5c..e0f0cf65a57 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -864,6 +864,10 @@ sizeof_pointer_memaccess_warning (location_t *sizeof_arg_loc, tree callee,
       || error_operand_p (sizeof_arg[idx]))
     return;
 
+  /* Unwrap the NOP_EXPR signalling that the sizeof was parenthesized.  */
+  if (TREE_CODE (sizeof_arg[idx]) == NOP_EXPR)
+    sizeof_arg[idx] = TREE_OPERAND (sizeof_arg[idx], 0);
+
   type = TYPE_P (sizeof_arg[idx])
 	 ? sizeof_arg[idx] : TREE_TYPE (sizeof_arg[idx]);
 
@@ -3099,3 +3103,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
 
   check_and_warn_address_or_pointer_of_packed_member (type, rhs);
 }
+
+/* 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);
+	}
+    }
+}
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c1d8fd336e8..9ab44550140 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -799,6 +799,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.
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a8bc301ffad..7c9c5188b9e 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -7888,12 +7888,20 @@ 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: 						      \
+      case TRUNC_DIV_EXPR:						      \
 	if (stack[sp - 1].expr.original_code == SIZEOF_EXPR		      \
 	    && stack[sp].expr.original_code == SIZEOF_EXPR)		      \
 	  {								      \
 	    tree type0 = stack[sp - 1].sizeof_arg;			      \
 	    tree type1 = stack[sp].sizeof_arg;				      \
+	    bool paren_p = false;					      \
+	    if (TREE_CODE (type0) == NOP_EXPR)				      \
+	      type0 = TREE_OPERAND (type0, 0);				      \
+	    if (TREE_CODE (type1) == NOP_EXPR)				      \
+	      {								      \
+		type1 = TREE_OPERAND (type1, 0);			      \
+		paren_p = true;						      \
+	      }								      \
 	    tree first_arg = type0;					      \
 	    if (!TYPE_P (type0))					      \
 	      type0 = TREE_TYPE (type0);				      \
@@ -7904,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)))  \
+		     && !paren_p)					      \
+	      maybe_warn_sizeof_array_div (stack[sp].loc, first_arg, type0,   \
+					   stack[sp].sizeof_arg, type1);      \
+	  }								      \
 	break;								      \
       default:								      \
 	break;								      \
@@ -9168,6 +9181,13 @@ c_parser_postfix_expression (c_parser *parser)
 	  expr = c_parser_expression (parser);
 	  if (TREE_CODE (expr.value) == MODIFY_EXPR)
 	    TREE_NO_WARNING (expr.value) = 1;
+	  /* If we parsed a SIZEOF_EXPR, wrap it in a NOP_EXPR to remember
+	     that it was parenthesized.  */
+	  if (expr.original_code == SIZEOF_EXPR
+	      && !error_operand_p (c_last_sizeof_arg))
+	    c_last_sizeof_arg = build1 (NOP_EXPR,
+					TREE_TYPE (c_last_sizeof_arg),
+					c_last_sizeof_arg);
 	  if (expr.original_code != C_MAYBE_CONST_EXPR
 	      && expr.original_code != SIZEOF_EXPR)
 	    expr.original_code = ERROR_MARK;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 10938cf0857..5b3751efb3a 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -664,6 +664,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);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index bb27099bfe1..8caf3dbf6db 100644
--- a/gcc/c/c-typeck.c
+++ b/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
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 9166156a5d5..7705f15c25f 100644
--- a/gcc/cp/typeck.c
+++ b/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
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e3e67c9b3bd..97ae25336ae 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -362,6 +362,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
@@ -5254,6 +5255,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
@@ -7946,6 +7948,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
diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
new file mode 100644
index 00000000000..84d9a730cba
--- /dev/null
+++ b/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;
+}
diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
index 83116183902..e9bad1fa420 100644
--- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
+++ b/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;
diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
new file mode 100644
index 00000000000..da220cd57ba
--- /dev/null
+++ b/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);
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
new file mode 100644
index 00000000000..7962c23522c
--- /dev/null
+++ b/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;)
+    {
+    }
+}

base-commit: 863e8d53eb2940e2c8850e632afe427e164f53cf
-- 
2.26.2

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

[Joseph Myers]

On Mon, 14 Sep 2020, Marek Polacek via Gcc-patches wrote:

> 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
> it was trickier; I've added a NOP_EXPR to discern between the non-()
> and () versions.

This sort of thing is normally handled for C via original_code in c_expr.  
I suppose that doesn't work in this case because the code dealing with 
parenthesized expressions has a special case for sizeof:

          if (expr.original_code != C_MAYBE_CONST_EXPR
              && expr.original_code != SIZEOF_EXPR)
            expr.original_code = ERROR_MARK;

Handling this in some way via c_expr seems better to me than generating 
NOP_EXPR.  I suppose you could invent a PAREN_SIZEOF_EXPR used by (sizeof 
foo) and ((sizeof foo)) etc. as an original_code setting (and handled the 
same as SIZEOF_EXPR by whatever other warnings look for SIZEOF_EXPR 
there), or else add fields to c_expr to allow more such information to be 
tracked there.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Marek Polacek]

On Mon, Sep 14, 2020 at 08:39:36PM +0000, Joseph Myers wrote:
> On Mon, 14 Sep 2020, Marek Polacek via Gcc-patches wrote:
> 
> > 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
> > it was trickier; I've added a NOP_EXPR to discern between the non-()
> > and () versions.
> 
> This sort of thing is normally handled for C via original_code in c_expr.  
> I suppose that doesn't work in this case because the code dealing with 
> parenthesized expressions has a special case for sizeof:
> 
>           if (expr.original_code != C_MAYBE_CONST_EXPR
>               && expr.original_code != SIZEOF_EXPR)
>             expr.original_code = ERROR_MARK;
> 
> Handling this in some way via c_expr seems better to me than generating 
> NOP_EXPR.  I suppose you could invent a PAREN_SIZEOF_EXPR used by (sizeof 
> foo) and ((sizeof foo)) etc. as an original_code setting (and handled the 
> same as SIZEOF_EXPR by whatever other warnings look for SIZEOF_EXPR 
> there), or else add fields to c_expr to allow more such information to be 
> tracked there.

I went with the last option.  But there was a snag: c_expr doesn't have
a default constructor, so the new member would remain uninitialized.
Instead of initializing .parenthesized_p everywhere where we initialize
.original_{type,type}, I only initialize .parenthesized_p when creating
a SIZEOF_EXPR.

It would seem appropriate to add a default constructor/NSDMIs for c_expr,
and then clean up the codebase so that we don't initialize c_expr's fields
manually.  Would you accept such a patch?  It's clearly out of scope for
this patch though.  Such a change would also mean that c_expr is no longer
a trivial type, meaning that we can't use memset on it, and that we couldn't
skip its initialization like we do e.g. in c_parser_alignof_expression.

Thanks for the review.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
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 parenthesized_p member to c_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.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 expr.parenthesized_p.
	* c-tree.h (struct c_expr): Add parenthesized_p member.
	(char_type_p): Declare.
	* c-typeck.c (c_expr_sizeof_expr): Initialize ret.parenthesized_p.
	(c_expr_sizeof_type): Likewise.
	(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.h                       |  1 +
 gcc/c-family/c-warn.c                         | 47 ++++++++++++++++
 gcc/c-family/c.opt                            |  5 ++
 gcc/c/c-parser.c                              | 33 ++++++-----
 gcc/c/c-tree.h                                |  6 ++
 gcc/c/c-typeck.c                              |  4 +-
 gcc/cp/typeck.c                               | 10 +++-
 gcc/doc/invoke.texi                           | 19 +++++++
 .../c-c++-common/Wsizeof-array-div1.c         | 56 +++++++++++++++++++
 .../c-c++-common/Wsizeof-pointer-div.c        |  2 +-
 .../g++.dg/warn/Wsizeof-array-div1.C          | 37 ++++++++++++
 .../g++.dg/warn/Wsizeof-array-div2.C          | 15 +++++
 12 files changed, 218 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 4fc64bc4aa6..443aaa2ca9c 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
 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 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
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index c32d8228b5c..6fdada5f9b7 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -3099,3 +3099,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
 
   check_and_warn_address_or_pointer_of_packed_member (type, rhs);
 }
+
+/* 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);
+	}
+    }
+}
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c1d8fd336e8..9ab44550140 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -799,6 +799,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.
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a8bc301ffad..0510a9b9fdc 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -7888,7 +7888,7 @@ 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: 						      \
+      case TRUNC_DIV_EXPR:						      \
 	if (stack[sp - 1].expr.original_code == SIZEOF_EXPR		      \
 	    && stack[sp].expr.original_code == SIZEOF_EXPR)		      \
 	  {								      \
@@ -7904,18 +7904,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.parenthesized_p)		      \
+	      maybe_warn_sizeof_array_div (stack[sp].loc, first_arg, type0,   \
+					   stack[sp].sizeof_arg, type1);      \
+	  }								      \
 	break;								      \
       default:								      \
 	break;								      \
@@ -9168,6 +9173,8 @@ c_parser_postfix_expression (c_parser *parser)
 	  expr = c_parser_expression (parser);
 	  if (TREE_CODE (expr.value) == MODIFY_EXPR)
 	    TREE_NO_WARNING (expr.value) = 1;
+	  if (expr.original_code == SIZEOF_EXPR)
+	    expr.parenthesized_p = true;
 	  if (expr.original_code != C_MAYBE_CONST_EXPR
 	      && expr.original_code != SIZEOF_EXPR)
 	    expr.original_code = ERROR_MARK;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 10938cf0857..4ef235fca41 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -147,6 +147,11 @@ struct c_expr
      etc), so we stash a copy here.  */
   source_range src_range;
 
+  /* True iff the sizeof expression was enclosed in parentheses.
+     NB: This member is currently only initialized when .original_code
+     is a SIZEOF_EXPR.  ??? Add a default constructor to this class.  */
+  bool parenthesized_p;
+
   /* Access to the first and last locations within the source spelling
      of this expression.  */
   location_t get_start () const { return src_range.m_start; }
@@ -664,6 +669,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);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index bb27099bfe1..ac8690d2fae 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -2938,6 +2938,7 @@ c_expr_sizeof_expr (location_t loc, struct c_expr expr)
       c_last_sizeof_loc = loc;
       ret.original_code = SIZEOF_EXPR;
       ret.original_type = NULL;
+      ret.parenthesized_p = false;
       if (c_vla_type_p (TREE_TYPE (folded_expr)))
 	{
 	  /* sizeof is evaluated when given a vla (C99 6.5.3.4p2).  */
@@ -2968,6 +2969,7 @@ c_expr_sizeof_type (location_t loc, struct c_type_name *t)
   c_last_sizeof_loc = loc;
   ret.original_code = SIZEOF_EXPR;
   ret.original_type = NULL;
+  ret.parenthesized_p = false;
   if ((type_expr || TREE_CODE (ret.value) == INTEGER_CST)
       && c_vla_type_p (type))
     {
@@ -3719,7 +3721,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
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 9166156a5d5..7705f15c25f 100644
--- a/gcc/cp/typeck.c
+++ b/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
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6d9ff2c3362..97fef872a54 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -362,6 +362,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
@@ -5255,6 +5256,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
@@ -7947,6 +7949,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
diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
new file mode 100644
index 00000000000..84d9a730cba
--- /dev/null
+++ b/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;
+}
diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
index 83116183902..e9bad1fa420 100644
--- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
+++ b/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;
diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
new file mode 100644
index 00000000000..da220cd57ba
--- /dev/null
+++ b/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);
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
new file mode 100644
index 00000000000..7962c23522c
--- /dev/null
+++ b/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;)
+    {
+    }
+}

base-commit: 0620f4d79e270f1a455a7ec099504d44dc6180e6
-- 
2.26.2

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

[Jakub Jelinek]

On Mon, Sep 14, 2020 at 09:30:44PM -0400, Marek Polacek via Gcc-patches wrote:
> --- a/gcc/c/c-tree.h
> +++ b/gcc/c/c-tree.h
> @@ -147,6 +147,11 @@ struct c_expr
>       etc), so we stash a copy here.  */
>    source_range src_range;
>  
> +  /* True iff the sizeof expression was enclosed in parentheses.
> +     NB: This member is currently only initialized when .original_code
> +     is a SIZEOF_EXPR.  ??? Add a default constructor to this class.  */
> +  bool parenthesized_p;
> +
>    /* Access to the first and last locations within the source spelling
>       of this expression.  */
>    location_t get_start () const { return src_range.m_start; }

I think a magic tree code would be better, c_expr is used in too many places
and returned by many functions, so it is copied over and over.
Even if you must add it, it would be better to change the struct layout,
because right now there are fields: tree, location_t, tree, 2xlocation_t,
which means 32-bit gap on 64-bit hosts before the second tree, so the new
field would fit in there.  But, if it is mostly uninitialized, it is kind of
unclean.

	Jakub

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

[Marek Polacek]

On Tue, Sep 15, 2020 at 09:04:41AM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Mon, Sep 14, 2020 at 09:30:44PM -0400, Marek Polacek via Gcc-patches wrote:
> > --- a/gcc/c/c-tree.h
> > +++ b/gcc/c/c-tree.h
> > @@ -147,6 +147,11 @@ struct c_expr
> >       etc), so we stash a copy here.  */
> >    source_range src_range;
> >  
> > +  /* True iff the sizeof expression was enclosed in parentheses.
> > +     NB: This member is currently only initialized when .original_code
> > +     is a SIZEOF_EXPR.  ??? Add a default constructor to this class.  */
> > +  bool parenthesized_p;
> > +
> >    /* Access to the first and last locations within the source spelling
> >       of this expression.  */
> >    location_t get_start () const { return src_range.m_start; }
> 
> I think a magic tree code would be better, c_expr is used in too many places
> and returned by many functions, so it is copied over and over.
> Even if you must add it, it would be better to change the struct layout,
> because right now there are fields: tree, location_t, tree, 2xlocation_t,
> which means 32-bit gap on 64-bit hosts before the second tree, so the new
> field would fit in there.  But, if it is mostly uninitialized, it is kind of
> unclean.

Ok, here's a version with PAREN_SIZEOF_EXPR.  It doesn't require changes to
c_expr, but adding a new tree code is always a pain...

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
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                       |  2 +
 gcc/c-family/c-common.def                     |  3 +
 gcc/c-family/c-common.h                       |  1 +
 gcc/c-family/c-warn.c                         | 47 ++++++++++++++++
 gcc/c-family/c.opt                            |  5 ++
 gcc/c/c-parser.c                              | 48 ++++++++++------
 gcc/c/c-tree.h                                |  1 +
 gcc/c/c-typeck.c                              |  2 +-
 gcc/cp/typeck.c                               | 10 +++-
 gcc/doc/invoke.texi                           | 19 +++++++
 .../c-c++-common/Wsizeof-array-div1.c         | 56 +++++++++++++++++++
 .../c-c++-common/Wsizeof-pointer-div.c        |  2 +-
 .../g++.dg/warn/Wsizeof-array-div1.C          | 37 ++++++++++++
 .../g++.dg/warn/Wsizeof-array-div2.C          | 15 +++++
 14 files changed, 226 insertions(+), 22 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 873bea9e2b2..b0a71d10e19 100644
--- a/gcc/c-family/c-common.c
+++ b/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:
@@ -8124,6 +8125,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);
 }
diff --git a/gcc/c-family/c-common.def b/gcc/c-family/c-common.def
index 7ceb9a22bd4..06f112e5683 100644
--- a/gcc/c-family/c-common.def
+++ b/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)
+
 /*
 Local variables:
 mode:c
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 4fc64bc4aa6..443aaa2ca9c 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
 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 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
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index c32d8228b5c..6fdada5f9b7 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -3099,3 +3099,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
 
   check_and_warn_address_or_pointer_of_packed_member (type, rhs);
 }
+
+/* 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);
+	}
+    }
+}
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c1d8fd336e8..9ab44550140 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -799,6 +799,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.
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a8bc301ffad..148832d388b 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -7870,7 +7870,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;
@@ -7888,9 +7888,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;				      \
@@ -7904,18 +7906,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;								      \
@@ -9171,6 +9178,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);
@@ -10786,7 +10796,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;
@@ -10809,7 +10820,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;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 10938cf0857..5b3751efb3a 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -664,6 +664,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);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index bb27099bfe1..8caf3dbf6db 100644
--- a/gcc/c/c-typeck.c
+++ b/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
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 9166156a5d5..7705f15c25f 100644
--- a/gcc/cp/typeck.c
+++ b/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
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6d9ff2c3362..97fef872a54 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -362,6 +362,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
@@ -5255,6 +5256,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
@@ -7947,6 +7949,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
diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
new file mode 100644
index 00000000000..84d9a730cba
--- /dev/null
+++ b/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;
+}
diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
index 83116183902..e9bad1fa420 100644
--- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
+++ b/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;
diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
new file mode 100644
index 00000000000..da220cd57ba
--- /dev/null
+++ b/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);
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
new file mode 100644
index 00000000000..7962c23522c
--- /dev/null
+++ b/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;)
+    {
+    }
+}

base-commit: d1a31689a736cdfb5e7cfa01f1168e338510e63b
-- 
2.26.2

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

[Marek Polacek]

Ping.

On Tue, Sep 15, 2020 at 04:33:05PM -0400, Marek Polacek via Gcc-patches wrote:
> On Tue, Sep 15, 2020 at 09:04:41AM +0200, Jakub Jelinek via Gcc-patches wrote:
> > On Mon, Sep 14, 2020 at 09:30:44PM -0400, Marek Polacek via Gcc-patches wrote:
> > > --- a/gcc/c/c-tree.h
> > > +++ b/gcc/c/c-tree.h
> > > @@ -147,6 +147,11 @@ struct c_expr
> > >       etc), so we stash a copy here.  */
> > >    source_range src_range;
> > >  
> > > +  /* True iff the sizeof expression was enclosed in parentheses.
> > > +     NB: This member is currently only initialized when .original_code
> > > +     is a SIZEOF_EXPR.  ??? Add a default constructor to this class.  */
> > > +  bool parenthesized_p;
> > > +
> > >    /* Access to the first and last locations within the source spelling
> > >       of this expression.  */
> > >    location_t get_start () const { return src_range.m_start; }
> > 
> > I think a magic tree code would be better, c_expr is used in too many places
> > and returned by many functions, so it is copied over and over.
> > Even if you must add it, it would be better to change the struct layout,
> > because right now there are fields: tree, location_t, tree, 2xlocation_t,
> > which means 32-bit gap on 64-bit hosts before the second tree, so the new
> > field would fit in there.  But, if it is mostly uninitialized, it is kind of
> > unclean.
> 
> Ok, here's a version with PAREN_SIZEOF_EXPR.  It doesn't require changes to
> c_expr, but adding a new tree code is always a pain...
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> 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                       |  2 +
>  gcc/c-family/c-common.def                     |  3 +
>  gcc/c-family/c-common.h                       |  1 +
>  gcc/c-family/c-warn.c                         | 47 ++++++++++++++++
>  gcc/c-family/c.opt                            |  5 ++
>  gcc/c/c-parser.c                              | 48 ++++++++++------
>  gcc/c/c-tree.h                                |  1 +
>  gcc/c/c-typeck.c                              |  2 +-
>  gcc/cp/typeck.c                               | 10 +++-
>  gcc/doc/invoke.texi                           | 19 +++++++
>  .../c-c++-common/Wsizeof-array-div1.c         | 56 +++++++++++++++++++
>  .../c-c++-common/Wsizeof-pointer-div.c        |  2 +-
>  .../g++.dg/warn/Wsizeof-array-div1.C          | 37 ++++++++++++
>  .../g++.dg/warn/Wsizeof-array-div2.C          | 15 +++++
>  14 files changed, 226 insertions(+), 22 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
>  create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
>  create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
> 
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 873bea9e2b2..b0a71d10e19 100644
> --- a/gcc/c-family/c-common.c
> +++ b/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:
> @@ -8124,6 +8125,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);
>  }
> diff --git a/gcc/c-family/c-common.def b/gcc/c-family/c-common.def
> index 7ceb9a22bd4..06f112e5683 100644
> --- a/gcc/c-family/c-common.def
> +++ b/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)
> +
>  /*
>  Local variables:
>  mode:c
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 4fc64bc4aa6..443aaa2ca9c 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
>  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 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
> diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
> index c32d8228b5c..6fdada5f9b7 100644
> --- a/gcc/c-family/c-warn.c
> +++ b/gcc/c-family/c-warn.c
> @@ -3099,3 +3099,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
>  
>    check_and_warn_address_or_pointer_of_packed_member (type, rhs);
>  }
> +
> +/* 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);
> +	}
> +    }
> +}
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index c1d8fd336e8..9ab44550140 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -799,6 +799,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.
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index a8bc301ffad..148832d388b 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -7870,7 +7870,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;
> @@ -7888,9 +7888,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;				      \
> @@ -7904,18 +7906,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;								      \
> @@ -9171,6 +9178,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);
> @@ -10786,7 +10796,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;
> @@ -10809,7 +10820,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;
> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
> index 10938cf0857..5b3751efb3a 100644
> --- a/gcc/c/c-tree.h
> +++ b/gcc/c/c-tree.h
> @@ -664,6 +664,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);
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index bb27099bfe1..8caf3dbf6db 100644
> --- a/gcc/c/c-typeck.c
> +++ b/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
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index 9166156a5d5..7705f15c25f 100644
> --- a/gcc/cp/typeck.c
> +++ b/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
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 6d9ff2c3362..97fef872a54 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -362,6 +362,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
> @@ -5255,6 +5256,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
> @@ -7947,6 +7949,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
> diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
> new file mode 100644
> index 00000000000..84d9a730cba
> --- /dev/null
> +++ b/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;
> +}
> diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
> index 83116183902..e9bad1fa420 100644
> --- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
> +++ b/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;
> diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
> new file mode 100644
> index 00000000000..da220cd57ba
> --- /dev/null
> +++ b/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);
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
> new file mode 100644
> index 00000000000..7962c23522c
> --- /dev/null
> +++ b/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;)
> +    {
> +    }
> +}
> 
> base-commit: d1a31689a736cdfb5e7cfa01f1168e338510e63b
> -- 
> 2.26.2
> 

Marek

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

[Jason Merrill]

On 9/22/20 1:29 PM, Marek Polacek wrote:
> Ping.

The C++ change is OK.

> On Tue, Sep 15, 2020 at 04:33:05PM -0400, Marek Polacek via Gcc-patches wrote:
>> On Tue, Sep 15, 2020 at 09:04:41AM +0200, Jakub Jelinek via Gcc-patches wrote:
>>> On Mon, Sep 14, 2020 at 09:30:44PM -0400, Marek Polacek via Gcc-patches wrote:
>>>> --- a/gcc/c/c-tree.h
>>>> +++ b/gcc/c/c-tree.h
>>>> @@ -147,6 +147,11 @@ struct c_expr
>>>>        etc), so we stash a copy here.  */
>>>>     source_range src_range;
>>>>   
>>>> +  /* True iff the sizeof expression was enclosed in parentheses.
>>>> +     NB: This member is currently only initialized when .original_code
>>>> +     is a SIZEOF_EXPR.  ??? Add a default constructor to this class.  */
>>>> +  bool parenthesized_p;
>>>> +
>>>>     /* Access to the first and last locations within the source spelling
>>>>        of this expression.  */
>>>>     location_t get_start () const { return src_range.m_start; }
>>>
>>> I think a magic tree code would be better, c_expr is used in too many places
>>> and returned by many functions, so it is copied over and over.
>>> Even if you must add it, it would be better to change the struct layout,
>>> because right now there are fields: tree, location_t, tree, 2xlocation_t,
>>> which means 32-bit gap on 64-bit hosts before the second tree, so the new
>>> field would fit in there.  But, if it is mostly uninitialized, it is kind of
>>> unclean.
>>
>> Ok, here's a version with PAREN_SIZEOF_EXPR.  It doesn't require changes to
>> c_expr, but adding a new tree code is always a pain...
>>
>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>
>> -- >8 --
>> 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                       |  2 +
>>   gcc/c-family/c-common.def                     |  3 +
>>   gcc/c-family/c-common.h                       |  1 +
>>   gcc/c-family/c-warn.c                         | 47 ++++++++++++++++
>>   gcc/c-family/c.opt                            |  5 ++
>>   gcc/c/c-parser.c                              | 48 ++++++++++------
>>   gcc/c/c-tree.h                                |  1 +
>>   gcc/c/c-typeck.c                              |  2 +-
>>   gcc/cp/typeck.c                               | 10 +++-
>>   gcc/doc/invoke.texi                           | 19 +++++++
>>   .../c-c++-common/Wsizeof-array-div1.c         | 56 +++++++++++++++++++
>>   .../c-c++-common/Wsizeof-pointer-div.c        |  2 +-
>>   .../g++.dg/warn/Wsizeof-array-div1.C          | 37 ++++++++++++
>>   .../g++.dg/warn/Wsizeof-array-div2.C          | 15 +++++
>>   14 files changed, 226 insertions(+), 22 deletions(-)
>>   create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
>>   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
>>   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
>>
>> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
>> index 873bea9e2b2..b0a71d10e19 100644
>> --- a/gcc/c-family/c-common.c
>> +++ b/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:
>> @@ -8124,6 +8125,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);
>>   }
>> diff --git a/gcc/c-family/c-common.def b/gcc/c-family/c-common.def
>> index 7ceb9a22bd4..06f112e5683 100644
>> --- a/gcc/c-family/c-common.def
>> +++ b/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)
>> +
>>   /*
>>   Local variables:
>>   mode:c
>> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
>> index 4fc64bc4aa6..443aaa2ca9c 100644
>> --- a/gcc/c-family/c-common.h
>> +++ b/gcc/c-family/c-common.h
>> @@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
>>   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 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
>> diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
>> index c32d8228b5c..6fdada5f9b7 100644
>> --- a/gcc/c-family/c-warn.c
>> +++ b/gcc/c-family/c-warn.c
>> @@ -3099,3 +3099,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
>>   
>>     check_and_warn_address_or_pointer_of_packed_member (type, rhs);
>>   }
>> +
>> +/* 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);
>> +	}
>> +    }
>> +}
>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>> index c1d8fd336e8..9ab44550140 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -799,6 +799,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.
>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>> index a8bc301ffad..148832d388b 100644
>> --- a/gcc/c/c-parser.c
>> +++ b/gcc/c/c-parser.c
>> @@ -7870,7 +7870,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;
>> @@ -7888,9 +7888,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;				      \
>> @@ -7904,18 +7906,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;								      \
>> @@ -9171,6 +9178,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);
>> @@ -10786,7 +10796,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;
>> @@ -10809,7 +10820,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;
>> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
>> index 10938cf0857..5b3751efb3a 100644
>> --- a/gcc/c/c-tree.h
>> +++ b/gcc/c/c-tree.h
>> @@ -664,6 +664,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);
>> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
>> index bb27099bfe1..8caf3dbf6db 100644
>> --- a/gcc/c/c-typeck.c
>> +++ b/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
>> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
>> index 9166156a5d5..7705f15c25f 100644
>> --- a/gcc/cp/typeck.c
>> +++ b/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
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 6d9ff2c3362..97fef872a54 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -362,6 +362,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
>> @@ -5255,6 +5256,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
>> @@ -7947,6 +7949,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
>> diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
>> new file mode 100644
>> index 00000000000..84d9a730cba
>> --- /dev/null
>> +++ b/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;
>> +}
>> diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
>> index 83116183902..e9bad1fa420 100644
>> --- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
>> +++ b/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;
>> diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
>> new file mode 100644
>> index 00000000000..da220cd57ba
>> --- /dev/null
>> +++ b/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);
>> +}
>> diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
>> new file mode 100644
>> index 00000000000..7962c23522c
>> --- /dev/null
>> +++ b/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;)
>> +    {
>> +    }
>> +}
>>
>> base-commit: d1a31689a736cdfb5e7cfa01f1168e338510e63b
>> -- 
>> 2.26.2
>>
> 
> Marek
> 

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

[Marek Polacek]

On Tue, Sep 22, 2020 at 04:07:41PM -0400, Jason Merrill wrote:
> On 9/22/20 1:29 PM, Marek Polacek wrote:
> > Ping.
> 
> The C++ change is OK.

Ping for the C parts.

> > On Tue, Sep 15, 2020 at 04:33:05PM -0400, Marek Polacek via Gcc-patches wrote:
> > > On Tue, Sep 15, 2020 at 09:04:41AM +0200, Jakub Jelinek via Gcc-patches wrote:
> > > > On Mon, Sep 14, 2020 at 09:30:44PM -0400, Marek Polacek via Gcc-patches wrote:
> > > > > --- a/gcc/c/c-tree.h
> > > > > +++ b/gcc/c/c-tree.h
> > > > > @@ -147,6 +147,11 @@ struct c_expr
> > > > >        etc), so we stash a copy here.  */
> > > > >     source_range src_range;
> > > > > +  /* True iff the sizeof expression was enclosed in parentheses.
> > > > > +     NB: This member is currently only initialized when .original_code
> > > > > +     is a SIZEOF_EXPR.  ??? Add a default constructor to this class.  */
> > > > > +  bool parenthesized_p;
> > > > > +
> > > > >     /* Access to the first and last locations within the source spelling
> > > > >        of this expression.  */
> > > > >     location_t get_start () const { return src_range.m_start; }
> > > > 
> > > > I think a magic tree code would be better, c_expr is used in too many places
> > > > and returned by many functions, so it is copied over and over.
> > > > Even if you must add it, it would be better to change the struct layout,
> > > > because right now there are fields: tree, location_t, tree, 2xlocation_t,
> > > > which means 32-bit gap on 64-bit hosts before the second tree, so the new
> > > > field would fit in there.  But, if it is mostly uninitialized, it is kind of
> > > > unclean.
> > > 
> > > Ok, here's a version with PAREN_SIZEOF_EXPR.  It doesn't require changes to
> > > c_expr, but adding a new tree code is always a pain...
> > > 
> > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > 
> > > -- >8 --
> > > 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                       |  2 +
> > >   gcc/c-family/c-common.def                     |  3 +
> > >   gcc/c-family/c-common.h                       |  1 +
> > >   gcc/c-family/c-warn.c                         | 47 ++++++++++++++++
> > >   gcc/c-family/c.opt                            |  5 ++
> > >   gcc/c/c-parser.c                              | 48 ++++++++++------
> > >   gcc/c/c-tree.h                                |  1 +
> > >   gcc/c/c-typeck.c                              |  2 +-
> > >   gcc/cp/typeck.c                               | 10 +++-
> > >   gcc/doc/invoke.texi                           | 19 +++++++
> > >   .../c-c++-common/Wsizeof-array-div1.c         | 56 +++++++++++++++++++
> > >   .../c-c++-common/Wsizeof-pointer-div.c        |  2 +-
> > >   .../g++.dg/warn/Wsizeof-array-div1.C          | 37 ++++++++++++
> > >   .../g++.dg/warn/Wsizeof-array-div2.C          | 15 +++++
> > >   14 files changed, 226 insertions(+), 22 deletions(-)
> > >   create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
> > >   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
> > >   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
> > > 
> > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > > index 873bea9e2b2..b0a71d10e19 100644
> > > --- a/gcc/c-family/c-common.c
> > > +++ b/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:
> > > @@ -8124,6 +8125,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);
> > >   }
> > > diff --git a/gcc/c-family/c-common.def b/gcc/c-family/c-common.def
> > > index 7ceb9a22bd4..06f112e5683 100644
> > > --- a/gcc/c-family/c-common.def
> > > +++ b/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)
> > > +
> > >   /*
> > >   Local variables:
> > >   mode:c
> > > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> > > index 4fc64bc4aa6..443aaa2ca9c 100644
> > > --- a/gcc/c-family/c-common.h
> > > +++ b/gcc/c-family/c-common.h
> > > @@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
> > >   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 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
> > > diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
> > > index c32d8228b5c..6fdada5f9b7 100644
> > > --- a/gcc/c-family/c-warn.c
> > > +++ b/gcc/c-family/c-warn.c
> > > @@ -3099,3 +3099,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
> > >     check_and_warn_address_or_pointer_of_packed_member (type, rhs);
> > >   }
> > > +
> > > +/* 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);
> > > +	}
> > > +    }
> > > +}
> > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > > index c1d8fd336e8..9ab44550140 100644
> > > --- a/gcc/c-family/c.opt
> > > +++ b/gcc/c-family/c.opt
> > > @@ -799,6 +799,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.
> > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> > > index a8bc301ffad..148832d388b 100644
> > > --- a/gcc/c/c-parser.c
> > > +++ b/gcc/c/c-parser.c
> > > @@ -7870,7 +7870,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;
> > > @@ -7888,9 +7888,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;				      \
> > > @@ -7904,18 +7906,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;								      \
> > > @@ -9171,6 +9178,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);
> > > @@ -10786,7 +10796,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;
> > > @@ -10809,7 +10820,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;
> > > diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
> > > index 10938cf0857..5b3751efb3a 100644
> > > --- a/gcc/c/c-tree.h
> > > +++ b/gcc/c/c-tree.h
> > > @@ -664,6 +664,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);
> > > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> > > index bb27099bfe1..8caf3dbf6db 100644
> > > --- a/gcc/c/c-typeck.c
> > > +++ b/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
> > > diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> > > index 9166156a5d5..7705f15c25f 100644
> > > --- a/gcc/cp/typeck.c
> > > +++ b/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
> > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > index 6d9ff2c3362..97fef872a54 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -362,6 +362,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
> > > @@ -5255,6 +5256,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
> > > @@ -7947,6 +7949,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
> > > diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
> > > new file mode 100644
> > > index 00000000000..84d9a730cba
> > > --- /dev/null
> > > +++ b/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;
> > > +}
> > > diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
> > > index 83116183902..e9bad1fa420 100644
> > > --- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
> > > +++ b/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;
> > > diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
> > > new file mode 100644
> > > index 00000000000..da220cd57ba
> > > --- /dev/null
> > > +++ b/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);
> > > +}
> > > diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
> > > new file mode 100644
> > > index 00000000000..7962c23522c
> > > --- /dev/null
> > > +++ b/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;)
> > > +    {
> > > +    }
> > > +}
> > > 
> > > base-commit: d1a31689a736cdfb5e7cfa01f1168e338510e63b
> > > -- 
> > > 2.26.2
> > > 
> > 
> > Marek
> > 
> 

Marek

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

[Marek Polacek]

Ping for the C parts.

On Mon, Sep 28, 2020 at 02:15:41PM -0400, Marek Polacek via Gcc-patches wrote:
> On Tue, Sep 22, 2020 at 04:07:41PM -0400, Jason Merrill wrote:
> > On 9/22/20 1:29 PM, Marek Polacek wrote:
> > > Ping.
> > 
> > The C++ change is OK.
> 
> Ping for the C parts.
> 
> > > On Tue, Sep 15, 2020 at 04:33:05PM -0400, Marek Polacek via Gcc-patches wrote:
> > > > On Tue, Sep 15, 2020 at 09:04:41AM +0200, Jakub Jelinek via Gcc-patches wrote:
> > > > > On Mon, Sep 14, 2020 at 09:30:44PM -0400, Marek Polacek via Gcc-patches wrote:
> > > > > > --- a/gcc/c/c-tree.h
> > > > > > +++ b/gcc/c/c-tree.h
> > > > > > @@ -147,6 +147,11 @@ struct c_expr
> > > > > >        etc), so we stash a copy here.  */
> > > > > >     source_range src_range;
> > > > > > +  /* True iff the sizeof expression was enclosed in parentheses.
> > > > > > +     NB: This member is currently only initialized when .original_code
> > > > > > +     is a SIZEOF_EXPR.  ??? Add a default constructor to this class.  */
> > > > > > +  bool parenthesized_p;
> > > > > > +
> > > > > >     /* Access to the first and last locations within the source spelling
> > > > > >        of this expression.  */
> > > > > >     location_t get_start () const { return src_range.m_start; }
> > > > > 
> > > > > I think a magic tree code would be better, c_expr is used in too many places
> > > > > and returned by many functions, so it is copied over and over.
> > > > > Even if you must add it, it would be better to change the struct layout,
> > > > > because right now there are fields: tree, location_t, tree, 2xlocation_t,
> > > > > which means 32-bit gap on 64-bit hosts before the second tree, so the new
> > > > > field would fit in there.  But, if it is mostly uninitialized, it is kind of
> > > > > unclean.
> > > > 
> > > > Ok, here's a version with PAREN_SIZEOF_EXPR.  It doesn't require changes to
> > > > c_expr, but adding a new tree code is always a pain...
> > > > 
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > 
> > > > -- >8 --
> > > > 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                       |  2 +
> > > >   gcc/c-family/c-common.def                     |  3 +
> > > >   gcc/c-family/c-common.h                       |  1 +
> > > >   gcc/c-family/c-warn.c                         | 47 ++++++++++++++++
> > > >   gcc/c-family/c.opt                            |  5 ++
> > > >   gcc/c/c-parser.c                              | 48 ++++++++++------
> > > >   gcc/c/c-tree.h                                |  1 +
> > > >   gcc/c/c-typeck.c                              |  2 +-
> > > >   gcc/cp/typeck.c                               | 10 +++-
> > > >   gcc/doc/invoke.texi                           | 19 +++++++
> > > >   .../c-c++-common/Wsizeof-array-div1.c         | 56 +++++++++++++++++++
> > > >   .../c-c++-common/Wsizeof-pointer-div.c        |  2 +-
> > > >   .../g++.dg/warn/Wsizeof-array-div1.C          | 37 ++++++++++++
> > > >   .../g++.dg/warn/Wsizeof-array-div2.C          | 15 +++++
> > > >   14 files changed, 226 insertions(+), 22 deletions(-)
> > > >   create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
> > > >   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
> > > >   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
> > > > 
> > > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > > > index 873bea9e2b2..b0a71d10e19 100644
> > > > --- a/gcc/c-family/c-common.c
> > > > +++ b/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:
> > > > @@ -8124,6 +8125,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);
> > > >   }
> > > > diff --git a/gcc/c-family/c-common.def b/gcc/c-family/c-common.def
> > > > index 7ceb9a22bd4..06f112e5683 100644
> > > > --- a/gcc/c-family/c-common.def
> > > > +++ b/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)
> > > > +
> > > >   /*
> > > >   Local variables:
> > > >   mode:c
> > > > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> > > > index 4fc64bc4aa6..443aaa2ca9c 100644
> > > > --- a/gcc/c-family/c-common.h
> > > > +++ b/gcc/c-family/c-common.h
> > > > @@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
> > > >   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 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
> > > > diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
> > > > index c32d8228b5c..6fdada5f9b7 100644
> > > > --- a/gcc/c-family/c-warn.c
> > > > +++ b/gcc/c-family/c-warn.c
> > > > @@ -3099,3 +3099,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
> > > >     check_and_warn_address_or_pointer_of_packed_member (type, rhs);
> > > >   }
> > > > +
> > > > +/* 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);
> > > > +	}
> > > > +    }
> > > > +}
> > > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > > > index c1d8fd336e8..9ab44550140 100644
> > > > --- a/gcc/c-family/c.opt
> > > > +++ b/gcc/c-family/c.opt
> > > > @@ -799,6 +799,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.
> > > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> > > > index a8bc301ffad..148832d388b 100644
> > > > --- a/gcc/c/c-parser.c
> > > > +++ b/gcc/c/c-parser.c
> > > > @@ -7870,7 +7870,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;
> > > > @@ -7888,9 +7888,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;				      \
> > > > @@ -7904,18 +7906,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;								      \
> > > > @@ -9171,6 +9178,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);
> > > > @@ -10786,7 +10796,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;
> > > > @@ -10809,7 +10820,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;
> > > > diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
> > > > index 10938cf0857..5b3751efb3a 100644
> > > > --- a/gcc/c/c-tree.h
> > > > +++ b/gcc/c/c-tree.h
> > > > @@ -664,6 +664,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);
> > > > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> > > > index bb27099bfe1..8caf3dbf6db 100644
> > > > --- a/gcc/c/c-typeck.c
> > > > +++ b/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
> > > > diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> > > > index 9166156a5d5..7705f15c25f 100644
> > > > --- a/gcc/cp/typeck.c
> > > > +++ b/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
> > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > index 6d9ff2c3362..97fef872a54 100644
> > > > --- a/gcc/doc/invoke.texi
> > > > +++ b/gcc/doc/invoke.texi
> > > > @@ -362,6 +362,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
> > > > @@ -5255,6 +5256,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
> > > > @@ -7947,6 +7949,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
> > > > diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
> > > > new file mode 100644
> > > > index 00000000000..84d9a730cba
> > > > --- /dev/null
> > > > +++ b/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;
> > > > +}
> > > > diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
> > > > index 83116183902..e9bad1fa420 100644
> > > > --- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
> > > > +++ b/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;
> > > > diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
> > > > new file mode 100644
> > > > index 00000000000..da220cd57ba
> > > > --- /dev/null
> > > > +++ b/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);
> > > > +}
> > > > diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
> > > > new file mode 100644
> > > > index 00000000000..7962c23522c
> > > > --- /dev/null
> > > > +++ b/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;)
> > > > +    {
> > > > +    }
> > > > +}
> > > > 
> > > > base-commit: d1a31689a736cdfb5e7cfa01f1168e338510e63b
> > > > -- 
> > > > 2.26.2
> > > > 
> > > 
> > > Marek
> > > 
> > 
> 
> Marek
> 

Marek

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

[Marek Polacek]

Ping.

On Thu, Oct 08, 2020 at 11:04:40AM -0400, Marek Polacek via Gcc-patches wrote:
> Ping for the C parts.
> 
> On Mon, Sep 28, 2020 at 02:15:41PM -0400, Marek Polacek via Gcc-patches wrote:
> > On Tue, Sep 22, 2020 at 04:07:41PM -0400, Jason Merrill wrote:
> > > On 9/22/20 1:29 PM, Marek Polacek wrote:
> > > > Ping.
> > > 
> > > The C++ change is OK.
> > 
> > Ping for the C parts.
> > 
> > > > On Tue, Sep 15, 2020 at 04:33:05PM -0400, Marek Polacek via Gcc-patches wrote:
> > > > > On Tue, Sep 15, 2020 at 09:04:41AM +0200, Jakub Jelinek via Gcc-patches wrote:
> > > > > > On Mon, Sep 14, 2020 at 09:30:44PM -0400, Marek Polacek via Gcc-patches wrote:
> > > > > > > --- a/gcc/c/c-tree.h
> > > > > > > +++ b/gcc/c/c-tree.h
> > > > > > > @@ -147,6 +147,11 @@ struct c_expr
> > > > > > >        etc), so we stash a copy here.  */
> > > > > > >     source_range src_range;
> > > > > > > +  /* True iff the sizeof expression was enclosed in parentheses.
> > > > > > > +     NB: This member is currently only initialized when .original_code
> > > > > > > +     is a SIZEOF_EXPR.  ??? Add a default constructor to this class.  */
> > > > > > > +  bool parenthesized_p;
> > > > > > > +
> > > > > > >     /* Access to the first and last locations within the source spelling
> > > > > > >        of this expression.  */
> > > > > > >     location_t get_start () const { return src_range.m_start; }
> > > > > > 
> > > > > > I think a magic tree code would be better, c_expr is used in too many places
> > > > > > and returned by many functions, so it is copied over and over.
> > > > > > Even if you must add it, it would be better to change the struct layout,
> > > > > > because right now there are fields: tree, location_t, tree, 2xlocation_t,
> > > > > > which means 32-bit gap on 64-bit hosts before the second tree, so the new
> > > > > > field would fit in there.  But, if it is mostly uninitialized, it is kind of
> > > > > > unclean.
> > > > > 
> > > > > Ok, here's a version with PAREN_SIZEOF_EXPR.  It doesn't require changes to
> > > > > c_expr, but adding a new tree code is always a pain...
> > > > > 
> > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > > 
> > > > > -- >8 --
> > > > > 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                       |  2 +
> > > > >   gcc/c-family/c-common.def                     |  3 +
> > > > >   gcc/c-family/c-common.h                       |  1 +
> > > > >   gcc/c-family/c-warn.c                         | 47 ++++++++++++++++
> > > > >   gcc/c-family/c.opt                            |  5 ++
> > > > >   gcc/c/c-parser.c                              | 48 ++++++++++------
> > > > >   gcc/c/c-tree.h                                |  1 +
> > > > >   gcc/c/c-typeck.c                              |  2 +-
> > > > >   gcc/cp/typeck.c                               | 10 +++-
> > > > >   gcc/doc/invoke.texi                           | 19 +++++++
> > > > >   .../c-c++-common/Wsizeof-array-div1.c         | 56 +++++++++++++++++++
> > > > >   .../c-c++-common/Wsizeof-pointer-div.c        |  2 +-
> > > > >   .../g++.dg/warn/Wsizeof-array-div1.C          | 37 ++++++++++++
> > > > >   .../g++.dg/warn/Wsizeof-array-div2.C          | 15 +++++
> > > > >   14 files changed, 226 insertions(+), 22 deletions(-)
> > > > >   create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
> > > > >   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
> > > > >   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
> > > > > 
> > > > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > > > > index 873bea9e2b2..b0a71d10e19 100644
> > > > > --- a/gcc/c-family/c-common.c
> > > > > +++ b/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:
> > > > > @@ -8124,6 +8125,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);
> > > > >   }
> > > > > diff --git a/gcc/c-family/c-common.def b/gcc/c-family/c-common.def
> > > > > index 7ceb9a22bd4..06f112e5683 100644
> > > > > --- a/gcc/c-family/c-common.def
> > > > > +++ b/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)
> > > > > +
> > > > >   /*
> > > > >   Local variables:
> > > > >   mode:c
> > > > > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> > > > > index 4fc64bc4aa6..443aaa2ca9c 100644
> > > > > --- a/gcc/c-family/c-common.h
> > > > > +++ b/gcc/c-family/c-common.h
> > > > > @@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
> > > > >   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 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
> > > > > diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
> > > > > index c32d8228b5c..6fdada5f9b7 100644
> > > > > --- a/gcc/c-family/c-warn.c
> > > > > +++ b/gcc/c-family/c-warn.c
> > > > > @@ -3099,3 +3099,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
> > > > >     check_and_warn_address_or_pointer_of_packed_member (type, rhs);
> > > > >   }
> > > > > +
> > > > > +/* 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);
> > > > > +	}
> > > > > +    }
> > > > > +}
> > > > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > > > > index c1d8fd336e8..9ab44550140 100644
> > > > > --- a/gcc/c-family/c.opt
> > > > > +++ b/gcc/c-family/c.opt
> > > > > @@ -799,6 +799,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.
> > > > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> > > > > index a8bc301ffad..148832d388b 100644
> > > > > --- a/gcc/c/c-parser.c
> > > > > +++ b/gcc/c/c-parser.c
> > > > > @@ -7870,7 +7870,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;
> > > > > @@ -7888,9 +7888,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;				      \
> > > > > @@ -7904,18 +7906,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;								      \
> > > > > @@ -9171,6 +9178,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);
> > > > > @@ -10786,7 +10796,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;
> > > > > @@ -10809,7 +10820,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;
> > > > > diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
> > > > > index 10938cf0857..5b3751efb3a 100644
> > > > > --- a/gcc/c/c-tree.h
> > > > > +++ b/gcc/c/c-tree.h
> > > > > @@ -664,6 +664,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);
> > > > > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> > > > > index bb27099bfe1..8caf3dbf6db 100644
> > > > > --- a/gcc/c/c-typeck.c
> > > > > +++ b/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
> > > > > diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> > > > > index 9166156a5d5..7705f15c25f 100644
> > > > > --- a/gcc/cp/typeck.c
> > > > > +++ b/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
> > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > > index 6d9ff2c3362..97fef872a54 100644
> > > > > --- a/gcc/doc/invoke.texi
> > > > > +++ b/gcc/doc/invoke.texi
> > > > > @@ -362,6 +362,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
> > > > > @@ -5255,6 +5256,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
> > > > > @@ -7947,6 +7949,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
> > > > > diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c
> > > > > new file mode 100644
> > > > > index 00000000000..84d9a730cba
> > > > > --- /dev/null
> > > > > +++ b/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;
> > > > > +}
> > > > > diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
> > > > > index 83116183902..e9bad1fa420 100644
> > > > > --- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c
> > > > > +++ b/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;
> > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C
> > > > > new file mode 100644
> > > > > index 00000000000..da220cd57ba
> > > > > --- /dev/null
> > > > > +++ b/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);
> > > > > +}
> > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C
> > > > > new file mode 100644
> > > > > index 00000000000..7962c23522c
> > > > > --- /dev/null
> > > > > +++ b/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;)
> > > > > +    {
> > > > > +    }
> > > > > +}
> > > > > 
> > > > > base-commit: d1a31689a736cdfb5e7cfa01f1168e338510e63b
> > > > > -- 
> > > > > 2.26.2
> > > > > 
> > > > 
> > > > Marek
> > > > 
> > > 
> > 
> > Marek
> > 
> 
> Marek
> 

Marek

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

[Joseph Myers]

The C parts are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com