Fixed bug on incorrectly revisiting same block.

Rationale:
Aart's fuzz tester found this particular bug, where
revisiting a block (after dynamic bce) would cause
static array length based bce to feed into itself
and thus incorrectly remove a needed bounds check.

bug=27376274

Change-Id: I9163f283af355d444b4cec707f194fe2b67c2572

diff --git a/compiler/optimizing/bounds_check_elimination.cc b/compiler/optimizing/bounds_check_elimination.cc
index a7a1c0f..288322e 100644 (file)
--- a/compiler/optimizing/bounds_check_elimination.cc
+++ b/compiler/optimizing/bounds_check_elimination.cc
@@ -1711,21 +1711,18 @@ void BoundsCheckElimination::Run() {
   // that value dominated by that instruction fits in that range. Range of that
   // value can be narrowed further down in the dominator tree.
   BCEVisitor visitor(graph_, side_effects_, induction_analysis_);
-  HBasicBlock* last_visited_block = nullptr;
   for (HReversePostOrderIterator it(*graph_); !it.Done(); it.Advance()) {
     HBasicBlock* current = it.Current();
-    if (current == last_visited_block) {
-      // We may insert blocks into the reverse post order list when processing
-      // a loop header. Don't process it again.
-      DCHECK(current->IsLoopHeader());
-      continue;
-    }
     if (visitor.IsAddedBlock(current)) {
       // Skip added blocks. Their effects are already taken care of.
       continue;
     }
     visitor.VisitBasicBlock(current);
-    last_visited_block = current;
+    // Skip forward to the current block in case new basic blocks were inserted
+    // (which always appear earlier in reverse post order) to avoid visiting the
+    // same basic block twice.
+    for ( ; !it.Done() && it.Current() != current; it.Advance()) {
+    }
   }
 
   // Perform cleanup.

diff --git a/test/449-checker-bce/src/Main.java b/test/449-checker-bce/src/Main.java
index 66e1d92..39467fc 100644 (file)
--- a/test/449-checker-bce/src/Main.java
+++ b/test/449-checker-bce/src/Main.java
@@ -1260,7 +1260,8 @@ public class Main {
       } else {
         assertIsManaged();
       }
-      array[i] = (array[i-2] + array[i-1] + array[i] + array[i+1] + array[i+2]) / 5;
+      // TODO: order matters, make it not so.
+      array[i] = (array[i] + array[i-2] + array[i-1] + array[i+1] + array[i+2]) / 5;
     }
   }
 

diff --git a/test/530-checker-loops/src/Main.java b/test/530-checker-loops/src/Main.java
index d5111b0..2e5fd25 100644 (file)
--- a/test/530-checker-loops/src/Main.java
+++ b/test/530-checker-loops/src/Main.java
@@ -633,6 +633,10 @@ public class Main {
     }
   }
 
+  //
+  // Verifier.
+  //
+
   public static void main(String[] args) {
     int[] empty = { };
     int[] x = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

diff --git a/test/578-bce-visit/expected.txt b/test/578-bce-visit/expected.txt
new file mode 100644 (file)
index 0000000..28fca2c
--- /dev/null
+++ b/test/578-bce-visit/expected.txt
@@ -0,0 +1,2 @@
+exception caught
+FUZZ result = 1001 16

diff --git a/test/578-bce-visit/info.txt b/test/578-bce-visit/info.txt
new file mode 100644 (file)
index 0000000..2462e1b
--- /dev/null
+++ b/test/578-bce-visit/info.txt
@@ -0,0 +1 @@
+Fuzz test that exposed bug in bounds check elimination visiting of blocks.

diff --git a/test/578-bce-visit/src/Main.java b/test/578-bce-visit/src/Main.java
new file mode 100644 (file)
index 0000000..b0e920e
--- /dev/null
+++ b/test/578-bce-visit/src/Main.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Automatically generated fuzz test that exposed bug in the way bounds
+ * check elimination visits basic blocks. If, after dynamic bce, the same
+ * block would be visited again, then static length based bce would incorrectly
+ * feed information back to itself and removed a necessary bounds check.
+ */
+public class Main {
+
+  private static int[][][] mA = new int[10][10][10];
+
+  private static int mX = 17;
+
+  private static int doit() {
+    int l0 = (((++mA[7][2][8]) <= mA[0][1][3]) ? (++mA[9][0][5]) : ((( -mA[0][7][0]) * ((mX == mX) ? 180 : mX)) + (mA[7][8][8]++)));
+    mA[1][0][4] -= mX;
+    int l1 = (((l0 >= ( ~mA[6][7][5])) && ((921 <= l0) && (mA[3][9][6] > l0))) ? mX : (l0--));
+    int l2 = ( -384);
+    for (int i0 = 7 - 1; i0 >= 1; i0--) {
+      mA[6][0][0] -= ((((l0++) == ( -mX)) ? (((mA[3][i0][1] > 503) || (mX <= i0)) ? (--l0) : (l0--)) : mX) - ( ~(mX--)));
+      int l3 = 24;
+      int l4 = ((l2--) & mX);
+      for (int i1 = i0-2 - 1; i1 >= 3; i1--) {
+        for (int i2 = 2; i2 < i0; i2++) {
+          mA[i0][4][l3] >>= 1;
+        }
+      }
+    }
+    return 1;
+  }
+
+  public static void main(String[] args) {
+    int k = 1;
+    for (int i0 = 0; i0 < 10; i0++)
+    for (int i1 = 0; i1 < 10; i1++)
+    for (int i2 = 0; i2 < 10; i2++)
+      mA[i0][i1][i2] = k++;
+    try {
+      k = doit();
+    } catch (Exception e) {
+      System.out.println("exception caught");
+    }
+    System.out.println("FUZZ result = " + k + " " + mX);
+  }
+}