Merge "Extend reflective analysis to include java.lang.Enum.valueOf"
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 90a098a..10e960c 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -217,6 +217,7 @@
public final LongMethods longMethods = new LongMethods();
public final ThrowableMethods throwableMethods = new ThrowableMethods();
public final ClassMethods classMethods = new ClassMethods();
+ public final EnumMethods enumMethods = new EnumMethods();
public final PrimitiveTypesBoxedTypeFields primitiveTypesBoxedTypeFields =
new PrimitiveTypesBoxedTypeFields();
public final AtomicFieldUpdaterMethods atomicFieldUpdaterMethods =
@@ -408,6 +409,20 @@
}
}
+ public class EnumMethods {
+
+ public DexMethod valueOf;
+
+ private EnumMethods() {
+ valueOf =
+ createMethod(
+ enumDescriptor,
+ valueOfMethodName,
+ enumDescriptor,
+ new DexString[] {classDescriptor, stringDescriptor});
+ }
+ }
+
/**
* All boxed types (Boolean, Byte, ...) have a field named TYPE which contains the Class object
* for the primitive type.
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index de42c18..0be9f91 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -68,6 +68,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -253,7 +254,6 @@
private void enqueueRootItems(Map<DexDefinition, ProguardKeepRule> items) {
items.entrySet().forEach(this::enqueueRootItem);
- pinnedItems.addAll(items.keySet());
}
private void enqueueRootItem(Entry<DexDefinition, ProguardKeepRule> root) {
@@ -261,7 +261,10 @@
}
private void enqueueRootItem(DexDefinition item, ProguardKeepRule rule) {
- KeepReason reason = KeepReason.dueToKeepRule(rule);
+ enqueueRootItem(item, KeepReason.dueToKeepRule(rule));
+ }
+
+ private void enqueueRootItem(DexDefinition item, KeepReason reason) {
if (item.isDexClass()) {
DexClass clazz = item.asDexClass();
workList.add(Action.markInstantiated(clazz, reason));
@@ -285,6 +288,7 @@
} else {
throw new IllegalArgumentException(item.toString());
}
+ pinnedItems.add(item);
}
private void enqueueHolderIfDependentNonStaticMember(
@@ -391,6 +395,10 @@
// Revisit the current method to implicitly add -keep rule for items with reflective access.
pendingReflectiveUses.add(currentMethod);
}
+ // See comment in handleJavaLangEnumValueOf.
+ if (method == appInfo.dexItemFactory.enumMethods.valueOf) {
+ pendingReflectiveUses.add(currentMethod);
+ }
if (!registerItemWithTarget(staticInvokes, method)) {
return false;
}
@@ -1150,6 +1158,25 @@
}
}
+ private DexMethod generatedEnumValuesMethod(DexClass enumClass) {
+ DexType arrayOfEnumClass =
+ appInfo.dexItemFactory.createType(
+ appInfo.dexItemFactory.createString("[" + enumClass.type.toDescriptorString()));
+ DexProto proto = appInfo.dexItemFactory.createProto(arrayOfEnumClass);
+ return appInfo.dexItemFactory.createMethod(
+ enumClass.type, proto, appInfo.dexItemFactory.createString("values"));
+ }
+
+ private void markEnumValuesAsReachable(DexClass clazz, KeepReason reason) {
+ DexEncodedMethod valuesMethod = clazz.lookupMethod(generatedEnumValuesMethod(clazz));
+ if (valuesMethod != null) {
+ // TODO(sgjesse): Does this have to be enqueued as a root item? Right now it is done as the
+ // marking of not renaming is in the root set.
+ enqueueRootItem(valuesMethod, reason);
+ rootSet.noObfuscation.add(valuesMethod);
+ }
+ }
+
private static void fillWorkList(Deque<DexType> worklist, DexType type) {
if (type.isInterface()) {
// We need to check if the method is shadowed by a class that directly implements
@@ -1534,15 +1561,23 @@
DexType originHolder = method.method.holder;
Origin origin = appInfo.originFor(originHolder);
IRCode code = method.buildIR(appInfo, appView.graphLense(), options, origin);
- code.instructionIterator().forEachRemaining(this::handleReflectiveBehavior);
+ Iterator<Instruction> iterator = code.instructionIterator();
+ while (iterator.hasNext()) {
+ Instruction instruction = iterator.next();
+ handleReflectiveBehavior(method, instruction);
+ }
}
- private void handleReflectiveBehavior(Instruction instruction) {
+ private void handleReflectiveBehavior(DexEncodedMethod method, Instruction instruction) {
if (!instruction.isInvokeMethod()) {
return;
}
InvokeMethod invoke = instruction.asInvokeMethod();
DexMethod invokedMethod = invoke.getInvokedMethod();
+ if (invokedMethod == appInfo.dexItemFactory.enumMethods.valueOf) {
+ handleJavaLangEnumValueOf(method, invoke);
+ return;
+ }
if (!isReflectionMethod(appInfo.dexItemFactory, invokedMethod)) {
return;
}
@@ -1577,6 +1612,20 @@
}
}
+ private void handleJavaLangEnumValueOf(DexEncodedMethod method, InvokeMethod invoke) {
+ // The use of java.lang.Enum.valueOf(java.lang.Class, java.lang.String) will indirectly
+ // access the values() method of the enum class passed as the first argument. The method
+ // SomeEnumClass.valueOf(java.lang.String) which is generated by javac for all enums will
+ // call this method.
+ if (invoke.inValues().get(0).isConstClass()) {
+ DexClass clazz =
+ appInfo.definitionFor(invoke.inValues().get(0).definition.asConstClass().getValue());
+ if (clazz.accessFlags.isEnum() && clazz.superType == appInfo.dexItemFactory.enumType) {
+ markEnumValuesAsReachable(clazz, KeepReason.invokedFrom(method));
+ }
+ }
+ }
+
private static class Action {
final Kind kind;
diff --git a/src/test/java/com/android/tools/r8/naming/EnumMinification.java b/src/test/java/com/android/tools/r8/naming/EnumMinification.java
index a8708eb..66cd45e 100644
--- a/src/test/java/com/android/tools/r8/naming/EnumMinification.java
+++ b/src/test/java/com/android/tools/r8/naming/EnumMinification.java
@@ -4,17 +4,19 @@
package com.android.tools.r8.naming;
-import static org.hamcrest.CoreMatchers.containsString;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ToolHelper.DexVm;
-import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.google.common.collect.ImmutableList;
import java.util.Arrays;
import java.util.Collection;
@@ -22,6 +24,12 @@
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.Type;
@RunWith(Parameterized.class)
public class EnumMinification extends TestBase {
@@ -37,29 +45,57 @@
this.backend = backend;
}
+ private AndroidApp buildApp(Class<?> mainClass, byte[] enumClassFile) throws Exception {
+ return ToolHelper.runR8(
+ R8Command.builder()
+ .addClassProgramData(ToolHelper.getClassAsBytes(mainClass), Origin.unknown())
+ .addClassProgramData(enumClassFile, Origin.unknown())
+ .addProguardConfiguration(
+ ImmutableList.of(keepMainProguardConfiguration(mainClass)), Origin.unknown())
+ .setProgramConsumer(emptyConsumer(backend))
+ .build());
+ }
+
+ public void runTest(
+ Class<?> mainClass, byte[] enumClass, String enumTypeName, boolean valueOfKept)
+ throws Exception {
+ AndroidApp output = buildApp(mainClass, enumClass);
+
+ CodeInspector inspector = new CodeInspector(output);
+ ClassSubject clazz = inspector.clazz(enumTypeName);
+ // The class and fields - including field $VALUES and method valueOf - can be renamed. Only
+ // the values() method needs to be
+ assertThat(clazz, isRenamed());
+ assertThat(clazz.field(enumTypeName, "VALUE1"), isRenamed());
+ assertThat(clazz.field(enumTypeName, "VALUE2"), isRenamed());
+ assertThat(clazz.field(enumTypeName + "[]", "$VALUES"), isRenamed());
+ assertThat(
+ clazz.method(enumTypeName, "valueOf", ImmutableList.of("java.lang.String")),
+ valueOfKept ? isRenamed() : not(isPresent()));
+ assertThat(clazz.method(enumTypeName + "[]", "values", ImmutableList.of()), not(isRenamed()));
+
+ assertEquals("VALUE1", runOnVM(output, mainClass, backend));
+ }
+
@Test
public void test() throws Exception {
- AndroidApp output =
- ToolHelper.runR8(
- R8Command.builder()
- .addClassProgramData(ToolHelper.getClassAsBytes(Main.class), Origin.unknown())
- .addClassProgramData(ToolHelper.getClassAsBytes(Enum.class), Origin.unknown())
- .addProguardConfiguration(
- ImmutableList.of(keepMainProguardConfiguration(Main.class)), Origin.unknown())
- .setProgramConsumer(emptyConsumer(backend))
- .build());
+ runTest(Main.class, ToolHelper.getClassAsBytes(Enum.class), Enum.class.getTypeName(), true);
+ }
- // TODO(117299356): valueOf on enum fails for minified enums.
- ProcessResult result = runOnVMRaw(output, Main.class, backend);
- assertEquals(1, result.exitCode);
- assertThat(
- result.stderr,
- containsString(
- backend == Backend.DEX
- ? ToolHelper.getDexVm().isNewerThan(DexVm.ART_4_4_4_HOST)
- ? "java.lang.NoSuchMethodException"
- : "java.lang.NullPointerException"
- : "java.lang.IllegalArgumentException"));
+ @Test
+ public void testAsmDump() throws Exception {
+ runTest(Main.class, EnumDump.dump(true), "com.android.tools.r8.naming.Enum", true);
+ }
+
+ @Test
+ public void testWithoutValuesMethod() throws Exception {
+ // This should not fail even if the values method is not present.
+ buildApp(Main.class, EnumDump.dump(false));
+ }
+
+ @Test
+ public void testJavaLangEnumValueOf() throws Exception {
+ runTest(Main2.class, ToolHelper.getClassAsBytes(Enum.class), Enum.class.getTypeName(), false);
}
}
@@ -67,7 +103,7 @@
public static void main(String[] args) {
Enum e = Enum.valueOf("VALUE1");
- System.out.println(e);
+ System.out.print(e);
}
}
@@ -75,3 +111,219 @@
VALUE1,
VALUE2
}
+
+class Main2 {
+ public static void main(String[] args) {
+ // Use java.lang.Enum.valueOf instead of com.android.tools.r8.naming.Enum.valueOf.
+ System.out.print(java.lang.Enum.valueOf(Enum.class, "VALUE1"));
+ }
+}
+/* Dump of javac generated code from the following enum class (the one just above):
+ *
+ * package com.android.tools.r8.naming;
+ *
+ * enum Enum {
+ * VALUE1,
+ * VALUE2
+ * }
+ *
+ */
+class EnumDump implements Opcodes {
+
+ public static byte[] dump(boolean includeValuesMethod) {
+ ClassWriter classWriter = new ClassWriter(0);
+ FieldVisitor fieldVisitor;
+ MethodVisitor methodVisitor;
+
+ classWriter.visit(
+ V1_8,
+ ACC_FINAL | ACC_SUPER | ACC_ENUM,
+ "com/android/tools/r8/naming/Enum",
+ "Ljava/lang/Enum<Lcom/android/tools/r8/naming/Enum;>;",
+ "java/lang/Enum",
+ null);
+
+ classWriter.visitSource("EnumMinification.java", null);
+
+ {
+ fieldVisitor =
+ classWriter.visitField(
+ ACC_PUBLIC | ACC_FINAL | ACC_STATIC | ACC_ENUM,
+ "VALUE1",
+ "Lcom/android/tools/r8/naming/Enum;",
+ null,
+ null);
+ fieldVisitor.visitEnd();
+ }
+ {
+ fieldVisitor =
+ classWriter.visitField(
+ ACC_PUBLIC | ACC_FINAL | ACC_STATIC | ACC_ENUM,
+ "VALUE2",
+ "Lcom/android/tools/r8/naming/Enum;",
+ null,
+ null);
+ fieldVisitor.visitEnd();
+ }
+ {
+ fieldVisitor =
+ classWriter.visitField(
+ ACC_PRIVATE | ACC_FINAL | ACC_STATIC | ACC_SYNTHETIC,
+ "$VALUES",
+ "[Lcom/android/tools/r8/naming/Enum;",
+ null,
+ null);
+ fieldVisitor.visitEnd();
+ }
+ if (includeValuesMethod) {
+ {
+ methodVisitor =
+ classWriter.visitMethod(
+ ACC_PUBLIC | ACC_STATIC,
+ "values",
+ "()[Lcom/android/tools/r8/naming/Enum;",
+ null,
+ null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(72, label0);
+ methodVisitor.visitFieldInsn(
+ GETSTATIC,
+ "com/android/tools/r8/naming/Enum",
+ "$VALUES",
+ "[Lcom/android/tools/r8/naming/Enum;");
+ methodVisitor.visitMethodInsn(
+ INVOKEVIRTUAL,
+ "[Lcom/android/tools/r8/naming/Enum;",
+ "clone",
+ "()Ljava/lang/Object;",
+ false);
+ methodVisitor.visitTypeInsn(CHECKCAST, "[Lcom/android/tools/r8/naming/Enum;");
+ methodVisitor.visitInsn(ARETURN);
+ methodVisitor.visitMaxs(1, 0);
+ methodVisitor.visitEnd();
+ }
+ }
+ {
+ methodVisitor =
+ classWriter.visitMethod(
+ ACC_PUBLIC | ACC_STATIC,
+ "valueOf",
+ "(Ljava/lang/String;)Lcom/android/tools/r8/naming/Enum;",
+ null,
+ null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(72, label0);
+ methodVisitor.visitLdcInsn(Type.getType("Lcom/android/tools/r8/naming/Enum;"));
+ methodVisitor.visitVarInsn(ALOAD, 0);
+ methodVisitor.visitMethodInsn(
+ INVOKESTATIC,
+ "java/lang/Enum",
+ "valueOf",
+ "(Ljava/lang/Class;Ljava/lang/String;)Ljava/lang/Enum;",
+ false);
+ methodVisitor.visitTypeInsn(CHECKCAST, "com/android/tools/r8/naming/Enum");
+ methodVisitor.visitInsn(ARETURN);
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLocalVariable("name", "Ljava/lang/String;", null, label0, label1, 0);
+ methodVisitor.visitMaxs(2, 1);
+ methodVisitor.visitEnd();
+ }
+ {
+ methodVisitor =
+ classWriter.visitMethod(ACC_PRIVATE, "<init>", "(Ljava/lang/String;I)V", "()V", null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(72, label0);
+ methodVisitor.visitVarInsn(ALOAD, 0);
+ methodVisitor.visitVarInsn(ALOAD, 1);
+ methodVisitor.visitVarInsn(ILOAD, 2);
+ methodVisitor.visitMethodInsn(
+ INVOKESPECIAL, "java/lang/Enum", "<init>", "(Ljava/lang/String;I)V", false);
+ methodVisitor.visitInsn(RETURN);
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLocalVariable(
+ "this", "Lcom/android/tools/r8/naming/Enum;", null, label0, label1, 0);
+ methodVisitor.visitMaxs(3, 3);
+ methodVisitor.visitEnd();
+ }
+ {
+ methodVisitor = classWriter.visitMethod(ACC_STATIC, "<clinit>", "()V", null, null);
+ methodVisitor.visitCode();
+ Label label0 = new Label();
+ methodVisitor.visitLabel(label0);
+ methodVisitor.visitLineNumber(73, label0);
+ methodVisitor.visitTypeInsn(NEW, "com/android/tools/r8/naming/Enum");
+ methodVisitor.visitInsn(DUP);
+ methodVisitor.visitLdcInsn("VALUE1");
+ methodVisitor.visitInsn(ICONST_0);
+ methodVisitor.visitMethodInsn(
+ INVOKESPECIAL,
+ "com/android/tools/r8/naming/Enum",
+ "<init>",
+ "(Ljava/lang/String;I)V",
+ false);
+ methodVisitor.visitFieldInsn(
+ PUTSTATIC,
+ "com/android/tools/r8/naming/Enum",
+ "VALUE1",
+ "Lcom/android/tools/r8/naming/Enum;");
+ Label label1 = new Label();
+ methodVisitor.visitLabel(label1);
+ methodVisitor.visitLineNumber(74, label1);
+ methodVisitor.visitTypeInsn(NEW, "com/android/tools/r8/naming/Enum");
+ methodVisitor.visitInsn(DUP);
+ methodVisitor.visitLdcInsn("VALUE2");
+ methodVisitor.visitInsn(ICONST_1);
+ methodVisitor.visitMethodInsn(
+ INVOKESPECIAL,
+ "com/android/tools/r8/naming/Enum",
+ "<init>",
+ "(Ljava/lang/String;I)V",
+ false);
+ methodVisitor.visitFieldInsn(
+ PUTSTATIC,
+ "com/android/tools/r8/naming/Enum",
+ "VALUE2",
+ "Lcom/android/tools/r8/naming/Enum;");
+ Label label2 = new Label();
+ methodVisitor.visitLabel(label2);
+ methodVisitor.visitLineNumber(72, label2);
+ methodVisitor.visitInsn(ICONST_2);
+ methodVisitor.visitTypeInsn(ANEWARRAY, "com/android/tools/r8/naming/Enum");
+ methodVisitor.visitInsn(DUP);
+ methodVisitor.visitInsn(ICONST_0);
+ methodVisitor.visitFieldInsn(
+ GETSTATIC,
+ "com/android/tools/r8/naming/Enum",
+ "VALUE1",
+ "Lcom/android/tools/r8/naming/Enum;");
+ methodVisitor.visitInsn(AASTORE);
+ methodVisitor.visitInsn(DUP);
+ methodVisitor.visitInsn(ICONST_1);
+ methodVisitor.visitFieldInsn(
+ GETSTATIC,
+ "com/android/tools/r8/naming/Enum",
+ "VALUE2",
+ "Lcom/android/tools/r8/naming/Enum;");
+ methodVisitor.visitInsn(AASTORE);
+ methodVisitor.visitFieldInsn(
+ PUTSTATIC,
+ "com/android/tools/r8/naming/Enum",
+ "$VALUES",
+ "[Lcom/android/tools/r8/naming/Enum;");
+ methodVisitor.visitInsn(RETURN);
+ methodVisitor.visitMaxs(4, 0);
+ methodVisitor.visitEnd();
+ }
+ classWriter.visitEnd();
+
+ return classWriter.toByteArray();
+ }
+}
