From bd5f13ebc3438577114daa748f5c9e11401e77cc Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Mon, 23 Jun 2014 09:11:45 +0000 Subject: [PATCH] Remove specialized access checks and overwrites altogether. They are already handled by GetOwnPropertyAttributes (and GetPropertyAttributesWithFailedAccessChecks) BUG= R=dcarney@chromium.org Review URL: https://codereview.chromium.org/331693006 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21922 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/objects.cc | 20 ------- src/objects.h | 3 - src/runtime.cc | 147 ++++++++++++++++-------------------------------- test/cctest/test-api.cc | 10 ---- 4 files changed, 48 insertions(+), 132 deletions(-) diff --git a/src/objects.cc b/src/objects.cc index 85149ef..9d0dec6 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -12201,26 +12201,6 @@ void JSObject::EnsureCanContainElements(Handle object, } -MaybeHandle JSObject::GetOwnPropertyAccessorPair( - Handle object, - Handle name) { - uint32_t index = 0; - if (name->AsArrayIndex(&index)) { - return GetOwnElementAccessorPair(object, index); - } - - Isolate* isolate = object->GetIsolate(); - LookupResult lookup(isolate); - object->LookupOwnRealNamedProperty(name, &lookup); - - if (lookup.IsPropertyCallbacks() && - lookup.GetCallbackObject()->IsAccessorPair()) { - return handle(AccessorPair::cast(lookup.GetCallbackObject()), isolate); - } - return MaybeHandle(); -} - - MaybeHandle JSObject::GetOwnElementAccessorPair( Handle object, uint32_t index) { diff --git a/src/objects.h b/src/objects.h index 8fd1294..22e0083 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2317,9 +2317,6 @@ class JSObject: public JSReceiver { } // These methods do not perform access checks! - MUST_USE_RESULT static MaybeHandle GetOwnPropertyAccessorPair( - Handle object, - Handle name); MUST_USE_RESULT static MaybeHandle GetOwnElementAccessorPair( Handle object, uint32_t index); diff --git a/src/runtime.cc b/src/runtime.cc index b5aa651..771d139 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -1887,86 +1887,6 @@ RUNTIME_FUNCTION(Runtime_IsInPrototypeChain) { } -static bool CheckAccessException(Object* callback, - v8::AccessType access_type) { - DisallowHeapAllocation no_gc; - ASSERT(!callback->IsForeign()); - if (callback->IsAccessorInfo()) { - AccessorInfo* info = AccessorInfo::cast(callback); - return - (access_type == v8::ACCESS_HAS && - (info->all_can_read() || info->all_can_write())) || - (access_type == v8::ACCESS_GET && info->all_can_read()) || - (access_type == v8::ACCESS_SET && info->all_can_write()); - } - return false; -} - - -template -static bool CheckGenericAccess( - Handle receiver, - Handle end, - Key key, - v8::AccessType access_type, - bool (Isolate::*mayAccess)(Handle, Key, v8::AccessType)) { - Isolate* isolate = receiver->GetIsolate(); - for (Handle current = receiver; - !current.is_identical_to(end); - current = Object::GetPrototype(isolate, current)) { - if (current->IsAccessCheckNeeded() && - !(isolate->*mayAccess)( - Handle::cast(current), key, access_type)) { - return false; - } - } - return true; -} - - -static void CheckPropertyAccess(Handle obj, - Handle name, - v8::AccessType access_type) { - Isolate* isolate = obj->GetIsolate(); - uint32_t index; - if (name->AsArrayIndex(&index)) { - Handle next(obj->GetPrototype(), isolate); - // TODO(1095): we should traverse hidden prototype hierachy as well. - if (!CheckGenericAccess( - obj, next, index, access_type, &Isolate::MayIndexedAccess)) { - obj->GetIsolate()->ReportFailedAccessCheck(obj, access_type); - } - return; - } - - LookupResult lookup(isolate); - obj->LookupOwn(name, &lookup, true); - - Handle next = lookup.IsProperty() - ? handle(lookup.holder()->GetPrototype(), isolate) - : Handle::cast(isolate->factory()->null_value()); - if (CheckGenericAccess >( - obj, next, name, access_type, &Isolate::MayNamedAccess)) { - return; - } - - // Access check callback denied the access, but some properties - // can have a special permissions which override callbacks decision - // (see v8::AccessControl). - // API callbacks can have per callback access exceptions. - if (lookup.IsFound() && lookup.type() == INTERCEPTOR) { - lookup.holder()->LookupOwnRealNamedProperty(name, &lookup); - } - - if (lookup.IsPropertyCallbacks() && - CheckAccessException(lookup.GetCallbackObject(), access_type)) { - return; - } - - isolate->ReportFailedAccessCheck(obj, access_type); -} - - // Enumerator used as indices into the array returned from GetOwnProperty enum PropertyDescriptorIndices { IS_ACCESSOR_INDEX, @@ -1986,36 +1906,65 @@ MUST_USE_RESULT static MaybeHandle GetOwnProperty(Isolate* isolate, Heap* heap = isolate->heap(); Factory* factory = isolate->factory(); - CheckPropertyAccess(obj, name, v8::ACCESS_HAS); - RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); + PropertyAttributes attrs; + uint32_t index = 0; + Handle value; + MaybeHandle maybe_accessors; + // TODO(verwaest): Unify once indexed properties can be handled by the + // LookupIterator. + if (name->AsArrayIndex(&index)) { + // Get attributes. + attrs = JSReceiver::GetOwnElementAttribute(obj, index); + if (attrs == ABSENT) { + RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); + return factory->undefined_value(); + } + + // Get AccessorPair if present. + maybe_accessors = JSObject::GetOwnElementAccessorPair(obj, index); + + // Get value if not an AccessorPair. + if (maybe_accessors.is_null()) { + ASSIGN_RETURN_ON_EXCEPTION(isolate, value, + Runtime::GetElementOrCharAt(isolate, obj, index), Object); + } + } else { + // Get attributes. + LookupIterator it(obj, name, LookupIterator::CHECK_OWN); + attrs = JSObject::GetPropertyAttributes(&it); + if (attrs == ABSENT) { + RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); + return factory->undefined_value(); + } + + // Get AccessorPair if present. + if (it.state() == LookupIterator::PROPERTY && + it.property_kind() == LookupIterator::ACCESSOR && + it.GetAccessors()->IsAccessorPair()) { + maybe_accessors = Handle::cast(it.GetAccessors()); + } - PropertyAttributes attrs = JSReceiver::GetOwnPropertyAttributes(obj, name); - if (attrs == ABSENT) { - RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); - return factory->undefined_value(); + // Get value if not an AccessorPair. + if (maybe_accessors.is_null()) { + ASSIGN_RETURN_ON_EXCEPTION( + isolate, value, Object::GetProperty(&it), Object); + } } ASSERT(!isolate->has_scheduled_exception()); - Handle accessors; - bool has_accessors = - JSObject::GetOwnPropertyAccessorPair(obj, name).ToHandle(&accessors); Handle elms = factory->NewFixedArray(DESCRIPTOR_SIZE); elms->set(ENUMERABLE_INDEX, heap->ToBoolean((attrs & DONT_ENUM) == 0)); elms->set(CONFIGURABLE_INDEX, heap->ToBoolean((attrs & DONT_DELETE) == 0)); - elms->set(IS_ACCESSOR_INDEX, heap->ToBoolean(has_accessors)); + elms->set(IS_ACCESSOR_INDEX, heap->ToBoolean(!maybe_accessors.is_null())); - if (!has_accessors) { - elms->set(WRITABLE_INDEX, heap->ToBoolean((attrs & READ_ONLY) == 0)); - // Runtime::GetObjectProperty does access check. - Handle value; - ASSIGN_RETURN_ON_EXCEPTION( - isolate, value, Runtime::GetObjectProperty(isolate, obj, name), - Object); - elms->set(VALUE_INDEX, *value); - } else { + Handle accessors; + if (maybe_accessors.ToHandle(&accessors)) { Handle getter(accessors->GetComponent(ACCESSOR_GETTER), isolate); Handle setter(accessors->GetComponent(ACCESSOR_SETTER), isolate); elms->set(GETTER_INDEX, *getter); elms->set(SETTER_INDEX, *setter); + } else { + elms->set(WRITABLE_INDEX, heap->ToBoolean((attrs & READ_ONLY) == 0)); + elms->set(VALUE_INDEX, *value); } return factory->NewJSArrayWithElements(elms); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 56979c6..a7cb454 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -9131,16 +9131,6 @@ TEST(AccessControl) { "Object.getOwnPropertyDescriptor(other, 'blocked_prop')"); ExpectFalse("propertyIsEnumerable.call(other, 'blocked_prop')"); - // Enable ACCESS_HAS - allowed_access_type[v8::ACCESS_HAS] = true; - ExpectUndefined("other.blocked_prop"); - // ... and now we can get the descriptor... - ExpectUndefined( - "Object.getOwnPropertyDescriptor(other, 'blocked_prop').value"); - // ... and enumerate the property. - ExpectTrue("propertyIsEnumerable.call(other, 'blocked_prop')"); - allowed_access_type[v8::ACCESS_HAS] = false; - // Access blocked element. CompileRun("other[239] = 1"); -- 2.7.4