[PATCH] c, c++: Implement -Wsizeof-array-div [PR91741]
Marek Polacek
polacek@redhat.com
Mon Sep 14 18:34:15 GMT 2020
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
More information about the Gcc-patches
mailing list