From 3997ae1b469617be5abbe517d61ea71481bc6638 Mon Sep 17 00:00:00 2001 From: ishell Date: Thu, 2 Jul 2015 02:05:39 -0700 Subject: [PATCH] Remove deprecated v8::Object::TurnOnAccessCheck() from the V8 API. The only right way to enable access checks is to install access check callbacks on an object template via v8::ObjectTemplate::SetAccessCheckCallbacks(). It does not make sense to enable access checks on an arbitrary object. Review URL: https://codereview.chromium.org/1217893012 Cr-Commit-Position: refs/heads/master@{#29439} --- include/v8.h | 17 ++------ src/api.cc | 26 +----------- src/deoptimizer.cc | 22 ---------- src/deoptimizer.h | 3 -- src/objects-printer.cc | 12 +++--- src/runtime/runtime-object.cc | 30 -------------- src/runtime/runtime.h | 2 - test/cctest/test-api-interceptors.cc | 76 +++++++++-------------------------- test/cctest/test-api.cc | 78 +----------------------------------- test/mjsunit/migrations.js | 12 ------ 10 files changed, 32 insertions(+), 246 deletions(-) diff --git a/include/v8.h b/include/v8.h index fafd168..c0c81de 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2859,13 +2859,6 @@ class V8_EXPORT Object : public Value { bool HasIndexedLookupInterceptor(); /** - * Turns on access check on the object if the object is an instance of - * a template that has access check callbacks. If an object has no - * access check info, the object cannot be accessed by anyone. - */ - V8_DEPRECATE_SOON("No alternative", void TurnOnAccessCheck()); - - /** * Returns the identity hash for this object. The current implementation * uses a hidden property on the object to store the identity hash. * @@ -4651,20 +4644,16 @@ class V8_EXPORT ObjectTemplate : public Template { void MarkAsUndetectable(); /** - * Sets access check callbacks on the object template. + * Sets access check callbacks on the object template and enables + * access checks. * * When accessing properties on instances of this object template, * the access check callback will be called to determine whether or * not to allow cross-context access to the properties. - * The last parameter specifies whether access checks are turned - * on by default on instances. If access checks are off by default, - * they can be turned on on individual instances by calling - * Object::TurnOnAccessCheck(). */ void SetAccessCheckCallbacks(NamedSecurityCallback named_handler, IndexedSecurityCallback indexed_handler, - Handle data = Handle(), - bool turned_on_by_default = true); + Handle data = Handle()); /** * Gets the number of internal fields for objects generated from diff --git a/src/api.cc b/src/api.cc index 49aeddb..62df2ca 100644 --- a/src/api.cc +++ b/src/api.cc @@ -1426,9 +1426,7 @@ void ObjectTemplate::MarkAsUndetectable() { void ObjectTemplate::SetAccessCheckCallbacks( NamedSecurityCallback named_callback, - IndexedSecurityCallback indexed_callback, - Handle data, - bool turned_on_by_default) { + IndexedSecurityCallback indexed_callback, Handle data) { i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate(); ENTER_V8(isolate); i::HandleScope scope(isolate); @@ -1449,7 +1447,7 @@ void ObjectTemplate::SetAccessCheckCallbacks( info->set_data(*Utils::OpenHandle(*data)); cons->set_access_check_info(*info); - cons->set_needs_access_check(turned_on_by_default); + cons->set_needs_access_check(true); } @@ -4262,26 +4260,6 @@ Maybe v8::Object::GetRealNamedPropertyAttributes( } -// Turns on access checks by copying the map and setting the check flag. -// Because the object gets a new map, existing inline cache caching -// the old map of this object will fail. -void v8::Object::TurnOnAccessCheck() { - i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate(); - ENTER_V8(isolate); - i::HandleScope scope(isolate); - i::Handle obj = Utils::OpenHandle(this); - - // When turning on access checks for a global object deoptimize all functions - // as optimized code does not always handle access checks. - i::Deoptimizer::DeoptimizeGlobalObject(*obj); - - i::Handle new_map = - i::Map::Copy(i::Handle(obj->map()), "APITurnOnAccessCheck"); - new_map->set_is_access_check_needed(true); - i::JSObject::MigrateToMap(obj, new_map); -} - - Local v8::Object::Clone() { auto self = Utils::OpenHandle(this); auto isolate = self->GetIsolate(); diff --git a/src/deoptimizer.cc b/src/deoptimizer.cc index 994904d..3ab10fc 100644 --- a/src/deoptimizer.cc +++ b/src/deoptimizer.cc @@ -454,28 +454,6 @@ void Deoptimizer::DeoptimizeMarkedCode(Isolate* isolate) { } -void Deoptimizer::DeoptimizeGlobalObject(JSObject* object) { - if (FLAG_trace_deopt) { - CodeTracer::Scope scope(object->GetHeap()->isolate()->GetCodeTracer()); - PrintF(scope.file(), "[deoptimize global object @ 0x%08" V8PRIxPTR "]\n", - reinterpret_cast(object)); - } - if (object->IsJSGlobalProxy()) { - PrototypeIterator iter(object->GetIsolate(), object); - // TODO(verwaest): This CHECK will be hit if the global proxy is detached. - CHECK(iter.GetCurrent()->IsJSGlobalObject()); - Context* native_context = - GlobalObject::cast(iter.GetCurrent())->native_context(); - MarkAllCodeForContext(native_context); - DeoptimizeMarkedCodeForContext(native_context); - } else if (object->IsGlobalObject()) { - Context* native_context = GlobalObject::cast(object)->native_context(); - MarkAllCodeForContext(native_context); - DeoptimizeMarkedCodeForContext(native_context); - } -} - - void Deoptimizer::MarkAllCodeForContext(Context* context) { Object* element = context->OptimizedCodeListHead(); while (!element->IsUndefined()) { diff --git a/src/deoptimizer.h b/src/deoptimizer.h index 5c0e6b1..ab76d41 100644 --- a/src/deoptimizer.h +++ b/src/deoptimizer.h @@ -484,9 +484,6 @@ class Deoptimizer : public Malloced { // Deoptimize all code in the given isolate. static void DeoptimizeAll(Isolate* isolate); - // Deoptimize code associated with the given global object. - static void DeoptimizeGlobalObject(JSObject* object); - // Deoptimizes all optimized code that has been previously marked // (via code->set_marked_for_deoptimization) and unlinks all functions that // refer to that code. diff --git a/src/objects-printer.cc b/src/objects-printer.cc index 8c7e991..35d1d66 100644 --- a/src/objects-printer.cc +++ b/src/objects-printer.cc @@ -418,12 +418,6 @@ void Map::MapPrint(std::ostream& os) { // NOLINT if (is_deprecated()) os << " - deprecated_map\n"; if (is_stable()) os << " - stable_map\n"; if (is_dictionary_map()) os << " - dictionary_map\n"; - if (is_prototype_map()) { - os << " - prototype_map\n"; - os << " - prototype info: " << Brief(prototype_info()); - } else { - os << " - back pointer: " << Brief(GetBackPointer()); - } if (is_hidden_prototype()) os << " - hidden_prototype\n"; if (has_named_interceptor()) os << " - named_interceptor\n"; if (has_indexed_interceptor()) os << " - indexed_interceptor\n"; @@ -432,6 +426,12 @@ void Map::MapPrint(std::ostream& os) { // NOLINT if (is_access_check_needed()) os << " - access_check_needed\n"; if (!is_extensible()) os << " - non-extensible\n"; if (is_observed()) os << " - observed\n"; + if (is_prototype_map()) { + os << " - prototype_map\n"; + os << " - prototype info: " << Brief(prototype_info()); + } else { + os << " - back pointer: " << Brief(GetBackPointer()); + } os << "\n - instance descriptors " << (owns_descriptors() ? "(own) " : "") << "#" << NumberOfOwnDescriptors() << ": " << Brief(instance_descriptors()); diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc index 3e0253c..8343958 100644 --- a/src/runtime/runtime-object.cc +++ b/src/runtime/runtime-object.cc @@ -386,36 +386,6 @@ RUNTIME_FUNCTION(Runtime_IsExtensible) { } -RUNTIME_FUNCTION(Runtime_DisableAccessChecks) { - HandleScope scope(isolate); - DCHECK(args.length() == 1); - CONVERT_ARG_HANDLE_CHECKED(HeapObject, object, 0); - Handle old_map(object->map()); - bool needs_access_checks = old_map->is_access_check_needed(); - if (needs_access_checks) { - // Copy map so it won't interfere constructor's initial map. - Handle new_map = Map::Copy(old_map, "DisableAccessChecks"); - new_map->set_is_access_check_needed(false); - JSObject::MigrateToMap(Handle::cast(object), new_map); - } - return isolate->heap()->ToBoolean(needs_access_checks); -} - - -RUNTIME_FUNCTION(Runtime_EnableAccessChecks) { - HandleScope scope(isolate); - DCHECK(args.length() == 1); - CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0); - Handle old_map(object->map()); - RUNTIME_ASSERT(!old_map->is_access_check_needed()); - // Copy map so it won't interfere constructor's initial map. - Handle new_map = Map::Copy(old_map, "EnableAccessChecks"); - new_map->set_is_access_check_needed(true); - JSObject::MigrateToMap(object, new_map); - return isolate->heap()->undefined_value(); -} - - RUNTIME_FUNCTION(Runtime_OptimizeObjectForAddingMultipleProperties) { HandleScope scope(isolate); DCHECK(args.length() == 2); diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index ba96ebd..53ef365 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -430,8 +430,6 @@ namespace internal { F(GetOwnProperty, 2, 1) \ F(PreventExtensions, 1, 1) \ F(IsExtensible, 1, 1) \ - F(DisableAccessChecks, 1, 1) \ - F(EnableAccessChecks, 1, 1) \ F(OptimizeObjectForAddingMultipleProperties, 2, 1) \ F(ObjectFreeze, 1, 1) \ F(ObjectSeal, 1, 1) \ diff --git a/test/cctest/test-api-interceptors.cc b/test/cctest/test-api-interceptors.cc index 7d2d4c6..2e9bc74 100644 --- a/test/cctest/test-api-interceptors.cc +++ b/test/cctest/test-api-interceptors.cc @@ -1661,6 +1661,12 @@ THREADED_TEST(IndexedInterceptorWithNoSetter) { } +static bool AccessAlwaysBlocked(Local global, Local name, + v8::AccessType type, Local data) { + return false; +} + + THREADED_TEST(IndexedInterceptorWithAccessorCheck) { v8::Isolate* isolate = CcTest::isolate(); v8::HandleScope scope(isolate); @@ -1668,9 +1674,10 @@ THREADED_TEST(IndexedInterceptorWithAccessorCheck) { templ->SetHandler( v8::IndexedPropertyHandlerConfiguration(IdentityIndexedPropertyGetter)); + templ->SetAccessCheckCallbacks(AccessAlwaysBlocked, nullptr); + LocalContext context; Local obj = templ->NewInstance(); - obj->TurnOnAccessCheck(); context->Global()->Set(v8_str("obj"), obj); const char* code = @@ -1689,46 +1696,6 @@ THREADED_TEST(IndexedInterceptorWithAccessorCheck) { } -THREADED_TEST(IndexedInterceptorWithAccessorCheckSwitchedOn) { - i::FLAG_allow_natives_syntax = true; - v8::Isolate* isolate = CcTest::isolate(); - v8::HandleScope scope(isolate); - Local templ = ObjectTemplate::New(isolate); - templ->SetHandler( - v8::IndexedPropertyHandlerConfiguration(IdentityIndexedPropertyGetter)); - - LocalContext context; - Local obj = templ->NewInstance(); - context->Global()->Set(v8_str("obj"), obj); - - const char* code = - "var result = 'PASSED';" - "for (var i = 0; i < 100; i++) {" - " var expected = i;" - " if (i == 5) {" - " %EnableAccessChecks(obj);" - " }" - " try {" - " var v = obj[i];" - " if (i == 5) {" - " result = 'Should not have reached this!';" - " break;" - " } else if (v != expected) {" - " result = 'Wrong value ' + v + ' at iteration ' + i;" - " break;" - " }" - " } catch (e) {" - " if (i != 5) {" - " result = e;" - " }" - " }" - " if (i == 5) %DisableAccessChecks(obj);" - "}" - "result"; - ExpectString(code, "PASSED"); -} - - THREADED_TEST(IndexedInterceptorWithDifferentIndices) { v8::Isolate* isolate = CcTest::isolate(); v8::HandleScope scope(isolate); @@ -2984,7 +2951,7 @@ THREADED_TEST(NamedAllCanReadInterceptor) { LocalContext context; AccessCheckData access_check_data; - access_check_data.result = false; + access_check_data.result = true; access_check_data.count = 0; ShouldInterceptData intercept_data_0; @@ -3016,7 +2983,7 @@ THREADED_TEST(NamedAllCanReadInterceptor) { auto checked = v8::ObjectTemplate::New(isolate); checked->SetAccessCheckCallbacks( SimpleAccessChecker, nullptr, - BuildWrappedObject(isolate, &access_check_data), false); + BuildWrappedObject(isolate, &access_check_data)); context->Global()->Set(v8_str("intercepted_0"), intercepted_0->NewInstance()); context->Global()->Set(v8_str("intercepted_1"), intercepted_1->NewInstance()); @@ -3027,14 +2994,12 @@ THREADED_TEST(NamedAllCanReadInterceptor) { "checked.__proto__ = intercepted_1;" "intercepted_1.__proto__ = intercepted_0;"); - checked_instance->TurnOnAccessCheck(); - CHECK_EQ(0, access_check_data.count); + CHECK_EQ(3, access_check_data.count); - access_check_data.result = true; ExpectInt32("checked.whatever", 17); CHECK(!CompileRun("Object.getOwnPropertyDescriptor(checked, 'whatever')") ->IsUndefined()); - CHECK_EQ(2, access_check_data.count); + CHECK_EQ(5, access_check_data.count); access_check_data.result = false; ExpectInt32("checked.whatever", intercept_data_0.value); @@ -3043,7 +3008,7 @@ THREADED_TEST(NamedAllCanReadInterceptor) { CompileRun("Object.getOwnPropertyDescriptor(checked, 'whatever')"); CHECK(try_catch.HasCaught()); } - CHECK_EQ(4, access_check_data.count); + CHECK_EQ(7, access_check_data.count); intercept_data_1.should_intercept = true; ExpectInt32("checked.whatever", intercept_data_1.value); @@ -3052,7 +3017,7 @@ THREADED_TEST(NamedAllCanReadInterceptor) { CompileRun("Object.getOwnPropertyDescriptor(checked, 'whatever')"); CHECK(try_catch.HasCaught()); } - CHECK_EQ(6, access_check_data.count); + CHECK_EQ(9, access_check_data.count); } @@ -3062,7 +3027,7 @@ THREADED_TEST(IndexedAllCanReadInterceptor) { LocalContext context; AccessCheckData access_check_data; - access_check_data.result = false; + access_check_data.result = true; access_check_data.count = 0; ShouldInterceptData intercept_data_0; @@ -3094,7 +3059,7 @@ THREADED_TEST(IndexedAllCanReadInterceptor) { auto checked = v8::ObjectTemplate::New(isolate); checked->SetAccessCheckCallbacks( SimpleAccessChecker, nullptr, - BuildWrappedObject(isolate, &access_check_data), false); + BuildWrappedObject(isolate, &access_check_data)); context->Global()->Set(v8_str("intercepted_0"), intercepted_0->NewInstance()); context->Global()->Set(v8_str("intercepted_1"), intercepted_1->NewInstance()); @@ -3105,14 +3070,13 @@ THREADED_TEST(IndexedAllCanReadInterceptor) { "checked.__proto__ = intercepted_1;" "intercepted_1.__proto__ = intercepted_0;"); - checked_instance->TurnOnAccessCheck(); - CHECK_EQ(0, access_check_data.count); + CHECK_EQ(3, access_check_data.count); access_check_data.result = true; ExpectInt32("checked[15]", 17); CHECK(!CompileRun("Object.getOwnPropertyDescriptor(checked, '15')") ->IsUndefined()); - CHECK_EQ(2, access_check_data.count); + CHECK_EQ(5, access_check_data.count); access_check_data.result = false; ExpectInt32("checked[15]", intercept_data_0.value); @@ -3121,7 +3085,7 @@ THREADED_TEST(IndexedAllCanReadInterceptor) { CompileRun("Object.getOwnPropertyDescriptor(checked, '15')"); CHECK(try_catch.HasCaught()); } - CHECK_EQ(4, access_check_data.count); + CHECK_EQ(7, access_check_data.count); intercept_data_1.should_intercept = true; ExpectInt32("checked[15]", intercept_data_1.value); @@ -3130,7 +3094,7 @@ THREADED_TEST(IndexedAllCanReadInterceptor) { CompileRun("Object.getOwnPropertyDescriptor(checked, '15')"); CHECK(try_catch.HasCaught()); } - CHECK_EQ(6, access_check_data.count); + CHECK_EQ(9, access_check_data.count); } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 00fd97b..5c844cb 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8278,7 +8278,6 @@ TEST(DetachedAccesses) { } Local env2_global = env2->Global(); - env2_global->TurnOnAccessCheck(); env2->DetachGlobal(); Local result; @@ -12760,71 +12759,6 @@ THREADED_TEST(AccessChecksReenabledCorrectly) { } -THREADED_TEST(TurnOnAccessCheck) { - v8::Isolate* isolate = CcTest::isolate(); - v8::HandleScope handle_scope(isolate); - - // Create an environment with access check to the global object disabled by - // default. - v8::Handle global_template = - v8::ObjectTemplate::New(isolate); - global_template->SetAccessCheckCallbacks(AccessAlwaysBlocked, NULL, - v8::Handle(), false); - v8::Local context = Context::New(isolate, NULL, global_template); - Context::Scope context_scope(context); - - // Set up a property and a number of functions. - context->Global()->Set(v8_str("a"), v8_num(1)); - CompileRun("function f1() {return a;}" - "function f2() {return a;}" - "function g1() {return h();}" - "function g2() {return h();}" - "function h() {return 1;}"); - Local f1 = - Local::Cast(context->Global()->Get(v8_str("f1"))); - Local f2 = - Local::Cast(context->Global()->Get(v8_str("f2"))); - Local g1 = - Local::Cast(context->Global()->Get(v8_str("g1"))); - Local g2 = - Local::Cast(context->Global()->Get(v8_str("g2"))); - Local h = - Local::Cast(context->Global()->Get(v8_str("h"))); - - // Get the global object. - v8::Handle global = context->Global(); - - // Call f1 one time and f2 a number of times. This will ensure that f1 still - // uses the runtime system to retreive property a whereas f2 uses global load - // inline cache. - CHECK(f1->Call(global, 0, NULL)->Equals(v8_num(1))); - for (int i = 0; i < 4; i++) { - CHECK(f2->Call(global, 0, NULL)->Equals(v8_num(1))); - } - - // Same for g1 and g2. - CHECK(g1->Call(global, 0, NULL)->Equals(v8_num(1))); - for (int i = 0; i < 4; i++) { - CHECK(g2->Call(global, 0, NULL)->Equals(v8_num(1))); - } - - // Detach the global and turn on access check. - Local hidden_global = Local::Cast( - context->Global()->GetPrototype()); - context->DetachGlobal(); - hidden_global->TurnOnAccessCheck(); - - // Failing access check results in exception. - CHECK(f1->Call(global, 0, NULL).IsEmpty()); - CHECK(f2->Call(global, 0, NULL).IsEmpty()); - CHECK(g1->Call(global, 0, NULL).IsEmpty()); - CHECK(g2->Call(global, 0, NULL).IsEmpty()); - - // No failing access check when just returning a constant. - CHECK(h->Call(global, 0, NULL)->Equals(v8_num(1))); -} - - // Tests that ScriptData can be serialized and deserialized. TEST(PreCompileSerialization) { v8::V8::Initialize(); @@ -16800,7 +16734,7 @@ TEST(GCInFailedAccessCheckCallback) { v8::Handle global_template = v8::ObjectTemplate::New(isolate); global_template->SetAccessCheckCallbacks(AccessAlwaysBlocked, NULL, - v8::Handle(), false); + v8::Handle()); // Create a context and set an x property on it's global object. LocalContext context0(NULL, global_template); @@ -19246,16 +19180,6 @@ TEST(JSONStringifyAccessCheck) { CHECK(CompileRun("JSON.stringify(other)").IsEmpty()); CHECK(CompileRun("JSON.stringify({ 'a' : other, 'b' : ['c'] })").IsEmpty()); CHECK(CompileRun("JSON.stringify([other, 'b', 'c'])").IsEmpty()); - - v8::Handle array = v8::Array::New(isolate, 2); - array->Set(0, v8_str("a")); - array->Set(1, v8_str("b")); - context1->Global()->Set(v8_str("array"), array); - ExpectString("JSON.stringify(array)", "[\"a\",\"b\"]"); - array->TurnOnAccessCheck(); - CHECK(CompileRun("JSON.stringify(array)").IsEmpty()); - CHECK(CompileRun("JSON.stringify([array])").IsEmpty()); - CHECK(CompileRun("JSON.stringify({'a' : array})").IsEmpty()); } } diff --git a/test/mjsunit/migrations.js b/test/mjsunit/migrations.js index 6a2ea64..288bc61 100644 --- a/test/mjsunit/migrations.js +++ b/test/mjsunit/migrations.js @@ -278,18 +278,6 @@ var migrations = [ migr: function(o, i) { Object.observe(o, function(){}); }, }, { - name: "%EnableAccessChecks", - migr: function(o, i) { - if (typeof (o) !== 'function') %EnableAccessChecks(o); - }, - }, - { - name: "%DisableAccessChecks", - migr: function(o, i) { - if ((typeof (o) !== 'function') && (o !== global)) %DisableAccessChecks(o); - }, - }, - { name: "seal", migr: function(o, i) { Object.seal(o); }, }, -- 2.7.4