From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9536 invoked by alias); 29 Nov 2017 15:56:52 -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 9527 invoked by uid 89); 29 Nov 2017 15:56:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-ot0-f194.google.com Received: from mail-ot0-f194.google.com (HELO mail-ot0-f194.google.com) (74.125.82.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 Nov 2017 15:56:49 +0000 Received: by mail-ot0-f194.google.com with SMTP id s4so3430190ote.4 for ; Wed, 29 Nov 2017 07:56:49 -0800 (PST) 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:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=3BAtFLtFh7Sgl0/h4a0pyEvBlX7THNtq5xdnGsVeuhg=; b=QbAgXSzLez1mpMtqfOLXe/mGzT/LiUt/NHmyB7QhWSGZsljnVH+qDQdTPM2/E5rwHQ H1XXawiFyI3JLpgAlR7njwF2wFBEezfYNDfp+d4Vr8bWAuMCvDVzQomjMvAiVeOW1Zc1 IodbFvYK4HDwDd8U0Vk8v6kBvTa0KKzh3pR0lzKZP3nqSS7lzz37FGVPlODybT+5bZRd J4LVee7s9gDD7PS4bQS0Q2yBWSPeL5K5XGEQtwW2S+xOat/iy3QZZhppw5iUN7bAsv7L l49pqYgnKIB9lAfR5zKUczkcbggFYzTusJdJwE49MrJ34zHmgDKWhrUAvRVbAJDz6Jza Df7g== X-Gm-Message-State: AJaThX5PTSUH0chC+emGFfNI/9LYg2gW4zFXy2JlbOm2YdpU0RdT7sWj CD0vxk6PNicDrHwHh0lCIslBUA== X-Google-Smtp-Source: AGs4zMYh/ARHZEPve0SBWztPjAN559cfFnxqC7P0BpREMqIr4BlrgE9RtqeosNWHkgh4GeFEhOQ8Lg== X-Received: by 10.157.6.162 with SMTP id 31mr2418267otx.343.1511971007338; Wed, 29 Nov 2017 07:56:47 -0800 (PST) Received: from localhost.localdomain (75-171-240-43.hlrn.qwest.net. [75.171.240.43]) by smtp.gmail.com with ESMTPSA id l3sm783417oif.49.2017.11.29.07.56.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 29 Nov 2017 07:56:46 -0800 (PST) Subject: Re: [RFC][PATCH] Extend DCE to remove unnecessary new/delete-pairs To: Jakub Jelinek References: <8305B5F4-2A96-4698-8C2E-3255658B5C12@theobroma-systems.com> <20171122103742.GN14653@tucnak> <20171129083045.GX2353@tucnak> Cc: =?UTF-8?Q?Dominik_Inf=c3=bchr?= , Richard Biener , GCC Patches From: Martin Sebor Message-ID: Date: Wed, 29 Nov 2017 16:29: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: <20171129083045.GX2353@tucnak> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg02502.txt.bz2 On 11/29/2017 01:30 AM, Jakub Jelinek wrote: > On Tue, Nov 28, 2017 at 09:11:00PM -0700, Martin Sebor wrote: >> On 11/27/2017 02:22 AM, Dominik Inführ wrote: >>> Thanks for all the reviews! I’ve revised the patch, the operator_delete_flag is now stored in tree_decl_with_vis (there already seem to be some FUNCTION_DECL-flags in there). I’ve also added the option -fallocation-dce to disable this optimization. It bootstraps and no regressions on aarch64 and x86_64. >>> >> It's great to be able to eliminate pairs of these calls. For >> unpaired calls, though, I think it would be even more useful to >> also issue a warning. Otherwise the elimination will mask bugs > > ?? I hope you're only talking about allocation where the returned > pointer can't leak elsewhere, doing allocation in one function > (e.g. constructor, or whatever other function) and deallocation in some > other one is so common such a warning would be not just useless, but > harmful with almost all occurrences being false positives. > > Warning on malloc/standard operator new or malloc/realloc-like function > when the return pointer can't escape the current function is reasonable. Yes, warn for leaks, or for calls to delete/free with no matching new/malloc (when they can be detected). From the test case included in the patch, warn on the first two of the following three functions: +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C @@ -0,0 +1,65 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-cddce-details" } */ + +#include + +void +new_without_use() { + int *x = new int; +} + +void +new_array_without_use() { + int *x = new int[5]; +} + +void +new_primitive() { + int *x = new int; + delete x; +} An obvious extension to such a checker would then be to also detect possible invalid deallocations, as in: void f (unsigned n) { void *p = n < 256 ? alloca (n) : malloc (n); // ... free (p); } David Malcolm was working on something like that earlier this year so he might have some thoughts on this as well. Martin