Message ID | o6n87219-90s3-758q-88op-s03oq16qsn57@fhfr.qr |
---|---|
State | Committed |
Commit | b2cd32b743ba440e75505ce30c6b5c592ed144ea |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 81DBB3858404 for <patchwork@sourceware.org>; Wed, 10 Nov 2021 10:10:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 81DBB3858404 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636539019; bh=87Z8HuMKsi5U3+YkKyPmEYM/0yV942MGna/GdmrEoIU=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=A6NNl+XIOdzAbo9acqr+JJVn5xGVgTslHc2kRm6PxGp0uS72JXul74raqgraTsn16 YjQdUyaLXtelmHn/6xHxVvWGmNdtA2KFm6KsJY6wYwq0D8PSgiRl29ntJRs8BHTY61 SkPmDIQQDTO8ZsQYA8vc76RLa3gEe0mWjg5lxItg= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id BF58E3858404 for <gcc-patches@gcc.gnu.org>; Wed, 10 Nov 2021 10:09:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BF58E3858404 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id B251D212B7 for <gcc-patches@gcc.gnu.org>; Wed, 10 Nov 2021 10:09:48 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 9C94B13B7F for <gcc-patches@gcc.gnu.org>; Wed, 10 Nov 2021 10:09:48 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id pvfpJGyai2GSLAAAMHmgww (envelope-from <rguenther@suse.de>) for <gcc-patches@gcc.gnu.org>; Wed, 10 Nov 2021 10:09:48 +0000 Date: Wed, 10 Nov 2021 11:09:48 +0100 (CET) To: gcc-patches@gcc.gnu.org Subject: [PATCH] testsuite/102690 - XFAIL g++.dg/warn/Warray-bounds-16.C Message-ID: <o6n87219-90s3-758q-88op-s03oq16qsn57@fhfr.qr> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Richard Biener <rguenther@suse.de> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
testsuite/102690 - XFAIL g++.dg/warn/Warray-bounds-16.C
|
|
Commit Message
Richard Biener
Nov. 10, 2021, 10:09 a.m. UTC
This XFAILs the bogus diagnostic test and rectifies the expectation on the optimization. Tested on x86_64-unknown-linux-gnu, pushed. 2021-11-10 Richard Biener <rguenther@suse.de> PR testsuite/102690 * g++.dg/warn/Warray-bounds-16.C: XFAIL diagnostic part and optimization. --- gcc/testsuite/g++.dg/warn/Warray-bounds-16.C | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Comments
On 11/10/21 3:09 AM, Richard Biener via Gcc-patches wrote: > This XFAILs the bogus diagnostic test and rectifies the expectation > on the optimization. > > Tested on x86_64-unknown-linux-gnu, pushed. > > 2021-11-10 Richard Biener <rguenther@suse.de> > > PR testsuite/102690 > * g++.dg/warn/Warray-bounds-16.C: XFAIL diagnostic part > and optimization. > --- > gcc/testsuite/g++.dg/warn/Warray-bounds-16.C | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > index 17b4d0d194e..89cbadb91c7 100644 > --- a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > @@ -19,11 +19,11 @@ struct S > p = (int*) new unsigned char [sizeof (int) * m]; > > for (int i = 0; i < m; i++) > - new (p + i) int (); > + new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* } } */ > } > }; > > S a (0); > > -/* Verify the loop has been eliminated. > - { dg-final { scan-tree-dump-not "goto" "optimized" } } */ > +/* The loop cannot be eliminated since the global 'new' can change 'm'. */ I don't understand this comment. Can you please explain how the global operator new (i.e., the one outside the loop below) can change the member of the class whose ctor calls the new? The member, or more precisely the enclosing object, doesn't yet exist at the time the global new is called because its ctor hasn't finished, so nothing outside the ctor can access it. A pointer to the S under construction can be used (and could be accessed by a replacement new) but it cannot be dereferenced to access its members because the object it points to doesn't exist until after the ctor completes. I copy the test below: inline void* operator new (__SIZE_TYPE__, void * v) { return v; } struct S { int* p; int m; S (int i) { m = i; p = (int*) new unsigned char [sizeof (int) * m]; for (int i = 0; i < m; i++) new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* } } */ } }; S a (0); Thanks Martin
On Wed, 10 Nov 2021, Martin Sebor wrote: > On 11/10/21 3:09 AM, Richard Biener via Gcc-patches wrote: > > This XFAILs the bogus diagnostic test and rectifies the expectation > > on the optimization. > > > > Tested on x86_64-unknown-linux-gnu, pushed. > > > > 2021-11-10 Richard Biener <rguenther@suse.de> > > > > PR testsuite/102690 > > * g++.dg/warn/Warray-bounds-16.C: XFAIL diagnostic part > > and optimization. > > --- > > gcc/testsuite/g++.dg/warn/Warray-bounds-16.C | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > > b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > > index 17b4d0d194e..89cbadb91c7 100644 > > --- a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > > +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > > @@ -19,11 +19,11 @@ struct S > > p = (int*) new unsigned char [sizeof (int) * m]; > > > > for (int i = 0; i < m; i++) > > - new (p + i) int (); > > + new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* } > > } */ > > } > > }; > > > > S a (0); > > > > -/* Verify the loop has been eliminated. > > - { dg-final { scan-tree-dump-not "goto" "optimized" } } */ > > +/* The loop cannot be eliminated since the global 'new' can change 'm'. */ > > I don't understand this comment. Can you please explain how > the global operator new (i.e., the one outside the loop below) > can change the member of the class whose ctor calls the new? > > The member, or more precisely the enclosing object, doesn't > yet exist at the time the global new is called because its > ctor hasn't finished, so nothing outside the ctor can access > it. A pointer to the S under construction can be used (and > could be accessed by a replacement new) but it cannot be > dereferenced to access its members because the object it > points to doesn't exist until after the ctor completes. Yes, that's the C++ legalise - which is why I XFAILed that part of the test rather than just removed it. The middle-end sees the object *this as existing and being global, thus accessible and mutable by '::new' which when replaced by the user could access and alter *this. Like maybe for S s; void *operator new(..) { s.m = 0; } main() { new (&s) (1); } that may be invalid C++ but this detail of C++ is not reflected in the GIMPLE IL. Before the change that regressed this if S::S() would call a global function foo() instead of new to do the allocation the behavior would be as after the change. Isn't the call to new or foo part of the construction and as such obviously allowed to access and alter the in-construction object? > I copy the test below: > > inline void* operator new (__SIZE_TYPE__, void * v) > { > return v; > } > > struct S > { > int* p; > int m; > > S (int i) > { > m = i; > p = (int*) new unsigned char [sizeof (int) * m]; > > for (int i = 0; i < m; i++) > new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* } } > */ > } > }; > > S a (0); > > Thanks > Martin >
On 11/11/21 1:18 AM, Richard Biener wrote: > On Wed, 10 Nov 2021, Martin Sebor wrote: > >> On 11/10/21 3:09 AM, Richard Biener via Gcc-patches wrote: >>> This XFAILs the bogus diagnostic test and rectifies the expectation >>> on the optimization. >>> >>> Tested on x86_64-unknown-linux-gnu, pushed. >>> >>> 2021-11-10 Richard Biener <rguenther@suse.de> >>> >>> PR testsuite/102690 >>> * g++.dg/warn/Warray-bounds-16.C: XFAIL diagnostic part >>> and optimization. >>> --- >>> gcc/testsuite/g++.dg/warn/Warray-bounds-16.C | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C >>> b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C >>> index 17b4d0d194e..89cbadb91c7 100644 >>> --- a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C >>> +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C >>> @@ -19,11 +19,11 @@ struct S >>> p = (int*) new unsigned char [sizeof (int) * m]; >>> >>> for (int i = 0; i < m; i++) >>> - new (p + i) int (); >>> + new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* } >>> } */ >>> } >>> }; >>> >>> S a (0); >>> >>> -/* Verify the loop has been eliminated. >>> - { dg-final { scan-tree-dump-not "goto" "optimized" } } */ >>> +/* The loop cannot be eliminated since the global 'new' can change 'm'. */ >> >> I don't understand this comment. Can you please explain how >> the global operator new (i.e., the one outside the loop below) >> can change the member of the class whose ctor calls the new? >> >> The member, or more precisely the enclosing object, doesn't >> yet exist at the time the global new is called because its >> ctor hasn't finished, so nothing outside the ctor can access >> it. A pointer to the S under construction can be used (and >> could be accessed by a replacement new) but it cannot be >> dereferenced to access its members because the object it >> points to doesn't exist until after the ctor completes. > > Yes, that's the C++ legalise - which is why I XFAILed that > part of the test rather than just removed it. The middle-end > sees the object *this as existing and being global, thus > accessible and mutable by '::new' which when replaced by > the user could access and alter *this. Like maybe for > > S s; > > void *operator new(..) { s.m = 0; } > > main() > { > new (&s) (1); > } > > that may be invalid C++ but this detail of C++ is not > reflected in the GIMPLE IL. Before the change that regressed > this if S::S() would call a global function foo() instead > of new to do the allocation the behavior would be as after > the change. Isn't the call to new or foo part of the > construction and as such obviously allowed to access > and alter the in-construction object? Here's my understanding. The lifetime of an object ends when its storage is reused to create another object of a class type, and the lifetime of the other object begins when its initialization is complete. In the window between the new ctor starting and completing the new object can be accessed in limited ways, but not the old object. (If it were otherwise, imagine the new object's ctor throwing in the middle of constructing the new object. What state would that leave the old object in? Which members could still be accessed?) The only way to access a subobject of the new object while it's still under construction is through a pointer to that subobject or through this (or a pointer derive from this). It seems that it should be possible to capture the constraint in the middle end that no member of an object under construction can be accessed unless a pointer to it has escaped that's derived from the this pointer. But this seems like a sufficiently obscure case that an expert on the C++ object model should confirm it. Martin > >> I copy the test below: >> >> inline void* operator new (__SIZE_TYPE__, void * v) >> { >> return v; >> } >> >> struct S >> { >> int* p; >> int m; >> >> S (int i) >> { >> m = i; >> p = (int*) new unsigned char [sizeof (int) * m]; >> >> for (int i = 0; i < m; i++) >> new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* } } >> */ >> } >> }; >> >> S a (0); >> >> Thanks >> Martin >> >
On Thu, 11 Nov 2021, Martin Sebor wrote: > On 11/11/21 1:18 AM, Richard Biener wrote: > > On Wed, 10 Nov 2021, Martin Sebor wrote: > > > >> On 11/10/21 3:09 AM, Richard Biener via Gcc-patches wrote: > >>> This XFAILs the bogus diagnostic test and rectifies the expectation > >>> on the optimization. > >>> > >>> Tested on x86_64-unknown-linux-gnu, pushed. > >>> > >>> 2021-11-10 Richard Biener <rguenther@suse.de> > >>> > >>> PR testsuite/102690 > >>> * g++.dg/warn/Warray-bounds-16.C: XFAIL diagnostic part > >>> and optimization. > >>> --- > >>> gcc/testsuite/g++.dg/warn/Warray-bounds-16.C | 6 +++--- > >>> 1 file changed, 3 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > >>> b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > >>> index 17b4d0d194e..89cbadb91c7 100644 > >>> --- a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > >>> +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C > >>> @@ -19,11 +19,11 @@ struct S > >>> p = (int*) new unsigned char [sizeof (int) * m]; > >>> > >>> for (int i = 0; i < m; i++) > >>> - new (p + i) int (); > >>> + new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* > >>> } > >>> } */ > >>> } > >>> }; > >>> > >>> S a (0); > >>> > >>> -/* Verify the loop has been eliminated. > >>> - { dg-final { scan-tree-dump-not "goto" "optimized" } } */ > >>> +/* The loop cannot be eliminated since the global 'new' can change 'm'. > >>> */ > >> > >> I don't understand this comment. Can you please explain how > >> the global operator new (i.e., the one outside the loop below) > >> can change the member of the class whose ctor calls the new? > >> > >> The member, or more precisely the enclosing object, doesn't > >> yet exist at the time the global new is called because its > >> ctor hasn't finished, so nothing outside the ctor can access > >> it. A pointer to the S under construction can be used (and > >> could be accessed by a replacement new) but it cannot be > >> dereferenced to access its members because the object it > >> points to doesn't exist until after the ctor completes. > > > > Yes, that's the C++ legalise - which is why I XFAILed that > > part of the test rather than just removed it. The middle-end > > sees the object *this as existing and being global, thus > > accessible and mutable by '::new' which when replaced by > > the user could access and alter *this. Like maybe for > > > > S s; > > > > void *operator new(..) { s.m = 0; } > > > > main() > > { > > new (&s) (1); > > } > > > > that may be invalid C++ but this detail of C++ is not > > reflected in the GIMPLE IL. Before the change that regressed > > this if S::S() would call a global function foo() instead > > of new to do the allocation the behavior would be as after > > the change. Isn't the call to new or foo part of the > > construction and as such obviously allowed to access > > and alter the in-construction object? > > Here's my understanding. > > The lifetime of an object ends when its storage is reused to > create another object of a class type, and the lifetime of > the other object begins when its initialization is complete. > > In the window between the new ctor starting and completing > the new object can be accessed in limited ways, but not > the old object. (If it were otherwise, imagine the new > object's ctor throwing in the middle of constructing the new > object. What state would that leave the old object in? Which > members could still be accessed?) > > The only way to access a subobject of the new object while > it's still under construction is through a pointer to that > subobject or through this (or a pointer derive from this). OK, so that might be the detail that one could exploit eventually. > It seems that it should be possible to capture the constraint > in the middle end that no member of an object under construction > can be accessed unless a pointer to it has escaped that's derived > from the this pointer. IIRC at some point we had this * restrict qualified in the CTOR but that broke cases where that's clearly not correct (but I forgot the details). Note for the middle-end that would just point at another missed optimization, namely that restrict does not work to disambiguate against calls. > But this seems like a sufficiently obscure case that an expert > on the C++ object model should confirm it. > > Martin > > > > >> I copy the test below: > >> > >> inline void* operator new (__SIZE_TYPE__, void * v) > >> { > >> return v; > >> } > >> > >> struct S > >> { > >> int* p; > >> int m; > >> > >> S (int i) > >> { > >> m = i; > >> p = (int*) new unsigned char [sizeof (int) * m]; > >> > >> for (int i = 0; i < m; i++) > >> new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* > >> } } > >> */ > >> } > >> }; > >> > >> S a (0); > >> > >> Thanks > >> Martin > >> > > > > >
diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C index 17b4d0d194e..89cbadb91c7 100644 --- a/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-16.C @@ -19,11 +19,11 @@ struct S p = (int*) new unsigned char [sizeof (int) * m]; for (int i = 0; i < m; i++) - new (p + i) int (); + new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* } } */ } }; S a (0); -/* Verify the loop has been eliminated. - { dg-final { scan-tree-dump-not "goto" "optimized" } } */ +/* The loop cannot be eliminated since the global 'new' can change 'm'. */ +/* { dg-final { scan-tree-dump-not "goto" "optimized" { xfail *-*-* } } } */