From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by sourceware.org (Postfix) with ESMTPS id 1F7083858C2C for ; Tue, 9 Nov 2021 00:09:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1F7083858C2C Received: by mail-pl1-x632.google.com with SMTP id n8so17602689plf.4 for ; Mon, 08 Nov 2021 16:09:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to; bh=u5kali128hkb3GzvDFxiIxoDjLdd8YPaLRj46dubYqo=; b=blu1+Q6jDjAYtuiVHKnmzf9C0aNY/mtkz+vd6XfzEBhGCADCMU2xGEYPD1o1e+5f+Q epRXEnX0KBsXyPXuLpoqy7hJAOzxLCMZ2L85aYbWQZaoeXx9p5gSZMBTWOvNLgUjg5Rr AiBPTrdkQu1nyOtjHP7CUwPFwmkrPz8QaxGyPiWIFKzNiJkDeAxarfSVwu8PL88TPzu9 mANKf6E7e/DmeGe+JrD+0aoVS0yLM+kB44HfNBDOWo8IO6LmwClNuzinWm9QpcWKRCn6 ZbnmvdMVUBXOr2D7yDU+ZPfEJVpqUDVovOPOes9vLUBcgNYuVh0MH21hiDeMCEjylV6I jotw== X-Gm-Message-State: AOAM532dt3BqidwPOKISQHLaOaS2oxRN59rijUsWpwBECzNKYuhI3vDw x4zcn2kNV/KJ8RF7GGgm5RA= X-Google-Smtp-Source: ABdhPJzYpiucS8x/6rdIix8wgeS5fzXkvZmfsaePsx4/L+FJk05LP+5K8utsHrc/iplV7EX4FOM5pg== X-Received: by 2002:a17:902:654b:b0:13d:c967:9cbd with SMTP id d11-20020a170902654b00b0013dc9679cbdmr2813915pln.88.1636416561027; Mon, 08 Nov 2021 16:09:21 -0800 (PST) Received: from [172.31.0.175] (c-98-202-48-222.hsd1.ut.comcast.net. [98.202.48.222]) by smtp.gmail.com with ESMTPSA id l1sm16559839pff.125.2021.11.08.16.09.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 08 Nov 2021 16:09:20 -0800 (PST) Message-ID: Date: Mon, 8 Nov 2021 17:09:18 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: Re: [PATCH] Convert strlen pass from evrp to ranger. Content-Language: en-US To: Aldy Hernandez , Andrew MacLeod , Jakub Jelinek Cc: Martin Sebor , GCC patches References: <20211008151222.37790-1-aldyh@redhat.com> <2952e615-3607-7d6a-d214-739c90744397@gmail.com> From: Jeff Law In-Reply-To: X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, HTML_MESSAGE, 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 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: Tue, 09 Nov 2021 00:09:24 -0000 On 10/15/2021 4:39 AM, Aldy Hernandez wrote: > > > On 10/15/21 2:47 AM, Andrew MacLeod wrote: >> On 10/14/21 6:07 PM, Martin Sebor via Gcc-patches wrote: >>> On 10/9/21 12:47 PM, Aldy Hernandez via Gcc-patches wrote: >>>> We seem to be passing a lot of context around in the strlen code.  I >>>> certainly don't want to contribute to more. >>>> >>>> Most of the handle_* functions are passing the gsi as well as either >>>> ptr_qry or rvals.  That looks a bit messy.  May I suggest putting all >>>> of that in the strlen pass object (well, the dom walker object, but we >>>> can rename it to be less dom centric)? >>>> >>>> Something like the attached (untested) patch could be the basis for >>>> further cleanups. >>>> >>>> Jakub, would this line of work interest you? >>> >>> You didn't ask me but since no one spoke up against it let me add >>> some encouragement: this is exactly what I was envisioning and in >>> line with other such modernization we have been doing elsewhere. >>> Could you please submit it for review? >>> >>> Martin >> >> I'm willing to bet he didn't submit it for review because he doesn't >> have time this release to polish and track it...  (I think the >> threader has been quite consuming).  Rather, it was offered as a >> starting point for someone else who might be interested in continuing >> to pursue this work...  *everyone* is interested in cleanup work >> others do :-) > > Exactly.  There's a lot of work that could be done in this area, and > I'm trying to avoid the situation with the threaders where what > started as refactoring ended up with me basically owning them ;-). > > That being said, I there are enough cleanups that are useful on their > own.  I've removed all the passing around of GSIs, as well as ptr_qry, > with the exception of anything dealing with the sprintf pass, since it > has a slightly different interface. > > This is patch 0001, which I'm formally submitting for inclusion. No > functional changes with this patch.  OK for trunk? > > Also, I am PINGing patch 0002, which is the strlen pass conversion to > the ranger.  As mentioned, this is just a change from an evrp client > to a ranger client.  The APIs are exactly the same, and besides, the > evrp analyzer is deprecated and slated for removal. OK for trunk? > > Aldy > > 0001-Convert-strlen-pass-from-evrp-to-ranger.patch > > From 152bc3a1dad9a960b7c0c53c65d6690532d9da5a Mon Sep 17 00:00:00 2001 > From: Aldy Hernandez > Date: Fri, 8 Oct 2021 15:54:23 +0200 > Subject: [PATCH] Convert strlen pass from evrp to ranger. > > The following patch converts the strlen pass from evrp to ranger, > leaving DOM as the last remaining user. > > 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. > > Basically the entire patch is just adjusting the calls to range_of_expr > to include context. The previous context of si->stmt was mostly > empty, so not really useful ;-). > > 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. > > On a positive note, these changes found two possible sprintf overflow > bugs in the C++ and Fortran front-ends which I have fixed below. > > Tested on x86-64 Linux. > > gcc/ChangeLog: > > * tree-ssa-strlen.c (compare_nonzero_chars): Pass statement > context to ranger. > (get_addr_stridx): Same. > (get_stridx): Same. > (get_range_strlen_dynamic): Same. > (handle_builtin_strlen): Same. > (handle_builtin_strchr): Same. > (handle_builtin_strcpy): Same. > (maybe_diag_stxncpy_trunc): Same. > (handle_builtin_stxncpy_strncat): > (handle_builtin_memcpy): Same. > (handle_builtin_strcat): Same. > (handle_alloc_call): Same. > (handle_builtin_memset): Same. > (handle_builtin_string_cmp): Same. > (handle_pointer_plus): Same. > (count_nonzero_bytes_addr): Same. > (count_nonzero_bytes): Same. > (handle_store): Same. > (fold_strstr_to_strncmp): Same. > (handle_integral_assign): Same. > (check_and_optimize_stmt): Same. > (class strlen_dom_walker): Replace evrp with ranger. > (strlen_dom_walker::before_dom_children): Remove evrp. > (strlen_dom_walker::after_dom_children): Remove evrp. > * gimple-ssa-warn-access.cc (maybe_check_access_sizes): > Restrict sprintf output. > > gcc/cp/ChangeLog: > > * ptree.c (cxx_print_xnode): Add more space to pfx array. > > gcc/fortran/ChangeLog: > > * misc.c (gfc_dummy_typename): Make sure ts->kind is > non-negative. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/builtin-sprintf-warn-22.c: XFAIL. OK.  Had I realized 99% was just adding the new argument to a bunch of call sites, I would have taken care of it earlier. jeff