Reviewers: Igor Sheludko,
Message:
PTAL.
Some tests needed minor updates because they're testing builtins that don't
expect to be called on internalized strings.
Description:
Internalize strings being stored into uninitialized property cells
Please review this at https://codereview.chromium.org/804993002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+34, -17 lines):
M src/lookup.h
M src/lookup.cc
M src/objects.h
M src/objects.cc
M test/cctest/test-strings.cc
M test/mjsunit/regress/regress-1757.js
M test/mjsunit/regress/regress-crbug-320922.js
M test/mjsunit/string-slices.js
Index: src/lookup.cc
diff --git a/src/lookup.cc b/src/lookup.cc
index
55e9d689a118765c97256aee5d5f98a8138d3ce2..6a540de4df621f82adda771e9007fa5ac9adae40
100644
--- a/src/lookup.cc
+++ b/src/lookup.cc
@@ -289,7 +289,7 @@ Handle<Object> LookupIterator::GetDataValue() const {
}
-void LookupIterator::WriteDataValue(Handle<Object> value) {
+Handle<Object> LookupIterator::WriteDataValue(Handle<Object> value) {
DCHECK_EQ(DATA, state_);
Handle<JSObject> holder = GetHolder<JSObject>();
if (holder_map_->is_dictionary_map()) {
@@ -297,7 +297,7 @@ void LookupIterator::WriteDataValue(Handle<Object>
value) {
if (holder->IsGlobalObject()) {
Handle<PropertyCell> 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<Object>
value) {
} else {
DCHECK_EQ(v8::internal::CONSTANT, property_details_.type());
}
+ return value;
}
Index: src/lookup.h
diff --git a/src/lookup.h b/src/lookup.h
index
a78bb5d91d658cd7d1e04c709d5c0fe9f731cf61..b197b76971cfe302cebe0898d1eb980b9b1791a9
100644
--- a/src/lookup.h
+++ b/src/lookup.h
@@ -136,7 +136,9 @@ class LookupIterator FINAL BASE_EMBEDDED {
Handle<PropertyCell> GetPropertyCell() const;
Handle<Object> GetAccessors() const;
Handle<Object> GetDataValue() const;
- void WriteDataValue(Handle<Object> value);
+ // Usually returns the value that was passed in, but may perform
+ // non-observable modifications on it, such as internalize strings.
+ Handle<Object> WriteDataValue(Handle<Object> value);
// Checks whether the receiver is an indexed exotic object
// and name is a special numeric index.
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
c6d5daaa7d42e49a653f9861224aad0a0244b944..5b1d18f8faea1eeec193f324074c985e968921d5
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -3096,7 +3096,7 @@ MaybeHandle<Object>
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>
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<Object>
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<Object>
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<HeapType>
PropertyCell::UpdatedType(Handle<PropertyCell> cell,
}
-void PropertyCell::SetValueInferType(Handle<PropertyCell> cell,
- Handle<Object> value) {
+Handle<Object> PropertyCell::SetValueInferType(Handle<PropertyCell> cell,
+ Handle<Object> 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<String>::cast(value));
+ }
cell->set_value(*value);
if (!HeapType::Any()->Is(cell->type())) {
Handle<HeapType> new_type = UpdatedType(cell, value);
cell->set_type(*new_type);
}
+ return value;
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
02b8a0b34549a4ea04a9f7d0f25d4901a4ee3790..01b9b87ee1e777c4cb4e53add91d5a0bcab4e9da
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -9658,8 +9658,8 @@ 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<PropertyCell> cell,
- Handle<Object> value);
+ static Handle<Object> SetValueInferType(Handle<PropertyCell> cell,
+ Handle<Object> value);
// Computes the new type of the cell's contents for the given value, but
// without actually modifying the 'type' field.
Index: test/cctest/test-strings.cc
diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc
index
7475a8e51e3ad1965b30b588b5bec72598617172..d1f23f75a6bf6b0a3f9de562015a6bef81463b73
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<v8::String> 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<v8::Value> result;
Handle<String> 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);
Index: test/mjsunit/regress/regress-1757.js
diff --git a/test/mjsunit/regress/regress-1757.js
b/test/mjsunit/regress/regress-1757.js
index
35e7355c3385c87fc5c879fe84442da0b33785a1..a850f70c6532dfb525462077dd145ccec33a326f
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));
Index: test/mjsunit/regress/regress-crbug-320922.js
diff --git a/test/mjsunit/regress/regress-crbug-320922.js
b/test/mjsunit/regress/regress-crbug-320922.js
index
9ba759a43ed819eec6d1701e9b7610882e01bfd1..f19962843a31067cc34c7ede51b639d24ba55338
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);
}
Index: test/mjsunit/string-slices.js
diff --git a/test/mjsunit/string-slices.js b/test/mjsunit/string-slices.js
index
c3f889bd991a1016d7b7f38440e20d59f870372c..52f15061806f97fa12e3441604ce7298bab23296
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);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
