8066103: C2's range check smearing allows out of bound array accesses

[Roland Westrelin]

Hi John,

Thanks for the suggestions. They sound very reasonable to me. Here is a new webrev:

http://cr.openjdk.java.net/~roland/8066103/webrev.03/

In the patch below:

> Or better yet a purely local cutout.  (This sketch has a bad goto; the value of 'prev_dom' carried by the goto is what I called 'most_recent_prev_check' above.)
> 
> diff --git a/src/share/vm/opto/ifnode.cpp b/src/share/vm/opto/ifnode.cpp
> --- a/src/share/vm/opto/ifnode.cpp
> +++ b/src/share/vm/opto/ifnode.cpp
> @@ -876,6 +876,7 @@
>     // Low and high offsets seen so far
>     jint off_lo = offset1;
>     jint off_hi = offset1;
> +    int nb_checks = 0;
> 
>     // Scan for the top 2 checks and collect range of offsets
>     for( int dist = 0; dist < 999; dist++ ) { // Range-Check scan limit
> @@ -890,6 +891,15 @@
>         // the same array bounds.
>         if( flip2 == flip1 && range2 == range1 && index2 == index1 &&
>             dom->outcnt() == 2 ) {
> +          if (nb_checks == 0 && dom->in(1) == in(1)) {
> +            // Found an immediately dominating test at the same offset.
> +            // This kind of back-to-back test can be eliminated locally,
> +            // and there is no need to search further for dominating tests.
> +            assert(offset2 == offset1, "");
> +            prev_dom = dom;
> +            goto got_prev_dom;  // FIXME: unstructured control flow
> +          }
> +          nb_checks++;
>           // Gather expanded bounds
>           off_lo = MIN2(off_lo,offset2);
>           off_hi = MAX2(off_hi,offset2);
> @@ -985,6 +995,7 @@
> 
>   } // End of Else scan for an equivalent test
> 
> + got_prev_dom:  // FIXME: unstructured control flow
>   // Hit!  Remove this IF
> #ifndef PRODUCT
>   if( TraceIterativeGVN ) {

I don’t think you need prev_dom. prev_dom is the projection, dom is the If node. IfNode::dominated_by needs the projection as input.

Roland.