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

Marek Polacek polacek@redhat.com
Thu Oct 8 15:04:40 GMT 2020


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



More information about the Gcc-patches mailing list