From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) by sourceware.org (Postfix) with ESMTPS id 9617F3835815 for ; Fri, 7 May 2021 09:34:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9617F3835815 Received: by mail-ed1-x535.google.com with SMTP id u13so9449095edd.3 for ; Fri, 07 May 2021 02:34:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3qZOtAoeDWIUAsf1/Dy+Ddqn7lVodpGQrlPyA5+RUQk=; b=NMK1UWs5iYs7XphueCspwUKEj56xZ75cIjBRQ41utbYsfKkReA2DPS6OQJZSfP5Moh 56ihJLxKjZ0wddUpz9/u3KZxUhzkVDg4RO0OCmDxDWU+mWVtb+urg7VYILOyUANFAJ1P Jwl+WHv22bGxX1NrI2lvXue6jcqVEBtx3GMqbWimGGj7axoXLSroSLR17b4BdbKxRZ3m deeyrBV0zKpsciSywJvXYv5ioB1t7uTyb1Y+79ZWWOdfVqrWRXA2xECEFDAbuG6VQGFV HwbBPqakOA2gGRvipC3461gCtAQtaUqRSRlTW80aoKH9ywGyG0U/FkzrYgxbQsmsudRJ YnQQ== X-Gm-Message-State: AOAM533614ZXtZL5rpW4Ze2F7brnL+FTg9NoqcyHzCOPhEAJvxgwl1RD He5S9DL+1a4HGSIGwUUSfGkK+Pe3YcKYqQ1WGDA= X-Google-Smtp-Source: ABdhPJxqIM9SzFWKdbwPIsacf2J747wxy8qTzQSyM3baPeID0qmqpZvSdob+Myodqx4f3kElSxAUbx0l4fekZ9Qi6ig= X-Received: by 2002:aa7:cb89:: with SMTP id r9mr10314115edt.245.1620380088188; Fri, 07 May 2021 02:34:48 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Fri, 7 May 2021 11:34:37 +0200 Message-ID: Subject: Re: [PATCH] run early sprintf warning after SSA (PR 100325) To: Martin Sebor Cc: Aldy Hernandez , gcc-patches , Andrew MacLeod Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, 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: Fri, 07 May 2021 09:34:51 -0000 On Fri, May 7, 2021 at 2:12 AM Martin Sebor wrote: > > On 5/6/21 8:32 AM, Aldy Hernandez wrote: > > > > > > On 5/5/21 9:26 AM, Richard Biener wrote: > >> On Wed, May 5, 2021 at 1:32 AM Martin Sebor via Gcc-patches > >> wrote: > >>> > >>> With no optimization, -Wformat-overflow and -Wformat-truncation > >>> runs early to detect a subset of simple bugs. But as it turns out, > >>> the pass runs just a tad too early, before SSA. That causes it to > >>> miss a class of problems that can easily be detected once code is > >>> in SSA form, and I would expect might also cause false positives. > >>> > >>> The attached change moves the sprintf pass just after pass_build_ssa, > >>> similar to other early flow-sensitive warnings (-Wnonnull-compare and > >>> -Wuninitialized). > >> > >> Makes sense. I suppose walloca might also benefit from SSA - it seems > >> to do range queries which won't work quite well w/o SSA? > > > > The early Walloca pass that runs without optimization doesn't do much, > > as we've never had ranges so early. All it does is diagnose _every_ > > call to alloca(), if -Walloca is passed: > > > > // The first time this pass is called, it is called before > > // optimizations have been run and range information is unavailable, > > // so we can only perform strict alloca checking. > > if (first_time_p) > > return warn_alloca != 0; > > > > Though, I suppose we could move the first alloca pass after SSA is > > available and make it the one and only pass, since ranger only needs > > SSA. However, I don't know how well this would work without value > > numbering or CSE. For example, for gcc.dg/Walloca-4.c the gimple is: > > > > : > > _1 = rear_ptr_9(D) - w_10(D); > > _2 = (long unsigned int) _1; > > if (_2 <= 4095) > > goto ; [INV] > > else > > goto ; [INV] > > > > : > > _3 = rear_ptr_9(D) - w_10(D); > > _4 = (long unsigned int) _3; > > src_16 = __builtin_alloca (_4); > > goto ; [INV] > > > > No ranges can be determined for _4. However, if either FRE or DOM run, > > as they do value numbering and CSE respectively, we could easily > > determine a range as the above would become: > > > > : > > _1 = rear_ptr_9(D) - w_10(D); > > _2 = (long unsigned int) _1; > > if (_2 <= 4095) > > goto ; [INV] > > else > > goto ; [INV] > > > > : > > src_16 = __builtin_alloca (_2); > > goto ; [INV] > > > > I'm inclined to leave the first alloca pass before SSA runs, as it > > doesn't do anything with ranges. If anyone's open to a simple -O0 CSE > > type pass, it would be a different story. Thoughts? > > Improving the analysis at -O0 and getting better warnings that are > more consistent with what is issued with optimization would be very > helpful (as as long as it doesn't compromise debugging experience > of course). I agree. It shouldn't be too difficult to for example run the VN propagation part without doing actual elimiation and keep value-numbers for consumption. do_rpo_vn (not exported) might even already support iterate = false, eliminate = false, it would just need factoring out the init/deinit somewhat. Of course it will be a lot more expensive to do since it cannot do "on-demand" value-numbering of interesting SSA names. I'm not sure that would be possible anyhow. Though for the alloca case quickly scanning the function whether there's any would of course be faster than throwing VN at it. Oh, and no - we don't want to perform CSE at -O0 (I mean affecting generated code). Richard. > Martin > > > > > Aldy > > >