From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 56476 invoked by alias); 7 Mar 2018 17:44:28 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 55429 invoked by uid 89); 7 Mar 2018 17:44:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-1.9 required=5.0 tests=BAYES_00,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=my, of=c2, The, walks?= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Mar 2018 17:44:27 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C6E8E23E6F3; Wed, 7 Mar 2018 17:44:25 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-22.rdu2.redhat.com [10.10.112.22]) by smtp.corp.redhat.com (Postfix) with ESMTP id 089287DE48; Wed, 7 Mar 2018 17:44:24 +0000 (UTC) Subject: Re: [PING] [PATCH] consider successor blocks when avoiding -Wstringop-truncation (PR 84468) To: Martin Sebor , Gcc Patch List References: <7116c7c3-43f3-0784-a05b-f68253cd7e5f@gmail.com> From: Jeff Law Message-ID: <7a716b2c-ecd8-3600-6cc4-ba681924126e@redhat.com> Date: Wed, 07 Mar 2018 17:44:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-03/txt/msg00341.txt.bz2 On 03/01/2018 02:17 PM, Martin Sebor wrote: > > I read you last reply as asking me to handle multiple edges. Sorry, I should have been clearer. You need to be prepared for the possibility of multiple edges and handle them in a conservatively correct way. The most likely way to get multiple outgoing edges is going to be via exception handling. In this code I think that's easily accomplished by making the code which walks into the successor block(s) conditional on single_succ_p (bb) and that the edge is not marked as EDGE_ABNORMAL. > The original patch handled just one edge and didn't bother > with EDGE_ABNORMAL because I wanted to keep it simple. Understood. But that's not safe in the sense that once you have multiple successors, you don't know which one you will transfer control to -- thus you have to check all of them for a suitable store. If any doesn't have a suitable store, then you'd have to warn. Essentially the question you need to answer is "given I'm in block X, do all paths from block X have a suitable store to terminate the string prior to a use". You can handle that in the trivial case with the code you're proposing in this patch, and that's fine for gcc-8. But the "right" way to answer that question is to look at the virtual operand chains. Though as we've discussed that may not necessarily be a good thing.   But > it sounds like you want me to go back to the first version > and just add a check for EDGE_ABNORMAL.  The attached patch > does that (plus it skips over any non-debug statements in > the successor block). Correct. I think your original patch with a check for single_succ_p is the direction I think we want for gcc-8.  Besides the usual I retested it with > the Linux kernel.  There are just three instances of > the warning in my default configuration but I know there > are a lot more in more extensive builds. I've actually got a fair amount of data here on kernel builds using the trunk on a variety of architectures. I haven't gone through it all yet, but there's certainly some string-op truncation warnings and a few others. I haven't figured out how best to aggregate that info and I don't want to duplicate Arnd's work. > > >> I glanced over the tests and I didn't see any that would benefit from >> handling multiple edges or the recursion (every one of the dg-bogus >> markers should be immediately transferring control to the null >> termination statement AFAICT). > > I've added some more test cases involving C++ exceptions (and > found one false positive(*)) but I don't have a test to exercise > blocks with multiple outgoing edges.  For future reference, how > would I create one? EH is the best bet to create them. You might also be able to get one if we've got fake edges in the CFG (necessary for things like post-dominator computation). async exceptions could likely create them fairly easily. My comment was more about not seeing the need to look into successor blocks if there's more than one. Can you file a bug report on the false positive so that we have a reminder to revisit looking at the virtual operand chains as a better solution here? I think this version is OK. Thanks, and again sorry for the confusion. Jeff