From: vegorov@chromium.org Date: Thu, 23 Sep 2010 11:25:01 +0000 (+0000) Subject: Fix getOwnPropertyDescriptor() support for index properties. X-Git-Tag: upstream/4.7.83~21171 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=622351fedd3c6decdd66b3d11a5031abf12ef1b6;p=platform%2Fupstream%2Fv8.git Fix getOwnPropertyDescriptor() support for index properties. Add support for index properties with getters, setters or indexed interceptors. For indexed interceptor case only fix crashes, do not guarantee any semantic soundness. Separate issue opened for this http://code.google.com/p/v8/issues/detail?id=877 BUG=http://code.google.com/p/v8/issues/detail?id=874 Review URL: http://codereview.chromium.org/3462005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5512 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/objects.cc b/src/objects.cc index dc784d3..3ab6ef5 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -6072,21 +6072,24 @@ bool JSObject::HasElementWithInterceptor(JSObject* receiver, uint32_t index) { } -bool JSObject::HasLocalElement(uint32_t index) { +JSObject::LocalElementType JSObject::HasLocalElement(uint32_t index) { // Check access rights if needed. if (IsAccessCheckNeeded() && !Top::MayIndexedAccess(this, index, v8::ACCESS_HAS)) { Top::ReportFailedAccessCheck(this, v8::ACCESS_HAS); - return false; + return UNDEFINED_ELEMENT; } // Check for lookup interceptor if (HasIndexedInterceptor()) { - return HasElementWithInterceptor(this, index); + return HasElementWithInterceptor(this, index) ? INTERCEPTED_ELEMENT + : UNDEFINED_ELEMENT; } // Handle [] on String objects. - if (this->IsStringObjectWithCharacterAt(index)) return true; + if (this->IsStringObjectWithCharacterAt(index)) { + return STRING_CHARACTER_ELEMENT; + } switch (GetElementsKind()) { case FAST_ELEMENTS: { @@ -6094,12 +6097,16 @@ bool JSObject::HasLocalElement(uint32_t index) { static_cast (Smi::cast(JSArray::cast(this)->length())->value()) : static_cast(FixedArray::cast(elements())->length()); - return (index < length) && - !FixedArray::cast(elements())->get(index)->IsTheHole(); + if ((index < length) && + !FixedArray::cast(elements())->get(index)->IsTheHole()) { + return FAST_ELEMENT; + } + break; } case PIXEL_ELEMENTS: { PixelArray* pixels = PixelArray::cast(elements()); - return (index < static_cast(pixels->length())); + if (index < static_cast(pixels->length())) return FAST_ELEMENT; + break; } case EXTERNAL_BYTE_ELEMENTS: case EXTERNAL_UNSIGNED_BYTE_ELEMENTS: @@ -6109,18 +6116,22 @@ bool JSObject::HasLocalElement(uint32_t index) { case EXTERNAL_UNSIGNED_INT_ELEMENTS: case EXTERNAL_FLOAT_ELEMENTS: { ExternalArray* array = ExternalArray::cast(elements()); - return (index < static_cast(array->length())); + if (index < static_cast(array->length())) return FAST_ELEMENT; + break; } case DICTIONARY_ELEMENTS: { - return element_dictionary()->FindEntry(index) - != NumberDictionary::kNotFound; + if (element_dictionary()->FindEntry(index) != + NumberDictionary::kNotFound) { + return DICTIONARY_ELEMENT; + } + break; } default: UNREACHABLE(); break; } - UNREACHABLE(); - return Heap::null_value(); + + return UNDEFINED_ELEMENT; } diff --git a/src/objects.h b/src/objects.h index c80a2ef..6c4af25 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1417,7 +1417,26 @@ class JSObject: public HeapObject { // Tells whether the index'th element is present. inline bool HasElement(uint32_t index); bool HasElementWithReceiver(JSObject* receiver, uint32_t index); - bool HasLocalElement(uint32_t index); + + // Tells whether the index'th element is present and how it is stored. + enum LocalElementType { + // There is no element with given index. + UNDEFINED_ELEMENT, + + // Element with given index is handled by interceptor. + INTERCEPTED_ELEMENT, + + // Element with given index is character in string. + STRING_CHARACTER_ELEMENT, + + // Element with given index is stored in fast backing store. + FAST_ELEMENT, + + // Element with given index is stored in slow backing store. + DICTIONARY_ELEMENT + }; + + LocalElementType HasLocalElement(uint32_t index); bool HasElementWithInterceptor(JSObject* receiver, uint32_t index); bool HasElementPostInterceptor(JSObject* receiver, uint32_t index); diff --git a/src/runtime.cc b/src/runtime.cc index 9cdecd1..ab2c2b5 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -644,46 +644,63 @@ static Object* Runtime_GetOwnProperty(Arguments args) { // This could be an element. uint32_t index; if (name->AsArrayIndex(&index)) { - if (!obj->HasLocalElement(index)) { - return Heap::undefined_value(); - } + switch (obj->HasLocalElement(index)) { + case JSObject::UNDEFINED_ELEMENT: + return Heap::undefined_value(); - // Special handling of string objects according to ECMAScript 5 15.5.5.2. - // Note that this might be a string object with elements other than the - // actual string value. This is covered by the subsequent cases. - if (obj->IsStringObjectWithCharacterAt(index)) { - JSValue* js_value = JSValue::cast(obj); - String* str = String::cast(js_value->value()); - elms->set(IS_ACCESSOR_INDEX, Heap::false_value()); - elms->set(VALUE_INDEX, str->SubString(index, index+1)); - elms->set(WRITABLE_INDEX, Heap::false_value()); - elms->set(ENUMERABLE_INDEX, Heap::false_value()); - elms->set(CONFIGURABLE_INDEX, Heap::false_value()); - return *desc; - } - - // This can potentially be an element in the elements dictionary or - // a fast element. - if (obj->HasDictionaryElements()) { - NumberDictionary* dictionary = obj->element_dictionary(); - int entry = dictionary->FindEntry(index); - PropertyDetails details = dictionary->DetailsAt(entry); - elms->set(IS_ACCESSOR_INDEX, Heap::false_value()); - elms->set(VALUE_INDEX, dictionary->ValueAt(entry)); - elms->set(WRITABLE_INDEX, Heap::ToBoolean(!details.IsReadOnly())); - elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!details.IsDontEnum())); - elms->set(CONFIGURABLE_INDEX, Heap::ToBoolean(!details.IsDontDelete())); - return *desc; - } else { - // Elements that are stored as array elements always has: - // writable: true, configurable: true, enumerable: true. - elms->set(IS_ACCESSOR_INDEX, Heap::false_value()); - Object* element = obj->GetElement(index); - elms->set(VALUE_INDEX, element); - elms->set(WRITABLE_INDEX, Heap::true_value()); - elms->set(ENUMERABLE_INDEX, Heap::true_value()); - elms->set(CONFIGURABLE_INDEX, Heap::true_value()); - return *desc; + case JSObject::STRING_CHARACTER_ELEMENT: { + // Special handling of string objects according to ECMAScript 5 + // 15.5.5.2. Note that this might be a string object with elements + // other than the actual string value. This is covered by the + // subsequent cases. + JSValue* js_value = JSValue::cast(obj); + String* str = String::cast(js_value->value()); + elms->set(IS_ACCESSOR_INDEX, Heap::false_value()); + elms->set(VALUE_INDEX, str->SubString(index, index+1)); + elms->set(WRITABLE_INDEX, Heap::false_value()); + elms->set(ENUMERABLE_INDEX, Heap::false_value()); + elms->set(CONFIGURABLE_INDEX, Heap::false_value()); + return *desc; + } + + case JSObject::INTERCEPTED_ELEMENT: + case JSObject::FAST_ELEMENT: + elms->set(IS_ACCESSOR_INDEX, Heap::false_value()); + elms->set(VALUE_INDEX, obj->GetElement(index)); + elms->set(WRITABLE_INDEX, Heap::true_value()); + elms->set(ENUMERABLE_INDEX, Heap::true_value()); + elms->set(CONFIGURABLE_INDEX, Heap::true_value()); + return *desc; + + case JSObject::DICTIONARY_ELEMENT: { + NumberDictionary* dictionary = obj->element_dictionary(); + int entry = dictionary->FindEntry(index); + ASSERT(entry != NumberDictionary::kNotFound); + PropertyDetails details = dictionary->DetailsAt(entry); + switch (details.type()) { + case CALLBACKS: { + // This is an accessor property with getter and/or setter. + FixedArray* callbacks = + FixedArray::cast(dictionary->ValueAt(entry)); + elms->set(IS_ACCESSOR_INDEX, Heap::true_value()); + elms->set(GETTER_INDEX, callbacks->get(0)); + elms->set(SETTER_INDEX, callbacks->get(1)); + break; + } + case NORMAL: + // This is a data property. + elms->set(IS_ACCESSOR_INDEX, Heap::false_value()); + elms->set(VALUE_INDEX, dictionary->ValueAt(entry)); + elms->set(WRITABLE_INDEX, Heap::ToBoolean(!details.IsReadOnly())); + break; + default: + UNREACHABLE(); + break; + } + elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!details.IsDontEnum())); + elms->set(CONFIGURABLE_INDEX, Heap::ToBoolean(!details.IsDontDelete())); + return *desc; + } } } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index d1acd0a..1963ff0 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -3022,6 +3022,27 @@ static v8::Handle IdentityIndexedPropertyGetter( } +THREADED_TEST(IndexedInterceptorWithGetOwnPropertyDescriptor) { + v8::HandleScope scope; + Local templ = ObjectTemplate::New(); + templ->SetIndexedPropertyHandler(IdentityIndexedPropertyGetter); + + LocalContext context; + context->Global()->Set(v8_str("obj"), templ->NewInstance()); + + // Check fast object case. + const char* fast_case_code = + "Object.getOwnPropertyDescriptor(obj, 0).value.toString()"; + ExpectString(fast_case_code, "0"); + + // Check slow case. + const char* slow_case_code = + "obj.x = 1; delete obj.x;" + "Object.getOwnPropertyDescriptor(obj, 1).value.toString()"; + ExpectString(slow_case_code, "1"); +} + + THREADED_TEST(IndexedInterceptorWithNoSetter) { v8::HandleScope scope; Local templ = ObjectTemplate::New(); diff --git a/test/mjsunit/regress/regress-874.js b/test/mjsunit/regress/regress-874.js new file mode 100644 index 0000000..384d9c7 --- /dev/null +++ b/test/mjsunit/regress/regress-874.js @@ -0,0 +1,37 @@ +// 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. + +var x = { }; + +var getter = function(){ return 42; }; +var setter = function(value){ }; +x.__defineGetter__(0, getter); +x.__defineSetter__(0, setter); + +assertEquals (undefined, Object.getOwnPropertyDescriptor(x, 0).value); +assertEquals (getter, Object.getOwnPropertyDescriptor(x, 0).get); +assertEquals (setter, Object.getOwnPropertyDescriptor(x, 0).set);