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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id E31BD385782B for ; Mon, 8 Feb 2021 23:11:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E31BD385782B Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-397-18PSHsJHOI2NP_CslfYvYg-1; Mon, 08 Feb 2021 18:11:35 -0500 X-MC-Unique: 18PSHsJHOI2NP_CslfYvYg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 860FE8030BB; Mon, 8 Feb 2021 23:11:34 +0000 (UTC) Received: from localhost.localdomain (ovpn-114-43.phx2.redhat.com [10.3.114.43]) by smtp.corp.redhat.com (Postfix) with ESMTP id 56DCA1002393; Mon, 8 Feb 2021 23:11:34 +0000 (UTC) Subject: Re: [PATCH] avoid -Warray-bounds checks for vtable assignments (PR 98266) To: Martin Sebor , gcc-patches References: <7fa3c1e0-8a09-3436-8669-6d1a577ca247@gmail.com> <55235bab-1a5a-4e3b-3a57-55d9a84ae00c@redhat.com> <042e2b8f-3031-519d-4a2f-1cb33e2004a7@gmail.com> <354bf06f-ad62-7966-a5d5-e4931ed1d5ed@redhat.com> <355d1e90-bba2-ad97-3544-c8fda473cfa0@gmail.com> From: Jeff Law Message-ID: <571fbfd1-d0f1-c1ca-1eef-4e804d7f8088@redhat.com> Date: Mon, 8 Feb 2021 16:11:33 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <355d1e90-bba2-ad97-3544-c8fda473cfa0@gmail.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, 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: Mon, 08 Feb 2021 23:11:41 -0000 On 2/8/21 3:44 PM, Martin Sebor wrote: > On 2/8/21 3:26 PM, Jeff Law wrote: >> >> >> On 2/8/21 2:56 PM, Martin Sebor wrote: >>> On 2/8/21 12:59 PM, Jeff Law wrote: >>>> >>>> >>>> On 1/19/21 5:56 PM, Martin Sebor via Gcc-patches wrote: >>>>> Similar to the problem reported for -Wstringop-overflow in pr98266 >>>>> and already fixed, -Warray-bounds is also susceptible to false >>>>> positives in assignments and copies involving virtual inheritance. >>>>> Because the two warnings don't share code yet (hopefully in GCC 12) >>>>> the attached patch adds its own workaround for this problem to >>>>> gimple-array-bounds.cc, this one slightly more crude because of >>>>> the limited insight the array bounds checking has into the checked >>>>> expressions. >>>>> >>>>> Tested on x86_64-linux. >>>>> >>>>> Martin >>>>> >>>>> gcc-98266.diff >>>>> >>>>> PR middle-end/98266 - bogus array subscript is partly outside array >>>>> bounds on virtual inheritance >>>>> >>>>> gcc/ChangeLog: >>>>> >>>>>      PR middle-end/98266 >>>>>      * gimple-array-bounds.cc >>>>> (array_bounds_checker::check_array_bounds): >>>>>      Avoid checking references involving artificial members. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>>      PR middle-end/98266 >>>>>      * g++.dg/warn/Warray-bounds-15.C: New test. >>>> It seems to me that we've got the full statement at some point  and >>>> thus >>>> the full expression so at some point couldn't we detect when >>>> TYPE_SIZE_UNIT!= DECL_SIZE_UNIT?  Or should we be using TYPE_SIZE_UNIT >>>> rather than DECL_SIZE_UNIT in gimple-array-bounds.cc >>>> >>>> Am I missing something? >>> >>> The expression we're looking at when the false positive is issued >>> is the MEM_REF in the LHS of: >>> >>> MEM[(struct D *)&D.2652 + 24B]._vptr.D = &MEM [(void >>> *)&_ZTC1E24_1D + 24B]; >>> >>> TREE_TYPE(LHS) is D, DECL_SIZE_UNIT (D.2652) is 24, and >>> TYPE_SIZE_UNIT(D) is also 24, so there's no discrepancy between >>> DECL_SIZE and TYPE_SIZE. >> So that seems like it's a different issue then, unrelated to 97595.  >> Right? > > I think the underlying problem is the same.  We're getting a size > that doesn't correspond to what's actually being accessed, and it > happens because of the virtual inheritance.  In pr97595 Jason > suggested to use the decl/type size inequality to identify this > case but I think we could have just as well used DECL_ARTIFICIAL > instead.  At least the test cases from pr97595 both pass with > this change. But in the 98266 case the type and decl sizes are the same.  So to be true that would mean that the underlying type we're using to access memory differs from its actual type.  Is that the case in the IL?  And does this have wider implications for diagnostics or optimizations that rely on accurate type sizing? I'm just trying to make sure I understand, not accepting or rejecting the patch yet. jeff