Use resolved target when searching for assume* rules.
Bug: 110178673
Change-Id: Icc8693df7f825edf23e1719fb0abbf99d367a809
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index e215f58..4596f3b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -263,10 +263,11 @@
ProguardMemberRuleLookup lookup = lookupMemberRule(definition);
if (lookup == null) {
- // Since -assumenosideeffects rules are always applied to all overriding methods, we can
- // simply check the target method, although this may be different from the dynamically
- // targeted method.
- DexEncodedMethod target = appView.definitionFor(invokedMethod);
+ // -assumenosideeffects rules are applied to upward visible and overriding methods, but only
+ // references that have actual definitions are marked by the root set builder. So, here, we
+ // try again with a resolved target, not the direct definition, which may not exist.
+ DexEncodedMethod target =
+ appView.appInfo().resolveMethod(invokedHolder, invokedMethod).asSingleTarget();
lookup = lookupMemberRule(target);
}
boolean invokeReplaced = false;
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithoutMatchingDefinitionTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithoutMatchingDefinitionTest.java
new file mode 100644
index 0000000..f008747
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithoutMatchingDefinitionTest.java
@@ -0,0 +1,153 @@
+// 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.assumenosideeffects;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.DexIndexedConsumer.ArchiveConsumer;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Streams;
+import java.nio.file.Path;
+import java.util.Collection;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+class LibraryBase {
+ boolean isInEditMode() {
+ return System.currentTimeMillis() <= 0;
+ }
+}
+
+class LibrarySub extends LibraryBase {}
+
+class ProgramClass extends LibrarySub {
+ private final String test;
+
+ private ProgramClass() {
+ if (isInEditMode()) {
+ test = "test";
+ } else {
+ test = "production";
+ }
+ }
+
+ public static void main(String... args) {
+ System.out.println(new ProgramClass().test);
+ }
+}
+
+@RunWith(Parameterized.class)
+public class AssumenosideeffectsWithoutMatchingDefinitionTest extends TestBase {
+ private static final Class<?> MAIN = ProgramClass.class;
+ private static final String EXPECTED_OUTPUT = StringUtils.lines("production");
+
+ enum TestConfig {
+ RULE_THAT_DIRECTLY_REFERS_BASE,
+ RULE_THAT_DIRECTLY_REFERS_SUB,
+ RULE_WITH_EXTENDS_BASE,
+ RULE_WITH_EXTENDS_SUB;
+
+ public String getKeepRule() {
+ switch (this) {
+ case RULE_THAT_DIRECTLY_REFERS_BASE:
+ return StringUtils.lines(
+ "-assumenosideeffects class **.LibraryBase {",
+ " boolean isInEditMode() return false;",
+ "}");
+ case RULE_THAT_DIRECTLY_REFERS_SUB:
+ return StringUtils.lines(
+ "-assumenosideeffects class **.LibrarySub {",
+ " boolean isInEditMode() return false;",
+ "}");
+ case RULE_WITH_EXTENDS_BASE:
+ return StringUtils.lines(
+ "-assumenosideeffects class * extends **.LibraryBase {",
+ " boolean isInEditMode() return false;",
+ "}");
+ case RULE_WITH_EXTENDS_SUB:
+ return StringUtils.lines(
+ "-assumenosideeffects class * extends **.LibrarySub {",
+ " boolean isInEditMode() return false;",
+ "}");
+ }
+ throw new Unreachable();
+ }
+
+ public void inspect(CodeInspector inspector) {
+ ClassSubject main = inspector.clazz(MAIN);
+ assertThat(main, isPresent());
+
+ MethodSubject init = main.init();
+ assertThat(init, isPresent());
+ assertEquals(
+ 0,
+ Streams.stream(init.iterateInstructions(
+ i -> i.isIf() || i.isIfNez() || i.isIfEqz())).count());
+ }
+ }
+
+ @Parameterized.Parameters(name = "{0} {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(getTestParameters().withAllRuntimes().build(), TestConfig.values());
+ }
+
+ private final TestParameters parameters;
+ private final TestConfig config;
+
+ public AssumenosideeffectsWithoutMatchingDefinitionTest(
+ TestParameters parameters, TestConfig config) {
+ this.parameters = parameters;
+ this.config = config;
+ }
+
+ @ClassRule
+ public static TemporaryFolder staticTemp = ToolHelper.getTemporaryFolderForTest();
+
+ private static Path libJarPath;
+ private static Path libDexPath;
+
+ @BeforeClass
+ public static void prepareLibJar() throws Exception {
+ libJarPath = staticTemp.newFile("lib.jar").toPath().toAbsolutePath();
+ writeClassesToJar(libJarPath, ImmutableList.of(LibraryBase.class, LibrarySub.class));
+ libDexPath = staticTemp.newFile("lib.dex").toPath().toAbsolutePath();
+ testForD8(staticTemp)
+ .addProgramFiles(libJarPath)
+ .setMinApi(AndroidApiLevel.B)
+ .setProgramConsumer(new ArchiveConsumer(libDexPath))
+ .compile();
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addLibraryFiles(libJarPath)
+ .addLibraryFiles(ToolHelper.getDefaultAndroidJar())
+ .addProgramClasses(MAIN)
+ .addKeepMainRule(MAIN)
+ .addKeepRules(config.getKeepRule())
+ .noMinification()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .addRunClasspathFiles(parameters.isDexRuntime() ? libDexPath : libJarPath)
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT)
+ .inspect(config::inspect);
+ }
+}
