From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x82a.google.com (mail-qt1-x82a.google.com [IPv6:2607:f8b0:4864:20::82a]) by sourceware.org (Postfix) with ESMTPS id 3F2003858409 for ; Sat, 9 Oct 2021 16:19:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3F2003858409 Received: by mail-qt1-x82a.google.com with SMTP id r17so7187283qtx.10 for ; Sat, 09 Oct 2021 09:19:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=QvQ+7uvGdo2CKmKOYFxurwrYDBPZGxjWXbzraFe+FbQ=; b=Rv5h57AqkZRANlirjEFBG8NTNt+tWRLYtnn3lfFAXp86MpFb1i7mknV2mttARsHyyD pZRoD9fwf0Kq4oWx76yTjjyPc0U3RQvC7eOFZQyv3efW/9ouPvYjPPOiuWlQ+C8jBp3D 31Lukt9wphzOV+qlQUYl+mnu+rqWfa2sGR9Rgx/o916iRRVVhwdqbVdYs4liqZC6Db/O KM3A3A/YNnnvY0C2SrwLQ21yI9VoiFm8hbRYFqsIOSEM2+fjBvPG9WupYFmdqZRLZq+D n0Tr9FzBRjIIc+pv+X8/GI5CkoHLA0Fb33XaV2GnAW5XW+Xib2tuoWMAvUUVoD1ojI+5 hkZw== X-Gm-Message-State: AOAM532UeIlBEgXMPlW1wBCZq2/NPp0MeETBNBlCJoGTGJF/iChU64wL Z6aYR9gZocUf9ypLMv4wzrpfXHkLX0E= X-Google-Smtp-Source: ABdhPJzZN3dc+i521hPJDwDYPQvCW8qGotnZxfAc/k1bFqpn8SkGBzl/h2JxvEWkfZhlfKcP2pkuGA== X-Received: by 2002:ac8:4155:: with SMTP id e21mr4985809qtm.312.1633796391675; Sat, 09 Oct 2021 09:19:51 -0700 (PDT) Received: from [192.168.0.41] (184-96-250-116.hlrn.qwest.net. [184.96.250.116]) by smtp.gmail.com with ESMTPSA id l20sm1715243qtk.19.2021.10.09.09.19.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 09 Oct 2021 09:19:51 -0700 (PDT) Subject: Re: [PATCH] Convert strlen pass from evrp to ranger. To: Aldy Hernandez Cc: Jakub Jelinek , Martin Sebor , GCC patches References: <20211008151222.37790-1-aldyh@redhat.com> From: Martin Sebor Message-ID: <217c8f4c-934f-f20f-b789-f19aa8a303f5@gmail.com> Date: Sat, 9 Oct 2021 10:19:49 -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=-3.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, 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: Sat, 09 Oct 2021 16:19:53 -0000 On 10/9/21 9:04 AM, Aldy Hernandez wrote: > On Fri, Oct 8, 2021 at 6:52 PM Martin Sebor wrote: >> >> On 10/8/21 9:12 AM, Aldy Hernandez via Gcc-patches wrote: >>> The following patch converts the strlen pass from evrp to ranger, >>> leaving DOM as the last remaining user. >> >> Thanks for doing this. I know I said I'd work on it but I'm still >> bogged down in my stage 1 work that's not going so great :( I just >> have a few minor comments/questions on the strlen change (inline) >> but I am a bit concerned about the test failure. >> >>> >>> No additional cleanups have been done. For example, the strlen pass >>> still has uses of VR_ANTI_RANGE, and the sprintf still passes around >>> pairs of integers instead of using a proper range. Fixing this >>> could further improve these passes. >>> >>> As a further enhancement, if the relevant maintainers deem useful, >>> the domwalk could be removed from strlen. That is, unless the pass >>> needs it for something else. >>> >>> With ranger we are now able to remove the range calculation from >>> before_dom_children entirely. Just working with the ranger on-demand >>> catches all the strlen and sprintf testcases with the exception of >>> builtin-sprintf-warn-22.c which is due to a limitation of the sprintf >>> code. I have XFAILed the test and documented what the problem is. >> >> builtin-sprintf-warn-22.c is a regression test for a false positive >> in Glibc. If it fails we'll have to deal with the Glibc failure >> again, which I would rather avoid. Have you checked to see if >> Glibc is affected by the change? Have you tested Glibc with the patch to see if the warning comes back? If it does there are other ways to suppress it, and I can take care of it. I just want us to avoid reintroducing it without doing our due diligence first. ... >> I think there still are quite a few calls to get_addr_stridx() and >> get_addr() that don't pass in rvals. I started doing this back in >> the GCC 11 cycle but didn't finish the job. Eventually, rvals >> should be passed to all their callers (or they made class members >> with a ranger member). I mention this in case it has an effect on >> the range propagation (since the pass also sets ranges). > > Definitely. You'll get even better ranges, though perhaps more false > positives as discussed above :-/. We want better ranges and better analysis. Ultimately, it will lead to better quality warnings, as has been one of the goals behind Ranger. If along the way it means a few false positives in some edge cases, we'll deal with them. I don't see this one as a significant problem. Martin