Fix correctness of BasicBlock.hasEquivalentCatchHandlers()
It should be checking that all successors are trampolines to the same
block, not that each pair have the same destinations.
I'm not sure the case this is guarding against can even happen, but I
think this is more correct.
Bug: b/246971330
Change-Id: Ie2cc03e60cf2bfc79e32e31838b161007232699a
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index e77e0fe..b55931c 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -1188,30 +1188,44 @@
return true;
}
- private boolean isExceptionTrampoline() {
- boolean ret = instructions.size() == 2 && entry().isMoveException() && exit().isGoto();
- assert !ret || !hasCatchHandlers() : "Trampoline should not have catch handlers";
- return ret;
+ private BasicBlock getExceptionTrampolineTarget() {
+ if (instructions.size() == 2 && entry().isMoveException() && exit().isGoto()) {
+ assert !hasCatchHandlers() : "Trampoline should not have catch handlers";
+ return getUniqueNormalSuccessor();
+ }
+ return null;
}
- /** Returns whether the given blocks are in the same try block. */
+ /**
+ * Returns whether the given blocks are in the same try block.
+ *
+ * <p>They are considered the same if all catch handlers have the same guards and are trampolines
+ * that point to the same block.
+ */
public boolean hasEquivalentCatchHandlers(BasicBlock other) {
if (this == other) {
return true;
}
List<Integer> targets1 = catchHandlers.getAllTargets();
List<Integer> targets2 = other.catchHandlers.getAllTargets();
+
int numHandlers = targets1.size();
if (numHandlers != targets2.size()) {
return false;
}
- // If all catch handlers are trampolines to the same block, then they are from the same try.
+ if (numHandlers == 0) {
+ return true;
+ }
+
+ List<DexType> guards1 = catchHandlers.getGuards();
+ List<DexType> guards2 = other.catchHandlers.getGuards();
for (int i = 0; i < numHandlers; ++i) {
BasicBlock catchBlock1 = successors.get(targets1.get(i));
BasicBlock catchBlock2 = other.successors.get(targets2.get(i));
- if (!catchBlock1.isExceptionTrampoline()
- || !catchBlock2.isExceptionTrampoline()
- || catchBlock1.getUniqueSuccessor() != catchBlock2.getUniqueSuccessor()) {
+ BasicBlock trampolineTarget = catchBlock1.getExceptionTrampolineTarget();
+ if (trampolineTarget == null
+ || trampolineTarget != catchBlock2.getExceptionTrampolineTarget()
+ || guards1.get(i).isNotIdenticalTo(guards2.get(i))) {
return false;
}
}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
index 7011411..80ea5f5 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
@@ -485,10 +485,16 @@
@NeverInline
private static void arrayInsideCatchHandler() {
try {
- // Test filled-new-array with a throwing instruction before the last array-put.
- int[] arr = new int[1];
- System.currentTimeMillis();
- arr[0] = 9;
+ int[] arr;
+ // Use nested try to test hasEquivalentCatchHandlers() with multiple targets.
+ try {
+ // Test filled-new-array with a throwing instruction before the last array-put.
+ arr = new int[1];
+ System.currentTimeMillis();
+ arr[0] = 9;
+ } catch (RuntimeException r) {
+ throw new RuntimeException(r);
+ }
System.out.println(Arrays.toString(arr));
} catch (Throwable t) {
throw new RuntimeException(t);
