Add more tests regarding member resolution with -overloadaggressively. Inspired from http://b/72858955#comment12 Bug: 72858955 Change-Id: I4eb2fea41673eb1568bf261c73afa755b60400d4
diff --git a/src/test/java/com/android/tools/r8/naming/overloadaggressively/A.java b/src/test/java/com/android/tools/r8/naming/overloadaggressively/A.java index af4c4b2..96b8a06 100644 --- a/src/test/java/com/android/tools/r8/naming/overloadaggressively/A.java +++ b/src/test/java/com/android/tools/r8/naming/overloadaggressively/A.java
@@ -4,7 +4,7 @@ package com.android.tools.r8.naming.overloadaggressively; public class A { - volatile int f1; + public volatile int f1; volatile Object f2; - volatile B f3; + public volatile B f3; }
diff --git a/src/test/java/com/android/tools/r8/naming/overloadaggressively/B.java b/src/test/java/com/android/tools/r8/naming/overloadaggressively/B.java index 78f5a24..6660196 100644 --- a/src/test/java/com/android/tools/r8/naming/overloadaggressively/B.java +++ b/src/test/java/com/android/tools/r8/naming/overloadaggressively/B.java
@@ -4,6 +4,19 @@ package com.android.tools.r8.naming.overloadaggressively; public class B { - volatile int f1; - volatile Object f2; + volatile int f1 = 8; + volatile Object f2 = "d8"; + volatile String f3 = "r8"; + + public int getF1() { + return f1; + } + + public Object getF2() { + return f2; + } + + public String getF3() { + return f3; + } }
diff --git a/src/test/java/com/android/tools/r8/naming/overloadaggressively/FieldResolution.java b/src/test/java/com/android/tools/r8/naming/overloadaggressively/FieldResolution.java new file mode 100644 index 0000000..26f5df6 --- /dev/null +++ b/src/test/java/com/android/tools/r8/naming/overloadaggressively/FieldResolution.java
@@ -0,0 +1,27 @@ +// Copyright (c) 2018, 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.naming.overloadaggressively; + +import java.lang.reflect.Field; +import java.util.Random; + +public class FieldResolution { + public static void main(String[] args) throws Exception { + A a = new A(); + B b = new B(); + + Field f3 = A.class.getField("f3"); + f3.set(a, b); + assert a.f3 != null; + assert a.f3 == b; + + Field f1 = A.class.getField("f1"); + Random random = new Random(); + int next = random.nextInt(); + f1.set(a, next); + a.f3.f1 = next; + int diff = a.f1 - b.f1; + System.out.println("diff: " + diff); + } +}
diff --git a/src/test/java/com/android/tools/r8/naming/overloadaggressively/Main.java b/src/test/java/com/android/tools/r8/naming/overloadaggressively/FieldUpdater.java similarity index 97% rename from src/test/java/com/android/tools/r8/naming/overloadaggressively/Main.java rename to src/test/java/com/android/tools/r8/naming/overloadaggressively/FieldUpdater.java index 956b198..8f5011c 100644 --- a/src/test/java/com/android/tools/r8/naming/overloadaggressively/Main.java +++ b/src/test/java/com/android/tools/r8/naming/overloadaggressively/FieldUpdater.java
@@ -7,7 +7,7 @@ import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; -public class Main { +public class FieldUpdater { @SuppressWarnings("unchecked") public static void main(String[] args) throws Exception { A a = new A();
diff --git a/src/test/java/com/android/tools/r8/naming/overloadaggressively/MethodResolution.java b/src/test/java/com/android/tools/r8/naming/overloadaggressively/MethodResolution.java new file mode 100644 index 0000000..c791031 --- /dev/null +++ b/src/test/java/com/android/tools/r8/naming/overloadaggressively/MethodResolution.java
@@ -0,0 +1,25 @@ +// Copyright (c) 2018, 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.naming.overloadaggressively; + +import java.lang.reflect.Method; + +public class MethodResolution { + public static void main(String[] args) throws Exception { + B b = new B(); + + int originalF1 = b.getF1(); + Method getF1 = B.class.getMethod("getF1", (Class[]) null); + int diff = ((Integer) getF1.invoke(b)) - originalF1; + System.out.println("diff: " + diff); + + Object originalF2 = b.getF2(); + Method getF2 = B.class.getMethod("getF2", (Class[]) null); + System.out.println(originalF2 + " v.s. " + getF2.invoke(b)); + + String originalF3 = b.getF3(); + Method getF3 = B.class.getMethod("getF3", (Class[]) null); + System.out.println(originalF3 + " v.s. " + getF3.invoke(b)); + } +}
diff --git a/src/test/java/com/android/tools/r8/naming/overloadaggressively/OverloadAggressivelyTest.java b/src/test/java/com/android/tools/r8/naming/overloadaggressively/OverloadAggressivelyTest.java index d0cdb28..1254d4b 100644 --- a/src/test/java/com/android/tools/r8/naming/overloadaggressively/OverloadAggressivelyTest.java +++ b/src/test/java/com/android/tools/r8/naming/overloadaggressively/OverloadAggressivelyTest.java
@@ -17,6 +17,7 @@ import com.android.tools.r8.ToolHelper.DexVm.Kind; import com.android.tools.r8.ToolHelper.ProcessResult; import com.android.tools.r8.graph.DexEncodedField; +import com.android.tools.r8.graph.DexEncodedMethod; import com.android.tools.r8.origin.Origin; import com.android.tools.r8.utils.AndroidApp; import com.android.tools.r8.utils.DexInspector; @@ -55,54 +56,144 @@ return testCases; } - @Test - public void overloadAggressivelyTest() throws Exception { - Assume.assumeTrue(ToolHelper.artSupported()); - byte[][] classes = { - ToolHelper.getClassAsBytes(Main.class), - ToolHelper.getClassAsBytes(A.class), - ToolHelper.getClassAsBytes(B.class) - }; - AndroidApp originalApp = buildAndroidApp(classes); - Path out = temp.getRoot().toPath(); - R8Command command = + private AndroidApp runR8(AndroidApp app, Class main, Path out) throws Exception { + R8Command command = ToolHelper.addProguardConfigurationConsumer( - ToolHelper.prepareR8CommandBuilder(originalApp), + ToolHelper.prepareR8CommandBuilder(app), pgConfig -> { pgConfig.setPrintMapping(true); pgConfig.setPrintMappingFile(out.resolve(ToolHelper.DEFAULT_PROGUARD_MAP_FILE)); }) .addProguardConfiguration( ImmutableList.copyOf(Iterables.concat(ImmutableList.of( - keepMainProguardConfiguration(Main.class), + keepMainProguardConfiguration(main), overloadaggressively ? "-overloadaggressively" : ""), CompatProguardCommandBuilder.REFLECTIONS)), Origin.unknown()) .setOutput(out, OutputMode.DexIndexed) .build(); - AndroidApp processedApp = ToolHelper.runR8(command); + return ToolHelper.runR8(command, o -> o.inlineAccessors = false); + } - DexInspector dexInspector = new DexInspector( - out.resolve(ToolHelper.DEFAULT_DEX_FILENAME), - out.resolve(ToolHelper.DEFAULT_PROGUARD_MAP_FILE).toString()); + @Test + public void fieldUpdater() throws Exception { + Assume.assumeTrue(ToolHelper.artSupported()); + byte[][] classes = { + ToolHelper.getClassAsBytes(FieldUpdater.class), + ToolHelper.getClassAsBytes(A.class), + ToolHelper.getClassAsBytes(B.class) + }; + AndroidApp originalApp = buildAndroidApp(classes); + Path out = temp.getRoot().toPath(); + AndroidApp processedApp = runR8(originalApp, FieldUpdater.class, out); + + DexInspector dexInspector = new DexInspector(processedApp); ClassSubject a = dexInspector.clazz(A.class.getCanonicalName()); DexEncodedField f1 = a.field("int", "f1").getField(); assertNotNull(f1); DexEncodedField f2 = a.field("java.lang.Object", "f2").getField(); assertNotNull(f2); + // TODO(b/72858955): due to the potential reflective access, they should have different names + // by R8's improved reflective access detection or via keep rules. assertEquals(overloadaggressively, f1.field.name == f2.field.name); DexEncodedField f3 = a.field(B.class.getCanonicalName(), "f3").getField(); assertNotNull(f3); + // TODO(b/72858955): ditto assertEquals(overloadaggressively, f1.field.name == f3.field.name); + // TODO(b/72858955): ditto assertEquals(overloadaggressively, f2.field.name == f3.field.name); - ProcessResult javaOutput = runOnJava(Main.class.getCanonicalName(), classes); - ProcessResult artOutput = runOnArtRaw(processedApp, Main.class.getCanonicalName(), dexVm); + String main = FieldUpdater.class.getCanonicalName(); + ProcessResult javaOutput = runOnJava(main, classes); + assertEquals(0, javaOutput.exitCode); + ProcessResult artOutput = runOnArtRaw(processedApp, main, dexVm); + // TODO(b/72858955): eventually, R8 should avoid this field resolution conflict. if (overloadaggressively) { assertNotEquals(0, artOutput.exitCode); assertTrue(artOutput.stderr.contains("ClassCastException")); } else { - assertEquals(0, javaOutput.exitCode); + assertEquals(0, artOutput.exitCode); + assertEquals(javaOutput.stdout.trim(), artOutput.stdout.trim()); + // ART may dump its own debugging info through stderr. + // assertEquals(javaOutput.stderr.trim(), artOutput.stderr.trim()); + } + } + + @Test + public void fieldResolution() throws Exception { + Assume.assumeTrue(ToolHelper.artSupported()); + byte[][] classes = { + ToolHelper.getClassAsBytes(FieldResolution.class), + ToolHelper.getClassAsBytes(A.class), + ToolHelper.getClassAsBytes(B.class) + }; + AndroidApp originalApp = buildAndroidApp(classes); + Path out = temp.getRoot().toPath(); + AndroidApp processedApp = runR8(originalApp, FieldResolution.class, out); + + DexInspector dexInspector = new DexInspector(processedApp); + ClassSubject a = dexInspector.clazz(A.class.getCanonicalName()); + DexEncodedField f1 = a.field("int", "f1").getField(); + assertNotNull(f1); + DexEncodedField f3 = a.field(B.class.getCanonicalName(), "f3").getField(); + assertNotNull(f3); + // TODO(b/72858955): due to the potential reflective access, they should have different names + // by R8's improved reflective access detection or via keep rules. + assertEquals(overloadaggressively, f1.field.name == f3.field.name); + + String main = FieldResolution.class.getCanonicalName(); + ProcessResult javaOutput = runOnJava(main, classes); + assertEquals(0, javaOutput.exitCode); + ProcessResult artOutput = runOnArtRaw(processedApp, main, dexVm); + // TODO(b/72858955): R8 should avoid field resolution conflict even w/ -overloadaggressively. + if (overloadaggressively) { + assertNotEquals(0, artOutput.exitCode); + assertTrue(artOutput.stderr.contains("IllegalArgumentException")); + } else { + assertEquals(0, artOutput.exitCode); + assertEquals(javaOutput.stdout.trim(), artOutput.stdout.trim()); + // ART may dump its own debugging info through stderr. + // assertEquals(javaOutput.stderr.trim(), artOutput.stderr.trim()); + } + } + + @Test + public void methodResolution() throws Exception { + Assume.assumeTrue(ToolHelper.artSupported()); + byte[][] classes = { + ToolHelper.getClassAsBytes(MethodResolution.class), + ToolHelper.getClassAsBytes(B.class) + }; + AndroidApp originalApp = buildAndroidApp(classes); + Path out = temp.getRoot().toPath(); + AndroidApp processedApp = runR8(originalApp, MethodResolution.class, out); + + DexInspector dexInspector = new DexInspector(processedApp); + ClassSubject b = dexInspector.clazz(B.class.getCanonicalName()); + DexEncodedMethod m1 = + b.method("int", "getF1", ImmutableList.of()).getMethod(); + assertNotNull(m1); + DexEncodedMethod m2 = + b.method("java.lang.Object", "getF2", ImmutableList.of()).getMethod(); + // TODO(b/72858955): due to the potential reflective access, they should have different names. + assertEquals(overloadaggressively, m1.method.name == m2.method.name); + DexEncodedMethod m3 = + b.method("java.lang.String", "getF3", ImmutableList.of()).getMethod(); + assertNotNull(m3); + // TODO(b/72858955): ditto + assertEquals(overloadaggressively, m1.method.name == m3.method.name); + // TODO(b/72858955): ditto + assertEquals(overloadaggressively, m2.method.name == m3.method.name); + + String main = MethodResolution.class.getCanonicalName(); + ProcessResult javaOutput = runOnJava(main, classes); + assertEquals(0, javaOutput.exitCode); + ProcessResult artOutput = runOnArtRaw(processedApp, main, dexVm); + // TODO(b/72858955): R8 should avoid method resolution conflict even w/ -overloadaggressively. + if (overloadaggressively) { + assertEquals(0, artOutput.exitCode); + assertNotEquals(javaOutput.stdout.trim(), artOutput.stdout.trim()); + } else { assertEquals(0, artOutput.exitCode); assertEquals(javaOutput.stdout.trim(), artOutput.stdout.trim()); // ART may dump its own debugging info through stderr.