From ce1e10f5fc8c2a608f476aabc4d4c9f3a9ce140a Mon Sep 17 00:00:00 2001 From: "mstarzinger@chromium.org" Date: Tue, 26 Feb 2013 10:46:00 +0000 Subject: [PATCH] Make __proto__ a foreign callback on Object.prototype. This moves the __proto__ property to Object.prototype and turns it into a callback property actually present in the descriptor array as opposed to a hack in the properties lookup. For now it still is a "magic" data property using foreign callbacks and not an accessor property visible to JavaScript. The second effect of this change is that JSON.parse() no longer treats the __proto__ property specially, it will be defined as any other data property. Note that object literals still have their special handling. R=rossberg@chromium.org BUG=v8:621,v8:1949,v8:2441 TEST=mjsunit,cctest,test262 Review URL: https://codereview.chromium.org/12212011 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13728 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/bootstrapper.cc | 22 +++++++++++++++++++--- src/factory.cc | 5 +++-- src/factory.h | 3 ++- src/json-parser.h | 16 +++++----------- src/objects.cc | 6 ------ src/property.h | 18 ++---------------- test/cctest/test-api.cc | 6 +++--- test/mjsunit/builtins.js | 2 +- test/mjsunit/harmony/object-observe.js | 4 ++-- test/mjsunit/harmony/proxies.js | 1 - test/mjsunit/json.js | 27 +++++++++++++++++---------- test/mjsunit/regress/regress-2441.js | 31 +++++++++++++++++++++++++++++++ test/test262/test262.status | 3 --- 13 files changed, 85 insertions(+), 59 deletions(-) create mode 100644 test/mjsunit/regress/regress-2441.js diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 6487dc9..b2c8c77 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -485,10 +485,26 @@ Handle Genesis::CreateEmptyFunction(Isolate* isolate) { native_context()->set_object_function(*object_fun); // Allocate a new prototype for the object function. - Handle prototype = factory->NewJSObject( - isolate->object_function(), - TENURED); + Handle object_prototype_map = + factory->NewMap(JS_OBJECT_TYPE, JSObject::kHeaderSize); + Handle prototype_descriptors( + factory->NewDescriptorArray(0, 1)); + DescriptorArray::WhitenessWitness witness(*prototype_descriptors); + Handle object_prototype( + factory->NewForeign(&Accessors::ObjectPrototype)); + PropertyAttributes attribs = static_cast( + DONT_ENUM | DONT_DELETE); + object_prototype_map->set_instance_descriptors(*prototype_descriptors); + + { // Add __proto__. + CallbacksDescriptor d(heap->Proto_symbol(), *object_prototype, attribs); + object_prototype_map->AppendDescriptor(&d, witness); + } + + Handle prototype = factory->NewJSObjectFromMap( + object_prototype_map, + TENURED); native_context()->set_initial_object_prototype(*prototype); SetPrototype(object_fun, prototype); } diff --git a/src/factory.cc b/src/factory.cc index 8be9f4c..6a4235e 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -950,10 +950,11 @@ Handle Factory::NewGlobalObject( -Handle Factory::NewJSObjectFromMap(Handle map) { +Handle Factory::NewJSObjectFromMap(Handle map, + PretenureFlag pretenure) { CALL_HEAP_FUNCTION( isolate(), - isolate()->heap()->AllocateJSObjectFromMap(*map, NOT_TENURED), + isolate()->heap()->AllocateJSObjectFromMap(*map, pretenure), JSObject); } diff --git a/src/factory.h b/src/factory.h index 6989396..f58008e 100644 --- a/src/factory.h +++ b/src/factory.h @@ -278,7 +278,8 @@ class Factory { // JS objects are pretenured when allocated by the bootstrapper and // runtime. - Handle NewJSObjectFromMap(Handle map); + Handle NewJSObjectFromMap(Handle map, + PretenureFlag pretenure = NOT_TENURED); // JS modules are pretenured. Handle NewJSModule(Handle context, diff --git a/src/json-parser.h b/src/json-parser.h index 22f4c70..8d9ddda 100644 --- a/src/json-parser.h +++ b/src/json-parser.h @@ -291,7 +291,6 @@ Handle JsonParser::ParseJsonValue() { // Parse a JSON object. Position must be right at '{'. template Handle JsonParser::ParseJsonObject() { - Handle prototype; Handle json_object = factory()->NewJSObject(object_constructor(), pretenure_); ASSERT_EQ(c0_, '{'); @@ -346,22 +345,17 @@ Handle JsonParser::ParseJsonObject() { Handle value = ParseJsonValue(); if (value.is_null()) return ReportUnexpectedCharacter(); - if (key->Equals(isolate()->heap()->Proto_symbol())) { - prototype = value; + if (JSObject::TryTransitionToField(json_object, key)) { + int index = json_object->LastAddedFieldIndex(); + json_object->FastPropertyAtPut(index, *value); } else { - if (JSObject::TryTransitionToField(json_object, key)) { - int index = json_object->LastAddedFieldIndex(); - json_object->FastPropertyAtPut(index, *value); - } else { - JSObject::SetLocalPropertyIgnoreAttributes( - json_object, key, value, NONE); - } + JSObject::SetLocalPropertyIgnoreAttributes( + json_object, key, value, NONE); } } while (MatchSkipWhiteSpace(',')); if (c0_ != '}') { return ReportUnexpectedCharacter(); } - if (!prototype.is_null()) SetPrototype(json_object, prototype); } AdvanceSkipWhitespace(); return json_object; diff --git a/src/objects.cc b/src/objects.cc index 85314e4..9b26fbc 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -4602,12 +4602,6 @@ void JSReceiver::LocalLookup( JSObject* js_object = JSObject::cast(this); - // Check __proto__ before interceptor. - if (name->Equals(heap->Proto_symbol()) && !IsJSContextExtensionObject()) { - result->ConstantResult(js_object); - return; - } - // Check for lookup interceptor except when bootstrapping. if (js_object->HasNamedInterceptor() && !heap->isolate()->bootstrapper()->IsActive()) { diff --git a/src/property.h b/src/property.h index 1cadd57..e79c87e 100644 --- a/src/property.h +++ b/src/property.h @@ -201,16 +201,6 @@ class LookupResult BASE_EMBEDDED { number_ = number; } - void ConstantResult(JSObject* holder) { - lookup_type_ = CONSTANT_TYPE; - holder_ = holder; - details_ = - PropertyDetails(static_cast(DONT_ENUM | - DONT_DELETE), - CALLBACKS); - number_ = -1; - } - void DictionaryResult(JSObject* holder, int entry) { lookup_type_ = DICTIONARY_TYPE; holder_ = holder; @@ -427,10 +417,7 @@ class LookupResult BASE_EMBEDDED { } Object* GetCallbackObject() { - if (lookup_type_ == CONSTANT_TYPE) { - return HEAP->prototype_accessors(); - } - ASSERT(!IsTransition()); + ASSERT(type() == CALLBACKS && !IsTransition()); return GetValue(); } @@ -466,8 +453,7 @@ class LookupResult BASE_EMBEDDED { TRANSITION_TYPE, DICTIONARY_TYPE, HANDLER_TYPE, - INTERCEPTOR_TYPE, - CONSTANT_TYPE + INTERCEPTOR_TYPE } lookup_type_; JSReceiver* holder_; diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 976c561..4938aae 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -1849,8 +1849,8 @@ THREADED_TEST(PropertyHandlerInPrototype) { Local top = templ->GetFunction()->NewInstance(); Local middle = templ->GetFunction()->NewInstance(); - bottom->Set(v8_str("__proto__"), middle); - middle->Set(v8_str("__proto__"), top); + bottom->SetPrototype(middle); + middle->SetPrototype(top); env->Global()->Set(v8_str("obj"), bottom); // Indexed and named get. @@ -9670,7 +9670,7 @@ THREADED_TEST(InterceptorCallICCacheableNotNeeded) { call_ic_function4 = v8_compile("function f(x) { return x - 1; }; f")->Run(); v8::Handle value = CompileRun( - "o.__proto__.x = function(x) { return x + 1; };" + "Object.getPrototypeOf(o).x = function(x) { return x + 1; };" "var result = 0;" "for (var i = 0; i < 1000; i++) {" " result = o.x(42);" diff --git a/test/mjsunit/builtins.js b/test/mjsunit/builtins.js index e43b589..062cfd5 100644 --- a/test/mjsunit/builtins.js +++ b/test/mjsunit/builtins.js @@ -54,7 +54,7 @@ function checkConstructor(func, name) { assertFalse(proto_desc.writable, name); assertFalse(proto_desc.configurable, name); var prototype = proto_desc.value; - assertEquals(null, prototype.__proto__, name); + assertEquals(null, Object.getPrototypeOf(prototype), name); for (var i = 0; i < propNames.length; i++) { var propName = propNames[i]; if (propName == "constructor") continue; diff --git a/test/mjsunit/harmony/object-observe.js b/test/mjsunit/harmony/object-observe.js index c902fb2..584d9e8 100644 --- a/test/mjsunit/harmony/object-observe.js +++ b/test/mjsunit/harmony/object-observe.js @@ -898,7 +898,7 @@ var q = {bar: 'no'}; obj.__proto__ = p; obj.__proto__ = p; // ignored obj.__proto__ = null; -obj.__proto__ = q; +obj.__proto__ = q; // the __proto__ accessor is gone // TODO(adamk): Add tests for objects with hidden prototypes // once we support observing the global object. Object.deliverChangeRecords(observer.callback); @@ -906,7 +906,7 @@ observer.assertCallbackRecords([ { object: obj, name: '__proto__', type: 'prototype', oldValue: Object.prototype }, { object: obj, name: '__proto__', type: 'prototype', oldValue: p }, - { object: obj, name: '__proto__', type: 'prototype', oldValue: null }, + { object: obj, name: '__proto__', type: 'new' }, ]); diff --git a/test/mjsunit/harmony/proxies.js b/test/mjsunit/harmony/proxies.js index 04fc769..f68e3bd 100644 --- a/test/mjsunit/harmony/proxies.js +++ b/test/mjsunit/harmony/proxies.js @@ -2294,7 +2294,6 @@ function TestConstructorWithProxyPrototype2(create, handler) { C.prototype = create(handler); var o = new C; - assertSame(C.prototype, o.__proto__); assertSame(C.prototype, Object.getPrototypeOf(o)); } diff --git a/test/mjsunit/json.js b/test/mjsunit/json.js index 6e91725..79826db 100644 --- a/test/mjsunit/json.js +++ b/test/mjsunit/json.js @@ -453,16 +453,23 @@ falseNum.__proto__ = Number.prototype; falseNum.toString = function() { return 42; }; assertEquals('"42"', JSON.stringify(falseNum)); -// We don't currently allow plain properties called __proto__ in JSON -// objects in JSON.parse. Instead we read them as we would JS object -// literals. If we change that, this test should change with it. -// -// Parse a non-object value as __proto__. This must not create a -// __proto__ property different from the original, and should not -// change the original. -var o = JSON.parse('{"__proto__":5}'); -assertEquals(Object.prototype, o.__proto__); // __proto__ isn't changed. -assertEquals(0, Object.keys(o).length); // __proto__ isn't added as enumerable. +// Parse an object value as __proto__. +var o1 = JSON.parse('{"__proto__":[]}'); +assertEquals([], o1.__proto__); +assertEquals(["__proto__"], Object.keys(o1)); +assertEquals([], Object.getOwnPropertyDescriptor(o1, "__proto__").value); +assertEquals(["__proto__"], Object.getOwnPropertyNames(o1)); +assertTrue(o1.hasOwnProperty("__proto__")); +assertTrue(Object.prototype.isPrototypeOf(o1)); + +// Parse a non-object value as __proto__. +var o2 = JSON.parse('{"__proto__":5}'); +assertEquals(5, o2.__proto__); +assertEquals(["__proto__"], Object.keys(o2)); +assertEquals(5, Object.getOwnPropertyDescriptor(o2, "__proto__").value); +assertEquals(["__proto__"], Object.getOwnPropertyNames(o2)); +assertTrue(o2.hasOwnProperty("__proto__")); +assertTrue(Object.prototype.isPrototypeOf(o2)); var json = '{"stuff before slash\\\\stuff after slash":"whatever"}'; assertEquals(json, JSON.stringify(JSON.parse(json))); diff --git a/test/mjsunit/regress/regress-2441.js b/test/mjsunit/regress/regress-2441.js new file mode 100644 index 0000000..72ce248 --- /dev/null +++ b/test/mjsunit/regress/regress-2441.js @@ -0,0 +1,31 @@ +// Copyright 2013 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. + +var o = {}; +Object.preventExtensions(o); +assertThrows("Object.defineProperty(o, 'foobarloo', {value:{}});", TypeError); +assertThrows("Object.defineProperty(o, '__proto__', {value:{}});", TypeError); diff --git a/test/test262/test262.status b/test/test262/test262.status index 8eaa365..4910939 100644 --- a/test/test262/test262.status +++ b/test/test262/test262.status @@ -30,9 +30,6 @@ def FAIL_OK = FAIL, OKAY ############################### BUGS ################################### -# '__proto__' should be treated as a normal property in JSON. -S15.12.2_A1: FAIL - # Sequencing of getter side effects on receiver and argument properties # is wrong. The receiver callback should be called before any arguments # are evaluated. -- 2.7.4