On 04/22/2016 03:57 PM, Jason Merrill wrote:
This looks good, but can we move the code into c-common rather than
duplicate it?
That would be this patch. Also passes testing on x86_64-linux.
Bernd
* doc/invoke.texi (Warning Options): Add -Wmemset-elt-size.
(-Wmemset-elt-size): New item.
c-family/
* c.opt (Wmemset-elt-size): New option.
* c-common.c (warn_for_memset): New function.
* c-common.h (warn_for_memset): Declare.
c/
* c-parser.c (c_parser_postfix_expression_after_primary): Call
warn_for_memset instead of warning directly here.
cp/
* parser.c (cp_parser_postfix_expression): Call
warn_for_memset instead of warning directly here.
testsuite/
* c-c++-common/memset-array.c: New test.
Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c (revision 235384)
+++ gcc/c/c-parser.c (working copy)
@@ -8282,18 +8282,15 @@ c_parser_postfix_expression_after_primar
expr.value, exprlist,
sizeof_arg,
sizeof_ptr_memacc_comptypes);
- if (warn_memset_transposed_args
- && TREE_CODE (expr.value) == FUNCTION_DECL
+ if (TREE_CODE (expr.value) == FUNCTION_DECL
&& DECL_BUILT_IN_CLASS (expr.value) == BUILT_IN_NORMAL
&& DECL_FUNCTION_CODE (expr.value) == BUILT_IN_MEMSET
- && vec_safe_length (exprlist) == 3
- && integer_zerop ((*exprlist)[2])
- && (literal_zero_mask & (1 << 2)) != 0
- && (!integer_zerop ((*exprlist)[1])
- || (literal_zero_mask & (1 << 1)) == 0))
- warning_at (expr_loc, OPT_Wmemset_transposed_args,
- "%<memset%> used with constant zero length parameter; "
- "this could be due to transposed parameters");
+ && vec_safe_length (exprlist) == 3)
+ {
+ tree arg0 = (*exprlist)[0];
+ tree arg2 = (*exprlist)[2];
+ warn_for_memset (expr_loc, arg0, arg2, literal_zero_mask);
+ }
start = expr.get_start ();
finish = parser->tokens_buf[0].get_finish ();
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c (revision 235384)
+++ gcc/c-family/c-common.c (working copy)
@@ -11767,6 +11767,49 @@ warn_for_div_by_zero (location_t loc, tr
warning_at (loc, OPT_Wdiv_by_zero, "division by zero");
}
+/* Warn for patterns where memset appears to be used incorrectly. The
+ warning location should be LOC. ARG0, and ARG2 are the first and
+ last arguments to the call, while LITERAL_ZERO_MASK has a 1 bit for
+ each argument that was a literal zero. */
+
+void
+warn_for_memset (location_t loc, tree arg0, tree arg2,
+ int literal_zero_mask)
+{
+ if (warn_memset_transposed_args
+ && integer_zerop (arg2)
+ && (literal_zero_mask & (1 << 2)) != 0
+ && (literal_zero_mask & (1 << 1)) == 0)
+ warning_at (loc, OPT_Wmemset_transposed_args,
+ "%<memset%> used with constant zero length "
+ "parameter; this could be due to transposed "
+ "parameters");
+
+ if (warn_memset_elt_size && TREE_CODE (arg2) == INTEGER_CST)
+ {
+ STRIP_NOPS (arg0);
+ if (TREE_CODE (arg0) == ADDR_EXPR)
+ arg0 = TREE_OPERAND (arg0, 0);
+ tree type = TREE_TYPE (arg0);
+ if (TREE_CODE (type) == ARRAY_TYPE)
+ {
+ tree elt_type = TREE_TYPE (type);
+ tree domain = TYPE_DOMAIN (type);
+ if (!integer_onep (TYPE_SIZE_UNIT (elt_type))
+ && TYPE_MAXVAL (domain)
+ && TYPE_MINVAL (domain)
+ && integer_zerop (TYPE_MINVAL (domain))
+ && integer_onep (fold_build2 (MINUS_EXPR, domain,
+ arg2,
+ TYPE_MAXVAL (domain))))
+ warning_at (loc, OPT_Wmemset_elt_size,
+ "%<memset%> used with length equal to "
+ "number of elements without multiplication "
+ "with element size");
+ }
+ }
+}
+
/* Subroutine of build_binary_op. Give warnings for comparisons
between signed and unsigned quantities that may fail. Do the
checking based on the original operand trees ORIG_OP0 and ORIG_OP1,
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h (revision 235384)
+++ gcc/c-family/c-common.h (working copy)
@@ -902,6 +902,7 @@ extern void c_parse_file (void);
extern void c_parse_final_cleanups (void);
extern void warn_for_omitted_condop (location_t, tree);
+extern void warn_for_memset (location_t, tree, tree, int);
/* These macros provide convenient access to the various _STMT nodes. */
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt (revision 235384)
+++ gcc/c-family/c.opt (working copy)
@@ -565,6 +565,10 @@ Wmemset-transposed-args
C ObjC C++ ObjC++ Var(warn_memset_transposed_args) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about suspicious calls to memset where the third argument is constant literal zero and the second is not.
+Wmemset-elt-size
+C ObjC C++ ObjC++ Var(warn_memset_elt_size) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about suspicious calls to memset where the third argument contains the number of elements not multiplied by the element size.
+
Wmisleading-indentation
C C++ Common Var(warn_misleading_indentation) Warning LangEnabledBy(C C++,Wall)
Warn when the indentation of the code does not reflect the block structure.
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c (revision 235384)
+++ gcc/cp/parser.c (working copy)
@@ -6829,20 +6829,19 @@ cp_parser_postfix_expression (cp_parser
}
}
- if (warn_memset_transposed_args)
+ if (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)
{
- if (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
- && TREE_CODE ((*args)[2]) == INTEGER_CST
- && integer_zerop ((*args)[2])
- && !(TREE_CODE ((*args)[1]) == INTEGER_CST
- && integer_zerop ((*args)[1])))
- warning (OPT_Wmemset_transposed_args,
- "%<memset%> used with constant zero length "
- "parameter; this could be due to transposed "
- "parameters");
+ tree arg0 = (*args)[0];
+ tree arg1 = (*args)[1];
+ tree arg2 = (*args)[2];
+ if (TREE_CODE (arg2) == CONST_DECL)
+ arg2 = DECL_INITIAL (arg2);
+ int literal_mask = ((!!integer_zerop (arg1) << 1)
+ | (!!integer_zerop (arg2) << 2));
+ warn_for_memset (input_location, arg0, arg2, literal_mask);
}
if (TREE_CODE (postfix_expression) == COMPONENT_REF)
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 235384)
+++ gcc/doc/invoke.texi (working copy)
@@ -273,7 +273,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-int-to-pointer-cast -Winvalid-memory-model -Wno-invalid-offsetof @gol
-Winvalid-pch -Wlarger-than=@var{len} @gol
-Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol
--Wmain -Wmaybe-uninitialized -Wmemset-transposed-args @gol
+-Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args @gol
-Wmisleading-indentation -Wmissing-braces @gol
-Wmissing-field-initializers -Wmissing-include-dirs @gol
-Wno-multichar -Wnonnull -Wnonnull-compare @gol
@@ -3547,6 +3547,7 @@ Options} and @ref{Objective-C and Object
-Wlogical-not-parentheses
-Wmain @r{(only for C/ObjC and unless} @option{-ffreestanding}@r{)} @gol
-Wmaybe-uninitialized @gol
+-Wmemset-elt-size @gol
-Wmemset-transposed-args @gol
-Wmisleading-indentation @r{(only for C/C++)} @gol
-Wmissing-braces @r{(only for C/ObjC)} @gol
@@ -5256,6 +5257,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-elt-size
+@opindex Wmemset-elt-size
+@opindex Wno-memset-elt-size
+Warn for suspicious calls to the @code{memset} built-in function, if the
+first argument references an array, and the third argument is a number
+equal to the number of elements, but not equal to the size of the array
+in memory. This indicates that the user has omitted a multiplication by
+the element size. This warning is enabled by @option{-Wall}.
+
@item -Wmemset-transposed-args
@opindex Wmemset-transposed-args
@opindex Wno-memset-transposed-args
Index: gcc/testsuite/c-c++-common/memset-array.c
===================================================================
--- gcc/testsuite/c-c++-common/memset-array.c (revision 0)
+++ gcc/testsuite/c-c++-common/memset-array.c (working copy)
@@ -0,0 +1,36 @@
+/* { dg-do compile } */
+/* { dg-options "-Wmemset-elt-size" } */
+enum a {
+ a_1,
+ a_2,
+ a_n
+};
+int t1[20];
+int t2[a_n];
+
+struct s
+{
+ int t[20];
+};
+
+void foo (struct s *s)
+{
+ __builtin_memset (t1, 0, 20); /* { dg-warning "element size" } */
+ __builtin_memset (t2, 0, a_n); /* { dg-warning "element size" } */
+ __builtin_memset (s->t, 0, 20); /* { dg-warning "element size" } */
+}
+
+char u1[20];
+char u2[a_n];
+
+struct s2
+{
+ char u[20];
+};
+
+void bar (struct s2 *s)
+{
+ __builtin_memset (u1, 0, 20);
+ __builtin_memset (u2, 0, a_n);
+ __builtin_memset (s->u, 0, 20);
+}
