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
jos...@codesourcery.com

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

[Marek Polacek via Gcc-patches]

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. .

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:
+