From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x332.google.com (mail-ot1-x332.google.com [IPv6:2607:f8b0:4864:20::332]) by sourceware.org (Postfix) with ESMTPS id 123273855014 for ; Wed, 7 Jul 2021 20:38:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 123273855014 Received: by mail-ot1-x332.google.com with SMTP id t24-20020a9d7f980000b029046f4a1a5ec4so3593920otp.1 for ; Wed, 07 Jul 2021 13:38:14 -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=hoI4XA99RrRDNEhSshyzgp+agXasUk9IufW2NR2EhBc=; b=cAwuK+Z7V3UUI7waa8ommdR0rW62s4gU97oGgrFo6GuQvFkjIHgiQYQQ0RFIOmR40i 41wyBiupw96zdaOiV8ebDSeUPWU2YPhqezmDsGC2wFrn5AIPrm3ZqvaSyHr7AgWeD3pf +JQdwKihYF+2nQmDw14fSaGFv3QxQDcMgX2vOLcBEZJsIWLRcZxFwe6AgdnEZqSuhAOh 3Z9kTzNBILYXoV+N/j5i1QvzFtKG6uyyQ/9H8iysVtOcFHPDiyYjc8Jf8Hw3T3fR/VBL OtqYmeB+jwAZ8SPfBjKMZijHIKcnexZokATJtUD+SpQW3eejccRg6uN6kYwBh5Ll0eF/ vzLQ== X-Gm-Message-State: AOAM531WGW4pWP7i1pUvIcIi5er7GXzvnuws9D63tJlJ3PmcrwSkeHbq v5t6y1HIEke3VJaMYZVXRVAxBsl93mY= X-Google-Smtp-Source: ABdhPJwM02wQwDdgX+FgFYDI0bFoJWGxSR+54KRcJpuiLHoumXbG+DSVM0p2xu6pyi0vvCfmEPdmQg== X-Received: by 2002:a05:6830:2470:: with SMTP id x48mr3088688otr.81.1625690293295; Wed, 07 Jul 2021 13:38:13 -0700 (PDT) Received: from [192.168.0.41] (75-166-102-22.hlrn.qwest.net. [75.166.102.22]) by smtp.gmail.com with ESMTPSA id a206sm228351oob.31.2021.07.07.13.38.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 07 Jul 2021 13:38:12 -0700 (PDT) Subject: Re: PING 2 [PATCH] correct handling of variable offset minus constant in -Warray-bounds (PR 100137) To: Richard Biener Cc: gcc-patches References: <63b26859-944b-cfbf-64a3-e70c73c09727@gmail.com> From: Martin Sebor Message-ID: <91a3a6e6-7297-d7e2-5df2-e25fc5d18fac@gmail.com> Date: Wed, 7 Jul 2021 14:38:11 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Jul 2021 20:38:15 -0000 On 7/7/21 1:38 AM, Richard Biener wrote: > On Tue, Jul 6, 2021 at 5:47 PM Martin Sebor via Gcc-patches > wrote: >> >> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-June/573349.html > > + if (TREE_CODE (axstype) != UNION_TYPE) > > what about QUAL_UNION_TYPE? (why constrain union type accesses > here - note you don't seem to constrain accesses of union members here) I didn't know a QUAL_UNION_TYPE was a thing. Removing the test doesn't seem to cause any regressions so let me do that in a followup. > > + if (tree access_size = TYPE_SIZE_UNIT (axstype)) > > + /* The byte size of the array has already been determined above > + based on a pointer ARG. Set ELTSIZE to the size of the type > + it points to and REFTYPE to the array with the size, rounded > + down as necessary. */ > + if (POINTER_TYPE_P (reftype)) > + reftype = TREE_TYPE (reftype); > + if (TREE_CODE (reftype) == ARRAY_TYPE) > + reftype = TREE_TYPE (reftype); > + if (tree refsize = TYPE_SIZE_UNIT (reftype)) > + if (TREE_CODE (refsize) == INTEGER_CST) > + eltsize = wi::to_offset (refsize); > > probably pre-existing but the pointer indirection is definitely confusing > me again and again given the variable is named 'reftype' - obviously > an access to a pointer does not have any element size. Possibly the > paths arriving here ensure somehow that the only case is when > reftype is not the access type but a pointer to the accessed memory. > "jump-threading" the source might help me avoiding to trip over this > again and again ... I agree (it is confusing). There's more to simplify here. It's on my to do list so let me see about this piece of code then. > > The patch removes a lot of odd code, I like that. You know this code best > and it's hard to spot errors. > > So OK, you'll deal with the fallout. I certainly will. Pushed in r12-2132. Thanks Martin > > Thanks, > Richard. > >> On 6/28/21 1:33 PM, Martin Sebor wrote: >>> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-June/573349.html >>> >>> On 6/21/21 4:25 PM, Martin Sebor wrote: >>>> -Warray-bounds relies on similar logic as -Wstringop-overflow et al., >>>> but using its own algorithm, including its own bugs such as PR 100137. >>>> The attached patch takes the first step toward unifying the logic >>>> between the warnings. It changes a subset of -Warray-bounds to call >>>> compute_objsize() to detect out-of-bounds indices. Besides fixing >>>> the bug this also nicely simplifies the code and improves >>>> the consistency between the informational messages printed by both >>>> classes of warnings. >>>> >>>> The changes to the test suite are extensive mainly because of >>>> the different format of the diagnostics resulting from slightly >>>> tighter bounds of offsets computed by the new algorithm, and in >>>> smaller part because the change lets -Warray-bounds diagnose some >>>> problems it previously missed due to the limitations of its own >>>> solution. >>>> >>>> The false positive reported in PR 100137 is a 10/11/12 regression >>>> but this change is too intrusive to backport. I have a smaller >>>> and more targeted patch I plan to backport in its stead. >>>> >>>> Tested on x86_64-linux. >>>> >>>> Martin >>> >>