From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23416 invoked by alias); 8 Jul 2014 12:50:33 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 23398 invoked by uid 89); 8 Jul 2014 12:50:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 08 Jul 2014 12:50:29 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s68CoPb7027082 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 8 Jul 2014 08:50:26 -0400 Received: from tucnak.zalov.cz (ovpn-116-32.ams2.redhat.com [10.36.116.32]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s68CoMsJ004812 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 8 Jul 2014 08:50:24 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.8/8.14.7) with ESMTP id s68CoKHe019136; Tue, 8 Jul 2014 14:50:21 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.8/8.14.8/Submit) id s68CoHIG019135; Tue, 8 Jul 2014 14:50:17 +0200 Date: Tue, 08 Jul 2014 12:50:00 -0000 From: Jakub Jelinek To: "Joseph S. Myers" , Jason Merrill , "Carlos O'Donell" , Siddhesh Poyarekar Cc: gcc-patches@gcc.gnu.org, libc-alpha@sourceware.org Subject: [RFC PATCH] -Wmemset-transposed-args (PR middle-end/61294) Message-ID: <20140708125017.GN31640@tucnak.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2014-07/txt/msg00546.txt.bz2 Hi! This is an attempt to move the warning about transposed memset arguments from the glibc headers to gcc FEs. The problem with the warning in glibc is that it uses __builtin_constant_p and e.g. jump threading very often makes the warning trigger even on code where it is very unlikely a user swapped arguments. See e.g. https://gcc.gnu.org/PR51744 https://gcc.gnu.org/PR56977 https://gcc.gnu.org/PR61294 https://bugzilla.redhat.com/452219 https://bugs.kde.org/show_bug.cgi?id=311098 https://bugzilla.mozilla.org/show_bug.cgi?id=581227 and many others. Thus, I'd like to warn in the FEs instead, and once we have a GCC release with that warning in, disable the glibc bits/string3.h: if (__builtin_constant_p (__len) && __len == 0 && (!__builtin_constant_p (__ch) || __ch != 0)) { __warn_memset_zero_len (); return __dest; } warning for GCC versions with that new warning in. Any thoughts on this? If you are ok with it, either we can add it only for 4.10/5.0 and later only, or perhaps 4.9.2 too, or even 4.9.1. For -D_FORTIFY_SOURCE=2 built code with glibc it shouldn't make a difference (other than having fewer false positives), but for other non-fortified -Wall compilation it would make a difference (introducing new warnings), so perhaps doing it only for 4.10/5.0+ is best. 2014-07-08 Jakub Jelinek PR middle-end/61294 gcc/c-family/ * c.opt (Wmemset-transposed-args): New warning. gcc/c/ * c-typeck.c (c_build_function_call_vec): Handle -Wmemset-transposed-args. gcc/cp/ * semantics.c (finish_call_expr): Handle -Wmemset-transposed-args. gcc/ * doc/invoke.texi (-Wmemset-transposed-args): Document. gcc/testsuite/ * c-c++-common/Wmemset-transposed-args1.c: New test. * g++.dg/warn/Wmemset-transposed-args-1.C: New test. --- gcc/c-family/c.opt.jj 2014-07-07 10:39:43.000000000 +0200 +++ gcc/c-family/c.opt 2014-07-08 13:12:07.755536537 +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 --- gcc/c/c-typeck.c.jj 2014-07-07 10:39:43.000000000 +0200 +++ gcc/c/c-typeck.c 2014-07-08 13:22:36.846564329 +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, + "% 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/semantics.c.jj 2014-07-02 09:04:13.000000000 +0200 +++ gcc/cp/semantics.c 2014-07-08 14:02:45.936782580 +0200 @@ -2361,6 +2361,18 @@ finish_call_expr (tree fn, vec used with constant zero length parameter; " + "this could be due to transposed parameters"); + /* A call to a namespace-scope function. */ result = build_new_function_call (fn, args, koenig_p, complain); } --- gcc/doc/invoke.texi.jj 2014-07-08 11:36:14.000000000 +0200 +++ gcc/doc/invoke.texi 2014-07-08 14:20:09.932799699 +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/c-c++-common/Wmemset-transposed-args1.c.jj 2014-07-08 13:46:19.381765644 +0200 +++ gcc/testsuite/c-c++-common/Wmemset-transposed-args1.c 2014-07-08 13:46:14.868798956 +0200 @@ -0,0 +1,20 @@ +/* { 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, 1, 1 - 1); /* { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } */ + memset (buf, 0, 0); + memset (buf, 1 - 1, 0); +} --- gcc/testsuite/g++.dg/warn/Wmemset-transposed-args-1.C.jj 2014-07-08 13:50:22.685624346 +0200 +++ gcc/testsuite/g++.dg/warn/Wmemset-transposed-args-1.C 2014-07-08 13:51:59.837968475 +0200 @@ -0,0 +1,25 @@ +// { dg-do compile } +// { dg-options "-Wall" } + +typedef __SIZE_TYPE__ size_t; +extern "C" void *memset (void *, int, size_t); +char buf[1024]; + +template +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, N); // { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } + memset (buf, 1, 1 - 1); // { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } + memset (buf, 1, N - N); // { dg-warning ".memset. used with constant zero length parameter; this could be due to transposed parameters" } + memset (buf, 0, 0); + memset (buf, 1 - 1, 0); +} + +void +bar () +{ + foo<0> (); +} Jakub