From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9885 invoked by alias); 2 Jul 2019 20:59:29 -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 9877 invoked by uid 89); 2 Jul 2019 20:59:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=blessing, settled, So 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 ESMTP; Tue, 02 Jul 2019 20:59:28 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4B1EFC00C7DD; Tue, 2 Jul 2019 20:59:22 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-19.rdu2.redhat.com [10.10.112.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id A7DA0414C; Tue, 2 Jul 2019 20:59:21 +0000 (UTC) Subject: Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549) To: Martin Sebor , 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: Jeff Law Openpgp: preference=signencrypt Message-ID: Date: Tue, 02 Jul 2019 20:59:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-07/txt/msg00185.txt.bz2 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. jeff