Fix supplementary character support.

Fixes all known bugs in our handling of supplementary characters. This change
introduces a performance regression on the assumption that it won't be released
without a corresponding JIT change to enable the code to be inlined back to
pretty much what it used to be.

Bug: 2587122
Change-Id: I3449c9718bbe32ebe53b6c10454ae1dc82105b59

5 files changed, 69 insertions, 91 deletions

diff --git a/libcore/luni/src/main/java/java/lang/String.java b/libcore/luni/src/main/java/java/lang/String.java
index cb39e2567..7de809445 100644
--- a/libcore/luni/src/main/java/java/lang/String.java
+++ b/libcore/luni/src/main/java/java/lang/String.java
@@ -1130,20 +1130,11 @@ public final class String implements Serializable, Comparable<String>,
      *         character isn't found.
      */
     public int indexOf(int c) {
-        // BEGIN android-changed
-        int _count = count;
-        if (0 < _count) {
-            int _offset = offset;
-            int last = _offset + _count;
-            char[] _value = value;
-            for (int i = _offset; i < last; i++) {
-                if (_value[i] == c) {
-                    return i - _offset;
-                }
-            }
+        // TODO: just "return indexOf(c, 0);" when the JIT can inline that deep.
+        if (c > 0xffff) {
+            return indexOfSupplementary(c, 0);
         }
-        return -1;
-        // END android-changed
+        return fastIndexOf(c, 0);
     }
 
     /**
@@ -1159,6 +1150,13 @@ public final class String implements Serializable, Comparable<String>,
      *         character isn't found.
      */
     public int indexOf(int c, int start) {
+        if (c > 0xffff) {
+            return indexOfSupplementary(c, start);
+        }
+        return fastIndexOf(c, start);
+    }
+
+    private int fastIndexOf(int c, int start) {
         // BEGIN android-changed
         int _count = count;
         if (start < _count) {
@@ -1178,6 +1176,15 @@ public final class String implements Serializable, Comparable<String>,
         // END android-changed
     }
 
+    private int indexOfSupplementary(int c, int start) {
+        if (!Character.isSupplementaryCodePoint(c)) {
+            return -1;
+        }
+        char[] chars = Character.toChars(c);
+        String needle = new String(0, chars.length, chars);
+        return indexOf(needle, start);
+    }
+
     /**
      * Searches in this string for the first index of the specified string. The
      * search for the string starts at the beginning and moves towards the end
@@ -1303,6 +1310,9 @@ public final class String implements Serializable, Comparable<String>,
      *         character isn't found.
      */
     public int lastIndexOf(int c) {
+        if (c > 0xffff) {
+            return lastIndexOfSupplementary(c, Integer.MAX_VALUE);
+        }
         // BEGIN android-changed
         int _count = count;
         int _offset = offset;
@@ -1329,6 +1339,9 @@ public final class String implements Serializable, Comparable<String>,
      *         character isn't found.
      */
     public int lastIndexOf(int c, int start) {
+        if (c > 0xffff) {
+            return lastIndexOfSupplementary(c, start);
+        }
         // BEGIN android-changed
         int _count = count;
         int _offset = offset;
@@ -1347,6 +1360,15 @@ public final class String implements Serializable, Comparable<String>,
         // END android-changed
     }
 
+    private int lastIndexOfSupplementary(int c, int start) {
+        if (!Character.isSupplementaryCodePoint(c)) {
+            return -1;
+        }
+        char[] chars = Character.toChars(c);
+        String needle = new String(0, chars.length, chars);
+        return lastIndexOf(needle, start);
+    }
+
     /**
      * Searches in this string for the last index of the specified string. The
      * search for the string starts at the end and moves towards the beginning
@@ -2181,15 +2203,9 @@ public final class String implements Serializable, Comparable<String>,
     }
 
     /**
-     * Retrieves the Unicode code point (character) value at the specified
-     * {@code index}.
+     * Returns the Unicode code point at the given {@code index}.
      * 
-     * @param index
-     *            the index to the {@code char} code unit within this string.
-     * @return the Unicode code point value.
-     * @throws IndexOutOfBoundsException
-     *             if {@code index} is negative or greater than or equal to
-     *             {@code length()}.
+     * @throws IndexOutOfBoundsException if {@code index < 0 || index >= length()}
      * @see Character#codePointAt(char[], int, int)
      * @since 1.5
      */
@@ -2197,20 +2213,13 @@ public final class String implements Serializable, Comparable<String>,
         if (index < 0 || index >= count) {
             throw new StringIndexOutOfBoundsException();
         }
-        int s = index + offset;
-        return Character.codePointAt(value, s, offset + count);
+        return Character.codePointAt(value, offset + index, offset + count);
     }
 
     /**
-     * Retrieves the Unicode code point value that precedes the specified
-     * {@code index}.
+     * Returns the Unicode code point that precedes the given {@code index}.
      * 
-     * @param index
-     *            the index to the {@code char} code unit within this string.
-     * @return the Unicode code point value.
-     * @throws IndexOutOfBoundsException
-     *             if {@code index} is less than 1 or greater than
-     *             {@code length()}.
+     * @throws IndexOutOfBoundsException if {@code index < 1 || index > length()}
      * @see Character#codePointBefore(char[], int, int)
      * @since 1.5
      */
@@ -2218,8 +2227,7 @@ public final class String implements Serializable, Comparable<String>,
         if (index < 1 || index > count) {
             throw new StringIndexOutOfBoundsException();
         }
-        int s = index + offset;
-        return Character.codePointBefore(value, s);
+        return Character.codePointBefore(value, offset + index, offset);
     }
 
     /**
@@ -2232,9 +2240,7 @@ public final class String implements Serializable, Comparable<String>,
      *            the exclusive end index of the subsequence.
      * @return the number of Unicode code points in the subsequence.
      * @throws IndexOutOfBoundsException
-     *             if {@code beginIndex} is negative or greater than {@code
-     *             endIndex} or {@code endIndex} is greater than {@code
-     *             length()}.
+     *         if {@code beginIndex < 0 || endIndex > length() || beginIndex > endIndex}
      * @see Character#codePointCount(CharSequence, int, int)
      * @since 1.5
      */
@@ -2242,8 +2248,7 @@ public final class String implements Serializable, Comparable<String>,
         if (beginIndex < 0 || endIndex > count || beginIndex > endIndex) {
             throw new StringIndexOutOfBoundsException();
         }
-        int s = beginIndex + offset;
-        return Character.codePointCount(value, s, endIndex - beginIndex);
+        return Character.codePointCount(value, offset + beginIndex, endIndex - beginIndex);
     }
 
     /**
@@ -2280,8 +2285,7 @@ public final class String implements Serializable, Comparable<String>,
      */
     public int offsetByCodePoints(int index, int codePointOffset) {
         int s = index + offset;
-        int r = Character.offsetByCodePoints(value, offset, count, s,
-                codePointOffset);
+        int r = Character.offsetByCodePoints(value, offset, count, s, codePointOffset);
         return r - offset;
     }
 
diff --git a/vm/DalvikVersion.h b/vm/DalvikVersion.h
index 6f67d7e8c..4f9eeb85f 100644
--- a/vm/DalvikVersion.h
+++ b/vm/DalvikVersion.h
@@ -32,6 +32,6 @@
  * way classes load changes, e.g. field ordering or vtable layout.  Changing
  * this guarantees that the optimized form of the DEX file is regenerated.
  */
-#define DALVIK_VM_BUILD         21
+#define DALVIK_VM_BUILD         22
 
 #endif /*_DALVIK_VERSION*/
diff --git a/vm/InlineNative.c b/vm/InlineNative.c
index 22d8fbb86..555e956de 100644
--- a/vm/InlineNative.c
+++ b/vm/InlineNative.c
@@ -407,8 +407,8 @@ static bool javaLangString_isEmpty(u4 arg0, u4 arg1, u4 arg2, u4 arg3,
  * Determine the index of the first character matching "ch".  The string
  * to search is described by "chars", "offset", and "count".
  *
- * The "ch" parameter is allowed to be > 0xffff.  Our Java-language
- * implementation does not currently handle this, so neither do we.
+ * The character must be <= 0xffff. Supplementary characters are handled in
+ * Java.
  *
  * The "start" parameter must be clamped to [0..count].
  *
@@ -455,27 +455,13 @@ static inline int indexOfCommon(Object* strObj, int ch, int start)
 }
 
 /*
- * public int indexOf(int c)
- *
- * Scan forward through the string for a matching character.
- */
-static bool javaLangString_indexOf_I(u4 arg0, u4 arg1, u4 arg2, u4 arg3,
-    JValue* pResult)
-{
-    /* null reference check on "this" */
-    if (!dvmValidateObject((Object*) arg0))
-        return false;
-
-    pResult->i = indexOfCommon((Object*) arg0, arg1, 0);
-    return true;
-}
-
-/*
  * public int indexOf(int c, int start)
  *
  * Scan forward through the string for a matching character.
+ * The character must be <= 0xffff; this method does not handle supplementary
+ * characters.
  */
-static bool javaLangString_indexOf_II(u4 arg0, u4 arg1, u4 arg2, u4 arg3,
+static bool javaLangString_fastIndexOf_II(u4 arg0, u4 arg1, u4 arg2, u4 arg3,
     JValue* pResult)
 {
     /* null reference check on "this" */
@@ -666,10 +652,8 @@ const InlineOperation gDvmInlineOpsTable[] = {
         "Ljava/lang/String;", "compareTo", "(Ljava/lang/String;)I" },
     { javaLangString_equals,
         "Ljava/lang/String;", "equals", "(Ljava/lang/Object;)Z" },
-    { javaLangString_indexOf_I,
-        "Ljava/lang/String;", "indexOf", "(I)I" },
-    { javaLangString_indexOf_II,
-        "Ljava/lang/String;", "indexOf", "(II)I" },
+    { javaLangString_fastIndexOf_II,
+        "Ljava/lang/String;", "fastIndexOf", "(II)I" },
     { javaLangString_isEmpty,
         "Ljava/lang/String;", "isEmpty", "()Z" },
     { javaLangString_length,
diff --git a/vm/InlineNative.h b/vm/InlineNative.h
index 890226e97..9b9ae3910 100644
--- a/vm/InlineNative.h
+++ b/vm/InlineNative.h
@@ -54,19 +54,18 @@ typedef enum NativeInlineOps {
     INLINE_STRING_CHARAT = 1,
     INLINE_STRING_COMPARETO = 2,
     INLINE_STRING_EQUALS = 3,
-    INLINE_STRING_INDEXOF_I = 4,
-    INLINE_STRING_INDEXOF_II = 5,
-    INLINE_STRING_IS_EMPTY = 6,
-    INLINE_STRING_LENGTH = 7,
-    INLINE_MATH_ABS_INT = 8,
-    INLINE_MATH_ABS_LONG = 9,
-    INLINE_MATH_ABS_FLOAT = 10,
-    INLINE_MATH_ABS_DOUBLE = 11,
-    INLINE_MATH_MIN_INT = 12,
-    INLINE_MATH_MAX_INT = 13,
-    INLINE_MATH_SQRT = 14,
-    INLINE_MATH_COS = 15,
-    INLINE_MATH_SIN = 16,
+    INLINE_STRING_FASTINDEXOF_II = 4,
+    INLINE_STRING_IS_EMPTY = 5,
+    INLINE_STRING_LENGTH = 6,
+    INLINE_MATH_ABS_INT = 7,
+    INLINE_MATH_ABS_LONG = 8,
+    INLINE_MATH_ABS_FLOAT = 9,
+    INLINE_MATH_ABS_DOUBLE = 10,
+    INLINE_MATH_MIN_INT = 11,
+    INLINE_MATH_MAX_INT = 12,
+    INLINE_MATH_SQRT = 13,
+    INLINE_MATH_COS = 14,
+    INLINE_MATH_SIN = 15,
 } NativeInlineOps;
 
 /*
diff --git a/vm/compiler/codegen/arm/CodegenDriver.c b/vm/compiler/codegen/arm/CodegenDriver.c
index 2f157a3bc..8d63c6623 100644
--- a/vm/compiler/codegen/arm/CodegenDriver.c
+++ b/vm/compiler/codegen/arm/CodegenDriver.c
@@ -2966,7 +2966,7 @@ static bool genInlinedCompareTo(CompilationUnit *cUnit, MIR *mir)
 #endif
 }
 
-static bool genInlinedIndexOf(CompilationUnit *cUnit, MIR *mir, bool singleI)
+static bool genInlinedFastIndexOf(CompilationUnit *cUnit, MIR *mir)
 {
 #if defined(USE_GLOBAL_STRING_DEFS)
     return false;
@@ -2976,12 +2976,8 @@ static bool genInlinedIndexOf(CompilationUnit *cUnit, MIR *mir, bool singleI)
 
     loadValueDirectFixed(cUnit, rlThis, r0);
     loadValueDirectFixed(cUnit, rlChar, r1);
-    if (!singleI) {
-        RegLocation rlStart = dvmCompilerGetSrc(cUnit, mir, 2);
-        loadValueDirectFixed(cUnit, rlStart, r2);
-    } else {
-        loadConstant(cUnit, r2, 0);
-    }
+    RegLocation rlStart = dvmCompilerGetSrc(cUnit, mir, 2);
+    loadValueDirectFixed(cUnit, rlStart, r2);
     /* Test objects for NULL */
     genNullCheck(cUnit, rlThis.sRegLow, r0, mir->offset, NULL);
     genDispatchToHandler(cUnit, TEMPLATE_STRING_INDEXOF);
@@ -3144,13 +3140,8 @@ static bool handleExecuteInline(CompilationUnit *cUnit, MIR *mir)
                         return false;
                     else
                         break;
-                case INLINE_STRING_INDEXOF_I:
-                    if (genInlinedIndexOf(cUnit, mir, true /* I */))
-                        return false;
-                    else
-                        break;
-                case INLINE_STRING_INDEXOF_II:
-                    if (genInlinedIndexOf(cUnit, mir, false /* I */))
+                case INLINE_STRING_FASTINDEXOF_II:
+                    if (genInlinedFastIndexOf(cUnit, mir))
                         return false;
                     else
                         break;