fix special index parsing

R=verwaest@chromium.org,dslomov@chromium.org BUG= Review URL: https://codereview.chromium.org/1038313004 Cr-Commit-Position: refs/heads/master@{#27518}

fix special index parsing
R=verwaest@chromium.org,dslomov@chromium.org BUG= Review URL: https://codereview.chromium.org/1038313004 Cr-Commit-Position: refs/heads/master@{#27518}
97981d94 · dcarney · Commit bot · bffde6f4 · 97981d94 · 97981d94
Commit 97981d94 authored Mar 30, 2015 by dcarney Committed by Commit bot Mar 30, 2015
6 changed files
--- a/src/conversions.cc
+++ b/src/conversions.cc
@@ -503,53 +503,60 @@ double StringToDouble(UnicodeCache* unicode_cache, Handle<String> string,
 }


-bool IsNonArrayIndexInteger(String* string) {
-  const int kBufferSize = 64;
-  const int kUint32MaxChars = 11;
+bool IsSpecialIndex(UnicodeCache* unicode_cache, String* string) {
+  // Max length of canonical double: -X.XXXXXXXXXXXXXXXXX-eXXX
+  const int kBufferSize = 24;
+  const int length = string->length();
+  if (length == 0 || length > kBufferSize) return false;
  uint16_t buffer[kBufferSize];
+  String::WriteToFlat(string, buffer, 0, length);
+  // If the first char is not a digit or a '-' or we can't match 'NaN' or
+  // '(-)Infinity', bailout immediately.
  int offset = 0;
-  const int length = string->length();
-  if (length == 0) return false;
-  // First iteration, check for minus, 0 followed by anything else, etc.
-  int to = std::min(offset + kUint32MaxChars, length);
-  {
-    String::WriteToFlat(string, buffer, offset, to);
-    bool negative = false;
-    if (buffer[offset] == '-') {
-      negative = true;
-      ++offset;
-      if (offset == to) return false;  // Just '-' is bad.
-    }
-    if (buffer[offset] == '0') {
-      return to == 2 && negative;  // Match just '-0'.
-    }
-    // Process positive integers.
-    if (!negative) {
-      uint64_t acc = 0;
-      for (; offset < to; ++offset) {
-        uint64_t digit = buffer[offset] - '0';
-        if (digit > 9) return false;
-        acc = 10 * acc + digit;
-      }
-      // String is consumed.  Evaluate what we have.
-      if (offset == length) {
-        return acc >
-               static_cast<uint64_t>(std::numeric_limits<uint32_t>::max());
+  if (!IsDecimalDigit(buffer[0])) {
+    if (buffer[0] == '-') {
+      if (length == 1) return false;  // Just '-' is bad.
+      if (!IsDecimalDigit(buffer[1])) {
+        if (buffer[1] == 'I' && length == 9) {
+          // Allow matching of '-Infinity' below.
+        } else {
+          return false;
+        }
      }
+      offset++;
+    } else if (buffer[0] == 'I' && length == 8) {
+      // Allow matching of 'Infinity' below.
+    } else if (buffer[0] == 'N' && length == 3) {
+      // Match NaN.
+      return buffer[1] == 'a' && buffer[2] == 'N';
+    } else {
+      return false;
    }
  }
-  // Consume rest of string.  If we get here, we're way out of uint32_t bounds
-  // or negative.
-  int i = offset;
-  while (true) {
-    for (; offset < to; ++offset, ++i) {
-      if (!IsDecimalDigit(buffer[i])) return false;
+  // Expected fast path: key is an integer.
+  static const int kRepresentableIntegerLength = 15;  // (-)XXXXXXXXXXXXXXX
+  if (length - offset <= kRepresentableIntegerLength) {
+    const int initial_offset = offset;
+    bool matches = true;
+    for (; offset < length; offset++) {
+      matches &= IsDecimalDigit(buffer[offset]);
    }
-    if (offset == length) break;
-    // Read next chunk.
-    to = std::min(offset + kBufferSize, length);
-    String::WriteToFlat(string, buffer, offset, to);
-    i = 0;
+    if (matches) {
+      // Match 0 and -0.
+      if (buffer[initial_offset] == '0') return initial_offset == length - 1;
+      return true;
+    }
+  }
+  // Slow path: test DoubleToString(StringToDouble(string)) == string.
+  Vector<const uint16_t> vector(buffer, length);
+  double d = StringToDouble(unicode_cache, vector, NO_FLAGS);
+  if (std::isnan(d)) return false;
+  // Compute reverse string.
+  char reverse_buffer[kBufferSize + 1];  // Result will be /0 terminated.
+  Vector<char> reverse_vector(reverse_buffer, arraysize(reverse_buffer));
+  const char* reverse_string = DoubleToCString(d, reverse_vector);
+  for (int i = 0; i < length; ++i) {
+    if (static_cast<uint16_t>(reverse_string[i]) != buffer[i]) return false;
  }
  return true;
 }

--- a/src/conversions.h
+++ b/src/conversions.h
@@ -237,7 +237,8 @@ inline size_t NumberToSize(Isolate* isolate,
 }


-bool IsNonArrayIndexInteger(String* string);
+// returns DoubleToString(StringToDouble(string)) == string
+bool IsSpecialIndex(UnicodeCache* unicode_cache, String* string);
 } }  // namespace v8::internal

 #endif  // V8_CONVERSIONS_H_
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -6066,7 +6066,8 @@ bool HOptimizedGraphBuilder::PropertyAccessInfo::LookupInPrototypes() {

 bool HOptimizedGraphBuilder::PropertyAccessInfo::IsIntegerIndexedExotic() {
  InstanceType instance_type = map_->instance_type();
-  return instance_type == JS_TYPED_ARRAY_TYPE && IsNonArrayIndexInteger(*name_);
+  return instance_type == JS_TYPED_ARRAY_TYPE &&
+         IsSpecialIndex(isolate()->unicode_cache(), *name_);
 }



--- a/src/lookup.cc
+++ b/src/lookup.cc
@@ -364,7 +364,7 @@ bool LookupIterator::IsIntegerIndexedExotic(JSReceiver* holder) {
  if (name()->IsString()) {
    Handle<String> name_string = Handle<String>::cast(name());
    if (name_string->length() != 0) {
-      result = IsNonArrayIndexInteger(*name_string);
+      result = IsSpecialIndex(isolate_->unicode_cache(), *name_string);
    }
  }
  exotic_index_state_ =

--- a/test/cctest/test-conversions.cc
+++ b/test/cctest/test-conversions.cc
@@ -367,53 +367,42 @@ TEST(BitField64) {
 static void CheckNonArrayIndex(bool expected, const char* chars) {
  auto isolate = CcTest::i_isolate();
  auto string = isolate->factory()->NewStringFromAsciiChecked(chars);
-  CHECK_EQ(expected, IsNonArrayIndexInteger(*string));
+  CHECK_EQ(expected, IsSpecialIndex(isolate->unicode_cache(), *string));
 }


-TEST(NonArrayIndexParsing) {
+TEST(SpecialIndexParsing) {
  auto isolate = CcTest::i_isolate();
  HandleScope scope(isolate);
  CheckNonArrayIndex(false, "");
  CheckNonArrayIndex(false, "-");
-  CheckNonArrayIndex(false, "0");
+  CheckNonArrayIndex(true, "0");
+  CheckNonArrayIndex(true, "-0");
  CheckNonArrayIndex(false, "01");
  CheckNonArrayIndex(false, "-01");
-  CheckNonArrayIndex(false, "4294967295");
-  CheckNonArrayIndex(false, "429496.7295");
-  CheckNonArrayIndex(false, "43s3");
-  CheckNonArrayIndex(true, "-0");
+  CheckNonArrayIndex(true, "0.5");
+  CheckNonArrayIndex(true, "-0.5");
+  CheckNonArrayIndex(true, "1");
  CheckNonArrayIndex(true, "-1");
+  CheckNonArrayIndex(true, "10");
+  CheckNonArrayIndex(true, "-10");
+  CheckNonArrayIndex(true, "NaN");
+  CheckNonArrayIndex(true, "Infinity");
+  CheckNonArrayIndex(true, "-Infinity");
+  CheckNonArrayIndex(true, "4294967295");
+  CheckNonArrayIndex(true, "429496.7295");
+  CheckNonArrayIndex(true, "1.3333333333333333");
+  CheckNonArrayIndex(false, "1.3333333333333339");
+  CheckNonArrayIndex(true, "1.333333333333331e+222");
+  CheckNonArrayIndex(true, "-1.3333333333333211e+222");
+  CheckNonArrayIndex(false, "-1.3333333333333311e+222");
+  CheckNonArrayIndex(true, "429496.7295");
+  CheckNonArrayIndex(false, "43s3");
  CheckNonArrayIndex(true, "4294967296");
  CheckNonArrayIndex(true, "-4294967296");
-  CheckNonArrayIndex(
-      true,
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296");
-  CheckNonArrayIndex(
-      true,
-      "-429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296"
-      "429496729642949672964294967296429496729642949672964294967296");
+  CheckNonArrayIndex(true, "999999999999999");
+  CheckNonArrayIndex(false, "9999999999999999");
+  CheckNonArrayIndex(true, "-999999999999999");
+  CheckNonArrayIndex(false, "-9999999999999999");
+  CheckNonArrayIndex(false, "42949672964294967296429496729694966");
 }
--- a/test/mjsunit/harmony/typedarrays.js
+++ b/test/mjsunit/harmony/typedarrays.js
@@ -530,10 +530,10 @@ function TestTypedArraysWithIllegalIndices() {
   * assertEquals(undefined, a[-Infinity]);
   */
  a[1.5] = 10;
-  assertEquals(10, a[1.5]);
+  assertEquals(undefined, a[1.5]);
  var nan = Math.sqrt(-1);
  a[nan] = 5;
-  assertEquals(5, a[nan]);
+  assertEquals(undefined, a[nan]);

  var x = 0;
  var y = -0;
@@ -579,10 +579,10 @@ function TestTypedArraysWithIllegalIndicesStrict() {
   * assertEquals(undefined, a[-Infinity]);
   */
  a[1.5] = 10;
-  assertEquals(10, a[1.5]);
+  assertEquals(undefined, a[1.5]);
  var nan = Math.sqrt(-1);
  a[nan] = 5;
-  assertEquals(5, a[nan]);
+  assertEquals(undefined, a[nan]);

  var x = 0;
  var y = -0;