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

3 files changed, 30 insertions, 0 deletions

diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 817ff673cc..84297ec557 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -2619,6 +2619,8 @@ static bool TryReplaceStringBuilderAppend(HInvoke* invoke) {
     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 eece2e4b08..7ed5bca947 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -6948,6 +6948,8 @@ class HStringBuilderAppend final : public HVariableInputSizeInstruction {
 
   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 9017433ce1..c63c328aa7 100644
--- a/test/697-checker-string-append/src/Main.java
+++ b/test/697-checker-string-append/src/Main.java
@@ -22,6 +22,7 @@ public class Main {
         testMiscelaneous();
         testNoArgs();
         testInline();
+        testEquals();
         System.out.println("passed");
     }
 
@@ -265,6 +266,31 @@ public class Main {
         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);