From patchwork Fri Apr 22 13:30:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Schmidt X-Patchwork-Id: 613608 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3qrxMv0MfQz9t5y for ; Fri, 22 Apr 2016 23:31:03 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=V//Cz/UB; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=Kb6pDNj16ry65TQhxV7ejONhQM+Z4XpXg3e4k9UCJ2FWZA+4Zs 8JiPh+iqEZx8pMvXkRVn6gUHdep2XAcCY+WGIIpMJmi71Bnnc38irQ4hAnaWqGif aOcma8FS1I+9eh7CbqsZTcvq+XpRKAuWMQ7h8y8RMf3KR+CqUJRwUqIpQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=tGEw0lVAQ5pxmANTsgpnesFyujk=; b=V//Cz/UBJvluak1JCSAo jsX9pFrvucY+mGAnfNG2G6Ps7fvIDl5DvHBxnMVcTzxP03AJw98Ny4jg/Zc+Qr/1 YDT6OaDwAsRkcMKkocBs2deCAfxnLcgpcuZOlqqknIcfwz3m9bhu41NRQvQocug1 13wwCNvraTmwKcs2/I21n/M= Received: (qmail 104235 invoked by alias); 22 Apr 2016 13:30:56 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 104226 invoked by uid 89); 22 Apr 2016 13:30:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL, BAYES_05, KAM_ASCII_DIVIDERS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=H*MI:online, bernd, dg-warning, retest X-HELO: mailout07.t-online.de Received: from mailout07.t-online.de (HELO mailout07.t-online.de) (194.25.134.83) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 22 Apr 2016 13:30:53 +0000 Received: from fwd02.aul.t-online.de (fwd02.aul.t-online.de [172.20.26.148]) by mailout07.t-online.de (Postfix) with SMTP id BF440394371; Fri, 22 Apr 2016 15:30:49 +0200 (CEST) Received: from sweetums.local (GEhKQOZJYhuFCcXqU0yd+e9VdtkOUVyUz3Slkn4b+EF7Zb5QFQuCwj+g++X1m2oQX4@[87.168.121.82]) by fwd02.t-online.de with (TLSv1.2:ECDHE-RSA-AES256-SHA encrypted) esmtp id 1atbAQ-495cxs0; Fri, 22 Apr 2016 15:30:42 +0200 To: GCC Patches , Bernd Schmidt , Jason Merrill , "Joseph S. Myers" From: Bernd Schmidt Subject: C, C++: New warning for memset without multiply by elt size Message-ID: <571A2776.6060502@t-online.de> Date: Fri, 22 Apr 2016 15:30:30 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.0 MIME-Version: 1.0 X-IsSubscribed: yes We had this problem in the C frontend until very recently: int array[some_count]; memset (array, 0, some_count); which forgets to multiply by sizeof int. The following patch implements a new warning option for this. Bootstrapped and tested (a while ago, will retest) on x86_64-linux. Ok for trunk? 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/ * c-parser.c (c_parser_postfix_expression_after_primary): Warn for memset with element count rather than array size. cp/ * parser.c (cp_parser_postfix_expression): Warn for memset with element count rather than array size. testsuite/ * c-c++-common/memset-array.c: New test. Index: gcc/c/c-parser.c =================================================================== --- gcc/c/c-parser.c (revision 234183) +++ gcc/c/c-parser.c (working copy) @@ -8240,18 +8240,46 @@ 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, - "% used with constant zero length parameter; " - "this could be due to transposed parameters"); + && vec_safe_length (exprlist) == 3) + { + if (warn_memset_transposed_args + && 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, + "% used with constant zero length " + "parameter; this could be due to transposed " + "parameters"); + if (warn_memset_elt_size + && TREE_CODE ((*exprlist)[2]) == INTEGER_CST) + { + tree ptr = (*exprlist)[0]; + STRIP_NOPS (ptr); + if (TREE_CODE (ptr) == ADDR_EXPR) + ptr = TREE_OPERAND (ptr, 0); + tree type = TREE_TYPE (ptr); + 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, + (*exprlist)[2], + TYPE_MAXVAL (domain)))) + warning_at (expr_loc, OPT_Wmemset_elt_size, + "% used with length equal to " + "number of elements without multiplication " + "with element size"); + } + } + } start = expr.get_start (); finish = parser->tokens_buf[0].get_finish (); Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 234183) +++ gcc/cp/parser.c (working copy) @@ -6829,20 +6829,48 @@ 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]))) + tree arg1 = (*args)[1]; + tree arg2 = (*args)[2]; + if (TREE_CODE (arg2) == CONST_DECL) + arg2 = DECL_INITIAL (arg2); + + if (warn_memset_transposed_args + && integer_zerop (arg2) + && !(TREE_CODE (arg1) == INTEGER_CST + && integer_zerop (arg1))) warning (OPT_Wmemset_transposed_args, "% used with constant zero length " "parameter; this could be due to transposed " "parameters"); + if (warn_memset_elt_size && TREE_CODE (arg2) == INTEGER_CST) + { + tree ptr = (*args)[0]; + STRIP_NOPS (ptr); + if (TREE_CODE (ptr) == ADDR_EXPR) + ptr = TREE_OPERAND (ptr, 0); + tree type = TREE_TYPE (ptr); + 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 (OPT_Wmemset_elt_size, + "% used with length equal to " + "number of elements without multiplication " + "with element size"); + } + } } if (TREE_CODE (postfix_expression) == COMPONENT_REF) 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); +} Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 234183) +++ 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 @@ -5248,6 +5249,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/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 234183) +++ gcc/c-family/c.opt (working copy) @@ -561,6 +561,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.