From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128361 invoked by alias); 11 Jul 2019 02:04:32 -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 128338 invoked by uid 89); 11 Jul 2019 02:04:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-6.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=Our=c2, corruption, discourage, If=c2?= X-HELO: mail-qt1-f193.google.com Received: from mail-qt1-f193.google.com (HELO mail-qt1-f193.google.com) (209.85.160.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 11 Jul 2019 02:04:30 +0000 Received: by mail-qt1-f193.google.com with SMTP id a15so4688616qtn.7 for ; Wed, 10 Jul 2019 19:04:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=040TWL+HySZD18hTh/qKsmxtPMd+f7mubUvT4e/I8cc=; b=VJWdU/urn2TWPqyqInuJXubpS3iiofi+JKqZbfHIPrnbKGUBeAlqpcoElyXhxax2Nc Yku3BXcETH+ukNjjaSL462q+42K+E+sLpxE3IIu0J9nfASq1FvaV0a78Y4OoMw6vOuOV eAtBeehqAyBxg65Qe6Jv/9HXqaw0E6T58MmE7uMKBdo6fwtayGj+YXFiyKlftHrbM5jO yEEG8Pj4r+8MPysuiMyIXmSy8yDnC6lH0RC5z15BePtsAYX4dXlbe8hNXLo99N03Wm2Z /JclrqkLLL2i275/m0QgVefe71CSCIjHkKccOdtLeIeGo1eMVddCjKcYJv8n8TpyEWk/ WUhg== Return-Path: Received: from [192.168.0.41] (174-16-116-9.hlrn.qwest.net. [174.16.116.9]) by smtp.gmail.com with ESMTPSA id x8sm2061825qka.106.2019.07.10.19.04.26 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Wed, 10 Jul 2019 19:04:27 -0700 (PDT) Subject: Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549) To: Jeff Law , gcc-patches References: <0b2e38e1-1fc7-f21e-0f26-596b50304e6e@gmail.com> <224f8161-e370-bcbc-3ee6-5cff5835e016@redhat.com> <87e654d9-6159-3467-d258-a2596f8d2b09@gmail.com> <0d4c1fb4-f9e7-7127-bf4c-051e0b7f3f5c@redhat.com> From: Martin Sebor Message-ID: <48fff7b4-5e43-f0c7-92b3-a0b0d436a772@gmail.com> Date: Thu, 11 Jul 2019 06:45:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 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: 2019-07/txt/msg00843.txt.bz2 On 7/2/19 2:59 PM, Jeff Law wrote: > On 6/30/19 3:50 PM, Martin Sebor wrote: >> On 6/26/19 6:11 PM, Jeff Law wrote: > [ Another big snip ] >> >>> Is there a strong need for an overloaded operator?  Our guidelines >>> generally discourage operator overloads.  This one seems on the border >>> to me.  Others may have different ideas where the line is.  If we really >>> don't need the overloaded operator, then we can avoid the controversy >>> completely. >> >> The copy assignment operator is necessary for the type to be stored >> in a hash_map.  Otherwise, the implicitly generated assignment will >> be used instead which is unsafe in vec and results in memory >> corription (I opened bug 90904 for this).  After working around this >> bug by providing the assignment operator I then ran into the hash_map >> bug 90959 that also results in memory corruption.  I spent a few fun >> hours tracking down the GCC miscompilations that they led to. > OK. THanks for explaining why we need the copy assignment operator. > > >> >> The golden rule in C++ is that every type should either be safe to >> copy and assign with the expected semantics or made not assignable >> and not copyable by declaring the copy ctor and copy assignment >> operator private (or deleted in more modern C++).  It would be very >> helpful to detect this kind of problem (classes that aren't safely >> assignable and copyable because they acquire/release resources in >> ctors/dtors).  Not just to us in GCC but I'm sure to others as well. >> It's something I've been hoping to implement for a while now. > You've got my blessing to do that :-) > >>> >>> So how do you deal with the case where one operand of a >>> {MIN,MAX,COND}_EXPR is the address of a local, but the other is not?  It >>> seems like we'll end up returning true from this function in that case >>> and potentially trigger changing the return value to NULL.  Or am I >>> missing something? >> >> I only briefly considered MIN/MAX but because in C and C++ the less >> than and greater than expressions are only defined for pointers into >> the same array/object or subobject and I didn't ponder it too hard. >> (In C they're undefined otherwise, in C++ the result is unspecified.) >> >> But you bring it up because you think the address should be returned >> regardless, even if the expression is invalid (or if it's COND_EXPR >> with mixed local/nonlocal addresses) so I've changed it to do that >> for all these explicitly handled codes and added tests to verify it. > THanks. > > So in the thread for strlen/sprintf the issue around unbounded walks up > through PHI argument's SSA_NAME_DEF_STMT was raised. > > I think we've got two options here. > > One would be to hold this patch until we sort out what the bounds should > look like, then adjust this patch to do something similar. > > Or go forward with this patch, then come back and adjust the walker once > we have settled on solution in the other thread. > > I'm OK with either. Your choice. I committed the patch as is until the question of which algorithms to put the limit on is answered (as discussed in the thread Re: sprintf/strlen integration). Martin