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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3440 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/code-stubs.cc

@@ -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)));
+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;
+}
 
-  Code* result;
-  {
-    v8::HandleScope scope;
 
-    // Update the static counter each time a new code stub is generated.
-    Counters::code_stubs.Increment();
+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);
+}
 
-    // Generate the new code.
-    MacroAssembler masm(NULL, 256);
 
-    // Nested stubs are not allowed for leafs.
-    masm.set_allow_stub_calls(AllowsStubCalls());
+void CodeStub::RecordCodeGeneration(Code* code, MacroAssembler* masm) {
+  code->set_major_key(MajorKey());
 
-    // Generate the code for the stub.
-    masm.set_generating_stub(true);
-    Generate(&masm);
+  // Add unresolved entries in the code to the fixup list.
+  Bootstrapper::AddFixup(code, masm);
 
-    // Create the code object.
-    CodeDesc desc;
-    masm.GetCode(&desc);
-
-    // Copy the generated code into a heap object, and store the major key.
-    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());
+  LOG(CodeCreateEvent(Logger::STUB_TAG, code, GetName()));
+  Counters::total_stubs_code_size.Increment(code->instruction_size());
 
 #ifdef ENABLE_DISASSEMBLER
-    if (FLAG_print_code_stubs) {
+  if (FLAG_print_code_stubs) {
 #ifdef DEBUG
-      Print();
+    Print();
 #endif
-      code->Disassemble(GetName());
-      PrintF("\n");
-    }
+    code->Disassemble(GetName());
+    PrintF("\n");
+  }
 #endif
+}
+
+
+Handle<Code> CodeStub::GetCode() {
+  Code* code;
+  if (!FindCodeInCache(&code)) {
+    v8::HandleScope scope;
+
+    // Generate the new code.
+    MacroAssembler masm(NULL, 256);
+    GenerateCode(&masm);
+
+    // Create the code object.
+    CodeDesc desc;
+    masm.GetCode(&desc);
 
-    if (custom_cache) {
-      SetCustomCache(*code);
+    // Copy the generated code into a heap object.
+    Code::Flags flags = Code::ComputeFlags(Code::STUB, InLoop());
+    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>(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 Handle<Code>(result);
+  return code;
 }
 
 

src/code-stubs.h

@@ -83,6 +83,11 @@ class CodeStub BASE_EMBEDDED {
   // 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 @@ class CodeStub BASE_EMBEDDED {
   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;

src/ia32/macro-assembler-ia32.cc

@@ -1015,17 +1015,37 @@ void MacroAssembler::TryGetFunctionPrototype(Register function,
 
 
 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) {
   ASSERT(argc >= 1 && generating_stub());
   ret((argc - 1) * kPointerSize);

src/ia32/macro-assembler-ia32.h

@@ -285,12 +285,22 @@ class MacroAssembler: public Assembler {
   // ---------------------------------------------------------------------------
   // 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);
 

src/ia32/stub-cache-ia32.cc

@@ -754,7 +754,7 @@ void StubCompiler::GenerateLoadField(JSObject* object,
 }
 
 
-void StubCompiler::GenerateLoadCallback(JSObject* object,
+bool StubCompiler::GenerateLoadCallback(JSObject* object,
                                         JSObject* holder,
                                         Register receiver,
                                         Register name_reg,
@@ -762,7 +762,8 @@ void StubCompiler::GenerateLoadCallback(JSObject* object,
                                         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 @@ void StubCompiler::GenerateLoadCallback(JSObject* object,
   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 @@ void StubCompiler::GenerateLoadCallback(JSObject* object,
   __ LeaveInternalFrame();
 
   __ ret(0);
+  return true;
 }
 
 
@@ -1420,10 +1429,10 @@ Object* LoadStubCompiler::CompileLoadField(JSObject* object,
 }
 
 
-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 @@ Object* LoadStubCompiler::CompileLoadCallback(JSObject* object,
   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 @@ Object* KeyedLoadStubCompiler::CompileLoadCallback(String* name,
   __ 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);

src/stub-cache.cc

@@ -120,7 +120,7 @@ Object* StubCache::ComputeLoadCallback(String* name,
   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));

src/stub-cache.h

@@ -405,7 +405,7 @@ class StubCompiler BASE_EMBEDDED {
                          String* name,
                          Label* miss);
 
-  void GenerateLoadCallback(JSObject* object,
+  bool GenerateLoadCallback(JSObject* object,
                             JSObject* holder,
                             Register receiver,
                             Register name_reg,
@@ -413,7 +413,8 @@ class StubCompiler BASE_EMBEDDED {
                             Register scratch2,
                             AccessorInfo* callback,
                             String* name,
-                            Label* miss);
+                            Label* miss,
+                            Failure** failure);
 
   void GenerateLoadConstant(JSObject* object,
                             JSObject* holder,
@@ -447,10 +448,10 @@ class LoadStubCompiler: public StubCompiler {
                            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,