Ensure that compile-time constants from library classes are ignored
Compile constants in library classes might still change at runtime. To avoid
using the values of these constants set all static field values in library
classes to unknown.
R=herhut@google.com
Change-Id: I6f0576b2a7d6814b2678415a2400d86ca5652380
diff --git a/src/main/java/com/android/tools/r8/graph/DexLibraryClass.java b/src/main/java/com/android/tools/r8/graph/DexLibraryClass.java
index 0c8ce8c..ffff701 100644
--- a/src/main/java/com/android/tools/r8/graph/DexLibraryClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexLibraryClass.java
@@ -17,6 +17,11 @@
DexEncodedMethod[] directMethods, DexEncodedMethod[] virtualMethods) {
super(sourceFile, interfaces, accessFlags, superType, type,
staticFields, instanceFields, directMethods, virtualMethods, annotations, origin);
+ // Set all static field values to unknown. We don't want to use the value from the library
+ // at compile time, as it can be different at runtime.
+ for (DexEncodedField staticField : staticFields) {
+ staticField.staticValue = DexValue.UNKNOWN;
+ }
}
@Override
diff --git a/src/main/java/com/android/tools/r8/graph/DexValue.java b/src/main/java/com/android/tools/r8/graph/DexValue.java
index 73b3242..415775d 100644
--- a/src/main/java/com/android/tools/r8/graph/DexValue.java
+++ b/src/main/java/com/android/tools/r8/graph/DexValue.java
@@ -17,6 +17,8 @@
public abstract class DexValue extends DexItem {
+ public static final UnknownDexValue UNKNOWN = UnknownDexValue.UNKNOWN;
+
public static final byte VALUE_BYTE = 0x00;
public static final byte VALUE_SHORT = 0x02;
public static final byte VALUE_CHAR = 0x03;
@@ -43,7 +45,7 @@
@Override
void collectMixedSectionItems(MixedSectionCollection mixedItems) {
// Should never be visited.
- assert false;
+ throw new Unreachable();
}
public abstract void sort();
@@ -108,6 +110,55 @@
return true;
}
+ static public class UnknownDexValue extends DexValue {
+
+ // Singleton instance.
+ public static final UnknownDexValue UNKNOWN = new UnknownDexValue();
+
+ private UnknownDexValue() {
+ }
+
+ @Override
+ public void collectIndexedItems(IndexedItemCollection indexedItems) {
+ throw new Unreachable();
+ }
+
+ @Override
+ public void sort() {
+ throw new Unreachable();
+ }
+
+ @Override
+ public boolean mayTriggerAllocation() {
+ return true;
+ }
+
+ @Override
+ public void writeTo(DexOutputBuffer dest, ObjectToOffsetMapping mapping) {
+ throw new Unreachable();
+ }
+
+ @Override
+ public int hashCode() {
+ return System.identityHashCode(this);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ return other == this;
+ }
+
+ @Override
+ public String toString() {
+ return "UNKNOWN";
+ }
+
+ @Override
+ public Instruction asConstInstruction(boolean hasClassInitializer, Value dest) {
+ return null;
+ }
+ }
+
static private abstract class SimpleDexValue extends DexValue {
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/PreloadedClassFileProvider.java b/src/main/java/com/android/tools/r8/utils/PreloadedClassFileProvider.java
index 8e3c638..b819a84 100644
--- a/src/main/java/com/android/tools/r8/utils/PreloadedClassFileProvider.java
+++ b/src/main/java/com/android/tools/r8/utils/PreloadedClassFileProvider.java
@@ -75,6 +75,13 @@
return builder.build();
}
+ public static ClassFileResourceProvider fromClassData(String descriptor, byte[] data)
+ throws IOException {
+ Builder builder = builder();
+ builder.addResource(descriptor, data);
+ return builder.build();
+ }
+
// Guess class descriptor from location of the class file.
static String guessTypeDescriptor(Path name) {
return guessTypeDescriptor(name.toString());
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index f2c4d15..ae06040 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -7,12 +7,15 @@
import static org.junit.Assert.assertEquals;
import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.dex.ApplicationReader;
+import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.shaking.FilteredClassPath;
import com.android.tools.r8.shaking.ProguardRuleParserException;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OutputMode;
+import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableList;
import com.google.common.io.ByteStreams;
import java.io.File;
@@ -88,6 +91,15 @@
}
/**
+ * Compile an application with D8.
+ */
+ protected AndroidApp compileWithD8(AndroidApp app)
+ throws CompilationException, ExecutionException, IOException {
+ D8Command command = ToolHelper.prepareD8CommandBuilder(app).build();
+ return ToolHelper.runD8(command);
+ }
+
+ /**
* Compile an application with R8.
*/
protected AndroidApp compileWithR8(Class... classes)
@@ -280,4 +292,14 @@
}
return result.stdout;
}
+
+
+ /**
+ * Disassemble the content of an application. Only works for an application with only dex code.
+ */
+ protected void disassemble(AndroidApp app) throws Exception {
+ InternalOptions options = new InternalOptions();
+ DexApplication dexApplication = new ApplicationReader(app, options, new Timing("XX")).read();
+ System.out.println(dexApplication.smali(options));
+ }
}
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 94d52f0..1d48748 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -475,6 +475,10 @@
return parser.getConfig();
}
+ public static D8Command.Builder prepareD8CommandBuilder(AndroidApp app) {
+ return D8Command.builder(app);
+ }
+
public static R8Command.Builder prepareR8CommandBuilder(AndroidApp app) {
return R8Command.builder(app);
}
diff --git a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
index ba5e370..8f3ace3 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
@@ -86,12 +86,20 @@
return new MethodSignature(name, returnJavaType, argumentJavaTypes);
}
- public FieldSignature addStaticField(String name, String type, String value) {
+ public FieldSignature addField(String flags, String name, String type, String value) {
fields.add(
- ".field public static " + name + " " + type + (value != null ? (" = " + value) : ""));
+ ".field " + flags + " " + name + " " + type + (value != null ? (" = " + value) : ""));
return new FieldSignature(name, type);
}
+ public FieldSignature addStaticField(String name, String type, String value) {
+ return addField("public static", name, type, value);
+ }
+
+ public FieldSignature addStaticFinalField(String name, String type, String value) {
+ return addField("public static final", name, type, value);
+ }
+
@Override
public String toString() {
StringBuilder builder = new StringBuilder();
diff --git a/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/LibraryClass.java b/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/LibraryClass.java
new file mode 100644
index 0000000..b124ee9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/LibraryClass.java
@@ -0,0 +1,13 @@
+// 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.rewrite.staticvalues.inlibraries;
+
+public class LibraryClass {
+ public static int x = 1;
+
+ static int getX() {
+ return x;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/StaticLibraryValuesChangeTest.java b/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/StaticLibraryValuesChangeTest.java
new file mode 100644
index 0000000..445c12c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/StaticLibraryValuesChangeTest.java
@@ -0,0 +1,77 @@
+// 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.rewrite.staticvalues.inlibraries;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.shaking.FilteredClassPath;
+import com.android.tools.r8.smali.SmaliTestBase.SmaliBuilder;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.PreloadedClassFileProvider;
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+
+public class StaticLibraryValuesChangeTest extends TestBase {
+ @Test
+ public void testStatic() throws Exception {
+ /*
+ * Three versions of the class LibraryClass is used in the test:
+ *
+ * * one in Java code, where field x is "public static int x = 1" (not final)
+ * * one in Jasmin code, where field x is "public static final int x = 2"
+ * * int in smali code, where field x is "public static final int x = 3"
+ *
+ * The first version is used to compile TestMain with javac. This causes the field accessor
+ * for x to be in the code for TestMain.main. As javac will inline compile-time constants
+ * it cannot be declared "final" here.
+ *
+ * The second version is used as a library class when compiling the javac compiled TestMain
+ * with R8.
+ *
+ * The third version is used for running the R8 compiled TestMain on Art.
+ */
+
+ // Build the second version of LibraryClass
+ JasminBuilder compileTimeLibrary = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = compileTimeLibrary.addClass(
+ "com.android.tools.r8.rewrite.staticvalues.inlibraries.LibraryClass");
+ clazz.addStaticFinalField("x", "I", "2");
+ clazz.addStaticMethod("getX", ImmutableList.of(), "I",
+ ".limit stack 1",
+ ".limit locals 0",
+ " iconst_2",
+ " ireturn");
+
+ // Compile TestMain with R8 using the second version of LibraryClass as library.
+ AndroidApp.Builder builder = AndroidApp.builder();
+ builder.addProgramFiles(
+ FilteredClassPath.unfiltered(ToolHelper.getClassFileForTestClass(TestMain.class)));
+ builder.addLibraryResourceProvider(PreloadedClassFileProvider.fromClassData(
+ "Lcom/android/tools/r8/rewrite/staticvalues/inlibraries/LibraryClass;",
+ compileTimeLibrary.buildClasses().get(0)));
+ AndroidApp app = compileWithR8(builder.build());
+
+ // Build the third version of LibraryClass
+ SmaliBuilder runtimeLibrary = new SmaliBuilder(LibraryClass.class.getCanonicalName());
+ runtimeLibrary.addStaticField("x", "I", "3");
+ runtimeLibrary.addStaticMethod(
+ "int",
+ "getX",
+ ImmutableList.of(),
+ 1,
+ " const/4 v0, 3",
+ " return v0"
+ );
+
+ // Merge the compiled TestMain with the runtime version of LibraryClass.
+ builder = AndroidApp.builder(app);
+ builder.addDexProgramData(runtimeLibrary.compile());
+ String result = runOnArt(builder.build(), TestMain.class);
+ assertEquals("33", result);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/TestMain.java b/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/TestMain.java
new file mode 100644
index 0000000..83f6c16
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/staticvalues/inlibraries/TestMain.java
@@ -0,0 +1,13 @@
+// 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.rewrite.staticvalues.inlibraries;
+
+public class TestMain {
+
+ public static void main(String[] args) {
+ System.out.print(LibraryClass.x);
+ System.out.print(LibraryClass.getX());
+ }
+}
