Version 2.0.69
Cherry pick: Guard for NPE when switch map and enum info is inconsistent
CL: https://r8-review.googlesource.com/c/r8/+/50477
Cherry pick: Reproduce NullPointerException during switch map optimization
CL: https://r8-review.googlesource.com/c/r8/+/50468
Bug: 154315490
Change-Id: I7f37d29668540f26e2076e7ee6d1405ee155ec4f
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index d386137..6f100e5 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.0.68";
+ public static final String LABEL = "2.0.69";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 0521d25..e1f01ff 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -1073,7 +1073,13 @@
int key = switchInsn.getKey(i);
DexField field = info.indexMap.get(key);
EnumValueInfo valueInfo = info.valueInfoMap.get(field);
- targetMap.put(valueInfo.ordinal, switchInsn.targetBlockIndices()[i]);
+ if (valueInfo != null) {
+ targetMap.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;
+ }
}
int[] keys = targetMap.keySet().toIntArray();
Arrays.sort(keys);
diff --git a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
index fd51973..f6cb711 100644
--- a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
@@ -333,9 +333,39 @@
return className.replace(DESCRIPTOR_PACKAGE_SEPARATOR, JAVA_PACKAGE_SEPARATOR);
}
- public static String getBinaryNameFromDescriptor(String classDescriptor) {
- assert isClassDescriptor(classDescriptor);
- return classDescriptor.substring(1, classDescriptor.length() - 1);
+ public static String getBinaryNameFromDescriptor(String descriptor) {
+ int numberOfLeadingSquareBrackets = getNumberOfLeadingSquareBrackets(descriptor);
+ if (numberOfLeadingSquareBrackets > 0) {
+ assert isClassDescriptor(descriptor.substring(numberOfLeadingSquareBrackets));
+ return descriptor.substring(0, numberOfLeadingSquareBrackets)
+ + descriptor.substring(numberOfLeadingSquareBrackets + 1, descriptor.length() - 1);
+ }
+ assert isClassDescriptor(descriptor);
+ return descriptor.substring(1, descriptor.length() - 1);
+ }
+
+ /**
+ * Convert an array or a class binary name to a descriptor.
+ *
+ * @param binaryName binary name i.e. "java/lang/Object" or "Ljava/lang/Object"
+ * @return a descriptor i.e. "Ljava/lang/Object;" or "[Ljava/lang/Object;
+ */
+ public static String getDescriptorFromArrayOrClassBinaryName(String binaryName) {
+ assert binaryName != null;
+ int numberOfLeadingSquareBrackets = getNumberOfLeadingSquareBrackets(binaryName);
+ if (numberOfLeadingSquareBrackets > 0) {
+ return binaryName.substring(0, numberOfLeadingSquareBrackets)
+ + getDescriptorFromClassBinaryName(binaryName.substring(numberOfLeadingSquareBrackets));
+ }
+ return getDescriptorFromClassBinaryName(binaryName);
+ }
+
+ private static int getNumberOfLeadingSquareBrackets(String string) {
+ int leadingSquareBrackets = 0;
+ while (string.charAt(leadingSquareBrackets) == '[') {
+ leadingSquareBrackets++;
+ }
+ return leadingSquareBrackets;
}
/**
@@ -346,7 +376,7 @@
*/
public static String getDescriptorFromClassBinaryName(String typeBinaryName) {
assert typeBinaryName != null;
- return ('L' + typeBinaryName + ';');
+ return 'L' + typeBinaryName + ';';
}
/**
diff --git a/src/main/java/com/android/tools/r8/utils/StringUtils.java b/src/main/java/com/android/tools/r8/utils/StringUtils.java
index fdf7e98..02e83de 100644
--- a/src/main/java/com/android/tools/r8/utils/StringUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/StringUtils.java
@@ -11,6 +11,8 @@
import java.util.Collection;
import java.util.List;
import java.util.function.Function;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
public class StringUtils {
public static char[] EMPTY_CHAR_ARRAY = {};
@@ -327,4 +329,8 @@
}
return string.length();
}
+
+ public static String replaceAll(String subject, String target, String replacement) {
+ return subject.replaceAll(Pattern.quote(target), Matcher.quoteReplacement(replacement));
+ }
}
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 3166939..ba45374 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -35,6 +35,7 @@
import com.android.tools.r8.graph.SmaliWriter;
import com.android.tools.r8.jasmin.JasminBuilder;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.references.ClassReference;
import com.android.tools.r8.references.FieldReference;
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.references.Reference;
@@ -233,6 +234,10 @@
return ClassFileTransformer.create(clazz);
}
+ public static ClassFileTransformer transformer(byte[] bytes, ClassReference classReference) {
+ return ClassFileTransformer.create(bytes, classReference);
+ }
+
// Actually running Proguard should only be during development.
private static final boolean RUN_PROGUARD = System.getProperty("run_proguard") != null;
// Actually running r8.jar in a forked process.
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 87b2806..c51142a 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -1043,6 +1043,11 @@
return paths;
}
+ public static Collection<Path> getClassFilesForInnerClasses(Class<?>... classes)
+ throws IOException {
+ return getClassFilesForInnerClasses(Arrays.asList(classes));
+ }
+
public static Path getFileNameForTestClass(Class clazz) {
List<String> parts = getNamePartsForTestClass(clazz);
return Paths.get("", parts.toArray(StringUtils.EMPTY_ARRAY));
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapWithMissingFieldTest.java b/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapWithMissingFieldTest.java
new file mode 100644
index 0000000..796b3d9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapWithMissingFieldTest.java
@@ -0,0 +1,98 @@
+// 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 static com.android.tools.r8.ToolHelper.getClassFilesForInnerClasses;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.references.ClassReference;
+import com.android.tools.r8.references.Reference;
+import java.io.IOException;
+import java.nio.file.Path;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * This tests that we gracefully handle the case where a switch map is incomplete, i.e., the
+ * corresponding enum has fields that are not present in the enum switch map.
+ */
+@RunWith(Parameterized.class)
+public class SwitchMapWithMissingFieldTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public SwitchMapWithMissingFieldTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramFiles(getSwitchMapProgramFile())
+ .addProgramClassFileData(
+ transformer(CompleteEnum.class)
+ .setClassDescriptor(descriptor(IncompleteEnum.class))
+ .transform())
+ .addKeepMainRule(TestClass.class)
+ .noMinification()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("B", "D");
+ }
+
+ private static ClassReference getSwitchMapClassReference() {
+ return Reference.classFromTypeName(SwitchMapWithMissingFieldTest.class.getTypeName() + "$1");
+ }
+
+ private static Path getSwitchMapProgramFile() throws IOException {
+ return getClassFilesForInnerClasses(SwitchMapWithMissingFieldTest.class).stream()
+ .filter(
+ file ->
+ file.toString().endsWith(getSwitchMapClassReference().getBinaryName() + ".class"))
+ .findFirst()
+ .get();
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ for (IncompleteEnum value : IncompleteEnum.values()) {
+ switch (value) {
+ case B:
+ System.out.println("B");
+ break;
+
+ case D:
+ System.out.println("D");
+ break;
+ }
+ }
+ }
+ }
+
+ enum CompleteEnum {
+ A,
+ B,
+ C,
+ D,
+ E;
+ }
+
+ enum IncompleteEnum {
+ B,
+ D;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapWithUnexpectedFieldTest.java b/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapWithUnexpectedFieldTest.java
new file mode 100644
index 0000000..0679ef6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/switches/SwitchMapWithUnexpectedFieldTest.java
@@ -0,0 +1,116 @@
+// 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 static com.android.tools.r8.ToolHelper.getClassFilesForInnerClasses;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.references.ClassReference;
+import com.android.tools.r8.references.Reference;
+import java.io.IOException;
+import java.nio.file.Path;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * This tests that we gracefully handle the case where a switch map refers to enum fields that are
+ * not present in the enum definition.
+ *
+ * <p>This situation may happen due to separate compilation. For example, a project may depend on a
+ * library that has been compiled against version X of a given enum but bundle version Y of the
+ * enum.
+ *
+ * <p>See also b/154315490.
+ */
+@RunWith(Parameterized.class)
+public class SwitchMapWithUnexpectedFieldTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public SwitchMapWithUnexpectedFieldTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramFiles(getSwitchMapProgramFile())
+ .addProgramClassFileData(
+ transformer(IncompleteEnum.class)
+ .setClassDescriptor(descriptor(CompleteEnum.class))
+ .transform())
+ .addKeepMainRule(TestClass.class)
+ .noMinification()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("B", "D");
+ }
+
+ private static ClassReference getSwitchMapClassReference() {
+ return Reference.classFromTypeName(SwitchMapWithUnexpectedFieldTest.class.getTypeName() + "$1");
+ }
+
+ private static Path getSwitchMapProgramFile() throws IOException {
+ return getClassFilesForInnerClasses(SwitchMapWithUnexpectedFieldTest.class).stream()
+ .filter(
+ file ->
+ file.toString().endsWith(getSwitchMapClassReference().getBinaryName() + ".class"))
+ .findFirst()
+ .get();
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ for (CompleteEnum value : CompleteEnum.values()) {
+ switch (value) {
+ case A:
+ System.out.println("A");
+ break;
+
+ case B:
+ System.out.println("B");
+ break;
+
+ case C:
+ System.out.println("C");
+ break;
+
+ case D:
+ System.out.println("D");
+ break;
+
+ case E:
+ System.out.println("E");
+ break;
+ }
+ }
+ }
+ }
+
+ enum CompleteEnum {
+ A,
+ B,
+ C,
+ D,
+ E;
+ }
+
+ enum IncompleteEnum {
+ B,
+ D;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
index ebe9ab2..0c8b4c1 100644
--- a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
+++ b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
@@ -3,6 +3,10 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.transformers;
+import static com.android.tools.r8.references.Reference.classFromTypeName;
+import static com.android.tools.r8.utils.DescriptorUtils.getBinaryNameFromDescriptor;
+import static com.android.tools.r8.utils.DescriptorUtils.getDescriptorFromArrayOrClassBinaryName;
+import static com.android.tools.r8.utils.StringUtils.replaceAll;
import static org.objectweb.asm.Opcodes.ASM7;
import com.android.tools.r8.TestRuntime.CfVm;
@@ -27,8 +31,10 @@
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Type;
public class ClassFileTransformer {
@@ -113,19 +119,21 @@
// Transformer utilities.
private final byte[] bytes;
+ private final ClassReference classReference;
private final List<ClassTransformer> classTransformers = new ArrayList<>();
private final List<MethodTransformer> methodTransformers = new ArrayList<>();
- private ClassFileTransformer(byte[] bytes) {
+ private ClassFileTransformer(byte[] bytes, ClassReference classReference) {
this.bytes = bytes;
+ this.classReference = classReference;
}
- public static ClassFileTransformer create(byte[] bytes) {
- return new ClassFileTransformer(bytes);
+ public static ClassFileTransformer create(byte[] bytes, ClassReference classReference) {
+ return new ClassFileTransformer(bytes, classReference);
}
public static ClassFileTransformer create(Class<?> clazz) throws IOException {
- return create(ToolHelper.getClassAsBytes(clazz));
+ return create(ToolHelper.getClassAsBytes(clazz), classFromTypeName(clazz.getTypeName()));
}
public byte[] transform() {
@@ -144,6 +152,10 @@
return this;
}
+ public ClassReference getClassReference() {
+ return classReference;
+ }
+
/** Unconditionally replace the implements clause of a class. */
public ClassFileTransformer setImplements(Class<?>... interfaces) {
return addClassTransformer(
@@ -170,27 +182,50 @@
}
/** Unconditionally replace the descriptor (ie, qualified name) of a class. */
- public ClassFileTransformer setClassDescriptor(String descriptor) {
- assert DescriptorUtils.isClassDescriptor(descriptor);
+ public ClassFileTransformer setClassDescriptor(String newDescriptor) {
+ assert DescriptorUtils.isClassDescriptor(newDescriptor);
+ String newBinaryName = getBinaryNameFromDescriptor(newDescriptor);
return addClassTransformer(
- new ClassTransformer() {
- @Override
- public void visit(
- int version,
- int access,
- String ignoredName,
- String signature,
- String superName,
- String[] interfaces) {
- super.visit(
- version,
- access,
- DescriptorUtils.getBinaryNameFromDescriptor(descriptor),
- signature,
- superName,
- interfaces);
- }
- });
+ new ClassTransformer() {
+ @Override
+ public void visit(
+ int version,
+ int access,
+ String binaryName,
+ String signature,
+ String superName,
+ String[] interfaces) {
+ super.visit(version, access, newBinaryName, signature, superName, interfaces);
+ }
+
+ @Override
+ public FieldVisitor visitField(
+ int access, String name, String descriptor, String signature, Object object) {
+ return super.visitField(
+ access,
+ name,
+ replaceAll(descriptor, getClassReference().getDescriptor(), newDescriptor),
+ signature,
+ object);
+ }
+
+ @Override
+ public MethodVisitor visitMethod(
+ int access,
+ String name,
+ String descriptor,
+ String signature,
+ String[] exceptions) {
+ return super.visitMethod(
+ access,
+ name,
+ replaceAll(descriptor, getClassReference().getDescriptor(), newDescriptor),
+ signature,
+ exceptions);
+ }
+ })
+ .replaceClassDescriptorInMethodInstructions(
+ getClassReference().getDescriptor(), newDescriptor);
}
public ClassFileTransformer setVersion(int newVersion) {
@@ -401,6 +436,66 @@
void apply(Label start, Label end, Label handler, String type);
}
+ public ClassFileTransformer replaceClassDescriptorInMethodInstructions(
+ String oldDescriptor, String newDescriptor) {
+ return addMethodTransformer(
+ new MethodTransformer() {
+ @Override
+ public void visitFieldInsn(int opcode, String owner, String name, String descriptor) {
+ super.visitFieldInsn(
+ opcode,
+ getBinaryNameFromDescriptor(
+ replaceAll(
+ getDescriptorFromArrayOrClassBinaryName(owner),
+ oldDescriptor,
+ newDescriptor)),
+ name,
+ replaceAll(descriptor, oldDescriptor, newDescriptor));
+ }
+
+ @Override
+ public void visitLdcInsn(Object value) {
+ if (value instanceof Type) {
+ Type type = (Type) value;
+ super.visitLdcInsn(
+ Type.getType(replaceAll(type.getDescriptor(), oldDescriptor, newDescriptor)));
+ } else {
+ super.visitLdcInsn(value);
+ }
+ }
+
+ @Override
+ public void visitMethodInsn(
+ int opcode, String owner, String name, String descriptor, boolean isInterface) {
+ super.visitMethodInsn(
+ opcode,
+ DescriptorUtils.isDescriptor(owner)
+ ? replaceAll(owner, oldDescriptor, newDescriptor)
+ : getBinaryNameFromDescriptor(
+ replaceAll(
+ getDescriptorFromArrayOrClassBinaryName(owner),
+ oldDescriptor,
+ newDescriptor)),
+ name,
+ replaceAll(descriptor, oldDescriptor, newDescriptor),
+ isInterface);
+ }
+
+ @Override
+ public void visitTypeInsn(int opcode, String type) {
+ super.visitTypeInsn(
+ opcode,
+ DescriptorUtils.isDescriptor(type)
+ ? replaceAll(type, oldDescriptor, newDescriptor)
+ : getBinaryNameFromDescriptor(
+ replaceAll(
+ getDescriptorFromArrayOrClassBinaryName(type),
+ oldDescriptor,
+ newDescriptor)));
+ }
+ });
+ }
+
public ClassFileTransformer transformMethodInsnInMethod(
String methodName, MethodInsnTransform transform) {
return addMethodTransformer(