Revision: 6205
Author: [email protected]
Date: Thu Jan 6 06:00:50 2011
Log: Avoid calling inherited setters when creating object literals and
their boilerplates.
Fix issue 1015.
Review URL: http://codereview.chromium.org/6118001
http://code.google.com/p/v8/source/detail?r=6205
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-1015.js
Deleted:
/branches/bleeding_edge/test/mjsunit/bugs/bug-1015.js
Modified:
/branches/bleeding_edge/src/accessors.cc
/branches/bleeding_edge/src/handles.cc
/branches/bleeding_edge/src/handles.h
/branches/bleeding_edge/src/heap-inl.h
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/mjsunit/regress/regress-192.js
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1015.js Thu Jan 6
06:00:50 2011
@@ -0,0 +1,66 @@
+// Copyright 2010 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// See: http://code.google.com/p/v8/issues/detail?id=1015
+
+// Object and array literals should be created using DefineOwnProperty, and
+// therefore not hit setters in the prototype.
+
+function mkFail(message) {
+ return function () { assertUnreachable(message); }
+}
+
+Object.defineProperty(Object.prototype, "foo",
+ {get: mkFail("oget"), set: mkFail("oset")});
+Object.defineProperty(Array.prototype, "2",
+ {get: mkFail("aget"), set: mkFail("aset")});
+
+function inFunction() {
+ for (var i = 0; i < 10; i++) {
+ // in loop.
+ var ja = JSON.parse('[1,2,3,4]');
+ var jo = JSON.parse('{"bar": 10, "foo": 20}')
+ var jop = JSON.parse('{"bar": 10, "__proto__": { }, "foo": 20}')
+ var a = [1,2,3,4];
+ var o = { bar: 10, foo: 20 };
+ var op = { __proto__: { set bar(v) { assertUnreachable("bset"); } },
+ bar: 10 };
+ }
+}
+
+for (var i = 0; i < 10; i++) {
+ // In global scope.
+ var ja = JSON.parse('[1,2,3,4]');
+ var jo = JSON.parse('{"bar": 10, "foo": 20}')
+ var jop = JSON.parse('{"bar": 10, "__proto__": { }, "foo": 20}')
+ var a = [1,2,3,4];
+ var o = { bar: 10, foo: 20 };
+ var op = { __proto__: { set bar(v) { assertUnreachable("bset"); } },
+ bar: 10 };
+ // In function scope.
+ inFunction();
+}
=======================================
--- /branches/bleeding_edge/test/mjsunit/bugs/bug-1015.js Tue Jan 4
05:14:39 2011
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright 2010 the V8 project authors. All rights reserved.
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-// * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-// * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following
-// disclaimer in the documentation and/or other materials provided
-// with the distribution.
-// * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived
-// from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// See: http://code.google.com/p/v8/issues/detail?id=1015
-
-// Object and array literals should be created using DefineOwnProperty, and
-// therefore not hit setters in the prototype.
-
-function mkFail(message) {
- return function () { assertUnreachable(message); }
-}
-
-Object.defineProperty(Object.prototype, "foo",
- {get: mkFail("oget"), set: mkFail("oset")});
-Object.defineProperty(Array.prototype, "2",
- {get: mkFail("aget"), set: mkFail("aset")});
-
-function inFunction() {
- for (var i = 0; i < 10; i++) {
- // in loop.
- var ja = JSON.parse('[1,2,3,4]');
- var jo = JSON.parse('{"bar": 10, "foo": 20}')
- var jop = JSON.parse('{"bar": 10, "__proto__": { }, "foo": 20}')
- var a = [1,2,3,4];
- var o = { bar: 10, foo: 20 };
- var op = { __proto__: { set bar(v) { assertUnreachable("bset"); } },
- bar: 10 };
- }
-}
-
-for (var i = 0; i < 10; i++) {
- // In global scope.
- var ja = JSON.parse('[1,2,3,4]');
- var jo = JSON.parse('{"bar": 10, "foo": 20}')
- var jop = JSON.parse('{"bar": 10, "__proto__": { }, "foo": 20}')
- var a = [1,2,3,4];
- var o = { bar: 10, foo: 20 };
- var op = { __proto__: { set bar(v) { assertUnreachable("bset"); } },
- bar: 10 };
- // In function scope.
- inFunction();
-}
=======================================
--- /branches/bleeding_edge/src/accessors.cc Tue Dec 7 03:31:57 2010
+++ /branches/bleeding_edge/src/accessors.cc Thu Jan 6 06:00:50 2011
@@ -126,8 +126,8 @@
// This means one of the object's prototypes is a JSArray and
// the object does not have a 'length' property.
// Calling SetProperty causes an infinite loop.
- return
object->IgnoreAttributesAndSetLocalProperty(Heap::length_symbol(),
- value, NONE);
+ return
object->SetLocalPropertyIgnoreAttributes(Heap::length_symbol(),
+ value, NONE);
}
}
return Top::Throw(*Factory::NewRangeError("invalid_array_length",
=======================================
--- /branches/bleeding_edge/src/handles.cc Tue Dec 7 03:31:57 2010
+++ /branches/bleeding_edge/src/handles.cc Thu Jan 6 06:00:50 2011
@@ -280,13 +280,13 @@
}
-Handle<Object> IgnoreAttributesAndSetLocalProperty(
+Handle<Object> SetLocalPropertyIgnoreAttributes(
Handle<JSObject> object,
Handle<String> key,
Handle<Object> value,
PropertyAttributes attributes) {
CALL_HEAP_FUNCTION(object->
- IgnoreAttributesAndSetLocalProperty(*key, *value, attributes),
Object);
+ SetLocalPropertyIgnoreAttributes(*key, *value, attributes), Object);
}
@@ -420,6 +420,15 @@
}
CALL_HEAP_FUNCTION(object->SetElement(index, *value), Object);
}
+
+
+Handle<Object> SetOwnElement(Handle<JSObject> object,
+ uint32_t index,
+ Handle<Object> value) {
+ ASSERT(!object->HasPixelElements());
+ ASSERT(!object->HasExternalArrayElements());
+ CALL_HEAP_FUNCTION(object->SetElement(index, *value, false), Object);
+}
Handle<JSObject> Copy(Handle<JSObject> obj) {
=======================================
--- /branches/bleeding_edge/src/handles.h Tue Dec 7 03:31:57 2010
+++ /branches/bleeding_edge/src/handles.h Thu Jan 6 06:00:50 2011
@@ -217,9 +217,10 @@
Handle<Object> ForceDeleteProperty(Handle<JSObject> object,
Handle<Object> key);
-Handle<Object> IgnoreAttributesAndSetLocalProperty(Handle<JSObject> object,
- Handle<String> key,
- Handle<Object> value,
+Handle<Object> SetLocalPropertyIgnoreAttributes(
+ Handle<JSObject> object,
+ Handle<String> key,
+ Handle<Object> value,
PropertyAttributes attributes);
Handle<Object> SetPropertyWithInterceptor(Handle<JSObject> object,
@@ -231,6 +232,10 @@
uint32_t index,
Handle<Object> value);
+Handle<Object> SetOwnElement(Handle<JSObject> object,
+ uint32_t index,
+ Handle<Object> value);
+
Handle<Object> GetProperty(Handle<JSObject> obj,
const char* name);
=======================================
--- /branches/bleeding_edge/src/heap-inl.h Tue Jan 4 04:19:55 2011
+++ /branches/bleeding_edge/src/heap-inl.h Thu Jan 6 06:00:50 2011
@@ -521,10 +521,6 @@
CALL_AND_RETRY(FUNCTION_CALL, return, return)
-#define CALL_HEAP_FUNCTION_INLINE(FUNCTION_CALL) \
- CALL_AND_RETRY(FUNCTION_CALL, break, break)
-
-
#ifdef DEBUG
inline bool Heap::allow_allocation(bool new_state) {
=======================================
--- /branches/bleeding_edge/src/objects.cc Tue Jan 4 05:59:34 2011
+++ /branches/bleeding_edge/src/objects.cc Thu Jan 6 06:00:50 2011
@@ -1823,8 +1823,9 @@
// We only need to deal with CALLBACKS and INTERCEPTORS
MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(LookupResult*
result,
String* name,
- Object* value) {
- if (!result->IsProperty()) {
+ Object* value,
+ bool
check_prototype) {
+ if (check_prototype && !result->IsProperty()) {
LookupCallbackSetterInPrototypes(name, result);
}
@@ -1850,7 +1851,8 @@
LookupResult r;
LookupRealNamedProperty(name, &r);
if (r.IsProperty()) {
- return SetPropertyWithFailedAccessCheck(&r, name, value);
+ return SetPropertyWithFailedAccessCheck(&r, name, value,
+ check_prototype);
}
break;
}
@@ -1891,7 +1893,7 @@
// Check access rights if needed.
if (IsAccessCheckNeeded()
&& !Top::MayNamedAccess(this, name, v8::ACCESS_SET)) {
- return SetPropertyWithFailedAccessCheck(result, name, value);
+ return SetPropertyWithFailedAccessCheck(result, name, value, true);
}
if (IsJSGlobalProxy()) {
@@ -1981,7 +1983,7 @@
// callback setter removed. The two lines looking up the LookupResult
// result are also added. If one of the functions is changed, the other
// should be.
-MaybeObject* JSObject::IgnoreAttributesAndSetLocalProperty(
+MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
String* name,
Object* value,
PropertyAttributes attributes) {
@@ -1993,14 +1995,14 @@
// Check access rights if needed.
if (IsAccessCheckNeeded()
&& !Top::MayNamedAccess(this, name, v8::ACCESS_SET)) {
- return SetPropertyWithFailedAccessCheck(&result, name, value);
+ return SetPropertyWithFailedAccessCheck(&result, name, value, false);
}
if (IsJSGlobalProxy()) {
Object* proto = GetPrototype();
if (proto->IsNull()) return value;
ASSERT(proto->IsJSGlobalObject());
- return JSObject::cast(proto)->IgnoreAttributesAndSetLocalProperty(
+ return JSObject::cast(proto)->SetLocalPropertyIgnoreAttributes(
name,
value,
attributes);
=======================================
--- /branches/bleeding_edge/src/objects.h Tue Jan 4 04:19:55 2011
+++ /branches/bleeding_edge/src/objects.h Thu Jan 6 06:00:50 2011
@@ -1341,7 +1341,8 @@
MUST_USE_RESULT MaybeObject* SetPropertyWithFailedAccessCheck(
LookupResult* result,
String* name,
- Object* value);
+ Object* value,
+ bool check_prototype);
MUST_USE_RESULT MaybeObject* SetPropertyWithCallback(Object* structure,
String* name,
Object* value,
@@ -1356,7 +1357,7 @@
String* name,
Object* value,
PropertyAttributes attributes);
- MUST_USE_RESULT MaybeObject* IgnoreAttributesAndSetLocalProperty(
+ MUST_USE_RESULT MaybeObject* SetLocalPropertyIgnoreAttributes(
String* key,
Object* value,
PropertyAttributes attributes);
=======================================
--- /branches/bleeding_edge/src/parser.cc Wed Jan 5 03:25:42 2011
+++ /branches/bleeding_edge/src/parser.cc Thu Jan 6 06:00:50 2011
@@ -3660,11 +3660,9 @@
if (value.is_null()) return Handle<Object>::null();
uint32_t index;
if (key->AsArrayIndex(&index)) {
- CALL_HEAP_FUNCTION_INLINE(
- (*json_object)->SetElement(index, *value, true));
+ SetOwnElement(json_object, index, value);
} else {
- CALL_HEAP_FUNCTION_INLINE(
- (*json_object)->SetPropertyPostInterceptor(*key, *value,
NONE));
+ SetLocalPropertyIgnoreAttributes(json_object, key, value, NONE);
}
} while (scanner_.Next() == Token::COMMA);
if (scanner_.current_token() != Token::RBRACE) {
=======================================
--- /branches/bleeding_edge/src/runtime.cc Thu Jan 6 05:14:32 2011
+++ /branches/bleeding_edge/src/runtime.cc Thu Jan 6 06:00:50 2011
@@ -330,13 +330,18 @@
Handle<Object> result;
uint32_t element_index = 0;
if (key->IsSymbol()) {
- // If key is a symbol it is not an array element.
- Handle<String> name(String::cast(*key));
- ASSERT(!name->AsArrayIndex(&element_index));
- result = SetProperty(boilerplate, name, value, NONE);
+ if (Handle<String>::cast(key)->AsArrayIndex(&element_index)) {
+ // Array index as string (uint32).
+ result = SetOwnElement(boilerplate, element_index, value);
+ } else {
+ Handle<String> name(String::cast(*key));
+ ASSERT(!name->AsArrayIndex(&element_index));
+ result = SetLocalPropertyIgnoreAttributes(boilerplate, name,
+ value, NONE);
+ }
} else if (key->ToArrayIndex(&element_index)) {
// Array index (uint32).
- result = SetElement(boilerplate, element_index, value);
+ result = SetOwnElement(boilerplate, element_index, value);
} else {
// Non-uint32 number.
ASSERT(key->IsNumber());
@@ -345,7 +350,8 @@
Vector<char> buffer(arr, ARRAY_SIZE(arr));
const char* str = DoubleToCString(num, buffer);
Handle<String> name = Factory::NewStringFromAscii(CStrVector(str));
- result = SetProperty(boilerplate, name, value, NONE);
+ result = SetLocalPropertyIgnoreAttributes(boilerplate, name,
+ value, NONE);
}
// If setting the property on the boilerplate throws an
// exception, the exception is converted to an empty handle in
@@ -984,7 +990,7 @@
// of callbacks in the prototype chain (this rules out using
// SetProperty). Also, we must use the handle-based version to
// avoid GC issues.
- IgnoreAttributesAndSetLocalProperty(global, name, value, attributes);
+ SetLocalPropertyIgnoreAttributes(global, name, value, attributes);
}
}
@@ -1099,7 +1105,7 @@
// to assign to the property. When adding the property we take
// special precautions to always add it as a local property even in
// case of callbacks in the prototype chain (this rules out using
- // SetProperty). We have IgnoreAttributesAndSetLocalProperty for
+ // SetProperty). We have SetLocalPropertyIgnoreAttributes for
// this.
// Note that objects can have hidden prototypes, so we need to traverse
// the whole chain of hidden prototypes to do a 'local' lookup.
@@ -1162,9 +1168,9 @@
global = Top::context()->global();
if (assign) {
- return global->IgnoreAttributesAndSetLocalProperty(*name,
- args[1],
- attributes);
+ return global->SetLocalPropertyIgnoreAttributes(*name,
+ args[1],
+ attributes);
}
return Heap::undefined_value();
}
@@ -1190,13 +1196,13 @@
// there, we add the property and take special precautions to always
// add it as a local property even in case of callbacks in the
// prototype chain (this rules out using SetProperty).
- // We use IgnoreAttributesAndSetLocalProperty instead
+ // We use SetLocalPropertyIgnoreAttributes instead
LookupResult lookup;
global->LocalLookup(*name, &lookup);
if (!lookup.IsProperty()) {
- return global->IgnoreAttributesAndSetLocalProperty(*name,
- *value,
- attributes);
+ return global->SetLocalPropertyIgnoreAttributes(*name,
+ *value,
+ attributes);
}
// Determine if this is a redeclaration of something not
@@ -1467,27 +1473,27 @@
PropertyAttributes writable =
static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE);
MaybeObject* result;
- result =
regexp->IgnoreAttributesAndSetLocalProperty(Heap::source_symbol(),
- source,
- final);
+ result = regexp->SetLocalPropertyIgnoreAttributes(Heap::source_symbol(),
+ source,
+ final);
ASSERT(!result->IsFailure());
- result =
regexp->IgnoreAttributesAndSetLocalProperty(Heap::global_symbol(),
- global,
- final);
+ result = regexp->SetLocalPropertyIgnoreAttributes(Heap::global_symbol(),
+ global,
+ final);
ASSERT(!result->IsFailure());
result =
-
regexp->IgnoreAttributesAndSetLocalProperty(Heap::ignore_case_symbol(),
- ignoreCase,
- final);
+ regexp->SetLocalPropertyIgnoreAttributes(Heap::ignore_case_symbol(),
+ ignoreCase,
+ final);
ASSERT(!result->IsFailure());
- result =
regexp->IgnoreAttributesAndSetLocalProperty(Heap::multiline_symbol(),
- multiline,
- final);
+ result =
regexp->SetLocalPropertyIgnoreAttributes(Heap::multiline_symbol(),
+ multiline,
+ final);
ASSERT(!result->IsFailure());
result =
-
regexp->IgnoreAttributesAndSetLocalProperty(Heap::last_index_symbol(),
- Smi::FromInt(0),
- writable);
+ regexp->SetLocalPropertyIgnoreAttributes(Heap::last_index_symbol(),
+ Smi::FromInt(0),
+ writable);
ASSERT(!result->IsFailure());
USE(result);
return regexp;
@@ -3571,9 +3577,9 @@
NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
// Use IgnoreAttributes version since a readonly property may be
// overridden and SetProperty does not allow this.
- return js_object->IgnoreAttributesAndSetLocalProperty(*name,
- *obj_value,
- attr);
+ return js_object->SetLocalPropertyIgnoreAttributes(*name,
+ *obj_value,
+ attr);
}
return Runtime::SetObjectProperty(js_object, name, obj_value, attr);
@@ -3674,9 +3680,9 @@
} else {
Handle<String> key_string = Handle<String>::cast(key);
key_string->TryFlatten();
- return js_object->IgnoreAttributesAndSetLocalProperty(*key_string,
- *value,
- attr);
+ return js_object->SetLocalPropertyIgnoreAttributes(*key_string,
+ *value,
+ attr);
}
}
@@ -3689,7 +3695,7 @@
if (name->AsArrayIndex(&index)) {
return js_object->SetElement(index, *value);
} else {
- return js_object->IgnoreAttributesAndSetLocalProperty(*name, *value,
attr);
+ return js_object->SetLocalPropertyIgnoreAttributes(*name, *value,
attr);
}
}
@@ -3771,7 +3777,7 @@
}
return object->
- IgnoreAttributesAndSetLocalProperty(name, args[2], attributes);
+ SetLocalPropertyIgnoreAttributes(name, args[2], attributes);
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-192.js Tue Dec 7
03:01:02 2010
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-192.js Thu Jan 6
06:00:50 2011
@@ -30,9 +30,16 @@
// See http://code.google.com/p/v8/issues/detail?id=192
+// UPDATE: This bug report is no longer valid. In ES5, creating object
+// literals MUST NOT trigger inherited accessors, but act as if creating
+// the properties using DefineOwnProperty.
+
Object.prototype.__defineGetter__("x", function() {});
-
-// Creating this object literal will throw an exception because we are
+Object.prototype.__defineSetter__("y",
+ function() {
assertUnreachable("setter"); });
+
+// Creating this object literal will *not* throw an exception because we
are
// assigning to a property that has only a getter.
-assertThrows("({ x: 42 })");
-
+var x = ({ x: 42, y: 37 });
+assertEquals(42, x.x);
+assertEquals(37, x.y);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
