Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument

[Prathamesh Kulkarni <bilbotheelffriend at gmail dot com>]

On Fri, May 2, 2014 at 5:50 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> The following apply to all versions of this patch:
>
> * New options need documenting in invoke.texi.
Added.
>
> * New options need nonempty help text in c.opt.  (It's unfortunate that
> the -Wsizeof-pointer-memaccess option immediately above got added without
> such help text.)
Added.
>
> * Don't use %s with IDENTIFIER_POINTER in diagnostics; instead, use %qE
> with the identifier passed directly to warning_at, as that will handle
> converting extended identifiers appropriately for the user's locale.
Thanks, modified the patch to use %qE.
>
> * What happens if some declarations of the function use a pointer and some
> use an array?  You should consider this case, decide what the best
> semantics are and ensure the testsuite verifies that they are implemented.
The warning is issued only if the parameter is declared as array in
function definition.
I hope that's reasonable, clang does the same thing.

Example:
int f1(int *p);
int f1(int p[]) { return (int) sizeof (p); } // warns

int f2(int p[])
int f2(int *p) { return (int) sizeof (p); } // does not warn

In this version of the patch, the warning is only enabled by passing
-Wsizeof-array-argument.
Should it be enabled by -Wextra or -Wall ?

Bootstrapped and tested on x86_64-unknown-linux-gnu.

[gcc/c]
  * c-tree.h (C_ARRAY_PARM): New macro, alias for DECL_LANG_FLAG_2.
  * c-decl.c (push_parm_decl): Set C_ARRAY_PARM (decl) if declarator
is an array parameter.
  * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning.

[gcc/c-family]
  * c.opt (Wsizeof-array-argument): New option.

[gcc/testsuite/gcc.dg]
  * sizeof-array-argument.c: New test-case.

[gcc/doc]
  * invoke.texi: Document Wsizeof-array-argument.

Thanks and Regards,
Prathamesh

>
> --
> Joseph S. Myers
> joseph@codesourcery.com

Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 210004)
+++ gcc/c/c-decl.c	(working copy)
@@ -4650,6 +4650,8 @@ push_parm_decl (const struct c_parm *par
   decl = grokdeclarator (parm->declarator, parm->specs, PARM, false, NULL,
 			 &attrs, expr, NULL, DEPRECATED_NORMAL);
   decl_attributes (&decl, attrs, 0);
+  
+  C_ARRAY_PARM (decl) = parm->declarator->kind == cdk_array;
 
   decl = pushdecl (decl);
 
Index: gcc/c/c-tree.h
===================================================================
--- gcc/c/c-tree.h	(revision 210004)
+++ gcc/c/c-tree.h	(working copy)
@@ -66,6 +66,9 @@ along with GCC; see the file COPYING3.
 /* For a FUNCTION_DECL, nonzero if it was an implicit declaration.  */
 #define C_DECL_IMPLICIT(EXP) DECL_LANG_FLAG_2 (EXP)
 
+/* For a PARM_DECL, nonzero if parameter was declared as array */
+#define C_ARRAY_PARM(NODE) DECL_LANG_FLAG_2 (NODE)
+
 /* For FUNCTION_DECLs, evaluates true if the decl is built-in but has
    been declared.  */
 #define C_DECL_DECLARED_BUILTIN(EXP)		\
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 210004)
+++ gcc/c/c-typeck.c	(working copy)
@@ -2732,6 +2732,13 @@ c_expr_sizeof_expr (location_t loc, stru
   else
     {
       bool expr_const_operands = true;
+
+      if (warn_sizeof_array_argument && TREE_CODE (expr.value) == PARM_DECL && C_ARRAY_PARM (expr.value)) 
+	{
+	  warning_at (loc, 0, "sizeof on array parameter %qE shall return size of %qT",
+		      DECL_NAME (expr.value), expr.original_type); 
+	  inform (DECL_SOURCE_LOCATION (expr.value), "declared here");
+	}
       tree folded_expr = c_fully_fold (expr.value, require_constant_value,
 				       &expr_const_operands);
       ret.value = c_sizeof (loc, TREE_TYPE (folded_expr));
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 210004)
+++ gcc/c-family/c.opt	(working copy)
@@ -517,6 +517,10 @@ Warn about missing fields in struct init
 Wsizeof-pointer-memaccess
 C ObjC C++ ObjC++ Var(warn_sizeof_pointer_memaccess) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 
+Wsizeof-array-argument
+C Var(warn_sizeof_array_argument) Warning
+Warn when sizeof is applied on a parameter declared as an array
+
 Wsuggest-attribute=format
 C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning
 Warn about functions which might be candidates for format attributes
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 210004)
+++ gcc/doc/invoke.texi	(working copy)
@@ -265,6 +265,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wreturn-type  -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wsign-conversion -Wfloat-conversion @gol
 -Wsizeof-pointer-memaccess @gol
+-Wsizeof-array-argument @gol
 -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
 -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
 -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
@@ -4639,6 +4640,12 @@ but a pointer, and suggests a possible f
 @code{memcpy (&foo, ptr, sizeof (&foo));}.  This warning is enabled by
 @option{-Wall}.
 
+@item -Wsizeof-array-argument
+@opindex Wsizeof-array-argument
+@opindex Wno-sizeof-array-argument
+Warn when sizeof is applied to parameter that is declared as an array
+in function definition.
+
 @item -Waddress
 @opindex Waddress
 @opindex Wno-address
Index: gcc/testsuite/gcc.dg/sizeof-array-argument.c
===================================================================
--- gcc/testsuite/gcc.dg/sizeof-array-argument.c	(revision 0)
+++ gcc/testsuite/gcc.dg/sizeof-array-argument.c	(working copy)
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-Wsizeof-array-argument" } */
+
+int foo(int a[])
+{
+  return (int) sizeof (a);  /* { dg-warning "sizeof on array parameter" } */
+}
+
+int bar(int x, int b[3])
+{
+  return x + (int) sizeof (b);  /* { dg-warning "sizeof on array parameter" } */
+}
+
+int f1(int *p)
+{
+  return (int) sizeof (p);
+}
+
+int f2(int *p);
+
+int f2(int p[])
+{
+  return (int) sizeof (p);  /* { dg-warning "sizeof on array parameter" } */
+}
+
+int f3(int x[]);
+
+int f3(int *x)
+{
+  return (int) sizeof (x);
+}