Regress78493232_WithPhi: Fix expectation for failing test
The test introduced in Ic278219f9 72548942a ("Remove trivial phis for
all string new-instance values.", 2018-06-01) only checks that the
stdout when running on ART is the same as when running on JVM.
Since the test program fails with empty stdout on JVM and all ART except
for version 6.0.1, the test would only fail (different stdout) when run
on Art 6.0.1. However, the behavior is only the same on ART 5.1.1 (the
test method returns null); on all other ART versions, the behavior is
different from the JVM:
* On 4.0.4, 4.4.4, 7.0.0 and tip-of-tree, the test program is rejected
by the Dalvik/ART verifier after being processed by D8.
* On 6.0.1 a different execution is observed:
"java.security.SecureRandom" is returned by the test method.
Ensure that the test does not throw NullPointerException on JVM and ART
5.1.1 by swapping a null and non-null argument to .equals(), and specify
the observed incorrect outcomes instead of using ensureSameOutput().
Also assert in AsmTestBase.ensureSameOutput() that exitCode is 0.
Bug: 78493232, 80118070, 109645295
Change-Id: I06392e5c0ec34de26cb154c4ce7178ad84f3ebe0
diff --git a/src/test/java/com/android/tools/r8/AsmTestBase.java b/src/test/java/com/android/tools/r8/AsmTestBase.java
index 9cc2b48..8038c98 100644
--- a/src/test/java/com/android/tools/r8/AsmTestBase.java
+++ b/src/test/java/com/android/tools/r8/AsmTestBase.java
@@ -64,6 +64,10 @@
Assert.assertEquals(javaResult.stdout, d8Result.stdout);
Assert.assertEquals(javaResult.stdout, r8Result.stdout);
Assert.assertEquals(javaResult.stdout, r8ShakenResult.stdout);
+ Assert.assertEquals(0, javaResult.exitCode);
+ Assert.assertEquals(0, d8Result.exitCode);
+ Assert.assertEquals(0, r8Result.exitCode);
+ Assert.assertEquals(0, r8ShakenResult.exitCode);
}
protected void ensureR8FailsWithCompilationError(String main, byte[]... classes)
diff --git a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Dump_WithPhi.java b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Dump_WithPhi.java
index 8a6bdcf..bfb0674 100644
--- a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Dump_WithPhi.java
+++ b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Dump_WithPhi.java
@@ -231,21 +231,25 @@
mv.visitJumpInsn(IFEQ, l7_b1);
mv.visitLabel(l7_b0);
mv.visitInsn(ACONST_NULL);
+ // At this point, NULL flows to l7_join.
mv.visitJumpInsn(GOTO, l7_join);
mv.visitLabel(l7_b1);
mv.visitVarInsn(ALOAD, 10);
- // Swap the new-instance or null, with the byte-array.
+ // At this point, an uninitialized String flows to l7_join.
mv.visitLabel(l7_join);
+ // At this point, stack contains [staticByteArray, NULL/uninitialized]
mv.visitInsn(SWAP);
- // Load the new-instacne and swap again with the byte-array.
+ // At this point, stack contains [NULL/uninitialized, staticByteArray]
mv.visitVarInsn(ALOAD, 10);
+ // At this point, stack contains [NULL/uninitialized, staticByteArray, uninitialized]
mv.visitInsn(SWAP);
+ // At this point, stack contains [NULL/uninitialized, uninitialized, staticByteArray]
mv.visitInsn(ICONST_0);
- // Invoke special will now always be on the new-instance as receiver.
+ // At this point, stack contains [NULL/uninitialized, uninitialized, staticByteArray, 0]
mv.visitMethodInsn(INVOKESPECIAL, "java/lang/String", "<init>", "([BI)V", false);
- // Return will be either new-instance or null.
+ // Just before ARETURN, stack contains [NULL/String].
// This will force a non-trivial phi of the new-instance on this block, ie, prior to <init>.
- // This phi must remain.
+ // This phi must remain since it is not trivial.
mv.visitInsn(ARETURN);
mv.visitLabel(l8);
mv.visitVarInsn(ILOAD, 0);
diff --git a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Utils.java b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Utils.java
index ff7da8f..ccf6467 100644
--- a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Utils.java
+++ b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232Utils.java
@@ -31,13 +31,14 @@
public static void compare(String output, int iterations) {
String expected = "java.security.SecureRandom";
- if (output.equals(expected)) {
+ if (expected.equals(output)) {
return;
}
System.out.println(
"After " + iterations + " iterations, expected \"" +
expected + "\", but got \"" + output + "\"");
- System.exit(1);
+ // Exit with code 0 to allow test to use ensureSameOutput().
+ System.exit(0);
}
public static void compareHash(int a, int b, byte[] c, int iterations) {
@@ -53,6 +54,7 @@
System.out.println("staticIntB: " + b);
System.out.print("staticIntByteArray: ");
printByteArray(c);
- System.exit(1);
+ // Exit with code 0 to allow test to use ensureSameOutput().
+ System.exit(0);
}
}
diff --git a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232_WithPhi.java b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232_WithPhi.java
index 3c7c465..3a56004 100644
--- a/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232_WithPhi.java
+++ b/src/test/java/com/android/tools/r8/regress/b78493232/Regress78493232_WithPhi.java
@@ -3,8 +3,15 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.regress.b78493232;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
import com.android.tools.r8.AsmTestBase;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.utils.AndroidApp;
import org.junit.Test;
// Variant of Regress78493232, but where the new-instance is forced to flow to a non-trivial phi
@@ -15,6 +22,57 @@
public void test() throws Exception {
// Run test on JVM and ART(x86) to ensure expected behavior.
// Running the same test on an ARM JIT causes errors.
+
+ // TODO(b/80118070): Remove this if-statement when fixed.
+ if (ToolHelper.getDexVm().getVersion() != Version.V5_1_1) {
+ AndroidApp app =
+ buildAndroidApp(
+ Regress78493232Dump_WithPhi.dump(),
+ ToolHelper.getClassAsBytes(Regress78493232Utils.class));
+ ProcessResult javaResult =
+ runOnJava(
+ Regress78493232Dump_WithPhi.CLASS_NAME,
+ Regress78493232Dump_WithPhi.dump(),
+ ToolHelper.getClassAsBytes(Regress78493232Utils.class));
+ ProcessResult d8Result =
+ runOnArtRaw(compileWithD8(app), Regress78493232Dump_WithPhi.CLASS_NAME);
+ ProcessResult r8Result =
+ runOnArtRaw(compileWithR8(app), Regress78493232Dump_WithPhi.CLASS_NAME);
+ String proguardConfig =
+ keepMainProguardConfiguration(Regress78493232Dump_WithPhi.CLASS_NAME)
+ + "-dontobfuscate\n";
+ ProcessResult r8ShakenResult =
+ runOnArtRaw(compileWithR8(app, proguardConfig), Regress78493232Dump_WithPhi.CLASS_NAME);
+ assertEquals(
+ "After 0 iterations, expected \"java.security.SecureRandom\", but got \"null\"\n",
+ javaResult.stdout);
+ assertEquals(0, javaResult.exitCode);
+ switch (ToolHelper.getDexVm().getVersion()) {
+ case V4_0_4:
+ case V4_4_4:
+ case V7_0_0:
+ case DEFAULT:
+ assertNotEquals(-1, d8Result.stderr.indexOf("java.lang.VerifyError"));
+ assertNotEquals(-1, r8Result.stderr.indexOf("java.lang.VerifyError"));
+ assertNotEquals(-1, r8ShakenResult.stderr.indexOf("java.lang.VerifyError"));
+ assertEquals(1, d8Result.exitCode);
+ assertEquals(1, r8Result.exitCode);
+ assertEquals(1, r8ShakenResult.exitCode);
+ break;
+ case V6_0_1:
+ assertEquals("Completed successfully after 1000 iterations\n", d8Result.stdout);
+ assertEquals("Completed successfully after 1000 iterations\n", r8Result.stdout);
+ assertEquals("Completed successfully after 1000 iterations\n", r8ShakenResult.stdout);
+ assertEquals(0, d8Result.exitCode);
+ assertEquals(0, r8Result.exitCode);
+ assertEquals(0, r8ShakenResult.exitCode);
+ break;
+ case V5_1_1:
+ default:
+ throw new Unreachable();
+ }
+ return;
+ }
ensureSameOutput(
Regress78493232Dump_WithPhi.CLASS_NAME,
Regress78493232Dump_WithPhi.dump(),
