Reviewers: Søren Gjesse,
Description:
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.
Please review this at http://codereview.chromium.org/2452007/show
Affected files:
M src/objects-inl.h
M test/cctest/test-strings.cc
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
c10c930d31958cce7cc7953f63da4119636f69c1..fe33e7ef5110a8ac6aa57e086cc0e5cbb2c603a1
100644
--- 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) { }
Index: test/cctest/test-strings.cc
diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc
index
0e30092dbbad90476188044a4821aa3945a8b684..062e8e3dbde808bc58a1582276c92c3070acc825
100644
--- a/test/cctest/test-strings.cc
+++ b/test/cctest/test-strings.cc
@@ -433,3 +433,50 @@ 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.
+ 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());
+ }
+ }
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
