[RFC PATCH] -Wmemset-transposed-args (PR middle-end/61294)
Jakub Jelinek
jakub@redhat.com
Wed Jul 9 10:33:00 GMT 2014
On Wed, Jul 09, 2014 at 12:26:09PM +0200, Richard Biener wrote:
> > I suppose we could use an INTEGER_CST distinct from the one in
> > TYPE_CACHED_VALUES for raw 0, with a TREE_LANG_FLAG set.
>
> Ick. (please no - at least make sure it doesn't survive anywhere to the
> middle-end, like fold or gimple).
Why? I don't think the middle-end ever compares INTEGER_CSTs for equality.
Here is my WIP on Jason's suggestion (untested of course other than the
testcase), the C++ FE is changed to do that, the C FE is not.
Sure, I could add another bool argument to
cp_parser_parenthesized_expression_list whether the caller expects to see
the literal zeros, and replace them with normal INTEGER_CSTs after the test.
--- gcc/c/c-typeck.c.jj 2014-07-08 17:38:09.612266656 +0200
+++ gcc/c/c-typeck.c 2014-07-09 10:41:38.931018088 +0200
@@ -2987,6 +2987,16 @@ c_build_function_call_vec (location_t lo
/* Convert anything with function type to a pointer-to-function. */
if (TREE_CODE (function) == FUNCTION_DECL)
{
+ if (warn_memset_transposed_args
+ && DECL_BUILT_IN_CLASS (function) == BUILT_IN_NORMAL
+ && DECL_FUNCTION_CODE (function) == BUILT_IN_MEMSET
+ && vec_safe_length (params) == 3
+ && integer_zerop ((*params)[2])
+ && !integer_zerop ((*params)[1]))
+ warning_at (loc, OPT_Wmemset_transposed_args,
+ "%<memset%> used with constant zero length parameter; "
+ "this could be due to transposed parameters");
+
/* Implement type-directed function overloading for builtins.
resolve_overloaded_builtin and targetm.resolve_overloaded_builtin
handle all the type checking. The result is a complete expression
--- gcc/cp/cp-tree.h.jj 2014-07-07 10:39:44.000000000 +0200
+++ gcc/cp/cp-tree.h 2014-07-09 11:48:12.534274257 +0200
@@ -4187,6 +4187,10 @@ more_aggr_init_expr_args_p (const aggr_i
#define SIZEOF_EXPR_TYPE_P(NODE) \
TREE_LANG_FLAG_0 (SIZEOF_EXPR_CHECK (NODE))
+/* True if INTEGER_CST is a zero literal seen in function argument list. */
+#define LITERAL_ZERO_P(NODE) \
+ (INTEGER_CST_CHECK (NODE)->base.nothrow_flag)
+
/* An enumeration of the kind of tags that C++ accepts. */
enum tag_types {
none_type = 0, /* Not a tag type. */
--- gcc/cp/parser.c.jj 2014-07-09 10:41:12.000000000 +0200
+++ gcc/cp/parser.c 2014-07-09 11:36:40.195693369 +0200
@@ -6142,6 +6142,19 @@ cp_parser_postfix_expression (cp_parser
}
}
+ if (warn_memset_transposed_args
+ && TREE_CODE (postfix_expression) == FUNCTION_DECL
+ && DECL_BUILT_IN_CLASS (postfix_expression) == BUILT_IN_NORMAL
+ && DECL_FUNCTION_CODE (postfix_expression) == BUILT_IN_MEMSET
+ && vec_safe_length (args) == 3
+ && integer_zerop ((*args)[2])
+ && LITERAL_ZERO_P ((*args)[2])
+ && !(integer_zerop ((*args)[1])
+ && LITERAL_ZERO_P ((*args)[1])))
+ warning (OPT_Wmemset_transposed_args,
+ "%<memset%> used with constant zero length parameter; "
+ "this could be due to transposed parameters");
+
if (TREE_CODE (postfix_expression) == COMPONENT_REF)
{
tree instance = TREE_OPERAND (postfix_expression, 0);
@@ -6630,6 +6643,10 @@ cp_parser_postfix_dot_deref_expression (
return postfix_expression;
}
+/* Cache of LITERAL_ZERO_P constants. */
+
+static GTY(()) tree literal_zeros[itk_none];
+
/* Parse a parenthesized expression-list.
expression-list:
@@ -6724,7 +6741,50 @@ cp_parser_parenthesized_expression_list
*non_constant_p = true;
}
else
- expr = cp_parser_assignment_expression (parser, cast_p, NULL);
+ {
+ expr = NULL_TREE;
+ cp_token *tok = cp_lexer_peek_token (parser->lexer);
+ switch (tok->type)
+ {
+ case CPP_NUMBER:
+ case CPP_CHAR:
+ case CPP_WCHAR:
+ case CPP_CHAR16:
+ case CPP_CHAR32:
+ /* If a parameter is literal zero alone, remember it
+ for -Wmemset-transposed-args warning. */
+ if (integer_zerop (tok->u.value)
+ && !TREE_OVERFLOW (tok->u.value)
+ && is_attribute_list == non_attr
+ && (cp_lexer_peek_nth_token (parser->lexer, 2)->type
+ == CPP_COMMA
+ || cp_lexer_peek_nth_token (parser->lexer, 2)->type
+ == CPP_CLOSE_PAREN))
+ {
+ unsigned int i;
+ for (i = 0; i < itk_none; ++i)
+ if (TREE_TYPE (tok->u.value) == integer_types[i])
+ break;
+ if (i < itk_none && literal_zeros[i])
+ expr = literal_zeros[i];
+ else
+ {
+ expr = copy_node (tok->u.value);
+ LITERAL_ZERO_P (expr) = 1;
+ if (i < itk_none)
+ literal_zeros[i] = expr;
+ }
+ /* Consume the 0 token (or '\0', 0LL etc.). */
+ cp_lexer_consume_token (parser->lexer);
+ }
+ break;
+ default:
+ break;
+ }
+ if (expr == NULL_TREE)
+ expr = cp_parser_assignment_expression (parser, cast_p,
+ NULL);
+ }
if (fold_expr_p)
expr = fold_non_dependent_expr (expr);
--- gcc/doc/invoke.texi.jj 2014-07-08 17:38:09.301268037 +0200
+++ gcc/doc/invoke.texi 2014-07-09 10:41:38.950017994 +0200
@@ -257,8 +257,8 @@ Objective-C and Objective-C++ Dialects}.
-Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
-Winvalid-pch -Wlarger-than=@var{len} -Wunsafe-loop-optimizations @gol
-Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol
--Wmain -Wmaybe-uninitialized -Wmissing-braces -Wmissing-field-initializers @gol
--Wmissing-include-dirs @gol
+-Wmain -Wmaybe-uninitialized -Wmemset-transposed-args -Wmissing-braces @gol
+-Wmissing-field-initializers -Wmissing-include-dirs @gol
-Wno-multichar -Wnonnull -Wno-overflow -Wopenmp-simd @gol
-Woverlength-strings -Wpacked -Wpacked-bitfield-compat -Wpadded @gol
-Wparentheses -Wpedantic-ms-format -Wno-pedantic-ms-format @gol
@@ -4683,6 +4683,15 @@ Warn when the @code{sizeof} operator is
declared as an array in a function definition. This warning is enabled by
default for C and C++ programs.
+@item -Wmemset-transposed-args
+@opindex Wmemset-transposed-args
+@opindex Wno-memset-transposed-args
+Warn for suspicious calls to the memset built-in function, if the
+second argument is not zero and third argument is zero. This warns e.g.@
+about @code{memset (buf, sizeof buf, 0);} where most probably
+@code{memset (buf, 0, sizeof buf);} was meant instead. This warning is
+enabled by @option{-Wall}.
+
@item -Waddress
@opindex Waddress
@opindex Wno-address
--- gcc/testsuite/g++.dg/warn/Wmemset-transposed-args-1.C.jj 2014-07-09 10:41:38.969017900 +0200
+++ gcc/testsuite/g++.dg/warn/Wmemset-transposed-args-1.C 2014-07-09 12:12:40.850993778 +0200
@@ -0,0 +1,74 @@
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+typedef __SIZE_TYPE__ size_t;
+extern "C" void *memset (void *, int, size_t);
+char buf[1024];
+namespace std
+{
+ extern "C" void *memset (void *, int, size_t);
+}
+
+template <int N>
+void
+foo ()
+{
+ memset (buf, sizeof buf, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, '\0'); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, L'\0'); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, N);
+ memset (buf, 1, 1 - 1);
+ memset (buf, 1, 0 - 0);
+ memset (buf, 1, N - N);
+ memset (buf, 0, 0);
+ memset (buf, 1 - 1, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, 0 - 0, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, 0L); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, 0UL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, 0LL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, 0ULL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, (int) 0);
+ memset (buf, sizeof buf, -0);
+}
+
+template <int N>
+void
+baz ()
+{
+ std::memset (buf, sizeof buf, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, '\0'); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, L'\0'); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, N);
+ std::memset (buf, 1, 1 - 1);
+ std::memset (buf, 1, 0 - 0);
+ std::memset (buf, 1, N - N);
+ std::memset (buf, 0, 0);
+ std::memset (buf, 1 - 1, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, 0 - 0, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, 0L); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, 0UL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, 0LL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, 0ULL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, (int) 0);
+ std::memset (buf, sizeof buf, -0);
+}
+
+void
+bar ()
+{
+ foo<0> ();
+ std::memset (buf, sizeof buf, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, '\0'); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, L'\0'); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, 1, 1 - 1);
+ std::memset (buf, 1, 0 - 0);
+ std::memset (buf, 0, 0);
+ std::memset (buf, 1 - 1, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, 0 - 0, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, 0L); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, 0UL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, 0LL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, 0ULL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ std::memset (buf, sizeof buf, (int) 0);
+ std::memset (buf, sizeof buf, -0);
+}
--- gcc/testsuite/c-c++-common/Wmemset-transposed-args1.c.jj 2014-07-09 10:41:38.969017900 +0200
+++ gcc/testsuite/c-c++-common/Wmemset-transposed-args1.c 2014-07-09 11:51:12.000000000 +0200
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+extern
+#ifdef __cplusplus
+"C"
+#endif
+void *memset (void *, int, size_t);
+char buf[1024];
+
+void
+foo ()
+{
+ memset (buf, sizeof buf, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, '\0'); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, L'\0'); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, 1, 1 - 1);
+ memset (buf, 1, 0 - 0);
+ memset (buf, 0, 0);
+ memset (buf, 1 - 1, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, 0 - 0, 0); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, 0L); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, 0UL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, 0LL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, 0ULL); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */
+ memset (buf, sizeof buf, (int) 0);
+ memset (buf, sizeof buf, -0);
+}
--- gcc/c-family/c.opt.jj 2014-07-08 17:38:09.249268240 +0200
+++ gcc/c-family/c.opt 2014-07-09 10:41:38.909018201 +0200
@@ -518,6 +518,10 @@ Wmain
LangEnabledBy(C ObjC C++ ObjC++,Wpedantic, 2, 0)
;
+Wmemset-transposed-args
+C ObjC C++ ObjC++ Var(warn_memset_transposed_args) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about suspicious call to memset where the third argument is constant zero and second is not zero
+
Wmissing-braces
C ObjC C++ ObjC++ Var(warn_missing_braces) Warning LangEnabledBy(C ObjC,Wall)
Warn about possibly missing braces around initializers
Jakub
More information about the Libc-alpha
mailing list