Never skip access checks in the lookup iterator

BUG=
R=yangguo@chromium.org

Review URL: https://codereview.chromium.org/536943002

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23661 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
verwaest@chromium.org 2014-09-03 14:05:55 +00:00
parent 96ae555e09
commit 5941bb4e73
10 changed files with 84 additions and 76 deletions

View File

@ -785,7 +785,7 @@ Handle<JSGlobalProxy> Genesis::CreateNewGlobals(
name, code, prototype, JS_GLOBAL_OBJECT_TYPE, JSGlobalObject::kSize);
#ifdef DEBUG
LookupIterator it(prototype, factory()->constructor_string(),
LookupIterator::OWN_PROPERTY);
LookupIterator::OWN_SKIP_INTERCEPTOR);
Handle<Object> value = JSReceiver::GetProperty(&it).ToHandleChecked();
DCHECK(it.IsFound());
DCHECK_EQ(*isolate()->object_function(), *value);
@ -2485,7 +2485,8 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
}
case CALLBACKS: {
Handle<Name> key(descs->GetKey(i));
LookupIterator it(to, key, LookupIterator::OWN_PROPERTY);
LookupIterator it(to, key, LookupIterator::OWN_SKIP_INTERCEPTOR);
CHECK_NE(LookupIterator::ACCESS_CHECK, it.state());
// If the property is already there we skip it
if (it.IsFound() && it.HasProperty()) continue;
HandleScope inner(isolate());
@ -2513,7 +2514,8 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
DCHECK(raw_key->IsName());
// If the property is already there we skip it.
Handle<Name> key(Name::cast(raw_key));
LookupIterator it(to, key, LookupIterator::OWN_PROPERTY);
LookupIterator it(to, key, LookupIterator::OWN_SKIP_INTERCEPTOR);
CHECK_NE(LookupIterator::ACCESS_CHECK, it.state());
if (it.IsFound() && it.HasProperty()) continue;
// Set the property.
Handle<Object> value = Handle<Object>(properties->ValueAt(i),

View File

@ -2182,7 +2182,8 @@ Handle<JSFunction> Factory::CreateApiFunction(
if (prototype->IsTheHole()) {
#ifdef DEBUG
LookupIterator it(handle(JSObject::cast(result->prototype())),
constructor_string(), LookupIterator::OWN_PROPERTY);
constructor_string(),
LookupIterator::OWN_SKIP_INTERCEPTOR);
MaybeHandle<Object> maybe_prop = Object::GetProperty(&it);
DCHECK(it.IsFound());
DCHECK(maybe_prop.ToHandleChecked().is_identical_to(result));

View File

@ -5292,16 +5292,30 @@ void HOptimizedGraphBuilder::VisitConditional(Conditional* expr) {
HOptimizedGraphBuilder::GlobalPropertyAccess
HOptimizedGraphBuilder::LookupGlobalProperty(Variable* var, LookupIterator* it,
PropertyAccessType access_type) {
DCHECK_EQ(*var->name(), *it->name());
if (var->is_this() || !current_info()->has_global_object()) {
return kUseGeneric;
}
if (!it->HasProperty() || it->property_kind() != LookupIterator::DATA ||
(access_type == STORE && it->IsReadOnly())) {
return kUseGeneric;
}
return kUseCell;
switch (it->state()) {
case LookupIterator::ACCESS_CHECK:
case LookupIterator::INTERCEPTOR:
case LookupIterator::NOT_FOUND:
return kUseGeneric;
case LookupIterator::PROPERTY:
if (!it->HasProperty()) return kUseGeneric;
switch (it->property_kind()) {
case LookupIterator::DATA:
if (access_type == STORE && it->IsReadOnly()) return kUseGeneric;
return kUseCell;
case LookupIterator::ACCESSOR:
return kUseGeneric;
}
case LookupIterator::JSPROXY:
case LookupIterator::TRANSITION:
UNREACHABLE();
}
UNREACHABLE();
return kUseGeneric;
}
@ -5343,14 +5357,10 @@ void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) {
}
Handle<GlobalObject> global(current_info()->global_object());
LookupIterator it(global, variable->name(), LookupIterator::OWN_PROPERTY);
LookupIterator it(global, variable->name(),
LookupIterator::OWN_SKIP_INTERCEPTOR);
GlobalPropertyAccess type = LookupGlobalProperty(variable, &it, LOAD);
if (type == kUseCell &&
current_info()->global_object()->IsAccessCheckNeeded()) {
type = kUseGeneric;
}
if (type == kUseCell) {
Handle<PropertyCell> cell = it.GetPropertyCell();
if (cell->type()->IsConstant()) {
@ -5797,7 +5807,8 @@ HInstruction* HOptimizedGraphBuilder::BuildLoadNamedField(
HConstant::cast(checked_object->ActualValue())->handle(isolate()));
if (object->IsJSObject()) {
LookupIterator it(object, info->name(), LookupIterator::OWN_PROPERTY);
LookupIterator it(object, info->name(),
LookupIterator::OWN_SKIP_INTERCEPTOR);
Handle<Object> value = JSObject::GetDataProperty(&it);
if (it.IsFound() && it.IsReadOnly() && !it.IsConfigurable()) {
return New<HConstant>(value);
@ -6461,7 +6472,7 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment(
HValue* value,
BailoutId ast_id) {
Handle<GlobalObject> global(current_info()->global_object());
LookupIterator it(global, var->name(), LookupIterator::OWN_PROPERTY);
LookupIterator it(global, var->name(), LookupIterator::OWN_SKIP_INTERCEPTOR);
GlobalPropertyAccess type = LookupGlobalProperty(var, &it, STORE);
if (type == kUseCell) {
Handle<PropertyCell> cell = it.GetPropertyCell();
@ -9047,10 +9058,10 @@ void HOptimizedGraphBuilder::VisitCall(Call* expr) {
// access check is not enabled we assume that the function will not change
// and generate optimized code for calling the function.
Handle<GlobalObject> global(current_info()->global_object());
LookupIterator it(global, var->name(), LookupIterator::OWN_PROPERTY);
LookupIterator it(global, var->name(),
LookupIterator::OWN_SKIP_INTERCEPTOR);
GlobalPropertyAccess type = LookupGlobalProperty(var, &it, LOAD);
if (type == kUseCell &&
!current_info()->global_object()->IsAccessCheckNeeded()) {
if (type == kUseCell) {
Handle<GlobalObject> global(current_info()->global_object());
known_global_function = expr->ComputeGlobalTarget(global, &it);
}
@ -10686,12 +10697,10 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
Handle<JSFunction> target = Handle<JSFunction>::null();
VariableProxy* proxy = expr->right()->AsVariableProxy();
bool global_function = (proxy != NULL) && proxy->var()->IsUnallocated();
if (global_function &&
current_info()->has_global_object() &&
!current_info()->global_object()->IsAccessCheckNeeded()) {
if (global_function && current_info()->has_global_object()) {
Handle<String> name = proxy->name();
Handle<GlobalObject> global(current_info()->global_object());
LookupIterator it(global, name, LookupIterator::OWN_PROPERTY);
LookupIterator it(global, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
Handle<Object> value = JSObject::GetDataProperty(&it);
if (it.IsFound() && value->IsJSFunction()) {
Handle<JSFunction> candidate = Handle<JSFunction>::cast(value);

View File

@ -283,7 +283,8 @@ bool IC::TryRemoveInvalidPrototypeDependentStub(Handle<Object> receiver,
if (receiver->IsGlobalObject()) {
Handle<GlobalObject> global = Handle<GlobalObject>::cast(receiver);
LookupIterator it(global, name, LookupIterator::OWN_PROPERTY);
LookupIterator it(global, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
if (it.state() == LookupIterator::ACCESS_CHECK) return false;
if (!it.IsFound() || !it.HasProperty()) return false;
Handle<PropertyCell> cell = it.GetPropertyCell();
return cell->type()->IsConstant();

View File

@ -1055,7 +1055,7 @@ void Isolate::DoThrow(Object* exception, MessageLocation* location) {
// probably not a valid Error object. In that case, we fall through
// and capture the stack trace at this throw site.
LookupIterator lookup(exception_handle, key,
LookupIterator::OWN_PROPERTY);
LookupIterator::OWN_SKIP_INTERCEPTOR);
Handle<Object> stack_trace_property;
if (Object::GetProperty(&lookup).ToHandle(&stack_trace_property) &&
stack_trace_property->IsJSArray()) {

View File

@ -36,12 +36,8 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* map) {
DisallowHeapAllocation no_gc;
switch (state_) {
case NOT_FOUND:
if (map->IsJSProxyMap()) {
return JSPROXY;
}
if (check_access_check() && map->is_access_check_needed()) {
return ACCESS_CHECK;
}
if (map->IsJSProxyMap()) return JSPROXY;
if (map->is_access_check_needed()) return ACCESS_CHECK;
// Fall through.
case ACCESS_CHECK:
if (check_interceptor() && map->has_named_interceptor()) {

View File

@ -16,21 +16,17 @@ class LookupIterator FINAL BASE_EMBEDDED {
public:
enum Configuration {
// Configuration bits.
kAccessCheck = 1 << 0,
kHidden = 1 << 1,
kInterceptor = 1 << 2,
kPrototypeChain = 1 << 3,
kHidden = 1 << 0,
kInterceptor = 1 << 1,
kPrototypeChain = 1 << 2,
// Convience combinations of bits.
OWN_PROPERTY = 0,
OWN_SKIP_INTERCEPTOR = kAccessCheck,
OWN = kAccessCheck | kInterceptor,
HIDDEN_PROPERTY = kHidden,
HIDDEN_SKIP_INTERCEPTOR = kAccessCheck | kHidden,
HIDDEN = kAccessCheck | kHidden | kInterceptor,
PROTOTYPE_CHAIN_PROPERTY = kHidden | kPrototypeChain,
PROTOTYPE_CHAIN_SKIP_INTERCEPTOR = kAccessCheck | kHidden | kPrototypeChain,
PROTOTYPE_CHAIN = kAccessCheck | kHidden | kPrototypeChain | kInterceptor
OWN_SKIP_INTERCEPTOR = 0,
OWN = kInterceptor,
HIDDEN_SKIP_INTERCEPTOR = kHidden,
HIDDEN = kHidden | kInterceptor,
PROTOTYPE_CHAIN_SKIP_INTERCEPTOR = kHidden | kPrototypeChain,
PROTOTYPE_CHAIN = kHidden | kPrototypeChain | kInterceptor
};
enum State {
@ -191,9 +187,6 @@ class LookupIterator FINAL BASE_EMBEDDED {
bool is_guaranteed_to_have_holder() const {
return !maybe_receiver_.is_null();
}
bool check_access_check() const {
return (configuration_ & kAccessCheck) != 0;
}
bool check_hidden() const { return (configuration_ & kHidden) != 0; }
bool check_interceptor() const {
return !IsBootstrapping() && (configuration_ & kInterceptor) != 0;

View File

@ -144,7 +144,8 @@ MaybeHandle<Object> Object::GetProperty(LookupIterator* it) {
Handle<Object> JSObject::GetDataProperty(Handle<JSObject> object,
Handle<Name> key) {
LookupIterator it(object, key, LookupIterator::PROTOTYPE_CHAIN_PROPERTY);
LookupIterator it(object, key,
LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
return GetDataProperty(&it);
}
@ -152,11 +153,13 @@ Handle<Object> JSObject::GetDataProperty(Handle<JSObject> object,
Handle<Object> JSObject::GetDataProperty(LookupIterator* it) {
for (; it->IsFound(); it->Next()) {
switch (it->state()) {
case LookupIterator::ACCESS_CHECK:
case LookupIterator::INTERCEPTOR:
case LookupIterator::NOT_FOUND:
case LookupIterator::TRANSITION:
UNREACHABLE();
case LookupIterator::ACCESS_CHECK:
if (it->HasAccess(v8::ACCESS_GET)) continue;
// Fall through.
case LookupIterator::JSPROXY:
it->NotFound();
return it->isolate()->factory()->undefined_value();
@ -3787,7 +3790,8 @@ void JSObject::WriteToField(int descriptor, Object* value) {
void JSObject::AddProperty(Handle<JSObject> object, Handle<Name> name,
Handle<Object> value,
PropertyAttributes attributes) {
LookupIterator it(object, name, LookupIterator::OWN_PROPERTY);
LookupIterator it(object, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
CHECK_NE(LookupIterator::ACCESS_CHECK, it.state());
#ifdef DEBUG
uint32_t index;
DCHECK(!object->IsJSProxy());
@ -4687,11 +4691,9 @@ void JSObject::DeleteHiddenProperty(Handle<JSObject> object, Handle<Name> key) {
bool JSObject::HasHiddenProperties(Handle<JSObject> object) {
Handle<Name> hidden = object->GetIsolate()->factory()->hidden_string();
LookupIterator it(object, hidden, LookupIterator::OWN_PROPERTY);
Maybe<PropertyAttributes> maybe = GetPropertyAttributes(&it);
// Cannot get an exception since the hidden_string isn't accessible to JS.
DCHECK(maybe.has_value);
return maybe.value != ABSENT;
LookupIterator it(object, hidden, LookupIterator::OWN_SKIP_INTERCEPTOR);
CHECK_NE(LookupIterator::ACCESS_CHECK, it.state());
return it.IsFound() && it.HasProperty();
}
@ -4722,7 +4724,8 @@ Object* JSObject::GetHiddenPropertiesHashTable() {
} else {
Isolate* isolate = GetIsolate();
LookupIterator it(handle(this), isolate->factory()->hidden_string(),
LookupIterator::OWN_PROPERTY);
LookupIterator::OWN_SKIP_INTERCEPTOR);
CHECK_NE(LookupIterator::ACCESS_CHECK, it.state());
if (it.IsFound() && it.HasProperty()) {
DCHECK_EQ(LookupIterator::DATA, it.property_kind());
return *it.GetDataValue();
@ -6163,7 +6166,8 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
setter->IsNull());
// At least one of the accessors needs to be a new value.
DCHECK(!getter->IsNull() || !setter->IsNull());
LookupIterator it(object, name, LookupIterator::OWN_PROPERTY);
LookupIterator it(object, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
CHECK_NE(LookupIterator::ACCESS_CHECK, it.state());
if (!getter->IsNull()) {
it.TransitionToAccessorProperty(ACCESSOR_GETTER, getter, attributes);
}
@ -12843,7 +12847,8 @@ bool JSArray::WouldChangeReadOnlyLength(Handle<JSArray> array,
CHECK(array->length()->ToArrayIndex(&length));
if (length <= index) {
LookupIterator it(array, array->GetIsolate()->factory()->length_string(),
LookupIterator::OWN_PROPERTY);
LookupIterator::OWN_SKIP_INTERCEPTOR);
CHECK_NE(LookupIterator::ACCESS_CHECK, it.state());
CHECK(it.IsFound());
CHECK(it.HasProperty());
return it.IsReadOnly();

View File

@ -2180,12 +2180,12 @@ static Object* DeclareGlobals(Isolate* isolate, Handle<GlobalObject> global,
PropertyAttributes attr, bool is_var,
bool is_const, bool is_function) {
// Do the lookup own properties only, see ES5 erratum.
LookupIterator it(global, name, LookupIterator::HIDDEN_PROPERTY);
LookupIterator it(global, name, LookupIterator::HIDDEN_SKIP_INTERCEPTOR);
Maybe<PropertyAttributes> maybe = JSReceiver::GetPropertyAttributes(&it);
DCHECK(maybe.has_value);
PropertyAttributes old_attributes = maybe.value;
if (!maybe.has_value) return isolate->heap()->exception();
if (old_attributes != ABSENT) {
if (it.IsFound()) {
PropertyAttributes old_attributes = maybe.value;
// The name was declared before; check for conflicting re-declarations.
if (is_const) return ThrowRedeclarationError(isolate, name);
@ -2310,7 +2310,7 @@ RUNTIME_FUNCTION(Runtime_InitializeConstGlobal) {
Handle<GlobalObject> global = isolate->global_object();
// Lookup the property as own on the global object.
LookupIterator it(global, name, LookupIterator::HIDDEN_PROPERTY);
LookupIterator it(global, name, LookupIterator::HIDDEN_SKIP_INTERCEPTOR);
Maybe<PropertyAttributes> maybe = JSReceiver::GetPropertyAttributes(&it);
DCHECK(maybe.has_value);
PropertyAttributes old_attributes = maybe.value;
@ -2460,7 +2460,7 @@ RUNTIME_FUNCTION(Runtime_InitializeLegacyConstLookupSlot) {
// code can run in between that modifies the declared property.
DCHECK(holder->IsJSGlobalObject() || holder->IsJSContextExtensionObject());
LookupIterator it(holder, name, LookupIterator::HIDDEN_PROPERTY);
LookupIterator it(holder, name, LookupIterator::HIDDEN_SKIP_INTERCEPTOR);
Maybe<PropertyAttributes> maybe = JSReceiver::GetPropertyAttributes(&it);
if (!maybe.has_value) return isolate->heap()->exception();
PropertyAttributes old_attributes = maybe.value;
@ -5040,14 +5040,14 @@ RUNTIME_FUNCTION(Runtime_DefineDataPropertyUnchecked) {
RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) == 0);
PropertyAttributes attr = static_cast<PropertyAttributes>(unchecked);
// Check access rights if needed.
if (js_object->IsAccessCheckNeeded() &&
!isolate->MayNamedAccess(js_object, name, v8::ACCESS_SET)) {
return isolate->heap()->undefined_value();
LookupIterator it(js_object, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
if (it.IsFound() && it.state() == LookupIterator::ACCESS_CHECK) {
if (!isolate->MayNamedAccess(js_object, name, v8::ACCESS_SET)) {
return isolate->heap()->undefined_value();
}
it.Next();
}
LookupIterator it(js_object, name, LookupIterator::OWN_PROPERTY);
// Take special care when attributes are different and there is already
// a property.
if (it.IsFound() && it.HasProperty() &&
@ -5293,9 +5293,9 @@ RUNTIME_FUNCTION(Runtime_AddNamedProperty) {
#ifdef DEBUG
uint32_t index = 0;
DCHECK(!key->ToArrayIndex(&index));
LookupIterator it(object, key, LookupIterator::OWN_PROPERTY);
LookupIterator it(object, key, LookupIterator::OWN_SKIP_INTERCEPTOR);
Maybe<PropertyAttributes> maybe = JSReceiver::GetPropertyAttributes(&it);
DCHECK(maybe.has_value);
if (!maybe.has_value) return isolate->heap()->exception();
RUNTIME_ASSERT(!it.IsFound());
#endif
@ -5325,7 +5325,7 @@ RUNTIME_FUNCTION(Runtime_AddPropertyForTemplate) {
bool duplicate;
if (key->IsName()) {
LookupIterator it(object, Handle<Name>::cast(key),
LookupIterator::OWN_PROPERTY);
LookupIterator::OWN_SKIP_INTERCEPTOR);
Maybe<PropertyAttributes> maybe = JSReceiver::GetPropertyAttributes(&it);
DCHECK(maybe.has_value);
duplicate = it.IsFound();

View File

@ -2044,7 +2044,8 @@ THREADED_TEST(ExecutableAccessorIsPreservedOnAttributeChange) {
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
i::LookupResult lookup(i_isolate);
i::Handle<i::String> name(v8::Utils::OpenHandle(*v8_str("length")));
i::LookupIterator it(a, name, i::LookupIterator::OWN_PROPERTY);
i::LookupIterator it(a, name, i::LookupIterator::OWN_SKIP_INTERCEPTOR);
CHECK_NE(i::LookupIterator::ACCESS_CHECK, it.state());
CHECK(it.HasProperty());
CHECK(it.GetAccessors()->IsExecutableAccessorInfo());
}