This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
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,