Fix bug that could cause a string to be incorrectly tagged as an array index.

We should only mark a string as an array index if we can store the entire value
of the number in the hash field. We sometimes failed to reject larger numbers.

Fixes http://code.google.com/p/v8/issues/detail?id=728

Review URL: http://codereview.chromium.org/2452007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4782 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/objects-inl.h b/src/objects-inl.h
index c10c930..fe33e7e 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -2986,7 +2986,8 @@ StringHasher::StringHasher(int length)
   : length_(length),
     raw_running_hash_(0),
     array_index_(0),
-    is_array_index_(0 < length_ && length_ <= String::kMaxArrayIndexSize),
+    is_array_index_(0 < length_ &&
+                    length_ <= String::kMaxCachedArrayIndexLength),
     is_first_char_(true),
     is_valid_(true) { }
 

diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc
index 0e30092..677b39d 100644 (file)
--- a/test/cctest/test-strings.cc
+++ b/test/cctest/test-strings.cc
@@ -433,3 +433,51 @@ TEST(ExternalShortStringAdd) {
   CHECK_EQ(0,
            v8::Script::Compile(v8::String::New(source))->Run()->Int32Value());
 }
+
+
+TEST(CachedHashOverflow) {
+  // We incorrectly allowed strings to be tagged as array indices even if their
+  // values didn't fit in the hash field.
+  // See http://code.google.com/p/v8/issues/detail?id=728
+  ZoneScope zone(DELETE_ON_EXIT);
+
+  InitializeVM();
+  v8::HandleScope handle_scope;
+  // Lines must be executed sequentially. Combining them into one script
+  // makes the bug go away.
+  const char* lines[] = {
+      "var x = [];",
+      "x[4] = 42;",
+      "var s = \"1073741828\";",
+      "x[s];",
+      "x[s] = 37;",
+      "x[4];",
+      "x[s];",
+      NULL
+  };
+
+  Handle<Smi> fortytwo(Smi::FromInt(42));
+  Handle<Smi> thirtyseven(Smi::FromInt(37));
+  Handle<Object> results[] = {
+      Factory::undefined_value(),
+      fortytwo,
+      Factory::undefined_value(),
+      Factory::undefined_value(),
+      thirtyseven,
+      fortytwo,
+      thirtyseven  // Bug yielded 42 here.
+  };
+
+  const char* line;
+  for (int i = 0; (line = lines[i]); i++) {
+    printf("%s\n", line);
+    v8::Local<v8::Value> result =
+        v8::Script::Compile(v8::String::New(line))->Run();
+    ASSERT_EQ(results[i]->IsUndefined(), result->IsUndefined());
+    ASSERT_EQ(results[i]->IsNumber(), result->IsNumber());
+    if (result->IsNumber()) {
+      ASSERT_EQ(Smi::cast(results[i]->ToSmi())->value(),
+                result->ToInt32()->Value());
+    }
+  }
+}