From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14604 invoked by alias); 21 Apr 2017 23:50:46 -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 14583 invoked by uid 89); 21 Apr 2017 23:50:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_MANYTO,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=H*Ad:U*msebor, our X-HELO: mail-qk0-f180.google.com Received: from mail-qk0-f180.google.com (HELO mail-qk0-f180.google.com) (209.85.220.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 21 Apr 2017 23:50:44 +0000 Received: by mail-qk0-f180.google.com with SMTP id h67so83708425qke.0 for ; Fri, 21 Apr 2017 16:50:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=quoATPtgZ1R+zlRTEqbfb0P7W71558n4x1wsh2YWkis=; b=lAblLxljTk+wyh1LbH/TGbvL1KnXFeCdRLr8L84VyQht+Nobi+k3tK8UsSUS1avJuO /6N0WFuTQbjfSToeIkuI85Ht0G0AIUx7hdSRGZ6X/EetbQL0mb3tW7FgRpsDnfAFTZwQ M7ow39UIAVUEbuCIJ1a1bLpEnvZMHNIqnucYtLrgL/JyasM3HGX3pXCAM95sbN3sQ+wK 7Ve4j1amwsptmnCPoX2l3LmoCDu3G4UuqkLMhcjCar3EZj91wTmJRKf881/aHD7FTvmD cah27faWtotBDRqTu9E9Ej+dU7yjiqiWn0N52lZZ9Wm/Yr9VgJuT/wDOfXzh1MeAwaM8 T4bQ== X-Gm-Message-State: AN3rC/4ergTjIVGxniAkn2mN1A57gKLjhVgmk5R/xq2+nAd8zJElEfEq zrgtK0l3ullAOg== X-Received: by 10.55.17.82 with SMTP id b79mr10513525qkh.18.1492818643879; Fri, 21 Apr 2017 16:50:43 -0700 (PDT) Received: from localhost.localdomain (75-166-101-229.hlrn.qwest.net. [75.166.101.229]) by smtp.gmail.com with ESMTPSA id 22sm5785108qkv.52.2017.04.21.16.50.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 21 Apr 2017 16:50:43 -0700 (PDT) Subject: Re: [PATCH] Implement a warning for bogus sizeof(pointer) / sizeof(pointer[0]) To: Bernd Edlinger , "gcc-patches@gcc.gnu.org" , Joseph Myers , Jason Merrill , Jeff Law , Richard Biener , Jakub Jelinek References: From: Martin Sebor Message-ID: <776bb206-7b8e-1878-5411-3f1cdaabac05@gmail.com> Date: Sat, 22 Apr 2017 06:51:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00971.txt.bz2 On 04/20/2017 02:35 PM, Bernd Edlinger wrote: > Hi! > > > This implements a new -Wall enabled warning for a rather common, but > completely wrong way to compute an array size by dividing the > sizeof(pointer) / sizeof(pointer[0]) or sizeof(*pointer). > > It is often hard to find this kind of error by simple code inspection > in real code, because using sizeof in this way is a quite common idiom > to get the array size of an array variable. And furthermore this > expression may be used in macros, which makes it even more important to > have this warning. > > There is a similar warning -Wsizeof-pointer-memaccess which helped in > implementing the infrastructure for the new warning in the C FE. > > However I noticed that the -Wsizeof-pointer-memaccess warning was > missing in C, when the sizeof is used inside parentheses, which is > different from C++, so I fixed that too. > > Of course, I added some test cases for that as well. > > To illustrate the usefulness of this warning, it revealed quite a few > places where bogus sizeof divisions were used in our testsuite. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? That seems like a useful warning. Just a few comments. First, -Wsizeof-array-argument already diagnoses a subset of the same problems. For example, with the patch applied, GCC issues the two warnings below for following test case. One should be sufficient. $ cat y.c && gcc -S -Wall y.c int f (int a[]) { return sizeof a / sizeof *a; } y.c: In function ‘f’: y.c:3:17: warning: ‘sizeof’ on array function parameter ‘a’ will return size of ‘int *’ [-Wsizeof-array-argument] return sizeof a / sizeof *a; ^ y.c:1:12: note: declared here int f (int a[]) ^ y.c:3:19: warning: dividing the pointer size by the element size [-Wsizeof-pointer-div] return sizeof a / sizeof *a; ^ Second, I would suggest mentioning the actual types of the operands rather than referring to "pointer size" and "element size." Maybe something like: division 'sizeof (int*) / sizeof (int)' does not compute the number of array elements I suggest avoiding "element size" because the pointed-to argument need not be an array. Mentioning the types should help users better understand the problem (especially in C++ where types are often obscured by layers of templates). It might also be a nice touch to add a note pointing to the declaration of the first sizeof operand (if it's an object). Martin