Special handling of referenced classes in Proguard compatibility mode
Keep classes and their default constructor for classes referenced when running
with --force-proguard-compatibility for the compat proguard command if the
class actually does have a default constructor.
This covers the special handling of const-class and include classes used in
check-cast.
For now the keep reason will be "invoked by the default constructor", which is
somewhat misleading, but until we know to what xtend this is actually needed
this was the simpel solution.
Bug: 68754879
Bug: 68689582
Change-Id: I6761fcd50156b19248c080ef8e57e43bcc570f15
Bug: 68754879
diff --git a/src/main/java/com/android/tools/r8/code/ConstClass.java b/src/main/java/com/android/tools/r8/code/ConstClass.java
index ef8b257..f140022 100644
--- a/src/main/java/com/android/tools/r8/code/ConstClass.java
+++ b/src/main/java/com/android/tools/r8/code/ConstClass.java
@@ -39,7 +39,7 @@
@Override
public void registerUse(UseRegistry registry) {
- registry.registerConstClass(getType());
+ registry.registerTypeReference(getType());
}
public DexType getType() {
diff --git a/src/main/java/com/android/tools/r8/graph/UseRegistry.java b/src/main/java/com/android/tools/r8/graph/UseRegistry.java
index f6736fa..78b2f56 100644
--- a/src/main/java/com/android/tools/r8/graph/UseRegistry.java
+++ b/src/main/java/com/android/tools/r8/graph/UseRegistry.java
@@ -65,8 +65,4 @@
throw new AssertionError();
}
}
-
- public boolean registerConstClass(DexType type) {
- return registerTypeReference(type);
- }
}
diff --git a/src/main/java/com/android/tools/r8/jar/JarRegisterEffectsVisitor.java b/src/main/java/com/android/tools/r8/jar/JarRegisterEffectsVisitor.java
index 46bcbd5..3321cb7 100644
--- a/src/main/java/com/android/tools/r8/jar/JarRegisterEffectsVisitor.java
+++ b/src/main/java/com/android/tools/r8/jar/JarRegisterEffectsVisitor.java
@@ -51,7 +51,7 @@
// Nothing to register for method type, it represents only a prototype not associated with a
// method name.
if (((Type) cst).getSort() != Type.METHOD) {
- registry.registerConstClass(application.getType((Type) cst));
+ registry.registerTypeReference(application.getType((Type) cst));
}
} else if (cst instanceof Handle) {
registerMethodHandleType((Handle) cst);
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 4c30d36..102accd 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -294,18 +294,7 @@
@Override
public boolean registerNewInstance(DexType type) {
- if (instantiatedTypes.contains(type)) {
- return false;
- }
- DexClass clazz = appInfo.definitionFor(type);
- if (clazz == null) {
- reportMissingClass(type);
- return false;
- }
- if (Log.ENABLED) {
- Log.verbose(getClass(), "Register new instatiation of `%s`.", clazz);
- }
- workList.add(Action.markInstantiated(clazz, KeepReason.instantiatedIn(currentMethod)));
+ markInstantiated(type, currentMethod);
return true;
}
@@ -343,30 +332,6 @@
}
return false;
}
-
- @Override
- public boolean registerConstClass(DexType type) {
- boolean result = registerTypeReference(type);
- // For Proguard compatibility mark default initializer for live type as live.
- if (options.forceProguardCompatibility) {
- if (type.isArrayType()) {
- return result;
- }
- assert type.isClassType();
- DexClass holder = appInfo.definitionFor(type);
- if (holder == null) {
- // Don't call reportMissingClass(type) here. That already happened in the call to
- // registerTypeReference(type).
- return result;
- }
- if (holder.hasDefaultInitializer()) {
- registerNewInstance(type);
- DexEncodedMethod init = holder.getDefaultInitializer();
- markDirectStaticOrConstructorMethodAsLive(init, KeepReason.reachableFromLiveType(type));
- }
- }
- return result;
- }
}
//
@@ -410,6 +375,9 @@
if (options.forceProguardCompatibility) {
if (holder.hasDefaultInitializer()) {
DexEncodedMethod init = holder.getDefaultInitializer();
+ // TODO(68246915): For now just register the default constructor as the source of
+ // instantiation.
+ markInstantiated(type, init);
markDirectStaticOrConstructorMethodAsLive(init, KeepReason.reachableFromLiveType(type));
}
}
@@ -587,6 +555,21 @@
enqueueRootItems(rootSet.getDependentItems(field));
}
+ private void markInstantiated(DexType type, DexEncodedMethod method) {
+ if (instantiatedTypes.contains(type)) {
+ return;
+ }
+ DexClass clazz = appInfo.definitionFor(type);
+ if (clazz == null) {
+ reportMissingClass(type);
+ return;
+ }
+ if (Log.ENABLED) {
+ Log.verbose(getClass(), "Register new instatiation of `%s`.", clazz);
+ }
+ workList.add(Action.markInstantiated(clazz, KeepReason.instantiatedIn(method)));
+ }
+
private void markDirectStaticOrConstructorMethodAsLive(
DexEncodedMethod encodedMethod, KeepReason reason) {
assert encodedMethod != null;
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index ea51c65..2128663 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -26,11 +26,15 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.Arrays;
+import java.util.Enumeration;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
import java.util.jar.JarOutputStream;
import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
import org.junit.Rule;
import org.junit.rules.TemporaryFolder;
@@ -93,6 +97,25 @@
}
/**
+ * Read the names of classes in a jar.
+ */
+ protected Set<String> readClassesInJar(Path jar) throws IOException {
+ Set<String> result = new HashSet<>();
+ try (ZipFile zipFile = new ZipFile(jar.toFile())) {
+ final Enumeration<? extends ZipEntry> entries = zipFile.entries();
+ while (entries.hasMoreElements()) {
+ ZipEntry entry = entries.nextElement();
+ String name = entry.getName();
+ if (name.endsWith(".class")) {
+ result.add(name.substring(0, name.length() - ".class".length()).replace('/', '.'));
+ }
+ }
+ }
+ return result;
+ }
+
+
+ /**
* Get the class name generated by javac.
*/
protected String getJavacGeneratedClassName(Class clazz) {
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 5753d84..8444ad1 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -970,7 +970,7 @@
}
}
- public static void runProguard(Path inJar, Path outJar, Path config) throws IOException {
+ public static String runProguard(Path inJar, Path outJar, Path config) throws IOException {
List<String> command = new ArrayList<>();
command.add(PROGUARD);
command.add("-forceprocessing"); // Proguard just checks the creation time on the in/out jars.
@@ -987,6 +987,7 @@
if (result.exitCode != 0) {
fail("Proguard failed, exit code " + result.exitCode + ", stderr:\n" + result.stderr);
}
+ return result.stdout;
}
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 dae1163..30d9111 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
@@ -20,6 +20,7 @@
import java.io.File;
import java.nio.file.Path;
import java.util.List;
+import java.util.Set;
import org.junit.Test;
public class ForceProguardCompatibilityTest extends TestBase {
@@ -125,4 +126,50 @@
runDefaultConstructorTest(false, TestClassWithDefaultConstructor.class, true);
runDefaultConstructorTest(false, TestClassWithoutDefaultConstructor.class, false);
}
+
+ public void testCheckCast(boolean forceProguardCompatibility, Class mainClass,
+ Class instantiatedClass, boolean containsCheckCast)
+ throws Exception {
+ R8Command.Builder builder = new CompatProguardCommandBuilder(forceProguardCompatibility, false);
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(mainClass));
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(instantiatedClass));
+ List<String> proguardConfig = ImmutableList.of(
+ "-keep class " + mainClass.getCanonicalName() + " {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ "-dontobfuscate");
+ builder.addProguardConfiguration(proguardConfig);
+
+ DexInspector inspector = new DexInspector(ToolHelper.runR8(builder.build()));
+ assertTrue(inspector.clazz(getJavacGeneratedClassName(mainClass)).isPresent());
+ ClassSubject clazz = inspector.clazz(getJavacGeneratedClassName(instantiatedClass));
+ assertEquals(containsCheckCast, clazz.isPresent());
+ assertEquals(containsCheckCast, clazz.isPresent());
+ if (clazz.isPresent()) {
+ assertEquals(forceProguardCompatibility && containsCheckCast, !clazz.isAbstract());
+ }
+
+ 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, instantiatedClass)),
+ proguardedJar, proguardConfigFile);
+ Set<String> classesAfterProguard = readClassesInJar(proguardedJar);
+ assertTrue(classesAfterProguard.contains(mainClass.getCanonicalName()));
+ assertEquals(
+ containsCheckCast, classesAfterProguard.contains(instantiatedClass.getCanonicalName()));
+ }
+ }
+
+ @Test
+ public void checkCastTest() throws Exception {
+ testCheckCast(true, TestMainWithCheckCast.class, TestClassWithDefaultConstructor.class, true);
+ testCheckCast(
+ true, TestMainWithoutCheckCast.class, TestClassWithDefaultConstructor.class, false);
+ testCheckCast(
+ false, TestMainWithCheckCast.class, TestClassWithDefaultConstructor.class, true);
+ testCheckCast(
+ false, TestMainWithoutCheckCast.class, TestClassWithDefaultConstructor.class, false);
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithCheckCast.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithCheckCast.java
new file mode 100644
index 0000000..d1eba28
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithCheckCast.java
@@ -0,0 +1,15 @@
+// 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 TestMainWithCheckCast {
+
+ public static void main(String[] args) throws Exception {
+ String thisPackage = TestMainWithCheckCast.class.getPackage().getName();
+ Object o = (TestClassWithDefaultConstructor)
+ Class.forName(thisPackage + ".TestClassWithDefaultConstructor").newInstance();
+ System.out.println("Instance of: " + o.getClass().getCanonicalName());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithoutCheckCast.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithoutCheckCast.java
new file mode 100644
index 0000000..1ba2549
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMainWithoutCheckCast.java
@@ -0,0 +1,14 @@
+// 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 TestMainWithoutCheckCast {
+
+ public static void main(String[] args) throws Exception {
+ String thisPackage = TestMainWithoutCheckCast.class.getPackage().getName();
+ Object o = Class.forName(thisPackage + ".TestClassWithDefaultConstructor").newInstance();
+ System.out.println("Instantiated " + o.getClass().getCanonicalName());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java b/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
index 8bff0b3..308ce69 100644
--- a/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
@@ -24,21 +24,6 @@
public class IncludeDescriptorClassesTest extends TestBase {
- private Set<String> readJarClasses(Path jar) throws IOException {
- Set<String> result = new HashSet<>();
- try (ZipFile zipFile = new ZipFile(jar.toFile())) {
- final Enumeration<? extends ZipEntry> entries = zipFile.entries();
- while (entries.hasMoreElements()) {
- ZipEntry entry = entries.nextElement();
- String name = entry.getName();
- if (name.endsWith(".class")) {
- result.add(name.substring(0, name.length() - ".class".length()).replace('/', '.'));
- }
- }
- }
- return result;
- }
-
private class Result {
final DexInspector inspector;
final Set<String> classesAfterProguard;
@@ -92,7 +77,7 @@
if (false) {
Path proguardedJar = temp.newFile("proguarded.jar").toPath();
ToolHelper.runProguard(jarTestClasses(classes), proguardedJar, proguardConfig);
- classesAfterProguard = readJarClasses(proguardedJar);
+ classesAfterProguard = readClassesInJar(proguardedJar);
}
return new Result(inspector, classesAfterProguard);
