On 12/13/2017 09:02 AM, David Malcolm wrote: > On Wed, 2017-12-13 at 08:46 -0700, Jeff Law wrote: >> On 12/13/2017 03:06 AM, Richard Biener wrote: >>> On December 12, 2017 9:50:38 PM GMT+01:00, David Malcolm <dmalcolm@ >>> redhat.com> wrote: >>>> PR tree-optimization/83312 reports a false positive from >>>> -Warray-bounds. >>>> The root cause is that VRP both: >>>> >>>> (a) updates a GIMPLE_COND to be always false, and >>>> >>>> (b) updates an ARRAY_REF in the now-unreachable other path to use >>>> an >>>> ASSERT_EXPR with a negative index: >>>> def_stmt j_6 = ASSERT_EXPR <j_9, j_9 < 0>; >>>> >>>> When vrp_prop::check_all_array_refs () runs, the CFG hasn't yet >>>> been >>>> updated to take account of (a), and so a false positive is >>>> emitted >>>> when (b) is encountered. >>>> >>>> This patch fixes the false warning by converting >>>> vrp_prop::check_all_array_refs >>>> from a simple iteration over all BBs to use a new dom_walker >>>> subclass, >>>> using the "skip_unreachable_blocks = true" mechanism to avoid >>>> analyzing >>>> (b). >>>> >>>> There didn't seem to be a pre-existing way to determine the >>>> unique >>>> out-edge after a GIMPLE_COND (if it has a constant cond), so I >>>> added >>>> a new gimple_cond_get_unique_successor_edge function. Similarly, >>>> something similar may apply for switches, so I put in a >>>> gimple_get_unique_successor_edge (though I wasn't able to create >>>> a >>>> reproducer that used a switch). >>>> >>>> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. >>>> >>>> OK for trunk? >>> >>> I don't like the GIMPLE.c bits a lot. Can't you use the existing >>> known taken edge helper (too lazy to look up from my phone...) >>> basically splitting out some code from cond processing in evrp for >>> example? >> >> I'm not sure those bits are needed at all. I think the right things >> will >> happen if we clear EDGE_EXECUTABLE on the appropriate edge in >> vrp_folder::fold_predicate_in. >> >> That should cause the block in question to become unreachable (zero >> preds). So later when we do the domwal dom_walker::walk will see the >> block as unreachable -- which avoids walking into it and also >> triggers >> the call to propagate_unreachable_to_edges which marks the outgoing >> edges as not executable. > > AIUI, dom_walker::bb_reachable only honors the EDGE_EXECUTABLE flags if > m_skip_unreachable_blocks is set on the dom_walker. > > However, dom_walker's ctor clears all of the EDGE_EXECUTABLE if that > bool is set. Ugh. How unfortunate, though I understand why it would be written that way.
> > So, as written any edge flags that are touched in > vrp_folder::fold_predicate_in will get reset when the dom walker is > created. > > So should the dom walker get created before the vrp_folder runs? That would probably work, but something doesn't feel right about it. Alternately we could to the dom_walker ctor that an initial state of EDGE_EXECUTABLE is already set. Jeff
