HStringBuilderAppend cannot be null.
Fix DCHECK() failure where we replaced an instruction
with non-null result (HInvoke StringBuilder.toString())
with an instruction that did not report that the result
cannot be null (HStringBuilderAppend) and then used the
result as both receiver and argument for String.equals().
The fix is to preserve the "cannot be null" invariant.
Test: Additional test in 697-checker-string-append.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Bug: 151107293
Bug: 19575890
Change-Id: I205f002bf8d2dfee6079ea0e14786ca0ab2e2e9c
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 817ff67..84297ec 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -2619,6 +2619,8 @@
append->SetArgumentAt(i, args[num_args - 1u - i]);
}
block->InsertInstructionBefore(append, invoke);
+ DCHECK(!invoke->CanBeNull());
+ DCHECK(!append->CanBeNull());
invoke->ReplaceWith(append);
// Copy environment, except for the StringBuilder uses.
for (HEnvironment* env = invoke->GetEnvironment(); env != nullptr; env = env->GetParent()) {
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index eece2e4..7ed5bca 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -6948,6 +6948,8 @@
bool CanThrow() const override { return true; }
+ bool CanBeNull() const override { return false; }
+
DECLARE_INSTRUCTION(StringBuilderAppend);
protected:
diff --git a/test/697-checker-string-append/src/Main.java b/test/697-checker-string-append/src/Main.java
index 9017433..c63c328 100644
--- a/test/697-checker-string-append/src/Main.java
+++ b/test/697-checker-string-append/src/Main.java
@@ -22,6 +22,7 @@
testMiscelaneous();
testNoArgs();
testInline();
+ testEquals();
System.out.println("passed");
}
@@ -265,6 +266,31 @@
assertEquals("", $noinline$appendNothing());
}
+ /// CHECK-START: boolean Main.$noinline$testAppendEquals(java.lang.String, int) instruction_simplifier (before)
+ /// CHECK-NOT: StringBuilderAppend
+
+ /// CHECK-START: boolean Main.$noinline$testAppendEquals(java.lang.String, int) instruction_simplifier (after)
+ /// CHECK: StringBuilderAppend
+ public static boolean $noinline$testAppendEquals(String s, int i) {
+ // Regression test for b/151107293 .
+ // When a string is used as both receiver and argument of String.equals(), we DCHECK()
+ // that it cannot be null. However, when replacing the call to StringBuilder.toString()
+ // with the HStringBuilderAppend(), the former reported CanBeNull() as false and
+ // therefore no explicit null checks were needed, but the replacement reported
+ // CanBeNull() as true, so when the result was used in String.equals() for both
+ // receiver and argument, the DCHECK() failed. This was fixed by overriding
+ // CanBeNull() in HStringBuilderAppend to correctly return false; the string that
+ // previously didn't require null check still does not require it.
+ String str = new StringBuilder().append(s).append(i).toString();
+ return str.equals(str);
+ }
+
+ public static void testEquals() {
+ if (!$noinline$testAppendEquals("Test", 42)) {
+ throw new Error("str.equals(str) is false");
+ }
+ }
+
public static void assertEquals(String expected, String actual) {
if (!expected.equals(actual)) {
throw new AssertionError("Expected: " + expected + ", actual: " + actual);
