Fix soundness in enum switch map removal
Bug: 172528424
Change-Id: Ieda83802b24b1d3de64399fef89c63938f2b072d
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java
index f1c2bdd..033a233 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java
@@ -228,16 +228,9 @@
continue;
}
- Int2IntMap ordinalToTargetMap = new Int2IntArrayMap(switchInsn.numberOfKeys());
- for (int i = 0; i < switchInsn.numberOfKeys(); i++) {
- assert switchInsn.targetBlockIndices()[i] != switchInsn.getFallthroughBlockIndex();
- DexField field = info.indexMap.get(switchInsn.getKey(i));
- EnumValueInfo valueInfo = info.valueInfoMap.getEnumValueInfo(field);
- if (valueInfo != null) {
- ordinalToTargetMap.put(valueInfo.ordinal, switchInsn.targetBlockIndices()[i]);
- } else {
- // The switch map refers to a field on the enum that does not exist in this compilation.
- }
+ Int2IntMap ordinalToTargetMap = computeOrdinalToTargetMap(switchInsn, info);
+ if (ordinalToTargetMap == null) {
+ continue;
}
int fallthroughBlockIndex = switchInsn.getFallthroughBlockIndex();
@@ -322,6 +315,24 @@
}
}
+ private Int2IntMap computeOrdinalToTargetMap(IntSwitch switchInsn, EnumSwitchInfo info) {
+ Int2IntMap ordinalToTargetMap = new Int2IntArrayMap(switchInsn.numberOfKeys());
+ for (int i = 0; i < switchInsn.numberOfKeys(); i++) {
+ assert switchInsn.targetBlockIndices()[i] != switchInsn.getFallthroughBlockIndex();
+ DexField field = info.indexMap.get(switchInsn.getKey(i));
+ EnumValueInfo valueInfo = info.valueInfoMap.getEnumValueInfo(field);
+ if (valueInfo != null) {
+ if (appView.appInfo().isPinned(field)) {
+ return null;
+ }
+ ordinalToTargetMap.put(valueInfo.ordinal, switchInsn.targetBlockIndices()[i]);
+ } else {
+ // The switch map refers to a field on the enum that does not exist in this compilation.
+ }
+ }
+ return ordinalToTargetMap;
+ }
+
private static final class EnumSwitchInfo {
final DexType enumClass;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapInvalidOrdinalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapInvalidOrdinalTest.java
new file mode 100644
index 0000000..cbbd4f0
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapInvalidOrdinalTest.java
@@ -0,0 +1,101 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.ir.optimize.switches;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class SwitchMapInvalidOrdinalTest extends TestBase {
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDexRuntimes().withAllApiLevels().build();
+ }
+
+ public SwitchMapInvalidOrdinalTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ testForD8()
+ .setMinApi(parameters.getApiLevel())
+ .addInnerClasses(SwitchMapInvalidOrdinalTest.class)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("a", "b", "0", "a");
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addKeepMainRule(Main.class)
+ .addKeepRules(
+ "-keep class"
+ + "com.android.tools.r8.ir.optimize.switches.SwitchMapInvalidOrdinalTest$MyEnum {"
+ + " static <fields>; }")
+ .addInnerClasses(SwitchMapInvalidOrdinalTest.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ // When the code reaches the switch the first time, then the switch map int[] gets
+ // initialized based on the values in the enum at this point creating a mapping ordinal to
+ // switch map entry. Here D and X have 3 as ordinal.
+ .assertSuccessWithOutputLines("a", "b", "0", "a");
+ }
+
+ @NeverClassInline
+ enum MyEnum {
+ A,
+ B,
+ C,
+ D;
+ }
+
+ public static class Main {
+ public static void main(String[] args) {
+ try {
+ // Use reflection to instantiate a new enum instance, and set it to A, using getFields
+ // and not getField since A is minified.
+ Constructor<MyEnum> constructor =
+ (Constructor<MyEnum>) MyEnum.class.getDeclaredConstructors()[0];
+ constructor.setAccessible(true);
+ MyEnum x = constructor.newInstance("X", 3);
+ Field f = MyEnum.class.getFields()[0];
+
+ // On Android, setAccessible allows to set a final field.
+ f.setAccessible(true);
+
+ f.set(null, x);
+ } catch (Exception e) {
+ System.out.println("Unexpected: " + e);
+ }
+ print(MyEnum.A);
+ print(MyEnum.B);
+ print(MyEnum.C);
+ print(MyEnum.D);
+ }
+
+ private static void print(MyEnum e) {
+ switch (e) {
+ case A:
+ System.out.println("a");
+ break;
+ case B:
+ System.out.println("b");
+ break;
+ default:
+ System.out.println("0");
+ }
+ }
+ }
+}
