Merge "Make DexSplitter not warn on main dex list classes missing from the application"
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstanceGet.java b/src/main/java/com/android/tools/r8/ir/code/InstanceGet.java
index 40756ae..14d7ffb 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstanceGet.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstanceGet.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.ir.code;
+import static com.android.tools.r8.optimize.MemberRebindingAnalysis.isVisibleFromOriginalContext;
+
import com.android.tools.r8.cf.LoadStoreHelper;
import com.android.tools.r8.cf.TypeVerificationHelper;
import com.android.tools.r8.cf.code.CfFieldInstruction;
@@ -19,6 +21,7 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
@@ -98,6 +101,24 @@
}
@Override
+ public boolean canBeDeadCode(AppInfo appInfo, IRCode code) {
+ // instance-get can be dead code as long as it cannot have any of the following:
+ // * NoSuchFieldError (resolution failure)
+ // * IllegalAccessError (not visible from the access context)
+ // * NullPointerException (null receiver).
+ // TODO(b/123857022): Should be possible to use definitionFor().
+ DexEncodedField resolvedField = appInfo.resolveFieldOn(getField().getHolder(), getField());
+ if (resolvedField == null) {
+ return false;
+ }
+ if (code == null
+ || !isVisibleFromOriginalContext(appInfo, code.method.method.getHolder(), resolvedField)) {
+ return false;
+ }
+ return object().getTypeLattice().nullability().isDefinitelyNotNull();
+ }
+
+ @Override
public int maxInValueRegister() {
return Constants.U4BIT_MAX;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/StaticGet.java b/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
index b79d076..80e74f3 100644
--- a/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
+++ b/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.code;
+import static com.android.tools.r8.optimize.MemberRebindingAnalysis.isVisibleFromOriginalContext;
+
import com.android.tools.r8.cf.LoadStoreHelper;
import com.android.tools.r8.cf.TypeVerificationHelper;
import com.android.tools.r8.cf.code.CfFieldInstruction;
@@ -18,6 +20,7 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
@@ -91,6 +94,25 @@
}
@Override
+ public boolean canBeDeadCode(AppInfo appInfo, IRCode code) {
+ // static-get can be dead as long as it cannot have any of the following:
+ // * NoSuchFieldError (resolution failure)
+ // * IllegalAccessError (not visible from the access context)
+ // * side-effects in <clinit>
+ // TODO(b/123857022): Should be possible to use definitionFor().
+ DexEncodedField resolvedField = appInfo.resolveFieldOn(getField().getHolder(), getField());
+ if (resolvedField == null) {
+ return false;
+ }
+ if (code == null
+ || !isVisibleFromOriginalContext(appInfo, code.method.method.getHolder(), resolvedField)) {
+ return false;
+ }
+ return appInfo.hasSubtyping()
+ && !getField().getHolder().classInitializationMayHaveSideEffects(appInfo);
+ }
+
+ @Override
public int maxInValueRegister() {
return Constants.U8BIT_MAX;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
index 608f915..60aec09 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.optimize;
+import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
@@ -291,14 +292,15 @@
Set<DexEncodedMethod> contexts = fieldsWithContexts.get(field);
if (target != null && target.field != field
&& contexts.stream().allMatch(context ->
- isVisibleFromOriginalContext(context.method.getHolder(), target))) {
+ isVisibleFromOriginalContext(appInfo, context.method.getHolder(), target))) {
builder.map(field,
lense.lookupField(validTargetFor(target.field, field, lookupTargetOnClass)));
}
}
}
- private boolean isVisibleFromOriginalContext(DexType context, DexEncodedField field) {
+ public static boolean isVisibleFromOriginalContext(
+ AppInfo appInfo, DexType context, DexEncodedField field) {
DexType holderType = field.field.getHolder();
DexClass holder = appInfo.definitionFor(holderType);
if (holder == null) {
diff --git a/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java b/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java
index 33f28fc..ea8ed0b 100644
--- a/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java
+++ b/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java
@@ -462,7 +462,7 @@
main.addMainMethod(
".limit stack 2",
".limit locals 1",
- " getstatic Empty/aMethod I",
+ " getstatic Empty/aField I",
" return");
ensureRuntimeException(builder, NoSuchFieldError.class);
diff --git a/src/test/java/com/android/tools/r8/shaking/FieldReadsJasminTest.java b/src/test/java/com/android/tools/r8/shaking/FieldReadsJasminTest.java
new file mode 100644
index 0000000..ac1fd90
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/FieldReadsJasminTest.java
@@ -0,0 +1,180 @@
+// Copyright (c) 2019, 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;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
+import com.android.tools.r8.jasmin.JasminTestBase;
+import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.util.Iterator;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class FieldReadsJasminTest extends JasminTestBase {
+ private static final String CLS = "Empty";
+ private static final String MAIN = "Main";
+ private final Backend backend;
+
+ @Parameterized.Parameters(name = "Backend: {0}")
+ public static Object[] data() {
+ return Backend.values();
+ }
+
+ public FieldReadsJasminTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void testInstanceGet_nonNullReceiver() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder empty = builder.addClass(CLS);
+ empty.addField("protected", "aField", "I", null);
+
+ ClassBuilder main = builder.addClass(MAIN);
+ main.addMainMethod(
+ ".limit stack 2",
+ ".limit locals 1",
+ " new Empty",
+ " dup",
+ " invokespecial Empty/<init>()V",
+ " getfield Empty/aField I",
+ " return");
+
+ ensureNoFields(builder, empty);
+ }
+
+ @Test
+ public void testStaticGet_noSideEffect() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder empty = builder.addClass(CLS);
+ empty.addStaticField("sField", "I");
+
+ ClassBuilder main = builder.addClass(MAIN);
+ main.addMainMethod(
+ ".limit stack 2",
+ ".limit locals 1",
+ " getstatic Empty/sField I",
+ " return");
+
+ ensureNoFields(builder, empty);
+ }
+
+ private void ensureNoFields(JasminBuilder app, ClassBuilder clazz) throws Exception {
+ testForR8(backend)
+ .addProgramClassFileData(app.buildClasses())
+ .addKeepRules("-keep class * { <methods>; }")
+ .compile()
+ .inspect(inspector -> {
+ ClassSubject classSubject = inspector.clazz(clazz.name);
+ assertThat(classSubject, isPresent());
+ classSubject.forAllFields(foundFieldSubject -> {
+ fail("Expect not to see any fields.");
+ });
+ });
+ }
+
+ @Test
+ public void testInstanceGet_nullableReceiver() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder empty = builder.addClass(CLS);
+ empty.addDefaultConstructor();
+ empty.addField("protected", "aField", "I", null);
+ MethodSignature foo = empty.addStaticMethod("foo", ImmutableList.of("L" + CLS + ";"), "V",
+ ".limit stack 2",
+ ".limit locals 1",
+ " aload 0",
+ " getfield Empty/aField I",
+ " return");
+
+ ClassBuilder main = builder.addClass(MAIN);
+ main.addMainMethod(
+ ".limit stack 2",
+ ".limit locals 1",
+ " aconst_null",
+ " invokestatic Empty/foo(L" + CLS + ";)V",
+ " return");
+
+ ensureFieldExists(builder, empty, foo, "aField");
+ }
+
+ @Test
+ public void testStaticGet_nonTrivialClinit() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder empty = builder.addClass(CLS);
+ empty.addDefaultConstructor();
+ empty.addStaticField("sField", "I");
+ empty.addClassInitializer(
+ ".limit stack 3",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " ldc \"hello\"",
+ " invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ " return");
+
+ ClassBuilder main = builder.addClass(MAIN);
+ MethodSignature mainMethod = main.addMainMethod(
+ ".limit stack 2",
+ ".limit locals 1",
+ " getstatic Empty/sField I",
+ " return");
+
+ ensureFieldExists(builder, main, mainMethod, "sField");
+ }
+
+ @Test
+ public void testStaticGet_allocation() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder empty = builder.addClass(CLS);
+ empty.addDefaultConstructor();
+ empty.addStaticField("sField", "Ljava/lang/String;", "\"8\"");
+
+ ClassBuilder main = builder.addClass(MAIN);
+ MethodSignature mainMethod = main.addMainMethod(
+ ".limit stack 2",
+ ".limit locals 1",
+ " getstatic Empty/sField Ljava/lang/String;",
+ " return");
+
+ ensureFieldExists(builder, main, mainMethod, "sField");
+ }
+
+ private void ensureFieldExists(
+ JasminBuilder app, ClassBuilder clazz, MethodSignature method, String fieldName)
+ throws Exception {
+ testForR8(backend)
+ .addProgramClassFileData(app.buildClasses())
+ .addKeepRules("-keep class * { <methods>; }")
+ .compile()
+ .inspect(inspector -> {
+ ClassSubject classSubject = inspector.clazz(clazz.name);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.uniqueMethodWithName(method.name);
+ assertThat(methodSubject, isPresent());
+ Iterator<InstructionSubject> it =
+ methodSubject.iterateInstructions(InstructionSubject::isFieldAccess);
+ assertTrue(it.hasNext());
+ assertEquals(
+ inspector.clazz(CLS).uniqueFieldWithName(fieldName).getFinalName(),
+ it.next().getField().name.toString());
+ });
+ }
+
+}
diff --git a/tools/apk_utils.py b/tools/apk_utils.py
index 5a6aa94..af24ec1 100644
--- a/tools/apk_utils.py
+++ b/tools/apk_utils.py
@@ -23,7 +23,8 @@
]
utils.RunCmd(cmd, quiet=quiet)
-def sign_with_apksigner(build_tools_dir, unsigned_apk, signed_apk, keystore, password):
+def sign_with_apksigner(
+ build_tools_dir, unsigned_apk, signed_apk, keystore, password, quiet=False):
cmd = [
os.path.join(build_tools_dir, 'apksigner'),
'sign',
@@ -34,5 +35,4 @@
'--out', signed_apk,
unsigned_apk
]
- utils.PrintCmd(cmd)
- subprocess.check_call(cmd)
+ utils.RunCmd(cmd, quiet=quiet)
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index 3a9bf8c..1d8b04a 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -102,11 +102,9 @@
},
'tivi': {
'app_id': 'app.tivi',
- # Forked from https://github.com/chrisbanes/tivi.git removing
- # signingConfigs.
'git_repo': 'https://github.com/sgjesse/tivi.git',
- # TODO(123047413): Fails with R8.
- 'skip': True,
+ 'min_sdk': 23,
+ 'compile_sdk': 28,
},
'Tusky': {
'app_id': 'com.keylesspalace.tusky',
@@ -419,14 +417,13 @@
if options.sign_apks and not os.path.isfile(signed_apk):
assert os.path.isfile(unsigned_apk)
if options.sign_apks:
- keystore = 'app.keystore'
- keystore_password = 'android'
apk_utils.sign_with_apksigner(
utils.ANDROID_BUILD_TOOLS,
unsigned_apk,
signed_apk,
- keystore,
- keystore_password)
+ options.keystore,
+ options.keystore_password,
+ quiet=options.quiet)
if os.path.isfile(signed_apk):
apk_dest = os.path.join(out_dir, signed_apk_name)
@@ -587,6 +584,12 @@
result.add_option('--app',
help='What app to run on',
choices=APPS.keys())
+ result.add_option('--keystore',
+ help='Path to app.keystore',
+ default='app.keystore')
+ result.add_option('--keystore-password', '--keystore_password',
+ help='Password for app.keystore',
+ default='android')
result.add_option('--monkey',
help='Whether to install and run app(s) with monkey',
default=False,
