From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 11637385702F for ; Fri, 25 Sep 2020 20:05:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 11637385702F Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-558-1GP0YR0LOl23bC9fiqyGXA-1; Fri, 25 Sep 2020 16:05:01 -0400 X-MC-Unique: 1GP0YR0LOl23bC9fiqyGXA-1 Received: by mail-qk1-f197.google.com with SMTP id 139so2866543qkl.11 for ; Fri, 25 Sep 2020 13:05:01 -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:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Qtmao/JvHhlYAWaD5isZJLmvPEJpxAk9X2Q53i1O8zA=; b=QNwLx49AU/FnSPC+3tWogDVkDK16QuiQJa+C1PHLVJmohVbGq3p98srws9vZEqQRRx M4tkUXeUAE2wJfuBN9bj8jrLltkPKwhs8u0Dh5Nl8/IaoFNUSbel2G/TYY4+pvX49soY 3QeyWikJG3MVB0aM/3eTbmdgvKiQJaYXDlCZNPy1XBo/ziEXW+h+dLteFsBVO1X+tx+0 aHu1bOpFZpX6ry0KPifOAC27LGFiMYIQ7hBvYguh0plqh/h1ENsEDpRID6euzoF+7Krk BNo42FM2NuQXg+P8/dnTkZXfsw5zq2BQnGDIX2g0DfVVSDeOBukizbYpFesTFATuGcyt bYIg== X-Gm-Message-State: AOAM531xS8/GiIHVZE+hTSwGTzcF5mHZr8/L43FoTc1NAuscqExpk5i6 OT6shJUjAQ2vTNiPx0Kn1nf7DkRmiedaKe+LoxOVw3Gqjllyd/3cCgL7a40w5oOP24M+8H4PqlW hDk+rMucgHEKhJIIAKw== X-Received: by 2002:a37:4711:: with SMTP id u17mr1724983qka.54.1601064300315; Fri, 25 Sep 2020 13:05:00 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxzKCQFLfqHRgwcS2wB+QytzQ3udafa+qfQfaVnhaTGemxe6QuvL3iRkP6AiGigjGlbrE85gw== X-Received: by 2002:a37:4711:: with SMTP id u17mr1724953qka.54.1601064299886; Fri, 25 Sep 2020 13:04:59 -0700 (PDT) Received: from [192.168.1.148] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id j25sm2597557qtr.83.2020.09.25.13.04.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 25 Sep 2020 13:04:58 -0700 (PDT) Subject: Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete To: Richard Biener Cc: gcc-patches@gcc.gnu.org, =?UTF-8?Q?Martin_Li=c5=a1ka?= , Jakub Jelinek , Jonathan Wakely References: <5878a705-6101-d7ac-fe11-13664a53b220@redhat.com> <0A92F1D5-5521-4F1A-B297-6637479FB278@suse.de> <5e4be9c0-3781-c983-ce05-09763bac67e5@redhat.com> <85ce021c-5730-b6e9-c7ad-3d9ae9afcba2@redhat.com> From: Jason Merrill Message-ID: Date: Fri, 25 Sep 2020 16:04:58 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Sep 2020 20:05:06 -0000 On 9/25/20 2:30 AM, Richard Biener wrote: > On Thu, 24 Sep 2020, Jason Merrill wrote: > >> On 9/24/20 3:43 AM, Richard Biener wrote: >>> On Wed, 23 Sep 2020, Jason Merrill wrote: >>> >>>> On 9/23/20 2:42 PM, Richard Biener wrote: >>>>> On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill >>>>> >>>>> wrote: >>>>>> On 9/23/20 4:14 AM, Richard Biener wrote: >>>>>>> C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P, >>>>>>> does not cause the deleted object to be escaped. It also has no >>>>>>> other interesting side-effects for PTA so skip it like we do >>>>>>> for BUILT_IN_FREE. >>>>>> >>>>>> Hmm, this is true of the default implementation, but since the function >>>>>> >>>>>> is replaceable, we don't know what a user definition might do with the >>>>>> pointer. >>>>> >>>>> But can the object still be 'used' after delete? Can delete fail / throw? >>>>> >>>>> What guarantee does the predicate give us? >>>> >>>> The deallocation function is called as part of a delete expression in order >>>> to >>>> release the storage for an object, ending its lifetime (if it was not ended >>>> by >>>> a destructor), so no, the object can't be used afterward. >>> >>> OK, but the delete operator can access the object contents if there >>> wasn't a destructor ... >> >>>> A deallocation function that throws has undefined behavior. >>> >>> OK, so it seems the 'replaceable' operators are the global ones >>> (for user-defined/class-specific placement variants I see arbitrary >>> extra arguments that we'd possibly need to handle). >>> >>> I'm happy to revert but I'd like to have a testcase that FAILs >>> with the patch ;) >>> >>> Now, the following aborts: >>> >>> struct X { >>> static struct X saved; >>> int *p; >>> X() { __builtin_memcpy (this, &saved, sizeof (X)); } >>> }; >>> void operator delete (void *p) >>> { >>> __builtin_memcpy (&X::saved, p, sizeof (X)); >>> } >>> int main() >>> { >>> int y = 1; >>> X *p = new X; >>> p->p = &y; >>> delete p; >>> X *q = new X; >>> *(q->p) = 2; >>> if (y != 2) >>> __builtin_abort (); >>> } >>> >>> and I could fix this by not making *p but what *p points to escape. >>> The testcase is of course maximally awkward, but hey ... ;) >>> >>> Now this would all be moot if operator delete may not access >>> the object (or if the object contents are undefined at that point). >>> >>> Oh, and the testcase segfaults when compiled with GCC 10 because >>> there we elide the new X / delete p pair ... which is invalid then? >>> Hmm, we emit >>> >>> MEM[(struct X *)_8] ={v} {CLOBBER}; >>> operator delete (_8, 8); >>> >>> so the object contents are undefined _before_ calling delete >>> even when I do not have a DTOR? That is, the above, >>> w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase. >> >> Yes, all classes have a destructor, even if it's trivial, so the object's >> lifetime definitely ends before the call to operator delete. This is less >> clear for scalar objects, but treating them similarly would be consistent with >> other recent changes, so I think it's fine for us to assume that scalar >> objects are also invalidated before the call to operator delete. But of >> course this doesn't apply to explicit calls to operator delete outside of a >> delete expression. > > OK, so change the testcase main slightly to > > int main() > { > int y = 1; > X *p = new X; > p->p = &y; > ::operator delete(p); > X *q = new X; > *(q->p) = 2; > if (y != 2) > __builtin_abort (); > } > > in this case the lifetime of *p does not end before calling > ::operator delete() and delete can stash the object contents > somewhere before ending its lifetime. For the very same reason > we may not elide a new/delete pair like in > > int main() > { > int *p = new int; > *p = 1; > ::operator delete (p); > } Correct; the permission to elide new/delete pairs are for the expressions, not the functions. > which we before the change did not do only because calling > operator delete made p escape. Unfortunately points-to analysis > cannot really reconstruct whether delete was called as part of > a delete expression or directly (and thus whether object lifetime > ended already), neither can DCE. So I guess we need to mark > the operator delete call in some way to make those transforms > safe. At least currently any operator delete call makes the > alias guarantee of a operator new call moot by forcing the object > to be aliased with all global and escaped memory ... > > Looks like there are some unallocated flags for CALL_EXPR we could > pick but I wonder if we can recycle protected_flag which is > > CALL_FROM_THUNK_P and > CALL_ALLOCA_FOR_VAR_P in > CALL_EXPR > > for calls to DECL_IS_OPERATOR_{NEW,DELETE}_P, thus whether > we have CALL_FROM_THUNK_P for those operators. Guess picking > a new flag is safer. We won't ever call those operators from a thunk, so it should be OK to reuse it. > But, does it seem correct that we need to distinguish > delete expressions from plain calls to operator delete? A reason for that distinction came up in the context of omitting new/delete pairs: we want to consider the operator first called by the new or delete expression, not a call from that first operator to another operator new/delete and exposed by inlining. https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543404.html > In this context I also wonder about non-replaceable operator delete, > specifically operator delete in classes - are there any semantic > differences between those or why did we choose to only mark > the replaceable ones? The standard says that for omitting a 'new' allocation, the operator new has to be a replaceable one, but does not say the same about 'delete'; it just says that if the allocation was omitted, the delete-expression does not call a deallocation function. It may not be necessary to make this distinction for delete. And this distinction could be local to the front end. In the front end, we currently have cxx_replaceable_global_alloc_fn that already ignores the replaceability of operator delete. And we have CALL_FROM_NEW_OR_DELETE_P, that would just need to move into the middle end. And perhaps get renamed to CALL_OMITTABLE_NEW_OR_DELETE_P, and not get set for calls to non-replaceable operator new. Jason