8215265: C2: range check elimination may allow illegal out of bound access

Reviewed-by: thartmann, kvn

src/share/vm/opto/loopTransform.cpp

@@ -1537,13 +1537,20 @@ bool IdealLoopTree::dominates_backedge(Node* ctrl) {
 
 //------------------------------adjust_limit-----------------------------------
 // Helper function for add_constraint().
-Node* PhaseIdealLoop::adjust_limit(int stride_con, Node * scale, Node *offset, Node *rc_limit, Node *loop_limit, Node *pre_ctrl) {
+Node* PhaseIdealLoop::adjust_limit(int stride_con, Node * scale, Node *offset, Node *rc_limit, Node *loop_limit, Node *pre_ctrl, bool round_up) {
   // Compute "I :: (limit-offset)/scale"
   Node *con = new (C) SubINode(rc_limit, offset);
   register_new_node(con, pre_ctrl);
   Node *X = new (C) DivINode(0, con, scale);
   register_new_node(X, pre_ctrl);
 
+  // When the absolute value of scale is greater than one, the integer
+  // division may round limit down so add one to the limit.
+  if (round_up) {
+    X = new (C) AddINode(X, _igvn.intcon(1));
+    register_new_node(X, pre_ctrl);
+  }
+
   // Adjust loop limit
   loop_limit = (stride_con > 0)
                ? (Node*)(new (C) MinINode(loop_limit, X))
@@ -1584,7 +1591,7 @@ void PhaseIdealLoop::add_constraint( int stride_con, int scale_con, Node *offset
     // (upper_limit-offset) may overflow or underflow.
     // But it is fine since main loop will either have
     // less iterations or will be skipped in such case.
-    *main_limit = adjust_limit(stride_con, scale, offset, upper_limit, *main_limit, pre_ctrl);
+    *main_limit = adjust_limit(stride_con, scale, offset, upper_limit, *main_limit, pre_ctrl, false);
 
     // The underflow limit: low_limit <= scale*I+offset.
     // For pre-loop compute
@@ -1620,7 +1627,8 @@ void PhaseIdealLoop::add_constraint( int stride_con, int scale_con, Node *offset
       // max(pre_limit, original_limit) is used in do_range_check().
     }
     // Pass (-stride) to indicate pre_loop_cond = NOT(main_loop_cond);
-    *pre_limit = adjust_limit((-stride_con), scale, offset, low_limit, *pre_limit, pre_ctrl);
+    *pre_limit = adjust_limit((-stride_con), scale, offset, low_limit, *pre_limit, pre_ctrl,
+                              scale_con > 1 && stride_con > 0);
 
   } else { // stride_con*scale_con < 0
     // For negative stride*scale pre-loop checks for overflow and
@@ -1646,7 +1654,8 @@ void PhaseIdealLoop::add_constraint( int stride_con, int scale_con, Node *offset
     Node *plus_one = new (C) AddINode(offset, one);
     register_new_node( plus_one, pre_ctrl );
     // Pass (-stride) to indicate pre_loop_cond = NOT(main_loop_cond);
-    *pre_limit = adjust_limit((-stride_con), scale, plus_one, upper_limit, *pre_limit, pre_ctrl);
+    *pre_limit = adjust_limit((-stride_con), scale, plus_one, upper_limit, *pre_limit, pre_ctrl,
+                              scale_con < -1 && stride_con > 0);
 
     if (low_limit->get_int() == -max_jint) {
       if (!RangeLimitCheck) return;
@@ -1681,7 +1690,8 @@ void PhaseIdealLoop::add_constraint( int stride_con, int scale_con, Node *offset
     //       I > (low_limit-(offset+1))/scale
     //   )
 
-    *main_limit = adjust_limit(stride_con, scale, plus_one, low_limit, *main_limit, pre_ctrl);
+    *main_limit = adjust_limit(stride_con, scale, plus_one, low_limit, *main_limit, pre_ctrl,
+                               false);
   }
 }
 

src/share/vm/opto/loopnode.hpp

@@ -959,7 +959,7 @@ public:
   // loop.  Scale_con, offset and limit are all loop invariant.
   void add_constraint( int stride_con, int scale_con, Node *offset, Node *low_limit, Node *upper_limit, Node *pre_ctrl, Node **pre_limit, Node **main_limit );
   // Helper function for add_constraint().
-  Node* adjust_limit( int stride_con, Node * scale, Node *offset, Node *rc_limit, Node *loop_limit, Node *pre_ctrl );
+  Node* adjust_limit(int stride_con, Node * scale, Node *offset, Node *rc_limit, Node *loop_limit, Node *pre_ctrl, bool round_up);
 
   // Partially peel loop up through last_peel node.
   bool partial_peel( IdealLoopTree *loop, Node_List &old_new );

test/compiler/rangechecks/RangeCheckEliminationScaleNotOne.java

+/*
+ * Copyright (c) 2018, Red Hat, Inc. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8215265
+ * @summary C2: range check elimination may allow illegal out of bound access
+ *
+ * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:-UseLoopPredicate RangeCheckEliminationScaleNotOne
+ *
+ */
+
+import java.util.Arrays;
+
+public class RangeCheckEliminationScaleNotOne {
+    public static void main(String[] args) {
+        {
+            int[] array = new int[199];
+            boolean[] flags = new boolean[100];
+            Arrays.fill(flags, true);
+            flags[0] = false;
+            flags[1] = false;
+            for (int i = 0; i < 20_000; i++) {
+                test1(100, array, 0, flags);
+            }
+            boolean ex = false;
+            try {
+                test1(100, array, -5, flags);
+            } catch (ArrayIndexOutOfBoundsException aie) {
+                ex = true;
+            }
+            if (!ex) {
+                throw new RuntimeException("no AIOOB exception");
+            }
+        }
+
+        {
+            int[] array = new int[199];
+            boolean[] flags = new boolean[100];
+            Arrays.fill(flags, true);
+            flags[0] = false;
+            flags[1] = false;
+            for (int i = 0; i < 20_000; i++) {
+                test2(100, array, 198, flags);
+            }
+            boolean ex = false;
+            try {
+                test2(100, array, 203, flags);
+            } catch (ArrayIndexOutOfBoundsException aie) {
+                ex = true;
+            }
+            if (!ex) {
+                throw new RuntimeException("no AIOOB exception");
+            }
+        }
+    }
+
+    private static int test1(int stop, int[] array, int offset, boolean[] flags) {
+        if (array == null) {}
+        int res = 0;
+        for (int i = 0; i < stop; i++) {
+            if (flags[i]) {
+                res += array[2 * i + offset];
+            }
+        }
+        return res;
+    }
+
+
+    private static int test2(int stop, int[] array, int offset, boolean[] flags) {
+        if (array == null) {}
+        int res = 0;
+        for (int i = 0; i < stop; i++) {
+            if (flags[i]) {
+                res += array[-2 * i + offset];
+            }
+        }
+        return res;
+    }
+}