From c060f4e26c6678c558df84492107a9ece984a585 Mon Sep 17 00:00:00 2001 From: jkummerow Date: Mon, 15 Dec 2014 07:46:01 -0800 Subject: [PATCH] Internalize strings being stored into uninitialized property cells Review URL: https://codereview.chromium.org/804993002 Cr-Commit-Position: refs/heads/master@{#25822} --- src/lookup.cc | 5 +++-- src/lookup.h | 4 +++- src/objects.cc | 21 ++++++++++++++------ src/objects.h | 6 ++++-- test/cctest/test-strings.cc | 5 +++-- test/mjsunit/regress/regress-1757.js | 3 ++- test/mjsunit/regress/regress-crbug-320922.js | 6 ++++-- test/mjsunit/string-slices.js | 3 ++- 8 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/lookup.cc b/src/lookup.cc index 55e9d689a1..6a540de4df 100644 --- a/src/lookup.cc +++ b/src/lookup.cc @@ -289,7 +289,7 @@ Handle LookupIterator::GetDataValue() const { } -void LookupIterator::WriteDataValue(Handle value) { +Handle LookupIterator::WriteDataValue(Handle value) { DCHECK_EQ(DATA, state_); Handle holder = GetHolder(); if (holder_map_->is_dictionary_map()) { @@ -297,7 +297,7 @@ void LookupIterator::WriteDataValue(Handle value) { if (holder->IsGlobalObject()) { Handle cell( PropertyCell::cast(property_dictionary->ValueAt(dictionary_entry()))); - PropertyCell::SetValueInferType(cell, value); + value = PropertyCell::SetValueInferType(cell, value); } else { property_dictionary->ValueAtPut(dictionary_entry(), *value); } @@ -306,6 +306,7 @@ void LookupIterator::WriteDataValue(Handle value) { } else { DCHECK_EQ(v8::internal::CONSTANT, property_details_.type()); } + return value; } diff --git a/src/lookup.h b/src/lookup.h index a78bb5d91d..b197b76971 100644 --- a/src/lookup.h +++ b/src/lookup.h @@ -136,7 +136,9 @@ class LookupIterator FINAL BASE_EMBEDDED { Handle GetPropertyCell() const; Handle GetAccessors() const; Handle GetDataValue() const; - void WriteDataValue(Handle value); + // Usually returns the value that was passed in, but may perform + // non-observable modifications on it, such as internalize strings. + Handle WriteDataValue(Handle value); // Checks whether the receiver is an indexed exotic object // and name is a special numeric index. diff --git a/src/objects.cc b/src/objects.cc index c6d5daaa7d..5b1d18f8fa 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3096,7 +3096,7 @@ MaybeHandle Object::SetDataProperty(LookupIterator* it, it->PrepareForDataProperty(value); // Write the property value. - it->WriteDataValue(value); + value = it->WriteDataValue(value); // Send the change record if there are observers. if (is_observed && !value->SameValue(*maybe_old.ToHandleChecked())) { @@ -3152,7 +3152,7 @@ MaybeHandle Object::AddDataProperty(LookupIterator* it, JSObject::AddSlowProperty(receiver, it->name(), value, attributes); } else { // Write the property value. - it->WriteDataValue(value); + value = it->WriteDataValue(value); } // Send the change record if there are observers. @@ -4039,7 +4039,7 @@ MaybeHandle JSObject::SetOwnPropertyIgnoreAttributes( it.ReconfigureDataProperty(value, attributes); it.PrepareForDataProperty(value); - it.WriteDataValue(value); + value = it.WriteDataValue(value); if (is_observed) { RETURN_ON_EXCEPTION( @@ -4064,7 +4064,7 @@ MaybeHandle JSObject::SetOwnPropertyIgnoreAttributes( it.ReconfigureDataProperty(value, attributes); it.PrepareForDataProperty(value); - it.WriteDataValue(value); + value = it.WriteDataValue(value); if (is_observed) { if (old_value->SameValue(*value)) { @@ -16887,13 +16887,22 @@ Handle PropertyCell::UpdatedType(Handle cell, } -void PropertyCell::SetValueInferType(Handle cell, - Handle value) { +Handle PropertyCell::SetValueInferType(Handle cell, + Handle value) { + // Heuristic: if a string is stored in a previously uninitialized + // property cell, internalize it. + if ((cell->type()->Is(HeapType::None()) || + cell->type()->Is(HeapType::Undefined())) && + value->IsString()) { + value = cell->GetIsolate()->factory()->InternalizeString( + Handle::cast(value)); + } cell->set_value(*value); if (!HeapType::Any()->Is(cell->type())) { Handle new_type = UpdatedType(cell, value); cell->set_type(*new_type); } + return value; } diff --git a/src/objects.h b/src/objects.h index 02b8a0b345..92d134f518 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9658,8 +9658,10 @@ class PropertyCell: public Cell { // of the cell's current type and the value's type. If the change causes // a change of the type of the cell's contents, code dependent on the cell // will be deoptimized. - static void SetValueInferType(Handle cell, - Handle value); + // Usually returns the value that was passed in, but may perform + // non-observable modifications on it, such as internalize strings. + static Handle SetValueInferType(Handle cell, + Handle value); // Computes the new type of the cell's contents for the given value, but // without actually modifying the 'type' field. diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc index 7475a8e51e..d1f23f75a6 100644 --- a/test/cctest/test-strings.cc +++ b/test/cctest/test-strings.cc @@ -1024,7 +1024,8 @@ TEST(JSONStringifySliceMadeExternal) { "var underlying = 'abcdefghijklmnopqrstuvwxyz';" "underlying")->ToString(CcTest::isolate()); v8::Handle slice = CompileRun( - "var slice = underlying.slice(1);" + "var slice = '';" + "slice = underlying.slice(1);" "slice")->ToString(CcTest::isolate()); CHECK(v8::Utils::OpenHandle(*slice)->IsSlicedString()); CHECK(v8::Utils::OpenHandle(*underlying)->IsSeqOneByteString()); @@ -1183,7 +1184,7 @@ TEST(SliceFromSlice) { v8::Local result; Handle string; const char* init = "var str = 'abcdefghijklmnopqrstuvwxyz';"; - const char* slice = "var slice = str.slice(1,-1); slice"; + const char* slice = "var slice = ''; slice = str.slice(1,-1); slice"; const char* slice_from_slice = "slice.slice(1,-1);"; CompileRun(init); diff --git a/test/mjsunit/regress/regress-1757.js b/test/mjsunit/regress/regress-1757.js index 35e7355c33..a850f70c65 100644 --- a/test/mjsunit/regress/regress-1757.js +++ b/test/mjsunit/regress/regress-1757.js @@ -27,6 +27,7 @@ // Flags: --string-slices --expose-externalize-string -var a = "abcdefghijklmnopqrstuvqxy"+"z"; +var a = "internalized dummy"; +a = "abcdefghijklmnopqrstuvqxy"+"z"; externalizeString(a, true); assertEquals('b', a.substring(1).charAt(0)); diff --git a/test/mjsunit/regress/regress-crbug-320922.js b/test/mjsunit/regress/regress-crbug-320922.js index 9ba759a43e..f19962843a 100644 --- a/test/mjsunit/regress/regress-crbug-320922.js +++ b/test/mjsunit/regress/regress-crbug-320922.js @@ -27,8 +27,10 @@ // Flags: --allow-natives-syntax -var string = "hello world"; -var expected = "Hello " + "world"; +var string = "internalized dummy"; +var expected = "internalized dummy"; +string = "hello world"; +expected = "Hello " + "world"; function Capitalize() { %_OneByteSeqStringSetChar(0, 0x48, string); } diff --git a/test/mjsunit/string-slices.js b/test/mjsunit/string-slices.js index c3f889bd99..52f1506180 100644 --- a/test/mjsunit/string-slices.js +++ b/test/mjsunit/string-slices.js @@ -193,7 +193,8 @@ assertEquals("\u03B2\u03B3\u03B4\u03B5\u03B4\u03B5\u03B6\u03B7", utf.substring(5,1) + utf.substring(3,7)); // Externalizing strings. -var a = "123456789" + "qwertyuiopasdfghjklzxcvbnm"; +var a = "internalized dummy"; +a = "123456789" + "qwertyuiopasdfghjklzxcvbnm"; var b = "23456789qwertyuiopasdfghjklzxcvbn" assertEquals(a.slice(1,-1), b);