Revision: 3440 Author: [email protected] Date: Wed Dec 9 06:54:34 2009 Log: Fix a crash caused by garbage collection during generation of a callback load (or keyed load) IC.
The problem was that the IC code calls a stub, which can allocate and thus trigger a GC if the stub is not already generated. Problem is solved by adding the ability to "try" to call a stub, trying to generate the stub code if necessary but signaling an allocation failure if generating the code is not possible. Review URL: http://codereview.chromium.org/472002 http://code.google.com/p/v8/source/detail?r=3440 Modified: /branches/bleeding_edge/src/code-stubs.cc /branches/bleeding_edge/src/code-stubs.h /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc /branches/bleeding_edge/src/ia32/macro-assembler-ia32.h /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc /branches/bleeding_edge/src/stub-cache.cc /branches/bleeding_edge/src/stub-cache.h ======================================= --- /branches/bleeding_edge/src/code-stubs.cc Wed Nov 11 06:32:14 2009 +++ /branches/bleeding_edge/src/code-stubs.cc Wed Dec 9 06:54:34 2009 @@ -35,82 +35,117 @@ namespace v8 { namespace internal { -Handle<Code> CodeStub::GetCode() { - bool custom_cache = has_custom_cache(); - - int index = 0; - uint32_t key = 0; - if (custom_cache) { - Code* cached; - if (GetCustomCache(&cached)) { - return Handle<Code>(cached); - } else { - index = NumberDictionary::kNotFound; - } - } else { - key = GetKey(); - index = Heap::code_stubs()->FindEntry(key); - if (index != NumberDictionary::kNotFound) - return Handle<Code>(Code::cast(Heap::code_stubs()->ValueAt(index))); - } - - Code* result; - { +bool CodeStub::FindCodeInCache(Code** code_out) { + if (has_custom_cache()) return GetCustomCache(code_out); + int index = Heap::code_stubs()->FindEntry(GetKey()); + if (index != NumberDictionary::kNotFound) { + *code_out = Code::cast(Heap::code_stubs()->ValueAt(index)); + return true; + } + return false; +} + + +void CodeStub::GenerateCode(MacroAssembler* masm) { + // Update the static counter each time a new code stub is generated. + Counters::code_stubs.Increment(); + // Nested stubs are not allowed for leafs. + masm->set_allow_stub_calls(AllowsStubCalls()); + // Generate the code for the stub. + masm->set_generating_stub(true); + Generate(masm); +} + + +void CodeStub::RecordCodeGeneration(Code* code, MacroAssembler* masm) { + code->set_major_key(MajorKey()); + + // Add unresolved entries in the code to the fixup list. + Bootstrapper::AddFixup(code, masm); + + LOG(CodeCreateEvent(Logger::STUB_TAG, code, GetName())); + Counters::total_stubs_code_size.Increment(code->instruction_size()); + +#ifdef ENABLE_DISASSEMBLER + if (FLAG_print_code_stubs) { +#ifdef DEBUG + Print(); +#endif + code->Disassemble(GetName()); + PrintF("\n"); + } +#endif +} + + +Handle<Code> CodeStub::GetCode() { + Code* code; + if (!FindCodeInCache(&code)) { v8::HandleScope scope; - // Update the static counter each time a new code stub is generated. - Counters::code_stubs.Increment(); - // Generate the new code. MacroAssembler masm(NULL, 256); - - // Nested stubs are not allowed for leafs. - masm.set_allow_stub_calls(AllowsStubCalls()); - - // Generate the code for the stub. - masm.set_generating_stub(true); - Generate(&masm); + GenerateCode(&masm); // Create the code object. CodeDesc desc; masm.GetCode(&desc); - // Copy the generated code into a heap object, and store the major key. + // Copy the generated code into a heap object. Code::Flags flags = Code::ComputeFlags(Code::STUB, InLoop()); - Handle<Code> code = Factory::NewCode(desc, NULL, flags, masm.CodeObject()); - code->set_major_key(MajorKey()); - - // Add unresolved entries in the code to the fixup list. - Bootstrapper::AddFixup(*code, &masm); - - LOG(CodeCreateEvent(Logger::STUB_TAG, *code, GetName())); - Counters::total_stubs_code_size.Increment(code->instruction_size()); - -#ifdef ENABLE_DISASSEMBLER - if (FLAG_print_code_stubs) { -#ifdef DEBUG - Print(); -#endif - code->Disassemble(GetName()); - PrintF("\n"); - } -#endif - - if (custom_cache) { - SetCustomCache(*code); + Handle<Code> new_object = + Factory::NewCode(desc, NULL, flags, masm.CodeObject()); + RecordCodeGeneration(*new_object, &masm); + + if (has_custom_cache()) { + SetCustomCache(*new_object); } else { // Update the dictionary and the root in Heap. Handle<NumberDictionary> dict = Factory::DictionaryAtNumberPut( Handle<NumberDictionary>(Heap::code_stubs()), - key, - code); + GetKey(), + new_object); Heap::public_set_code_stubs(*dict); } - result = *code; + code = *new_object; } - return Handle<Code>(result); + return Handle<Code>(code); +} + + +Object* CodeStub::TryGetCode() { + Code* code; + if (!FindCodeInCache(&code)) { + // Generate the new code. + MacroAssembler masm(NULL, 256); + GenerateCode(&masm); + + // Create the code object. + CodeDesc desc; + masm.GetCode(&desc); + + // Try to copy the generated code into a heap object. + Code::Flags flags = Code::ComputeFlags(Code::STUB, InLoop()); + Object* new_object = + Heap::CreateCode(desc, NULL, flags, masm.CodeObject()); + if (new_object->IsFailure()) return new_object; + code = Code::cast(new_object); + RecordCodeGeneration(code, &masm); + + if (has_custom_cache()) { + SetCustomCache(code); + } else { + // Try to update the code cache but do not fail if unable. + new_object = Heap::code_stubs()->AtNumberPut(GetKey(), code); + if (!new_object->IsFailure()) { + Heap::public_set_code_stubs(NumberDictionary::cast(new_object)); + } + } + } + + return code; } ======================================= --- /branches/bleeding_edge/src/code-stubs.h Wed Dec 2 23:56:21 2009 +++ /branches/bleeding_edge/src/code-stubs.h Wed Dec 9 06:54:34 2009 @@ -83,6 +83,11 @@ // Retrieve the code for the stub. Generate the code if needed. Handle<Code> GetCode(); + // Retrieve the code for the stub if already generated. Do not + // generate the code if not already generated and instead return a + // retry after GC Failure object. + Object* TryGetCode(); + static Major MajorKeyFromKey(uint32_t key) { return static_cast<Major>(MajorKeyBits::decode(key)); }; @@ -104,9 +109,20 @@ static const int kMinorBits = kBitsPerInt - kSmiTagSize - kMajorBits; private: + // Lookup the code in the (possibly custom) cache. + bool FindCodeInCache(Code** code_out); + + // Nonvirtual wrapper around the stub-specific Generate function. Call + // this function to set up the macro assembler and generate the code. + void GenerateCode(MacroAssembler* masm); + // Generates the assembler code for the stub. virtual void Generate(MacroAssembler* masm) = 0; + // Perform bookkeeping required after code generation when stub code is + // initially generated. + void RecordCodeGeneration(Code* code, MacroAssembler* masm); + // Returns information for computing the number key. virtual Major MajorKey() = 0; virtual int MinorKey() = 0; ======================================= --- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Wed Dec 9 01:35:41 2009 +++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Wed Dec 9 06:54:34 2009 @@ -1015,15 +1015,35 @@ void MacroAssembler::CallStub(CodeStub* stub) { - ASSERT(allow_stub_calls()); // calls are not allowed in some stubs + ASSERT(allow_stub_calls()); // Calls are not allowed in some stubs. call(stub->GetCode(), RelocInfo::CODE_TARGET); } + + +Object* MacroAssembler::TryCallStub(CodeStub* stub) { + ASSERT(allow_stub_calls()); // Calls are not allowed in some stubs. + Object* result = stub->TryGetCode(); + if (!result->IsFailure()) { + call(Handle<Code>(Code::cast(result)), RelocInfo::CODE_TARGET); + } + return result; +} void MacroAssembler::TailCallStub(CodeStub* stub) { - ASSERT(allow_stub_calls()); // calls are not allowed in some stubs + ASSERT(allow_stub_calls()); // Calls are not allowed in some stubs. jmp(stub->GetCode(), RelocInfo::CODE_TARGET); } + + +Object* MacroAssembler::TryTailCallStub(CodeStub* stub) { + ASSERT(allow_stub_calls()); // Calls are not allowed in some stubs. + Object* result = stub->TryGetCode(); + if (!result->IsFailure()) { + jmp(Handle<Code>(Code::cast(result)), RelocInfo::CODE_TARGET); + } + return result; +} void MacroAssembler::StubReturn(int argc) { ======================================= --- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.h Wed Dec 2 23:56:21 2009 +++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.h Wed Dec 9 06:54:34 2009 @@ -285,12 +285,22 @@ // --------------------------------------------------------------------------- // Runtime calls - // Call a code stub. + // Call a code stub. Generate the code if necessary. void CallStub(CodeStub* stub); - // Tail call a code stub (jump). + // Call a code stub and return the code object called. Try to generate + // the code if necessary. Do not perform a GC but instead return a retry + // after GC failure. + Object* TryCallStub(CodeStub* stub); + + // Tail call a code stub (jump). Generate the code if necessary. void TailCallStub(CodeStub* stub); + // Tail call a code stub (jump) and return the code object called. Try to + // generate the code if necessary. Do not perform a GC but instead return + // a retry after GC failure. + Object* TryTailCallStub(CodeStub* stub); + // Return from a code stub after popping its arguments. void StubReturn(int argc); ======================================= --- /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Tue Nov 24 06:10:06 2009 +++ /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Wed Dec 9 06:54:34 2009 @@ -754,7 +754,7 @@ } -void StubCompiler::GenerateLoadCallback(JSObject* object, +bool StubCompiler::GenerateLoadCallback(JSObject* object, JSObject* holder, Register receiver, Register name_reg, @@ -762,7 +762,8 @@ Register scratch2, AccessorInfo* callback, String* name, - Label* miss) { + Label* miss, + Failure** failure) { // Check that the receiver isn't a smi. __ test(receiver, Immediate(kSmiTagMask)); __ j(zero, miss, not_taken); @@ -798,7 +799,14 @@ Address getter_address = v8::ToCData<Address>(callback->getter()); ApiFunction fun(getter_address); ApiGetterEntryStub stub(callback_handle, &fun); - __ CallStub(&stub); + // Calling the stub may try to allocate (if the code is not already + // generated). Do not allow the call to perform a garbage + // collection but instead return the allocation failure object. + Object* result = masm()->TryCallStub(&stub); + if (result->IsFailure()) { + *failure = Failure::cast(result); + return false; + } // We need to avoid using eax since that now holds the result. Register tmp = other.is(eax) ? reg : other; @@ -806,6 +814,7 @@ __ LeaveInternalFrame(); __ ret(0); + return true; } @@ -1420,10 +1429,10 @@ } -Object* LoadStubCompiler::CompileLoadCallback(JSObject* object, +Object* LoadStubCompiler::CompileLoadCallback(String* name, + JSObject* object, JSObject* holder, - AccessorInfo* callback, - String* name) { + AccessorInfo* callback) { // ----------- S t a t e ------------- // -- ecx : name // -- esp[0] : return address @@ -1432,8 +1441,11 @@ Label miss; __ mov(eax, Operand(esp, kPointerSize)); - GenerateLoadCallback(object, holder, eax, ecx, ebx, edx, - callback, name, &miss); + Failure* failure = Failure::InternalError(); + bool success = GenerateLoadCallback(object, holder, eax, ecx, ebx, edx, + callback, name, &miss, &failure); + if (!success) return failure; + __ bind(&miss); GenerateLoadMiss(masm(), Code::LOAD_IC); @@ -1597,8 +1609,11 @@ __ cmp(Operand(eax), Immediate(Handle<String>(name))); __ j(not_equal, &miss, not_taken); - GenerateLoadCallback(receiver, holder, ecx, eax, ebx, edx, - callback, name, &miss); + Failure* failure = Failure::InternalError(); + bool success = GenerateLoadCallback(receiver, holder, ecx, eax, ebx, edx, + callback, name, &miss, &failure); + if (!success) return failure; + __ bind(&miss); __ DecrementCounter(&Counters::keyed_load_callback, 1); GenerateLoadMiss(masm(), Code::KEYED_LOAD_IC); ======================================= --- /branches/bleeding_edge/src/stub-cache.cc Wed Nov 25 08:39:18 2009 +++ /branches/bleeding_edge/src/stub-cache.cc Wed Dec 9 06:54:34 2009 @@ -120,7 +120,7 @@ Object* code = receiver->map()->FindInCodeCache(name, flags); if (code->IsUndefined()) { LoadStubCompiler compiler; - code = compiler.CompileLoadCallback(receiver, holder, callback, name); + code = compiler.CompileLoadCallback(name, receiver, holder, callback); if (code->IsFailure()) return code; LOG(CodeCreateEvent(Logger::LOAD_IC_TAG, Code::cast(code), name)); Object* result = receiver->map()->UpdateCodeCache(name, Code::cast(code)); ======================================= --- /branches/bleeding_edge/src/stub-cache.h Tue Nov 24 06:10:06 2009 +++ /branches/bleeding_edge/src/stub-cache.h Wed Dec 9 06:54:34 2009 @@ -405,7 +405,7 @@ String* name, Label* miss); - void GenerateLoadCallback(JSObject* object, + bool GenerateLoadCallback(JSObject* object, JSObject* holder, Register receiver, Register name_reg, @@ -413,7 +413,8 @@ Register scratch2, AccessorInfo* callback, String* name, - Label* miss); + Label* miss, + Failure** failure); void GenerateLoadConstant(JSObject* object, JSObject* holder, @@ -447,10 +448,10 @@ JSObject* holder, int index, String* name); - Object* CompileLoadCallback(JSObject* object, + Object* CompileLoadCallback(String* name, + JSObject* object, JSObject* holder, - AccessorInfo* callback, - String* name); + AccessorInfo* callback); Object* CompileLoadConstant(JSObject* object, JSObject* holder, Object* value, -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
