On Mon, Aug 17, 2015 at 7:31 PM, Jeff Law <l...@redhat.com> 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: >>>> >>>> <bb 2>: >>>> ... >>>> 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 > >
