This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: warn on memset with constant length 0


Andrew Pinski <pinskia@gmail.com> writes:
> I don't think we should be warning inside builtins.c.  Also this really
> should gets own option to enable the warning and be part of -Wall and
> not -Wextra.

Thanks for your feedback.  What are your thoughts on the appended patch?

> With your patch as it is, it will trigger a warning with:
>
> static inline void f(int *a, int size)
> {
>   memset(a, 0, size);
> }
> 
> void g(void)
> {
>   f(0, 0);
> }
> 
> Which is very hard to avoid in some cases (my case is just simplified
> case).

I added that as a testcase to make sure that it didn't give any
warnings.  Looking forward to your feedback.

/assar

gcc/ChangeLog

2006-12-15  Assar  <assar@permabit.com>

	* c-common.h: Add warn_memset.
	* c-opts.c: Parse -Wmemset.
	
	* c-common.c: Add check for calling memset with constant third
	argument of 0.

	* c-typeck.c (build_function_call): Call check_builtin_arguments
	to get warnings.

	* c.opt: Add -Wmemset.

	* doc/invoke.texi: Document -Wmemset.

gcc/testsuite/ChangeLog

2006-12-15  Assar Westerlund  <assar@kth.se>

	* gcc.dg/memset-constant0-1.c: Test for "memset with 0" warning.
	* gcc.dg/memset-constant0-2.c: Likewise.
	* gcc.dg/memset-constant0-3.c: Test for not getting "memset with
	0" warning.

gcc/cp/ChangeLog

2006-12-15  Assar  <assar@permabit.com>

	* typeck.c (build_function_call): Call check_builtin_arguments to
	get warnings.
	* call.c (build_over_call): Same.

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 119393)
+++ gcc/doc/invoke.texi	(working copy)
@@ -234,7 +234,7 @@
 -Wno-int-to-pointer-cast @gol
 -Wno-invalid-offsetof  -Winvalid-pch @gol
 -Wlarger-than-@var{len}  -Wunsafe-loop-optimizations  -Wlong-long @gol
--Wmain  -Wmissing-braces  -Wmissing-field-initializers @gol
+-Wmain  -Wmemset  -Wmissing-braces  -Wmissing-field-initializers @gol
 -Wmissing-format-attribute  -Wmissing-include-dirs @gol
 -Wmissing-noreturn @gol
 -Wno-multichar  -Wnonnull  -Wno-overflow @gol
@@ -2507,6 +2507,12 @@
 arguments, two, or three arguments of appropriate types.
 This warning is enabled by @option{-Wall}.
 
+@item -Wmemset
+@opindex -Wmemset
+Warn if the third argument to @samp{memset} is a constant 0.  This is
+usually caused by swapping arguments two and three.
+This warning is enabled by @option{-Wall}.
+
 @item -Wmissing-braces
 @opindex Wmissing-braces
 Warn if an aggregate or union initializer is not fully bracketed.  In
Index: gcc/testsuite/gcc.dg/memset-constant0-2.c
===================================================================
--- gcc/testsuite/gcc.dg/memset-constant0-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/memset-constant0-2.c	(revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wmemset" } */
+
+typedef unsigned size_t;
+void *memset(void *s, int c, size_t n);
+
+void clear(void *p, size_t len)
+{
+  memset(p, len, 0); /* { dg-warning "memset called with a constant 0 length" } */
+}
Index: gcc/testsuite/gcc.dg/memset-constant0-1.c
===================================================================
--- gcc/testsuite/gcc.dg/memset-constant0-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/memset-constant0-1.c	(revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-Wmemset" } */
+
+typedef unsigned size_t;
+void *memset(void *s, int c, size_t n);
+
+int main()
+{
+  int foo;
+  memset(&foo, sizeof(foo), 0); /* { dg-warning "memset called with a constant 0 length" } */
+  return foo;
+}
Index: gcc/testsuite/gcc.dg/memset-constant0-3.c
===================================================================
--- gcc/testsuite/gcc.dg/memset-constant0-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/memset-constant0-3.c	(revision 0)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-Wextra -O3" } */
+
+typedef unsigned size_t;
+void *memset(void *s, int c, size_t n);
+
+static inline void f(int *a, int size)
+{
+  memset(a, 0, size);
+}
+
+void g(void)
+{
+  f(0, 0);
+}
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 119393)
+++ gcc/cp/typeck.c	(working copy)
@@ -2691,6 +2691,8 @@
   check_function_arguments (TYPE_ATTRIBUTES (fntype), coerced_params,
 			    TYPE_ARG_TYPES (fntype));
 
+  check_builtin_arguments (fndecl, coerced_params);
+
   return build_cxx_call (function, coerced_params);
 }
 
Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 119393)
+++ gcc/cp/call.c	(working copy)
@@ -4889,6 +4889,8 @@
   check_function_arguments (TYPE_ATTRIBUTES (TREE_TYPE (fn)),
 			    converted_args, TYPE_ARG_TYPES (TREE_TYPE (fn)));
 
+  check_builtin_arguments (fn, converted_args);
+
   /* Avoid actually calling copy constructors and copy assignment operators,
      if possible.  */
 
Index: gcc/c.opt
===================================================================
--- gcc/c.opt	(revision 119393)
+++ gcc/c.opt	(working copy)
@@ -255,6 +255,10 @@
 C ObjC
 Warn about suspicious declarations of \"main\"
 
+Wmemset
+C objC C++ ObjC++
+Warn about possible bad uses of \"memset\"
+
 Wmissing-braces
 C ObjC C++ ObjC++ Var(warn_missing_braces)
 Warn about possibly missing braces around initializers
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 119393)
+++ gcc/c-typeck.c	(working copy)
@@ -2328,6 +2328,8 @@
   check_function_arguments (TYPE_ATTRIBUTES (fntype), coerced_params,
 			    TYPE_ARG_TYPES (fntype));
 
+  check_builtin_arguments (fundecl, coerced_params);
+
   if (require_constant_value)
     {
       result = fold_build3_initializer (CALL_EXPR, TREE_TYPE (fntype),
Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c	(revision 119393)
+++ gcc/c-opts.c	(working copy)
@@ -385,6 +385,7 @@
       set_Wunused (value);
       set_Wformat (value);
       set_Wimplicit (value);
+      warn_memset = value;
       warn_char_subscripts = value;
       warn_missing_braces = value;
       warn_parentheses = value;
@@ -459,6 +460,10 @@
       set_Wformat (atoi (arg));
       break;
 
+    case OPT_Wmemset:
+      warn_memset = value;
+      break;
+
     case OPT_Wimplicit:
       set_Wimplicit (value);
       break;
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 119393)
+++ gcc/c-common.c	(working copy)
@@ -274,6 +274,10 @@
 
 int warn_format;
 
+/* Warn about likely bad uses of memset. */
+
+int warn_memset;
+
 /* Warn about using __null (as NULL in C++) as sentinel.  For code compiled
    with GCC this doesn't matter as __null is guaranteed to have the right
    size.  */
@@ -5827,6 +5831,30 @@
   return NULL_TREE;
 }
 
+/* Check arguments passed to memset.  */
+static void
+check_function_memset (tree params)
+{
+  tree param;
+  int param_num;
+
+  for (param = params, param_num = 1;
+       param;
+       param_num++, param = TREE_CHAIN (param))
+    if (param_num == 3 && integer_zerop (TREE_VALUE (param)))
+      warning (OPT_Wextra, "memset called with a constant 0 length");
+}
+
+void
+check_builtin_arguments (tree fundecl, tree params)
+{
+  if (!fundecl || !DECL_BUILT_IN(fundecl))
+    return;
+
+  if (warn_memset && DECL_FUNCTION_CODE(fundecl) == BUILT_IN_MEMSET)
+    check_function_memset (params);
+}
+
 /* Check for valid arguments being passed to a function.  */
 void
 check_function_arguments (tree attrs, tree params, tree typelist)
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h	(revision 119393)
+++ gcc/c-common.h	(working copy)
@@ -417,7 +417,11 @@
 
 extern int warn_format;
 
+/* Warn about likely bad uses of memset. */
 
+extern int warn_memset;
+
+
 /* C/ObjC language option variables.  */
 
 
@@ -634,6 +638,7 @@
 extern const char *fname_as_string (int);
 extern tree fname_decl (unsigned, tree);
 
+extern void check_builtin_arguments (tree, tree);
 extern void check_function_arguments (tree, tree, tree);
 extern void check_function_arguments_recurse (void (*)
 					      (void *, tree,


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]