Mark field type as live.
We assume that a field type is equal to, or at least a super type of,
the type used to initialize that field. If the type relation is broken
for some reasons, that's not the case anymore. It is safer to mark the
type of field as live when we mark the field as live.
Bug: 78788577, 80206476, 80130830
Change-Id: I114601eb02d6b8f0802cd50805beb07a637266df
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 2765a85..7e483a7 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -786,6 +786,9 @@
// Mark the type live here, so that the class exists at runtime. Note that this also marks all
// supertypes as live, so even if the field is actually on a supertype, its class will be live.
markTypeAsLive(field.clazz);
+ if (field.type.isClassType()) {
+ markTypeAsLive(field.type);
+ }
// Find the actual field.
DexEncodedField encodedField = appInfo.resolveFieldOn(field.clazz, field);
if (encodedField == null) {
@@ -815,6 +818,9 @@
private void markInstanceFieldAsLive(DexEncodedField field, KeepReason reason) {
assert field != null;
markTypeAsLive(field.field.clazz);
+ if (field.field.type.isClassType()) {
+ markTypeAsLive(field.field.type);
+ }
if (Log.ENABLED) {
Log.verbose(getClass(), "Adding instance field `%s` to live set.", field.field);
}
diff --git a/src/test/examples/multidex005/ref-list-4-r8.txt b/src/test/examples/multidex005/ref-list-4-r8.txt
new file mode 100644
index 0000000..ea7b0f7
--- /dev/null
+++ b/src/test/examples/multidex005/ref-list-4-r8.txt
@@ -0,0 +1,8 @@
+Lmultidex005/DirectlyReferenced;
+Lmultidex005/FieldReference;
+Lmultidex005/Interface1;
+Lmultidex005/Interface2;
+Lmultidex005/Interface3;
+Lmultidex005/SuperClass;
+Lmultidex005/SuperInterface;
+Lmultidex005/SuperSuperClass;
\ No newline at end of file
diff --git a/src/test/examples/multidex005/ref-list-4.txt b/src/test/examples/multidex005/ref-list-4.txt
index ea7b0f7..7733fba 100644
--- a/src/test/examples/multidex005/ref-list-4.txt
+++ b/src/test/examples/multidex005/ref-list-4.txt
@@ -1,5 +1,8 @@
Lmultidex005/DirectlyReferenced;
Lmultidex005/FieldReference;
+Lmultidex005/IndirectlyReferenced;
+Lmultidex005/IndirectlyReferencedInterface;
+Lmultidex005/IndirectlyReferencedSuperClass;
Lmultidex005/Interface1;
Lmultidex005/Interface2;
Lmultidex005/Interface3;
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java
index 43b4c0f..374d58f 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.GenerateMainDexListCommand;
import com.android.tools.r8.OutputMode;
import com.android.tools.r8.R8Command;
+import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ir.desugar.LambdaRewriter;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -30,19 +31,15 @@
import java.util.function.Consumer;
import java.util.stream.Collectors;
import org.junit.Assert;
-import org.junit.Rule;
import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-public class MainDexTracingTest {
+public class MainDexTracingTest extends TestBase {
private static final String EXAMPLE_BUILD_DIR = ToolHelper.EXAMPLES_BUILD_DIR;
private static final String EXAMPLE_O_BUILD_DIR = ToolHelper.EXAMPLES_ANDROID_O_BUILD_DIR;
private static final String EXAMPLE_SRC_DIR = ToolHelper.EXAMPLES_DIR;
private static final String EXAMPLE_O_SRC_DIR = ToolHelper.EXAMPLES_ANDROID_O_DIR;
- @Rule public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
-
@Test
public void traceMainDexList001_whyareyoukeeping() throws Throwable {
PrintStream stdout = System.out;
@@ -54,6 +51,7 @@
EXAMPLE_BUILD_DIR,
Paths.get(EXAMPLE_SRC_DIR, "multidex", "main-dex-rules-whyareyoukeeping.txt"),
Paths.get(EXAMPLE_SRC_DIR, "multidex001", "ref-list-1.txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex001", "ref-list-1.txt"),
AndroidApiLevel.I);
String output = new String(baos.toByteArray(), Charset.defaultCharset());
Assert.assertTrue(output.contains("is live because referenced in keep rule:"));
@@ -68,6 +66,7 @@
EXAMPLE_BUILD_DIR,
Paths.get(EXAMPLE_SRC_DIR, "multidex", "main-dex-rules.txt"),
Paths.get(EXAMPLE_SRC_DIR, "multidex001", "ref-list-1.txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex001", "ref-list-1.txt"),
AndroidApiLevel.I);
}
@@ -79,6 +78,7 @@
EXAMPLE_BUILD_DIR,
Paths.get(EXAMPLE_SRC_DIR, "multidex001", "main-dex-rules-2.txt"),
Paths.get(EXAMPLE_SRC_DIR, "multidex001", "ref-list-2.txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex001", "ref-list-2.txt"),
AndroidApiLevel.I);
}
@@ -90,6 +90,7 @@
EXAMPLE_BUILD_DIR,
Paths.get(EXAMPLE_SRC_DIR, "multidex", "main-dex-rules.txt"),
Paths.get(EXAMPLE_SRC_DIR, "multidex002", "ref-list-1.txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex002", "ref-list-1.txt"),
AndroidApiLevel.I);
}
@@ -101,6 +102,7 @@
EXAMPLE_BUILD_DIR,
Paths.get(EXAMPLE_SRC_DIR, "multidex", "main-dex-rules.txt"),
Paths.get(EXAMPLE_SRC_DIR, "multidex003", "ref-list-1.txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex003", "ref-list-1.txt"),
AndroidApiLevel.I);
}
@@ -112,6 +114,7 @@
EXAMPLE_O_BUILD_DIR,
Paths.get(EXAMPLE_SRC_DIR, "multidex", "main-dex-rules.txt"),
Paths.get(EXAMPLE_O_SRC_DIR, "multidex004", "ref-list-1.txt"),
+ Paths.get(EXAMPLE_O_SRC_DIR, "multidex004", "ref-list-1.txt"),
AndroidApiLevel.I);
}
@@ -132,7 +135,14 @@
@Test
public void traceMainDexList005_4() throws Throwable {
- doTest5(4);
+ doTest(
+ "traceMainDexList005",
+ "multidex005",
+ EXAMPLE_BUILD_DIR,
+ Paths.get(EXAMPLE_SRC_DIR, "multidex005", "main-dex-rules-4.txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex005", "ref-list-4-r8.txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex005", "ref-list-4.txt"),
+ AndroidApiLevel.I);
}
@Test
@@ -158,6 +168,7 @@
EXAMPLE_BUILD_DIR,
Paths.get(EXAMPLE_SRC_DIR, "multidex006", "main-dex-rules-1.txt"),
Paths.get(EXAMPLE_SRC_DIR, "multidex006", "ref-list-1.txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex006", "ref-list-1.txt"),
AndroidApiLevel.I);
}
@@ -168,6 +179,7 @@
EXAMPLE_BUILD_DIR,
Paths.get(EXAMPLE_SRC_DIR, "multidex005", "main-dex-rules-" + variant + ".txt"),
Paths.get(EXAMPLE_SRC_DIR, "multidex005", "ref-list-" + variant + ".txt"),
+ Paths.get(EXAMPLE_SRC_DIR, "multidex005", "ref-list-" + variant + ".txt"),
AndroidApiLevel.I);
}
@@ -176,6 +188,7 @@
String packageName,
String buildDir,
Path mainDexRules,
+ Path expectedR8MainDexList,
Path expectedMainDexList,
AndroidApiLevel minSdk)
throws Throwable {
@@ -184,6 +197,7 @@
packageName,
buildDir,
mainDexRules,
+ expectedR8MainDexList,
expectedMainDexList,
minSdk,
(options) -> {
@@ -196,6 +210,7 @@
String packageName,
String buildDir,
Path mainDexRules,
+ Path expectedR8MainDexList,
Path expectedMainDexList,
AndroidApiLevel minSdk,
Consumer<InternalOptions> optionsConsumer)
@@ -259,17 +274,22 @@
.map(this::mainDexStringToDescriptor)
.sorted()
.collect(Collectors.toList());
- // Check that both generated lists are the same as the reference list, except for lambda
+ // Check that generated lists are the same as the reference list, except for lambda
// classes which are only produced when running R8.
+ String[] r8RefList = new String(Files.readAllBytes(
+ expectedR8MainDexList), StandardCharsets.UTF_8).split("\n");
+ for (int i = 0; i < r8RefList.length; i++) {
+ String reference = r8RefList[i].trim();
+ if (r8MainDexList.size() <= i) {
+ Assert.fail("R8 main dex list is missing '" + reference + "'");
+ }
+ checkSameMainDexEntry(reference, r8MainDexList.get(i));
+ }
String[] refList = new String(Files.readAllBytes(
expectedMainDexList), StandardCharsets.UTF_8).split("\n");
int nonLambdaOffset = 0;
for (int i = 0; i < refList.length; i++) {
String reference = refList[i].trim();
- if (r8MainDexList.size() <= i) {
- Assert.fail("R8 main dex list is missing '" + reference + "'");
- }
- checkSameMainDexEntry(reference, r8MainDexList.get(i));
// The main dex list generator does not do any lambda desugaring.
if (!isLambda(reference)) {
if (mainDexGeneratorMainDexList.size() <= i - nonLambdaOffset) {
diff --git a/src/test/java/com/android/tools/r8/shaking/FieldTypeTest.java b/src/test/java/com/android/tools/r8/shaking/FieldTypeTest.java
index 574ec6d..831d664 100644
--- a/src/test/java/com/android/tools/r8/shaking/FieldTypeTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/FieldTypeTest.java
@@ -21,22 +21,15 @@
import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.google.common.collect.ImmutableList;
import java.nio.file.Path;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
@RunWith(VmTestRunner.class)
public class FieldTypeTest extends TestBase {
- @Ignore("b/78788577")
- @Test
- public void test_brokenTypeHierarchy() throws Exception {
- JasminBuilder jasminBuilder = new JasminBuilder();
- // interface Itf
- ClassBuilder itf = jasminBuilder.addInterface("Itf");
- MethodSignature foo = itf.addAbstractMethod("foo", ImmutableList.of(), "V");
- // class Impl /* implements Itf */
- ClassBuilder impl = jasminBuilder.addClass("Impl");
+ private ClassBuilder addImplementor(
+ JasminBuilder jasminBuilder, String name, String superName, String... interfaces) {
+ ClassBuilder impl = jasminBuilder.addClass(name, superName, interfaces);
impl.addDefaultConstructor();
impl.addVirtualMethod("foo", ImmutableList.of(), "V",
".limit locals 2",
@@ -50,15 +43,43 @@
".limit stack 2",
"ldc \"" + impl.name + "\"",
"areturn");
+ return impl;
+ }
+
+ @Test
+ public void test_brokenTypeHierarchy() throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ // interface Itf1
+ ClassBuilder itf1 = jasminBuilder.addInterface("Itf1");
+ MethodSignature foo1 = itf1.addAbstractMethod("foo", ImmutableList.of(), "V");
+ // class Impl1 /* implements Itf1 */
+ ClassBuilder impl1 = addImplementor(jasminBuilder, "Impl1", "java/lang/Object");
+
+ // Another interface and implementer with a correct relation.
+ ClassBuilder itf2 = jasminBuilder.addInterface("Itf2");
+ MethodSignature foo2 = itf2.addAbstractMethod("foo", ImmutableList.of(), "V");
+ ClassBuilder impl2 = addImplementor(jasminBuilder, "Impl2", "java/lang/Object", itf2.name);
+
ClassBuilder client = jasminBuilder.addClass("Client");
- FieldSignature obj = client.addStaticFinalField("obj", itf.getDescriptor(), null);
+ client.setAccess("final");
+ client.addDefaultConstructor();
+ FieldSignature a = client.addStaticFinalField("a", "Ljava/lang/Object;", null);
+ FieldSignature obj1 = client.addField("private static", "obj1", itf1.getDescriptor(), null);
+ FieldSignature obj2 = client.addStaticFinalField("obj2", itf2.getDescriptor(), null);
client.addClassInitializer(
".limit locals 1",
".limit stack 2",
- "new " + impl.name,
+ "aconst_null",
+ "putstatic " + client.name + "/" + a.name + " " + "Ljava/lang/Object;",
+ "new " + impl1.name,
"dup",
- "invokespecial " + impl.name + "/<init>()V",
- "putstatic " + client.name + "/" + obj.name + " " + itf.getDescriptor(),
+ "invokespecial " + impl1.name + "/<init>()V",
+ // Unused, i.e., not read, field, yet still remained in the output.
+ "putstatic " + client.name + "/" + obj1.name + " " + itf1.getDescriptor(),
+ "new " + impl2.name,
+ "dup",
+ "invokespecial " + impl2.name + "/<init>()V",
+ "putstatic " + client.name + "/" + obj2.name + " " + itf2.getDescriptor(),
"return"
);
@@ -67,8 +88,8 @@
".limit locals 2",
".limit stack 2",
"getstatic java/lang/System/out Ljava/io/PrintStream;",
- "getstatic " + client.name + "/" + obj.name + " " + itf.getDescriptor(),
/*
+ "getstatic " + client.name + "/" + obj1.name + " " + itf1.getDescriptor(),
"astore_0",
"aload_0",
// java.lang.IncompatibleClassChangeError:
@@ -76,19 +97,24 @@
"invokeinterface " + itf.name + "/" + foo.name + "()V 1",
"aload_0",
*/
+ "getstatic " + client.name + "/" + obj2.name + " " + itf2.getDescriptor(),
"invokevirtual java/io/PrintStream/print(Ljava/lang/Object;)V",
"return"
);
final String mainClassName = mainClass.name;
- String proguardConfig = keepMainProguardConfiguration(mainClass.name, false, false);
+ String proguardConfig =
+ keepMainProguardConfiguration(mainClass.name, false, false)
+ // AGP default is to not turn optimizations on, which disables MemberValuePropagation,
+ // resulting in the problematic putstatic being remained.
+ + "-dontoptimize\n";
// Run input program on java.
Path outputDirectory = temp.newFolder().toPath();
jasminBuilder.writeClassFiles(outputDirectory);
ProcessResult javaResult = ToolHelper.runJava(outputDirectory, mainClassName);
assertEquals(0, javaResult.exitCode);
- assertThat(javaResult.stdout, containsString(impl.name));
+ assertThat(javaResult.stdout, containsString(impl2.name));
AndroidApp processedApp = compileWithR8(jasminBuilder.build(), proguardConfig,
// Disable inlining to avoid the (short) tested method from being inlined then removed.
@@ -97,12 +123,12 @@
// Run processed (output) program on ART
ProcessResult artResult = runOnArtRaw(processedApp, mainClassName);
assertEquals(0, artResult.exitCode);
- assertThat(artResult.stdout, containsString(impl.name));
+ assertThat(artResult.stdout, containsString(impl2.name));
assertEquals(-1, artResult.stderr.indexOf("DoFieldPut"));
DexInspector inspector = new DexInspector(processedApp);
- ClassSubject itfSubject = inspector.clazz(itf.name);
- assertThat(itfSubject, isPresent());
+ ClassSubject itf1Subject = inspector.clazz(itf1.name);
+ assertThat(itf1Subject, isPresent());
}
}
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 d578815..cae3b06 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
@@ -94,12 +94,13 @@
Result result = runTest(mainClass, proguardConfig);
- // Without includedescriptorclasses return type argument type and field type are removed.
+ // Without includedescriptorclasses return type and argument type are removed.
result.assertKept(ClassWithNativeMethods.class);
result.assertRemoved(NativeArgumentType.class);
result.assertRemoved(NativeReturnType.class);
- result.assertRemoved(InstanceFieldType.class);
- result.assertRemoved(StaticFieldType.class);
+ // Field type is not removed due to the concern about the broken type hierarchy.
+ result.assertRenamed(InstanceFieldType.class);
+ result.assertRenamed(StaticFieldType.class);
}
}
