From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25340 invoked by alias); 18 Aug 2015 08:45:26 -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 25330 invoked by uid 89); 18 Aug 2015 08:45:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ig0-f173.google.com Received: from mail-ig0-f173.google.com (HELO mail-ig0-f173.google.com) (209.85.213.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 18 Aug 2015 08:45:24 +0000 Received: by igfj19 with SMTP id j19so77495279igf.0 for ; Tue, 18 Aug 2015 01:45:22 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.50.78.133 with SMTP id b5mr22498558igx.32.1439887522025; Tue, 18 Aug 2015 01:45:22 -0700 (PDT) Received: by 10.107.32.140 with HTTP; Tue, 18 Aug 2015 01:45:21 -0700 (PDT) In-Reply-To: <55D21A8D.50004@redhat.com> References: <20150814112006.GR3335@redhat.com> <20150814132945.GS3335@redhat.com> <55CE002E.6000108@redhat.com> <20150814153224.GU3335@redhat.com> <55CE0A5A.4070802@redhat.com> <20150814195836.GB2093@redhat.com> <55CE5054.5080400@redhat.com> <20150814204649.GC2093@redhat.com> <55D21A8D.50004@redhat.com> Date: Tue, 18 Aug 2015 08:47:00 -0000 Message-ID: Subject: Re: [PATCH] Fix middle-end/67133, part 1 From: Richard Biener To: Jeff Law Cc: Marek Polacek , GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00959.txt.bz2 On Mon, Aug 17, 2015 at 7:31 PM, Jeff Law wrote: > On 08/14/2015 02:46 PM, Marek Polacek wrote: >>>> >>>> Then in isolate-paths in find_explicit_erroneous_behaviour we're walking >>>> the >>>> stmts in bb 2 and we find a null dereference, so >>>> insert_trap_and_remove_trailing_statements >>>> comes in play and turns bb 2 into: >>>> >>>> : >>>> ... >>>> SR.5_10 = MEM[(const struct A &)0B]; >>>> __builtin_trap (); >>>> >>>> i.e. it removs the defining statement for c_13. Then >>>> find_explicit_erroneous_behaviour >>>> walks over bb 3, hits the fn1 (&D.2434.OutBufCur, &b, c_13); statement, >>>> and >>>> ICEs on the c_13 argument: it's a released SSA name with NULL TREE_TYPE. >>>> >>>> The question now is what to do with that. Skip SSA_NAME_IN_FREE_LIST? >>>> That >>>> sounds weird. Note that we're going to remove bb 3 and bb 4 anyway... >>> >>> Jeez, looking at the code N years later, I feel like a complete idiot. Of >>> course that's not going to work. >>> >>> We certainly don't want to peek at SSA_NAME_IN_FREE_LIST. >> >> >> Yeh, I thought as much. >> >>> I wonder if we should be walking in backwards dominator order to avoid >>> these >>> effects. Or maybe just ignoring any BB with no preds. I'll ponder those >>> over the weekend. >> >> >> I suppose both ought to work. Or at least theoretically we could run e.g. >> cleanup_cfg to prune the IR after we've inserted trap and removed trailing >> stmts so that it gets rid of unreachable bbs. Would that make sense? >> >> Anyway, if you think of how would you like to solve this I can take a >> crack >> at it next week. > > The funny thing here is we remove the statements after the trap to avoid > this exact situation! Not sure, when I came along that code in the past I wondered why we don't just do like other passes - split the block, insert the unreachable() at the end of the first and leave the actual cleanup to cfg-cleanup. > I think the problem with schemes that either change the order of block > processing, or which ignore some blocks are going to run into issues. By > walking blocks and statements in a backwards order, we address 99% of the > problems, including uses in PHIs in a direct successor block. > > What's not handled is a use in a PHI at the frontier of a subgraph that > becomes unreachable. We'd have to do the usual unreachable block analysis > to catch and handle those properly. So you are after second-level effects that require earlier unreachable paths to be pruned? > I don't particularly like that idea.... > > But in walking through all that, I think I've stumbled on a simpler > solution. Specifically do as a little as possible and let the standard > mechanisms clean things up :-) > > 1. Delete the code that removes instructions after the trap. > > 2. Split the block immediately after the trap and remove the edge > from the original block (with the trap) to the new block. cfg-cleanup will do that for you if you have a not returning stmt ending the previous block. Richard. > > THen let the standard mechanisms handle things when that pass is complete. > > By setting cfg_altered, we'll get unreachable code removal which will > capture most of the intended effect. DCE fires a couple more passes down in > the pipeline to pick up the remaining tidbits. > > Do you want to try and tackle this? > > jeff > >