From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com [IPv6:2607:f8b0:4864:20::72f]) by sourceware.org (Postfix) with ESMTPS id D59403846075 for ; Fri, 7 May 2021 00:12:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D59403846075 Received: by mail-qk1-x72f.google.com with SMTP id o5so6902915qkb.0 for ; Thu, 06 May 2021 17:12:20 -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=aG7xgX7cLnuBBXqYCMUNQFupOwazfVhWjnBz1Xot6ig=; b=YRhfHja/L4qD8vquqgqphpWGrPbDjhucBgANIg50h8Jma/o1/Xqql6Y/+nHeCHuhAW 6HTxINroRiJpKFfBXJRohzyAQ9LkahHAxIPp636Kn1nDJH2kkIHqZLyKXWz+j9HzMsau CEMMYttynjlj2fKvYXdZklIOP7O/8f4kQYvbr/17q9l3VjUUG5dio+jub9JEl5LBD6S0 DztzTScZ86CVUmS0Y6XywAfBfSuLZuzqA+/n5iz2LMlf6rGsG7baSGn4aZVXP46qyZmV tm5QjWmbcEuzCwgF8j/hIXMUPsytSHsIR7mhbP/maCH7B+7mo0eksjq0jb3oo8hh0Aux h+Cg== X-Gm-Message-State: AOAM533ie/PyuhKxJmAlArTj64iswlJHahh0tm/hSThHnyrqSy941DKy V9khUbMwk1Sfl9yCDfCoD1I= X-Google-Smtp-Source: ABdhPJyN7BWXS8rqEatfoy8VPnWmSgBICptbrpMYFbnIieUuBnRFam7JVw7GIUvww2BPBoqv8VoIAg== X-Received: by 2002:a37:bb84:: with SMTP id l126mr6721432qkf.311.1620346340409; Thu, 06 May 2021 17:12:20 -0700 (PDT) Received: from [192.168.0.41] (71-218-14-121.hlrn.qwest.net. [71.218.14.121]) by smtp.gmail.com with ESMTPSA id b15sm3263079qkj.95.2021.05.06.17.12.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 06 May 2021 17:12:19 -0700 (PDT) Subject: Re: [PATCH] run early sprintf warning after SSA (PR 100325) To: Aldy Hernandez , Richard Biener Cc: gcc-patches , Andrew MacLeod References: From: Martin Sebor Message-ID: Date: Thu, 6 May 2021 18:12:18 -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: 8bit 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, NICE_REPLY_A, 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 00:12:22 -0000 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). Martin > > Aldy >