Revision: 21917
Author: [email protected]
Date: Mon Jun 23 08:53:27 2014 UTC
Log: Simplify access checks performed by GetOwnProperty
BUG=
[email protected]
Review URL: https://codereview.chromium.org/339553002
http://code.google.com/p/v8/source/detail?r=21917
Modified:
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/runtime.cc Fri Jun 20 15:40:21 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc Mon Jun 23 08:53:27 2014 UTC
@@ -1914,83 +1914,64 @@
template<class Key>
static bool CheckGenericAccess(
Handle<JSObject> receiver,
- Handle<JSObject> holder,
+ Handle<Object> end,
Key key,
v8::AccessType access_type,
bool (Isolate::*mayAccess)(Handle<JSObject>, Key, v8::AccessType)) {
Isolate* isolate = receiver->GetIsolate();
- for (Handle<JSObject> current = receiver;
- true;
- current = handle(JSObject::cast(current->GetPrototype()), isolate))
{
+ for (Handle<Object> current = receiver;
+ !current.is_identical_to(end);
+ current = Object::GetPrototype(isolate, current)) {
if (current->IsAccessCheckNeeded() &&
- !(isolate->*mayAccess)(current, key, access_type)) {
+ !(isolate->*mayAccess)(
+ Handle<JSObject>::cast(current), key, access_type)) {
return false;
}
- if (current.is_identical_to(holder)) break;
}
return true;
}
-enum AccessCheckResult {
- ACCESS_FORBIDDEN,
- ACCESS_ALLOWED,
- ACCESS_ABSENT
-};
-
-
-static AccessCheckResult CheckPropertyAccess(Handle<JSObject> obj,
- Handle<Name> name,
- v8::AccessType access_type) {
+static void CheckPropertyAccess(Handle<JSObject> obj,
+ Handle<Name> name,
+ v8::AccessType access_type) {
+ Isolate* isolate = obj->GetIsolate();
uint32_t index;
if (name->AsArrayIndex(&index)) {
+ Handle<Object> next(obj->GetPrototype(), isolate);
// TODO(1095): we should traverse hidden prototype hierachy as well.
- if (CheckGenericAccess(
- obj, obj, index, access_type, &Isolate::MayIndexedAccess)) {
- return ACCESS_ALLOWED;
+ if (!CheckGenericAccess(
+ obj, next, index, access_type, &Isolate::MayIndexedAccess)) {
+ obj->GetIsolate()->ReportFailedAccessCheck(obj, access_type);
}
-
- obj->GetIsolate()->ReportFailedAccessCheck(obj, access_type);
- return ACCESS_FORBIDDEN;
+ return;
}
- Isolate* isolate = obj->GetIsolate();
LookupResult lookup(isolate);
obj->LookupOwn(name, &lookup, true);
- if (!lookup.IsProperty()) return ACCESS_ABSENT;
- Handle<JSObject> holder(lookup.holder(), isolate);
+ Handle<Object> next = lookup.IsProperty()
+ ? handle(lookup.holder()->GetPrototype(), isolate)
+ : Handle<Object>::cast(isolate->factory()->null_value());
if (CheckGenericAccess<Handle<Object> >(
- obj, holder, name, access_type, &Isolate::MayNamedAccess)) {
- return ACCESS_ALLOWED;
+ 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 descision
// (currently see v8::AccessControl).
// API callbacks can have per callback access exceptions.
- switch (lookup.type()) {
- case CALLBACKS:
- if (CheckAccessException(lookup.GetCallbackObject(), access_type)) {
- return ACCESS_ALLOWED;
- }
- break;
- case INTERCEPTOR:
- // If the object has an interceptor, try real named properties.
- // Overwrite the result to fetch the correct property later.
- holder->LookupRealNamedProperty(name, &lookup);
- if (lookup.IsProperty() && lookup.IsPropertyCallbacks()) {
- if (CheckAccessException(lookup.GetCallbackObject(), access_type))
{
- return ACCESS_ALLOWED;
- }
- }
- break;
- default:
- break;
+ 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);
- return ACCESS_FORBIDDEN;
}
@@ -2012,16 +1993,9 @@
Handle<Name>
name) {
Heap* heap = isolate->heap();
Factory* factory = isolate->factory();
- // Due to some WebKit tests, we want to make sure that we do not log
- // more than one access failure here.
- AccessCheckResult access_check_result =
- CheckPropertyAccess(obj, name, v8::ACCESS_HAS);
+
+ CheckPropertyAccess(obj, name, v8::ACCESS_HAS);
RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
- switch (access_check_result) {
- case ACCESS_FORBIDDEN: return factory->false_value();
- case ACCESS_ALLOWED: break;
- case ACCESS_ABSENT: return factory->undefined_value();
- }
PropertyAttributes attrs = JSReceiver::GetOwnPropertyAttributes(obj,
name);
if (attrs == ABSENT) {
@@ -2032,7 +2006,7 @@
Handle<AccessorPair> accessors;
bool has_accessors =
JSObject::GetOwnPropertyAccessorPair(obj, name).ToHandle(&accessors);
- Handle<FixedArray> elms =
isolate->factory()->NewFixedArray(DESCRIPTOR_SIZE);
+ Handle<FixedArray> 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));
@@ -2046,27 +2020,13 @@
Object);
elms->set(VALUE_INDEX, *value);
} else {
- // Access checks are performed for both accessors separately.
- // When they fail, the respective field is not set in the descriptor.
Handle<Object> getter(accessors->GetComponent(ACCESSOR_GETTER),
isolate);
Handle<Object> setter(accessors->GetComponent(ACCESSOR_SETTER),
isolate);
-
- if (!getter->IsMap() && CheckPropertyAccess(obj, name,
v8::ACCESS_GET)) {
- ASSERT(!isolate->has_scheduled_exception());
- elms->set(GETTER_INDEX, *getter);
- } else {
- RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
- }
-
- if (!setter->IsMap() && CheckPropertyAccess(obj, name,
v8::ACCESS_SET)) {
- ASSERT(!isolate->has_scheduled_exception());
- elms->set(SETTER_INDEX, *setter);
- } else {
- RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
- }
+ elms->set(GETTER_INDEX, *getter);
+ elms->set(SETTER_INDEX, *setter);
}
- return isolate->factory()->NewJSArrayWithElements(elms);
+ return factory->NewJSArrayWithElements(elms);
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Mon Jun 23 08:50:54
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-api.cc Mon Jun 23 08:53:27
2014 UTC
@@ -9184,17 +9184,6 @@
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p')");
- // Enable ACCESS_HAS.
- allowed_access_type[v8::ACCESS_HAS] = true;
- ExpectUndefined("other.js_accessor_p");
- ExpectUndefined(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').get");
- ExpectUndefined(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').set");
- ExpectUndefined(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').value");
- allowed_access_type[v8::ACCESS_HAS] = false;
-
// Enable both ACCESS_HAS and ACCESS_GET.
allowed_access_type[v8::ACCESS_HAS] = true;
allowed_access_type[v8::ACCESS_GET] = true;
@@ -9202,45 +9191,13 @@
ExpectString("other.js_accessor_p", "getter");
ExpectObject(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').get",
getter);
- ExpectUndefined(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').set");
- ExpectUndefined(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').value");
-
- allowed_access_type[v8::ACCESS_GET] = false;
- allowed_access_type[v8::ACCESS_HAS] = false;
-
- // Enable both ACCESS_HAS and ACCESS_SET.
- allowed_access_type[v8::ACCESS_HAS] = true;
- allowed_access_type[v8::ACCESS_SET] = true;
-
- ExpectUndefined("other.js_accessor_p");
- ExpectUndefined(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').get");
ExpectObject(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').set",
setter);
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').value");
- allowed_access_type[v8::ACCESS_SET] = false;
allowed_access_type[v8::ACCESS_HAS] = false;
-
- // Enable both ACCESS_HAS, ACCESS_GET and ACCESS_SET.
- allowed_access_type[v8::ACCESS_HAS] = true;
- allowed_access_type[v8::ACCESS_GET] = true;
- allowed_access_type[v8::ACCESS_SET] = true;
-
- ExpectString("other.js_accessor_p", "getter");
- ExpectObject(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').get",
getter);
- ExpectObject(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').set",
setter);
- ExpectUndefined(
- "Object.getOwnPropertyDescriptor(other, 'js_accessor_p').value");
-
- allowed_access_type[v8::ACCESS_SET] = false;
allowed_access_type[v8::ACCESS_GET] = false;
- allowed_access_type[v8::ACCESS_HAS] = false;
// Access an element with JS accessor.
CompileRun("other[42] = 2");
@@ -9248,51 +9205,17 @@
ExpectUndefined("other[42]");
ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42')");
- // Enable ACCESS_HAS.
- allowed_access_type[v8::ACCESS_HAS] = true;
- ExpectUndefined("other[42]");
- ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').get");
- ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').set");
- ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').value");
- allowed_access_type[v8::ACCESS_HAS] = false;
-
// Enable both ACCESS_HAS and ACCESS_GET.
allowed_access_type[v8::ACCESS_HAS] = true;
allowed_access_type[v8::ACCESS_GET] = true;
ExpectString("other[42]", "el_getter");
ExpectObject("Object.getOwnPropertyDescriptor(other, '42').get",
el_getter);
- ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').set");
- ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').value");
-
- allowed_access_type[v8::ACCESS_GET] = false;
- allowed_access_type[v8::ACCESS_HAS] = false;
-
- // Enable both ACCESS_HAS and ACCESS_SET.
- allowed_access_type[v8::ACCESS_HAS] = true;
- allowed_access_type[v8::ACCESS_SET] = true;
-
- ExpectUndefined("other[42]");
- ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').get");
ExpectObject("Object.getOwnPropertyDescriptor(other, '42').set",
el_setter);
ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').value");
- allowed_access_type[v8::ACCESS_SET] = false;
allowed_access_type[v8::ACCESS_HAS] = false;
-
- // Enable both ACCESS_HAS, ACCESS_GET and ACCESS_SET.
- allowed_access_type[v8::ACCESS_HAS] = true;
- allowed_access_type[v8::ACCESS_GET] = true;
- allowed_access_type[v8::ACCESS_SET] = true;
-
- ExpectString("other[42]", "el_getter");
- ExpectObject("Object.getOwnPropertyDescriptor(other, '42').get",
el_getter);
- ExpectObject("Object.getOwnPropertyDescriptor(other, '42').set",
el_setter);
- ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').value");
-
- allowed_access_type[v8::ACCESS_SET] = false;
allowed_access_type[v8::ACCESS_GET] = false;
- allowed_access_type[v8::ACCESS_HAS] = false;
v8::Handle<Value> value;
--
--
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.
