Merge "Start looking for specific code patterns in the enqueuer"
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 dcf29a4..2c3a656 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -109,6 +109,7 @@
public final DexString getClassMethodName = createString("getClass");
public final DexString ordinalMethodName = createString("ordinal");
public final DexString desiredAssertionStatusMethodName = createString("desiredAssertionStatus");
+ public final DexString forNameMethodName = createString("forName");
public final DexString getNameName = createString("getName");
public final DexString getSimpleNameName = createString("getSimpleName");
public final DexString assertionsDisabled = createString("$assertionsDisabled");
@@ -246,12 +247,15 @@
public class ClassMethods {
public DexMethod desiredAssertionStatus;
+ public DexMethod forName;
public DexMethod getName;
public DexMethod getSimpleName;
private ClassMethods() {
desiredAssertionStatus = createMethod(classDescriptor,
desiredAssertionStatusMethodName, booleanDescriptor, DexString.EMPTY_ARRAY);
+ forName = createMethod(classDescriptor,
+ forNameMethodName, classDescriptor, new DexString[]{stringDescriptor});
getName = createMethod(classDescriptor, getNameName, stringDescriptor, DexString.EMPTY_ARRAY);
getSimpleName = createMethod(classDescriptor,
getSimpleNameName, stringDescriptor, DexString.EMPTY_ARRAY);
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 ad00c86..2d0ddb8 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking;
+import com.android.tools.r8.ApiLevelException;
import com.android.tools.r8.dex.IndexedItemCollection;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
@@ -28,8 +29,12 @@
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.KeyedDexItem;
import com.android.tools.r8.graph.PresortedComparable;
+import com.android.tools.r8.ir.code.ConstString;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
+import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableMap;
@@ -144,6 +149,12 @@
private Queue<Action> proguardCompatibilityWorkList = Queues.newArrayDeque();
/**
+ * A set of methods that need code inspection for Proguard compatibility rules.
+ */
+ private final Set<DexEncodedMethod> pendingProguardReflectiveCompatibility =
+ Sets.newLinkedHashSet();
+
+ /**
* A cache for DexMethod that have been marked reachable.
*/
private Set<DexMethod> virtualTargetsMarkedAsReachable = Sets.newIdentityHashSet();
@@ -237,6 +248,10 @@
@Override
public boolean registerInvokeStatic(DexMethod method) {
+ if (options.forceProguardCompatibility &&
+ method == appInfo.dexItemFactory.classMethods.forName) {
+ pendingProguardReflectiveCompatibility.add(currentMethod);
+ }
if (!registerItemWithTarget(staticInvokes, method)) {
return false;
}
@@ -381,13 +396,7 @@
// For Proguard compatibility keep the default initializer for live types.
if (options.forceProguardCompatibility) {
if (holder.isProgramClass() && holder.hasDefaultInitializer()) {
- DexEncodedMethod init = holder.getDefaultInitializer();
- ProguardKeepRule rule =
- ProguardConfigurationUtils.buildDefaultInitializerKeepRule(holder);
- proguardCompatibilityWorkList.add(
- Action.markInstantiated(holder, KeepReason.dueToProguardCompatibilityKeepRule(rule)));
- proguardCompatibilityWorkList.add(
- Action.markMethodLive(init, KeepReason.dueToProguardCompatibilityKeepRule(rule)));
+ markClassAsInstantiatedWithCompatRule(holder);
}
}
}
@@ -870,11 +879,14 @@
}
// Continue fix-point processing while there are additional work items to ensure
// Proguard compatibility.
- if (proguardCompatibilityWorkList.isEmpty()) {
+ if (proguardCompatibilityWorkList.isEmpty() &&
+ pendingProguardReflectiveCompatibility.isEmpty()) {
break;
}
+ pendingProguardReflectiveCompatibility.forEach(this::handleProguardReflectiveBehavior);
workList.addAll(proguardCompatibilityWorkList);
proguardCompatibilityWorkList.clear();
+ pendingProguardReflectiveCompatibility.clear();
}
if (Log.ENABLED) {
Set<DexEncodedMethod> allLive = Sets.newIdentityHashSet();
@@ -1049,6 +1061,49 @@
collectReachedFields(staticFieldsWritten, this::tryLookupStaticField)));
}
+ private void markClassAsInstantiatedWithCompatRule(DexClass clazz) {
+ ProguardKeepRule rule =
+ ProguardConfigurationUtils.buildDefaultInitializerKeepRule(clazz);
+ proguardCompatibilityWorkList.add(
+ Action.markInstantiated(clazz, KeepReason.dueToProguardCompatibilityKeepRule(rule)));
+ if (clazz.hasDefaultInitializer()) {
+ proguardCompatibilityWorkList.add(
+ Action.markMethodLive(
+ clazz.getDefaultInitializer(), KeepReason.dueToProguardCompatibilityKeepRule(rule)));
+ }
+ }
+
+ private void handleProguardReflectiveBehavior(DexEncodedMethod method) {
+ try {
+ IRCode code = method.buildIR(options);
+ code.instructionIterator().forEachRemaining(this::handleProguardReflectiveBehavior);
+ } catch (ApiLevelException e) {
+ // Ignore this exception here. It will be hit again further in the pipeline when
+ // generating code.
+ }
+ }
+
+ private void handleProguardReflectiveBehavior(Instruction instruction) {
+ // Handle actual invokes of Class.forName(className) where className is a const string
+ // representing a known class as an implicit keep rule on that class.
+ if (instruction.isInvokeStatic()
+ && (instruction.asInvokeStatic().getInvokedMethod() ==
+ appInfo.dexItemFactory.classMethods.forName)
+ && instruction.asInvokeStatic().arguments().get(0).isConstant()) {
+ assert instruction.asInvokeStatic().arguments().get(0).getConstInstruction().isConstString();
+ ConstString constString =
+ instruction.asInvokeStatic().arguments().get(0).getConstInstruction().asConstString();
+ DexString forNameArgument = constString.getValue();
+ DexString forNameDescriptor = appInfo.dexItemFactory.createString(
+ DescriptorUtils.javaTypeToDescriptor(forNameArgument.toString()));
+ DexType forNameType = appInfo.dexItemFactory.createType(forNameDescriptor);
+ DexClass forNameClass = appInfo.definitionFor(forNameType);
+ if (forNameClass != null) {
+ markClassAsInstantiatedWithCompatRule(forNameClass);
+ }
+ }
+ }
+
private static class Action {
final Kind kind;
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java
index e00d36a..9fea981 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationUtils.java
@@ -18,11 +18,13 @@
ProguardClassNameList.Builder classNameListBuilder = ProguardClassNameList.builder();
classNameListBuilder.addClassName(false, ProguardTypeMatcher.create(clazz.type));
builder.setClassNames(classNameListBuilder.build());
- ProguardMemberRule.Builder memberRuleBuilder = ProguardMemberRule.builder();
- memberRuleBuilder.setRuleType(ProguardMemberType.INIT);
- memberRuleBuilder.setName("<init>");
- memberRuleBuilder.setArguments(ImmutableList.of());
- builder.getMemberRules().add(memberRuleBuilder.build());
+ if (clazz.hasDefaultInitializer()) {
+ ProguardMemberRule.Builder memberRuleBuilder = ProguardMemberRule.builder();
+ memberRuleBuilder.setRuleType(ProguardMemberType.INIT);
+ memberRuleBuilder.setName("<init>");
+ memberRuleBuilder.setArguments(ImmutableList.of());
+ builder.getMemberRules().add(memberRuleBuilder.build());
+ }
return builder.build();
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
index 62ca811..a920ec7 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.compatproguard.CompatProguardCommandBuilder;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.shaking.ProguardClassNameList;
import com.android.tools.r8.shaking.ProguardConfiguration;
@@ -204,4 +205,78 @@
testCheckCast(
false, TestMainWithoutCheckCast.class, TestClassWithDefaultConstructor.class, false);
}
+
+ public void testClassForName(boolean forceProguardCompatibility) throws Exception {
+ CompatProguardCommandBuilder builder =
+ new CompatProguardCommandBuilder(forceProguardCompatibility, false);
+ Class mainClass = TestMainWithClassForName.class;
+ Class forNameClass1 = TestClassWithDefaultConstructor.class;
+ Class forNameClass2 = TestClassWithoutDefaultConstructor.class;
+ List<Class> forNameClasses = ImmutableList.of(forNameClass1, forNameClass2);
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(mainClass));
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(forNameClass1));
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(forNameClass2));
+ List<String> proguardConfig = ImmutableList.of(
+ "-keep class " + mainClass.getCanonicalName() + " {",
+ " <init>();", // Add <init>() so it does not become a compatibility rule below.
+ " public static void main(java.lang.String[]);",
+ "}",
+ "-dontobfuscate");
+ builder.addProguardConfiguration(proguardConfig, Origin.unknown());
+ Path proguardCompatibilityRules = temp.newFile().toPath();
+ builder.setProguardCompatibilityRulesOutput(proguardCompatibilityRules);
+
+ DexInspector inspector = new DexInspector(ToolHelper.runR8(builder.build()));
+ assertTrue(inspector.clazz(getJavacGeneratedClassName(mainClass)).isPresent());
+ forNameClasses.forEach(clazz -> {
+ assertEquals(forceProguardCompatibility,
+ inspector.clazz(getJavacGeneratedClassName(clazz)).isPresent());
+ });
+
+ // Check the Proguard compatibility rules generated.
+ ProguardConfigurationParser parser =
+ new ProguardConfigurationParser(new DexItemFactory(), null);
+ parser.parse(proguardCompatibilityRules);
+ ProguardConfiguration configuration = parser.getConfigRawForTesting();
+ if (forceProguardCompatibility) {
+ assertEquals(2, configuration.getRules().size());
+ configuration.getRules().forEach(rule -> {
+ Set<ProguardMemberRule> memberRules = rule.getMemberRules();
+ ProguardClassNameList classNames = rule.getClassNames();
+ assertEquals(1, classNames.size());
+ DexType type = classNames.asSpecificDexTypes().get(0);
+ if (type.toSourceString().equals(forNameClass1.getCanonicalName())) {
+ assertEquals(1, memberRules.size());
+ assertEquals(ProguardMemberType.INIT, memberRules.iterator().next().getRuleType());
+ } else {
+ assertTrue(type.toSourceString().equals(forNameClass2.getCanonicalName()));
+ // During parsing we add in the default constructor if there are otherwise no single
+ // member rule.
+ assertEquals(1, memberRules.size());
+ assertEquals(ProguardMemberType.INIT, memberRules.iterator().next().getRuleType());
+ }
+ });
+ } else {
+ assertEquals(0, configuration.getRules().size());
+ }
+
+ if (RUN_PROGUARD) {
+ Path proguardedJar = File.createTempFile("proguarded", ".jar", temp.getRoot()).toPath();
+ Path proguardConfigFile = File.createTempFile("proguard", ".config", temp.getRoot()).toPath();
+ FileUtils.writeTextFile(proguardConfigFile, proguardConfig);
+ ToolHelper.runProguard(jarTestClasses(
+ ImmutableList.of(mainClass, forNameClass1, forNameClass2)),
+ proguardedJar, proguardConfigFile);
+ Set<String> classesAfterProguard = readClassesInJar(proguardedJar);
+ assertTrue(classesAfterProguard.contains(mainClass.getCanonicalName()));
+ assertTrue(classesAfterProguard.contains(forNameClass1.getCanonicalName()));
+ assertTrue(classesAfterProguard.contains(forNameClass2.getCanonicalName()));
+ }
+ }
+
+ @Test
+ public void classForNameTest() throws Exception {
+ testClassForName(true);
+ testClassForName(false);
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithClassForName.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithClassForName.java
new file mode 100644
index 0000000..bebcf94
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithClassForName.java
@@ -0,0 +1,18 @@
+// Copyright (c) 2017, 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.shaking.forceproguardcompatibility;
+
+public class TestMainWithClassForName {
+ private static void method1() throws Exception {
+ System.out.println(Class.forName(
+ "com.android.tools.r8.shaking.forceproguardcompatibility.TestClassWithoutDefaultConstructor"));
+ }
+
+ public static void main(String[] args) throws Exception {
+ System.out.println(Class.forName(
+ "com.android.tools.r8.shaking.forceproguardcompatibility.TestClassWithDefaultConstructor"));
+ TestMainWithClassForName.method1();
+ }
+}
