From 5f78dc14c823791d976c1eefa952e1acb3169597 Mon Sep 17 00:00:00 2001 From: "kaznacheev@chromium.org" Date: Thu, 5 Aug 2010 08:37:12 +0000 Subject: [PATCH] Avoid GC when compiling CallIC stubs. In rare cases GC could be called from ComputeCallMiss function thus breaking CallIC::LoadFunction. Review URL: http://codereview.chromium.org/3047027 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5174 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/stub-cache-arm.cc | 26 ++++--- src/heap-inl.h | 6 ++ src/heap.cc | 4 ++ src/heap.h | 23 +++++++ src/ia32/stub-cache-ia32.cc | 35 ++++++---- src/ic.cc | 164 ++++++++++++++++++++++++-------------------- src/stub-cache.cc | 7 -- src/stub-cache.h | 8 +-- src/x64/stub-cache-x64.cc | 28 +++++--- 9 files changed, 182 insertions(+), 119 deletions(-) diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index ff3007c..8c8e702 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -1252,9 +1252,11 @@ void CallStubCompiler::GenerateNameCheck(String* name, Label* miss) { } -void CallStubCompiler::GenerateMissBranch() { - Handle ic = ComputeCallMiss(arguments().immediate(), kind_); - __ Jump(ic, RelocInfo::CODE_TARGET); +Object* CallStubCompiler::GenerateMissBranch() { + Object* obj = StubCache::ComputeCallMiss(arguments().immediate(), kind_); + if (obj->IsFailure()) return obj; + __ Jump(Handle(Code::cast(obj)), RelocInfo::CODE_TARGET); + return obj; } @@ -1286,7 +1288,8 @@ Object* CallStubCompiler::CompileCallField(JSObject* object, // Handle call cache miss. __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(FIELD, name); @@ -1337,7 +1340,8 @@ Object* CallStubCompiler::CompileArrayPushCall(Object* object, // Handle call cache miss. __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1388,7 +1392,8 @@ Object* CallStubCompiler::CompileArrayPopCall(Object* object, // Handle call cache miss. __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1561,7 +1566,8 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, } __ bind(&miss_in_smi_check); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1610,7 +1616,8 @@ Object* CallStubCompiler::CompileCallInterceptor(JSObject* object, // Handle call cache miss. __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(INTERCEPTOR, name); @@ -1694,7 +1701,8 @@ Object* CallStubCompiler::CompileCallGlobal(JSObject* object, // Handle call cache miss. __ bind(&miss); __ IncrementCounter(&Counters::call_global_inline_miss, 1, r1, r3); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(NORMAL, name); diff --git a/src/heap-inl.h b/src/heap-inl.h index 5cb24ee..27c6f1c 100644 --- a/src/heap-inl.h +++ b/src/heap-inl.h @@ -435,6 +435,12 @@ inline bool Heap::allow_allocation(bool new_state) { return old; } +inline bool Heap::allow_gc(bool new_state) { + bool old = gc_allowed_; + gc_allowed_ = new_state; + return old; +} + #endif diff --git a/src/heap.cc b/src/heap.cc index 9f27a49..a7416c6 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -134,6 +134,7 @@ Heap::SurvivalRateTrend Heap::survival_rate_trend_ = Heap::STABLE; #ifdef DEBUG bool Heap::allocation_allowed_ = true; +bool Heap::gc_allowed_ = true; int Heap::allocation_timeout_ = 0; bool Heap::disallow_allocation_failure_ = false; @@ -319,6 +320,9 @@ void Heap::ReportStatisticsAfterGC() { void Heap::GarbageCollectionPrologue() { +#ifdef DEBUG + ASSERT(gc_allowed_); +#endif TranscendentalCache::Clear(); ClearJSFunctionResultCaches(); gc_count_++; diff --git a/src/heap.h b/src/heap.h index 2994a6d..b5bfb92 100644 --- a/src/heap.h +++ b/src/heap.h @@ -877,6 +877,8 @@ class Heap : public AllStatic { #ifdef DEBUG static bool IsAllocationAllowed() { return allocation_allowed_; } static inline bool allow_allocation(bool enable); + static bool IsGCAllowed() { return gc_allowed_; } + static inline bool allow_gc(bool enable); static bool disallow_allocation_failure() { return disallow_allocation_failure_; @@ -1078,6 +1080,7 @@ class Heap : public AllStatic { #ifdef DEBUG static bool allocation_allowed_; + static bool gc_allowed_; // If the --gc-interval flag is set to a positive value, this // variable holds the value indicating the number of allocations @@ -1680,6 +1683,20 @@ class AssertNoAllocation { bool old_state_; }; +class AssertNoGC { + public: + AssertNoGC() { + old_state_ = Heap::allow_gc(false); + } + + ~AssertNoGC() { + Heap::allow_gc(old_state_); + } + + private: + bool old_state_; +}; + class DisableAssertNoAllocation { public: DisableAssertNoAllocation() { @@ -1702,6 +1719,12 @@ class AssertNoAllocation { ~AssertNoAllocation() { } }; +class AssertNoGC { + public: + AssertNoGC() { } + ~AssertNoGC() { } +}; + class DisableAssertNoAllocation { public: DisableAssertNoAllocation() { } diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index 48f08a9..c21dd4f 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -1287,9 +1287,11 @@ void CallStubCompiler::GenerateNameCheck(String* name, Label* miss) { } -void CallStubCompiler::GenerateMissBranch() { - Handle ic = ComputeCallMiss(arguments().immediate(), kind_); - __ jmp(ic, RelocInfo::CODE_TARGET); +Object* CallStubCompiler::GenerateMissBranch() { + Object* obj = StubCache::ComputeCallMiss(arguments().immediate(), kind_); + if (obj->IsFailure()) return obj; + __ jmp(Handle(Code::cast(obj)), RelocInfo::CODE_TARGET); + return obj; } @@ -1340,7 +1342,8 @@ Object* CallStubCompiler::CompileCallField(JSObject* object, // Handle call cache miss. __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(FIELD, name); @@ -1487,7 +1490,8 @@ Object* CallStubCompiler::CompileArrayPushCall(Object* object, } __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1570,7 +1574,8 @@ Object* CallStubCompiler::CompileArrayPopCall(Object* object, 1); __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1633,8 +1638,8 @@ Object* CallStubCompiler::CompileStringCharCodeAtCall(Object* object, __ ret((argc + 1) * kPointerSize); __ bind(&miss); - - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1700,9 +1705,8 @@ Object* CallStubCompiler::CompileStringCharAtCall(Object* object, __ ret((argc + 1) * kPointerSize); __ bind(&miss); - // Restore function name in ecx. - - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1856,7 +1860,8 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, FreeSpaceForFastApiCall(masm(), eax); } __ bind(&miss_in_smi_check); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1920,7 +1925,8 @@ Object* CallStubCompiler::CompileCallInterceptor(JSObject* object, // Handle load cache miss. __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(INTERCEPTOR, name); @@ -2005,7 +2011,8 @@ Object* CallStubCompiler::CompileCallGlobal(JSObject* object, // Handle call cache miss. __ bind(&miss); __ IncrementCounter(&Counters::call_global_inline_miss, 1); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(NORMAL, name); diff --git a/src/ic.cc b/src/ic.cc index a5370a6..aace456 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -501,20 +501,24 @@ Object* CallICBase::LoadFunction(State state, // Lookup the property in the object. LookupResult lookup; - LookupForRead(*object, *name, &lookup); + { + AssertNoGC nogc; // GC could invalidate the pointers held in lookup. - if (!lookup.IsProperty()) { - // If the object does not have the requested property, check which - // exception we need to throw. - if (IsContextual(object)) { - return ReferenceError("not_defined", name); + LookupForRead(*object, *name, &lookup); + + if (!lookup.IsProperty()) { + // If the object does not have the requested property, check which + // exception we need to throw. + if (IsContextual(object)) { + return ReferenceError("not_defined", name); + } + return TypeError("undefined_method", object, name); } - return TypeError("undefined_method", object, name); - } - // Lookup is valid: Update inline cache and stub cache. - if (FLAG_use_ic) { - UpdateCaches(&lookup, state, object, name); + // Lookup is valid: Update inline cache and stub cache. + if (FLAG_use_ic) { + UpdateCaches(&lookup, state, object, name); + } } // Get the property. @@ -787,67 +791,71 @@ Object* LoadIC::Load(State state, Handle object, Handle name) { // Named lookup in the object. LookupResult lookup; - LookupForRead(*object, *name, &lookup); + { + AssertNoGC nogc; // GC could invalidate the pointers held in lookup. - // If we did not find a property, check if we need to throw an exception. - if (!lookup.IsProperty()) { - if (FLAG_strict || IsContextual(object)) { - return ReferenceError("not_defined", name); + LookupForRead(*object, *name, &lookup); + + // If we did not find a property, check if we need to throw an exception. + if (!lookup.IsProperty()) { + if (FLAG_strict || IsContextual(object)) { + return ReferenceError("not_defined", name); + } + LOG(SuspectReadEvent(*name, *object)); } - LOG(SuspectReadEvent(*name, *object)); - } - bool can_be_inlined = - FLAG_use_ic && - state == PREMONOMORPHIC && - lookup.IsProperty() && - lookup.IsCacheable() && - lookup.holder() == *object && - lookup.type() == FIELD && - !object->IsAccessCheckNeeded(); - - if (can_be_inlined) { - Map* map = lookup.holder()->map(); - // Property's index in the properties array. If negative we have - // an inobject property. - int index = lookup.GetFieldIndex() - map->inobject_properties(); - if (index < 0) { - // Index is an offset from the end of the object. - int offset = map->instance_size() + (index * kPointerSize); - if (PatchInlinedLoad(address(), map, offset)) { - set_target(megamorphic_stub()); -#ifdef DEBUG - if (FLAG_trace_ic) { - PrintF("[LoadIC : inline patch %s]\n", *name->ToCString()); + bool can_be_inlined = + FLAG_use_ic && + state == PREMONOMORPHIC && + lookup.IsProperty() && + lookup.IsCacheable() && + lookup.holder() == *object && + lookup.type() == FIELD && + !object->IsAccessCheckNeeded(); + + if (can_be_inlined) { + Map* map = lookup.holder()->map(); + // Property's index in the properties array. If negative we have + // an inobject property. + int index = lookup.GetFieldIndex() - map->inobject_properties(); + if (index < 0) { + // Index is an offset from the end of the object. + int offset = map->instance_size() + (index * kPointerSize); + if (PatchInlinedLoad(address(), map, offset)) { + set_target(megamorphic_stub()); + #ifdef DEBUG + if (FLAG_trace_ic) { + PrintF("[LoadIC : inline patch %s]\n", *name->ToCString()); + } + #endif + return lookup.holder()->FastPropertyAt(lookup.GetFieldIndex()); + #ifdef DEBUG + } else { + if (FLAG_trace_ic) { + PrintF("[LoadIC : no inline patch %s (patching failed)]\n", + *name->ToCString()); + } } -#endif - return lookup.holder()->FastPropertyAt(lookup.GetFieldIndex()); -#ifdef DEBUG } else { if (FLAG_trace_ic) { - PrintF("[LoadIC : no inline patch %s (patching failed)]\n", + PrintF("[LoadIC : no inline patch %s (not inobject)]\n", *name->ToCString()); } } } else { - if (FLAG_trace_ic) { - PrintF("[LoadIC : no inline patch %s (not inobject)]\n", - *name->ToCString()); - } - } - } else { - if (FLAG_use_ic && state == PREMONOMORPHIC) { - if (FLAG_trace_ic) { - PrintF("[LoadIC : no inline patch %s (not inlinable)]\n", - *name->ToCString()); -#endif + if (FLAG_use_ic && state == PREMONOMORPHIC) { + if (FLAG_trace_ic) { + PrintF("[LoadIC : no inline patch %s (not inlinable)]\n", + *name->ToCString()); + #endif + } } } - } - // Update inline cache and stub cache. - if (FLAG_use_ic) { - UpdateCaches(&lookup, state, object, name); + // Update inline cache and stub cache. + if (FLAG_use_ic) { + UpdateCaches(&lookup, state, object, name); + } } PropertyAttributes attr; @@ -1037,17 +1045,21 @@ Object* KeyedLoadIC::Load(State state, // Named lookup. LookupResult lookup; - LookupForRead(*object, *name, &lookup); + { + AssertNoGC nogc; // GC could invalidate the pointers held in lookup. - // If we did not find a property, check if we need to throw an exception. - if (!lookup.IsProperty()) { - if (FLAG_strict || IsContextual(object)) { - return ReferenceError("not_defined", name); + LookupForRead(*object, *name, &lookup); + + // If we did not find a property, check if we need to throw an exception. + if (!lookup.IsProperty()) { + if (FLAG_strict || IsContextual(object)) { + return ReferenceError("not_defined", name); + } } - } - if (FLAG_use_ic) { - UpdateCaches(&lookup, state, object, name); + if (FLAG_use_ic) { + UpdateCaches(&lookup, state, object, name); + } } PropertyAttributes attr; @@ -1245,6 +1257,8 @@ Object* StoreIC::Store(State state, // Lookup the property locally in the receiver. if (FLAG_use_ic && !receiver->IsJSGlobalProxy()) { + AssertNoGC nogc; // GC could invalidate the pointers held in lookup. + LookupResult lookup; if (LookupForWrite(*receiver, *name, &lookup)) { @@ -1418,13 +1432,17 @@ Object* KeyedStoreIC::Store(State state, return *value; } - // Lookup the property locally in the receiver. - LookupResult lookup; - receiver->LocalLookup(*name, &lookup); + { + AssertNoGC nogc; // GC could invalidate the pointers held in lookup. - // Update inline cache and stub cache. - if (FLAG_use_ic) { - UpdateCaches(&lookup, state, receiver, name, value); + // Lookup the property locally in the receiver. + LookupResult lookup; + receiver->LocalLookup(*name, &lookup); + + // Update inline cache and stub cache. + if (FLAG_use_ic) { + UpdateCaches(&lookup, state, receiver, name, value); + } } // Set the property. diff --git a/src/stub-cache.cc b/src/stub-cache.cc index bc29d06..6a0c93e 100644 --- a/src/stub-cache.cc +++ b/src/stub-cache.cc @@ -822,13 +822,6 @@ void StubCache::Clear() { // StubCompiler implementation. -// Support function for computing call IC miss stubs. -Handle ComputeCallMiss(int argc, Code::Kind kind) { - CALL_HEAP_FUNCTION(StubCache::ComputeCallMiss(argc, kind), Code); -} - - - Object* LoadCallbackProperty(Arguments args) { ASSERT(args[0]->IsJSObject()); ASSERT(args[1]->IsJSObject()); diff --git a/src/stub-cache.h b/src/stub-cache.h index 8c00ee8..0be32f1 100644 --- a/src/stub-cache.h +++ b/src/stub-cache.h @@ -336,10 +336,6 @@ Object* CallInterceptorProperty(Arguments args); Object* KeyedLoadPropertyWithInterceptor(Arguments args); -// Support function for computing call IC miss stubs. -Handle ComputeCallMiss(int argc, Code::Kind kind); - - // The stub compiler compiles stubs for the stub cache. class StubCompiler BASE_EMBEDDED { public: @@ -688,7 +684,9 @@ class CallStubCompiler: public StubCompiler { void GenerateNameCheck(String* name, Label* miss); - void GenerateMissBranch(); + // Generates a jump to CallIC miss stub. Returns Failure if the jump cannot + // be generated. + Object* GenerateMissBranch(); }; diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index e67000a..4c15715 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -820,9 +820,11 @@ void CallStubCompiler::GenerateNameCheck(String* name, Label* miss) { } -void CallStubCompiler::GenerateMissBranch() { - Handle ic = ComputeCallMiss(arguments().immediate(), kind_); - __ Jump(ic, RelocInfo::CODE_TARGET); +Object* CallStubCompiler::GenerateMissBranch() { + Object* obj = StubCache::ComputeCallMiss(arguments().immediate(), kind_); + if (obj->IsFailure()) return obj; + __ Jump(Handle(Code::cast(obj)), RelocInfo::CODE_TARGET); + return obj; } @@ -975,7 +977,8 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, // Handle call cache miss. __ bind(&miss_in_smi_check); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1029,7 +1032,8 @@ Object* CallStubCompiler::CompileCallField(JSObject* object, // Handle call cache miss. __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(FIELD, name); @@ -1186,8 +1190,8 @@ Object* CallStubCompiler::CompileArrayPushCall(Object* object, } __ bind(&miss); - - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1270,8 +1274,8 @@ Object* CallStubCompiler::CompileArrayPopCall(Object* object, argc + 1, 1); __ bind(&miss); - - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(function); @@ -1357,7 +1361,8 @@ Object* CallStubCompiler::CompileCallInterceptor(JSObject* object, // Handle load cache miss. __ bind(&miss); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(INTERCEPTOR, name); @@ -1442,7 +1447,8 @@ Object* CallStubCompiler::CompileCallGlobal(JSObject* object, // Handle call cache miss. __ bind(&miss); __ IncrementCounter(&Counters::call_global_inline_miss, 1); - GenerateMissBranch(); + Object* obj = GenerateMissBranch(); + if (obj->IsFailure()) return obj; // Return the generated code. return GetCode(NORMAL, name); -- 2.7.4