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 [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 215FE3971C38 for ; Thu, 25 Feb 2021 23:16:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 215FE3971C38 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-24-DOcQ4dn9Nt2bFYLG_ttUGw-1; Thu, 25 Feb 2021 18:15:58 -0500 X-MC-Unique: DOcQ4dn9Nt2bFYLG_ttUGw-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id A3A561936B65; Thu, 25 Feb 2021 23:15:57 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-98.phx2.redhat.com [10.3.112.98]) by smtp.corp.redhat.com (Postfix) with ESMTP id 61E2250C0D; Thu, 25 Feb 2021 23:15:57 +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> From: Jeff Law Message-ID: <87e9e0b9-7959-d418-112c-748f0903466c@redhat.com> Date: Thu, 25 Feb 2021 16:15:56 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <042e2b8f-3031-519d-4a2f-1cb33e2004a7@gmail.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 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.5 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_H4, 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: Thu, 25 Feb 2021 23:16:04 -0000 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 it's a related, but not necessary exactly the same as pr97595 where we do have the discrepancy between DECL_SIZE and TYPE_SIZE. I think Jason's comment in pr97595 is the key here.  For a base subject with virtual bases the offset is bounded by the most-derived object size -- which runs a bit counter to some of the concepts we rely on. So I think that raises two key questions. First, can we detect when we've got an access to a base subobject with virtual bases.  I think that's what you're trying to do with the DECL_ARTIFICIAL check.  I don't know offhand if that's the best way to detect these kinds of accesses or not.  That's probably the most important question here, particularly for gcc-11. Second, if we can detect those kinds of accesses, do we have any way to get the most derived object size?  If so, obviously we'd want to use it (gcc-12 perhaps), if not, we'd punt like your patch does. Jeff