From e664f42a5a53d3ffd8ea44788188012d5e4fc1ba Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Tue, 25 Feb 2014 12:18:30 +0000 Subject: [PATCH] Revert r19430, r19459: "Reland "Allow ICs to be generated for own global proxy."" Causing ClusterFuzz crash (issue 343928) TBR=verwaest@chromium.org Review URL: https://codereview.chromium.org/179643003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19540 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/stub-cache-arm.cc | 19 +++++++++++++++++++ src/code-stubs-hydrogen.cc | 13 +++++-------- src/code-stubs.h | 26 +++++++------------------- src/ic.cc | 40 +++++++++++++++++++++------------------- src/isolate.cc | 4 ++-- src/mips/stub-cache-mips.cc | 19 +++++++++++++++++++ src/objects-inl.h | 13 +++---------- src/objects.h | 4 ++++ src/runtime.cc | 2 +- 9 files changed, 81 insertions(+), 59 deletions(-) diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 1bb772e..0c7df71 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -1307,6 +1307,21 @@ void StoreStubCompiler::GenerateStoreViaSetter( Handle StoreStubCompiler::CompileStoreInterceptor( Handle object, Handle name) { + Label miss; + + // Check that the map of the object hasn't changed. + __ CheckMap(receiver(), scratch1(), Handle(object->map()), &miss, + DO_SMI_CHECK); + + // Perform global security token check if needed. + if (object->IsJSGlobalProxy()) { + __ CheckAccessGlobalProxy(receiver(), scratch1(), &miss); + } + + // Stub is never generated for non-global objects that require access + // checks. + ASSERT(object->IsJSGlobalProxy() || !object->IsAccessCheckNeeded()); + __ Push(receiver(), this->name(), value()); // Do tail-call to the runtime system. @@ -1314,6 +1329,10 @@ Handle StoreStubCompiler::CompileStoreInterceptor( ExternalReference(IC_Utility(IC::kStoreInterceptorProperty), isolate()); __ TailCallExternalReference(store_ic_property, 3, 1); + // Handle store cache miss. + __ bind(&miss); + TailCallBuiltin(masm(), MissBuiltin(kind())); + // Return the generated code. return GetCode(kind(), Code::FAST, name); } diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index b7247eb..e1d11fb 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -1051,16 +1051,13 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { Handle placeholder_cell = isolate()->factory()->NewPropertyCell(placeholer_value); + HParameter* receiver = GetParameter(0); HParameter* value = GetParameter(2); - if (stub->check_global()) { - // Check that the map of the global has not changed: use a placeholder map - // that will be replaced later with the global object's map. - Handle placeholder_map = isolate()->factory()->meta_map(); - HValue* global = Add( - StoreGlobalStub::global_placeholder(isolate())); - Add(global, placeholder_map, top_info()); - } + // Check that the map of the global has not changed: use a placeholder map + // that will be replaced later with the global object's map. + Handle placeholder_map = isolate()->factory()->meta_map(); + Add(receiver, placeholder_map, top_info()); HValue* cell = Add(placeholder_cell); HObjectAccess access(HObjectAccess::ForCellPayload(isolate())); diff --git a/src/code-stubs.h b/src/code-stubs.h index 79043d5..3ac51e7 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -963,27 +963,19 @@ class LoadFieldStub: public HandlerStub { class StoreGlobalStub : public HandlerStub { public: - explicit StoreGlobalStub(bool is_constant, bool check_global) { - bit_field_ = IsConstantBits::encode(is_constant) | - CheckGlobalBits::encode(check_global); - } - - static Handle global_placeholder(Isolate* isolate) { - return isolate->factory()->uninitialized_value(); + explicit StoreGlobalStub(bool is_constant) { + bit_field_ = IsConstantBits::encode(is_constant); } Handle GetCodeCopyFromTemplate(Isolate* isolate, - GlobalObject* global, + Map* receiver_map, PropertyCell* cell) { Handle code = CodeStub::GetCodeCopyFromTemplate(isolate); - if (check_global()) { - // Replace the placeholder cell and global object map with the actual - // global cell and receiver map. - code->ReplaceNthObject(1, global_placeholder(isolate)->map(), global); - code->ReplaceNthObject(1, isolate->heap()->meta_map(), global->map()); - } + // Replace the placeholder cell and global object map with the actual global + // cell and receiver map. Map* cell_map = isolate->heap()->global_property_cell_map(); code->ReplaceNthObject(1, cell_map, cell); + code->ReplaceNthObject(1, isolate->heap()->meta_map(), receiver_map); return code; } @@ -995,12 +987,9 @@ class StoreGlobalStub : public HandlerStub { Isolate* isolate, CodeStubInterfaceDescriptor* descriptor); - bool is_constant() const { + bool is_constant() { return IsConstantBits::decode(bit_field_); } - bool check_global() const { - return CheckGlobalBits::decode(bit_field_); - } void set_is_constant(bool value) { bit_field_ = IsConstantBits::update(bit_field_, value); } @@ -1017,7 +1006,6 @@ class StoreGlobalStub : public HandlerStub { class IsConstantBits: public BitField {}; class RepresentationBits: public BitField {}; - class CheckGlobalBits: public BitField {}; DISALLOW_COPY_AND_ASSIGN(StoreGlobalStub); }; diff --git a/src/ic.cc b/src/ic.cc index 8ba0274..14fc6a9 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -1072,7 +1072,7 @@ MaybeObject* KeyedLoadIC::Load(Handle object, Handle key) { maybe_object = LoadIC::Load(object, Handle::cast(key)); if (maybe_object->IsFailure()) return maybe_object; } else if (FLAG_use_ic && !object->IsAccessCheckNeeded()) { - ASSERT(!object->IsAccessCheckNeeded()); + ASSERT(!object->IsJSGlobalProxy()); if (object->IsString() && key->IsNumber()) { if (state() == UNINITIALIZED) stub = string_stub(); } else if (object->IsJSObject()) { @@ -1111,17 +1111,21 @@ static bool LookupForWrite(Handle receiver, Handle holder = receiver; receiver->Lookup(*name, lookup); if (lookup->IsFound()) { - if (lookup->IsInterceptor() && !HasInterceptorSetter(lookup->holder())) { - receiver->LocalLookupRealNamedProperty(*name, lookup); - if (!lookup->IsFound()) return false; + if (lookup->IsReadOnly() || !lookup->IsCacheable()) return false; + + if (lookup->holder() == *receiver) { + if (lookup->IsInterceptor() && !HasInterceptorSetter(*receiver)) { + receiver->LocalLookupRealNamedProperty(*name, lookup); + return lookup->IsFound() && + !lookup->IsReadOnly() && + lookup->CanHoldValue(value) && + lookup->IsCacheable(); + } + return lookup->CanHoldValue(value); } - if (lookup->IsReadOnly() || !lookup->IsCacheable()) return false; - if (lookup->holder() == *receiver) return lookup->CanHoldValue(value); if (lookup->IsPropertyCallbacks()) return true; - // JSGlobalProxy either stores on the global object in the prototype, or - // goes into the runtime if access checks are needed, so this is always - // safe. + // JSGlobalProxy always goes via the runtime, so it's safe to cache. if (receiver->IsJSGlobalProxy()) return true; // Currently normal holders in the prototype chain are not supported. They // would require a runtime positive lookup and verification that the details @@ -1307,7 +1311,7 @@ Handle StoreIC::CompileHandler(LookupResult* lookup, Handle name, Handle value, InlineCacheHolderFlag cache_holder) { - if (object->IsAccessCheckNeeded()) return slow_stub(); + if (object->IsJSGlobalProxy()) return slow_stub(); ASSERT(cache_holder == OWN_MAP); // This is currently guaranteed by checks in StoreIC::Store. Handle receiver = Handle::cast(object); @@ -1331,19 +1335,17 @@ Handle StoreIC::CompileHandler(LookupResult* lookup, } case NORMAL: if (kind() == Code::KEYED_STORE_IC) break; - if (receiver->IsJSGlobalProxy() || receiver->IsGlobalObject()) { + if (receiver->IsGlobalObject()) { // The stub generated for the global object picks the value directly // from the property cell. So the property must be directly on the // global object. - Handle global = receiver->IsJSGlobalProxy() - ? handle(GlobalObject::cast(receiver->GetPrototype())) - : Handle::cast(receiver); + Handle global = Handle::cast(receiver); Handle cell(global->GetPropertyCell(lookup), isolate()); Handle union_type = PropertyCell::UpdatedType(cell, value); - StoreGlobalStub stub( - union_type->IsConstant(), receiver->IsJSGlobalProxy()); + StoreGlobalStub stub(union_type->IsConstant()); + Handle code = stub.GetCodeCopyFromTemplate( - isolate(), *global, *cell); + isolate(), receiver->map(), *cell); // TODO(verwaest): Move caching of these NORMAL stubs outside as well. HeapObject::UpdateMapCodeCache(receiver, name, code); return code; @@ -1384,7 +1386,7 @@ Handle StoreIC::CompileHandler(LookupResult* lookup, } case INTERCEPTOR: if (kind() == Code::KEYED_STORE_IC) break; - ASSERT(HasInterceptorSetter(*holder)); + ASSERT(HasInterceptorSetter(*receiver)); return compiler.CompileStoreInterceptor(receiver, name); case CONSTANT: break; @@ -1680,7 +1682,7 @@ MaybeObject* KeyedStoreIC::Store(Handle object, } if (use_ic) { - ASSERT(!object->IsAccessCheckNeeded()); + ASSERT(!object->IsJSGlobalProxy()); if (object->IsJSObject()) { Handle receiver = Handle::cast(object); diff --git a/src/isolate.cc b/src/isolate.cc index d81e50e..a9a5161 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -778,7 +778,7 @@ static MayAccessDecision MayAccessPreCheck(Isolate* isolate, bool Isolate::MayNamedAccess(JSObject* receiver, Object* key, v8::AccessType type) { - ASSERT(receiver->IsJSGlobalProxy() || receiver->IsAccessCheckNeeded()); + ASSERT(receiver->IsAccessCheckNeeded()); // The callers of this method are not expecting a GC. DisallowHeapAllocation no_gc; @@ -829,7 +829,7 @@ bool Isolate::MayNamedAccess(JSObject* receiver, Object* key, bool Isolate::MayIndexedAccess(JSObject* receiver, uint32_t index, v8::AccessType type) { - ASSERT(receiver->IsJSGlobalProxy() || receiver->IsAccessCheckNeeded()); + ASSERT(receiver->IsAccessCheckNeeded()); // Check for compatibility between the security tokens in the // current lexical context and the accessed object. ASSERT(context()); diff --git a/src/mips/stub-cache-mips.cc b/src/mips/stub-cache-mips.cc index b1f2126..9e00692 100644 --- a/src/mips/stub-cache-mips.cc +++ b/src/mips/stub-cache-mips.cc @@ -1293,6 +1293,21 @@ void StoreStubCompiler::GenerateStoreViaSetter( Handle StoreStubCompiler::CompileStoreInterceptor( Handle object, Handle name) { + Label miss; + + // Check that the map of the object hasn't changed. + __ CheckMap(receiver(), scratch1(), Handle(object->map()), &miss, + DO_SMI_CHECK); + + // Perform global security token check if needed. + if (object->IsJSGlobalProxy()) { + __ CheckAccessGlobalProxy(receiver(), scratch1(), &miss); + } + + // Stub is never generated for non-global objects that require access + // checks. + ASSERT(object->IsJSGlobalProxy() || !object->IsAccessCheckNeeded()); + __ Push(receiver(), this->name(), value()); // Do tail-call to the runtime system. @@ -1300,6 +1315,10 @@ Handle StoreStubCompiler::CompileStoreInterceptor( ExternalReference(IC_Utility(IC::kStoreInterceptorProperty), isolate()); __ TailCallExternalReference(store_ic_property, 3, 1); + // Handle store cache miss. + __ bind(&miss); + TailCallBuiltin(masm(), MissBuiltin(kind())); + // Return the generated code. return GetCode(kind(), Code::FAST, name); } diff --git a/src/objects-inl.h b/src/objects-inl.h index 4c49170..2db83f2 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -927,8 +927,7 @@ bool Object::IsJSGlobalProxy() { bool result = IsHeapObject() && (HeapObject::cast(this)->map()->instance_type() == JS_GLOBAL_PROXY_TYPE); - ASSERT(!result || - HeapObject::cast(this)->map()->is_access_check_needed()); + ASSERT(!result || IsAccessCheckNeeded()); return result; } @@ -953,14 +952,8 @@ bool Object::IsUndetectableObject() { bool Object::IsAccessCheckNeeded() { - if (!IsHeapObject()) return false; - if (IsJSGlobalProxy()) { - JSGlobalProxy* proxy = JSGlobalProxy::cast(this); - GlobalObject* global = - proxy->GetIsolate()->context()->global_object(); - return proxy->IsDetachedFrom(global); - } - return HeapObject::cast(this)->map()->is_access_check_needed(); + return IsHeapObject() + && HeapObject::cast(this)->map()->is_access_check_needed(); } diff --git a/src/objects.h b/src/objects.h index fcc6436..1ecacd8 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2361,6 +2361,10 @@ class JSObject: public JSReceiver { // been modified since it was created. May give false positives. bool IsDirty(); + // If the receiver is a JSGlobalProxy this method will return its prototype, + // otherwise the result is the receiver itself. + inline Object* BypassGlobalProxy(); + // Accessors for hidden properties object. // // Hidden properties are not local properties of the object itself. diff --git a/src/runtime.cc b/src/runtime.cc index 55013f1..306042f 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -14735,7 +14735,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_IsAccessAllowedForObserver) { ASSERT(args.length() == 3); CONVERT_ARG_HANDLE_CHECKED(JSFunction, observer, 0); CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 1); - ASSERT(object->map()->is_access_check_needed()); + ASSERT(object->IsAccessCheckNeeded()); Handle key = args.at(2); SaveContext save(isolate); isolate->set_context(observer->context()); -- 2.7.4