Fixed deoptimization of inlined getters.
authorsvenpanne@chromium.org <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 7 Sep 2012 09:01:54 +0000 (09:01 +0000)
committersvenpanne@chromium.org <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 7 Sep 2012 09:01:54 +0000 (09:01 +0000)
It is necessary to explicitly handle the internal frame lying between the caller
of the getter and the getter itself in the deoptimizer: When the getter is
inlined, leaving the internal frame restores the correct context.

BUG=http://crbug/134609
TEST=mjsunit/regress/regress-crbug-134609

Review URL: https://chromiumcodereview.appspot.com/10910110

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

25 files changed:
include/v8.h
src/arm/deoptimizer-arm.cc
src/arm/lithium-codegen-arm.cc
src/arm/stub-cache-arm.cc
src/builtins.cc
src/builtins.h
src/deoptimizer.cc
src/deoptimizer.h
src/flag-definitions.h
src/heap.h
src/hydrogen-instructions.h
src/hydrogen.cc
src/hydrogen.h
src/ia32/deoptimizer-ia32.cc
src/ia32/lithium-codegen-ia32.cc
src/ia32/stub-cache-ia32.cc
src/mips/deoptimizer-mips.cc
src/mips/lithium-codegen-mips.cc
src/mips/stub-cache-mips.cc
src/objects.cc
src/stub-cache.h
src/x64/deoptimizer-x64.cc
src/x64/lithium-codegen-x64.cc
src/x64/stub-cache-x64.cc
test/mjsunit/regress/regress-crbug-134609.js [new file with mode: 0644]

index 3e1b427..ddde388 100644 (file)
@@ -4042,7 +4042,7 @@ class Internals {
   static const int kNullValueRootIndex = 7;
   static const int kTrueValueRootIndex = 8;
   static const int kFalseValueRootIndex = 9;
-  static const int kEmptySymbolRootIndex = 115;
+  static const int kEmptySymbolRootIndex = 116;
 
   static const int kJSObjectType = 0xaa;
   static const int kFirstNonstringType = 0x80;
index bf55be2..5339be1 100644 (file)
@@ -595,30 +595,34 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslationIterator* iterator,
 }
 
 
-void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
-                                           int frame_index) {
-  JSFunction* setter = JSFunction::cast(ComputeLiteral(iterator->Next()));
-  // The receiver and the implicit return value are expected in registers by the
-  // StoreIC, so they don't belong to the output stack frame. This means that we
-  // have to use a height of 0.
+void Deoptimizer::DoComputeAccessorStubFrame(TranslationIterator* iterator,
+                                             int frame_index,
+                                             bool is_setter_stub_frame) {
+  JSFunction* accessor = JSFunction::cast(ComputeLiteral(iterator->Next()));
+  // The receiver (and the implicit return value, if any) are expected in
+  // registers by the LoadIC/StoreIC, so they don't belong to the output stack
+  // frame. This means that we have to use a height of 0.
   unsigned height = 0;
   unsigned height_in_bytes = height * kPointerSize;
+  const char* kind = is_setter_stub_frame ? "setter" : "getter";
   if (FLAG_trace_deopt) {
-    PrintF("  translating setter stub => height=%u\n", height_in_bytes);
+    PrintF("  translating %s stub => height=%u\n", kind, height_in_bytes);
   }
 
   // We need 5 stack entries from StackFrame::INTERNAL (lr, fp, cp, frame type,
-  // code object, see MacroAssembler::EnterFrame) + 1 stack entry from setter
-  // stub (implicit return value, see StoreStubCompiler::CompileStoreViaSetter).
-  unsigned fixed_frame_size = (5 + 1) * kPointerSize;
+  // code object, see MacroAssembler::EnterFrame). For a setter stub frames we
+  // need one additional entry for the implicit return value, see
+  // StoreStubCompiler::CompileStoreViaSetter.
+  unsigned fixed_frame_entries = 5 + (is_setter_stub_frame ? 1 : 0);
+  unsigned fixed_frame_size = fixed_frame_entries * kPointerSize;
   unsigned output_frame_size = height_in_bytes + fixed_frame_size;
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, setter);
+      new(output_frame_size) FrameDescription(output_frame_size, accessor);
   output_frame->SetFrameType(StackFrame::INTERNAL);
 
-  // A frame for a setter stub can not be the topmost or bottommost one.
+  // A frame for an accessor stub can not be the topmost or bottommost one.
   ASSERT(frame_index > 0 && frame_index < output_count_ - 1);
   ASSERT(output_[frame_index] == NULL);
   output_[frame_index] = output_frame;
@@ -668,15 +672,17 @@ void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
   output_frame->SetFrameSlot(output_offset, value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08" V8PRIxPTR ": [top + %u] <- 0x%08" V8PRIxPTR
-           " ; function (setter sentinel)\n",
-           top_address + output_offset, output_offset, value);
+           " ; function (%s sentinel)\n",
+           top_address + output_offset, output_offset, value, kind);
   }
 
-  // Get Code object from setter stub.
+  // Get Code object from accessor stub.
   output_offset -= kPointerSize;
-  Code* setter_stub =
-      isolate_->builtins()->builtin(Builtins::kStoreIC_Setter_ForDeopt);
-  value = reinterpret_cast<intptr_t>(setter_stub);
+  Builtins::Name name = is_setter_stub_frame ?
+      Builtins::kStoreIC_Setter_ForDeopt :
+      Builtins::kLoadIC_Getter_ForDeopt;
+  Code* accessor_stub = isolate_->builtins()->builtin(name);
+  value = reinterpret_cast<intptr_t>(accessor_stub);
   output_frame->SetFrameSlot(output_offset, value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08" V8PRIxPTR ": [top + %u] <- 0x%08" V8PRIxPTR
@@ -689,16 +695,20 @@ void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
       static_cast<Translation::Opcode>(iterator->Next());
   iterator->Skip(Translation::NumberOfOperandsFor(opcode));
 
-  // The implicit return value was part of the artificial setter stub
-  // environment.
-  output_offset -= kPointerSize;
-  DoTranslateCommand(iterator, frame_index, output_offset);
+  if (is_setter_stub_frame) {
+    // The implicit return value was part of the artificial setter stub
+    // environment.
+    output_offset -= kPointerSize;
+    DoTranslateCommand(iterator, frame_index, output_offset);
+  }
 
   ASSERT(0 == output_offset);
 
+  Smi* offset = is_setter_stub_frame ?
+      isolate_->heap()->setter_stub_deopt_pc_offset() :
+      isolate_->heap()->getter_stub_deopt_pc_offset();
   intptr_t pc = reinterpret_cast<intptr_t>(
-      setter_stub->instruction_start() +
-      isolate_->heap()->setter_stub_deopt_pc_offset()->value());
+      accessor_stub->instruction_start() + offset->value());
   output_frame->SetPc(pc);
 }
 
index 9f4a789..f11044d 100644 (file)
@@ -484,6 +484,11 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
     case JS_CONSTRUCT:
       translation->BeginConstructStubFrame(closure_id, translation_size);
       break;
+    case JS_GETTER:
+      ASSERT(translation_size == 1);
+      ASSERT(height == 0);
+      translation->BeginGetterStubFrame(closure_id);
+      break;
     case JS_SETTER:
       ASSERT(translation_size == 2);
       ASSERT(height == 0);
index 058c29b..66714f8 100644 (file)
@@ -2951,6 +2951,43 @@ Handle<Code> LoadStubCompiler::CompileLoadCallback(
 }
 
 
+#undef __
+#define __ ACCESS_MASM(masm)
+
+
+void LoadStubCompiler::GenerateLoadViaGetter(MacroAssembler* masm,
+                                             Handle<JSFunction> getter) {
+  // ----------- S t a t e -------------
+  //  -- r0    : receiver
+  //  -- r2    : name
+  //  -- lr    : return address
+  // -----------------------------------
+  {
+    FrameScope scope(masm, StackFrame::INTERNAL);
+
+    if (!getter.is_null()) {
+      // Call the JavaScript getter with the receiver on the stack.
+      __ push(r0);
+      ParameterCount actual(0);
+      __ InvokeFunction(getter, actual, CALL_FUNCTION, NullCallWrapper(),
+                        CALL_AS_METHOD);
+    } else {
+      // If we generate a global code snippet for deoptimization only, remember
+      // the place to continue after deoptimization.
+      masm->isolate()->heap()->SetGetterStubDeoptPCOffset(masm->pc_offset());
+    }
+
+    // Restore context register.
+    __ ldr(cp, MemOperand(fp, StandardFrameConstants::kContextOffset));
+  }
+  __ Ret();
+}
+
+
+#undef __
+#define __ ACCESS_MASM(masm())
+
+
 Handle<Code> LoadStubCompiler::CompileLoadViaGetter(
     Handle<String> name,
     Handle<JSObject> receiver,
@@ -2967,19 +3004,7 @@ Handle<Code> LoadStubCompiler::CompileLoadViaGetter(
   __ JumpIfSmi(r0, &miss);
   CheckPrototypes(receiver, r0, holder, r3, r4, r1, name, &miss);
 
-  {
-    FrameScope scope(masm(), StackFrame::INTERNAL);
-
-    // Call the JavaScript getter with the receiver on the stack.
-    __ push(r0);
-    ParameterCount actual(0);
-    __ InvokeFunction(getter, actual, CALL_FUNCTION, NullCallWrapper(),
-                      CALL_AS_METHOD);
-
-    // Restore context register.
-    __ ldr(cp, MemOperand(fp, StandardFrameConstants::kContextOffset));
-  }
-  __ Ret();
+  GenerateLoadViaGetter(masm(), getter);
 
   __ bind(&miss);
   GenerateLoadMiss(masm(), Code::LOAD_IC);
index fa30549..6b0b7ed 100644 (file)
@@ -1292,6 +1292,11 @@ static void Generate_LoadIC_Normal(MacroAssembler* masm) {
 }
 
 
+static void Generate_LoadIC_Getter_ForDeopt(MacroAssembler* masm) {
+  LoadStubCompiler::GenerateLoadViaGetter(masm, Handle<JSFunction>());
+}
+
+
 static void Generate_KeyedLoadIC_Initialize(MacroAssembler* masm) {
   KeyedLoadIC::GenerateInitialize(masm);
 }
index 9bdb0ad..ca70ae5 100644 (file)
@@ -123,6 +123,8 @@ enum BuiltinExtraArguments {
                                     Code::kNoExtraICState)              \
   V(LoadIC_Megamorphic,             LOAD_IC, MEGAMORPHIC,               \
                                     Code::kNoExtraICState)              \
+  V(LoadIC_Getter_ForDeopt,         LOAD_IC, MONOMORPHIC,               \
+                                    Code::kNoExtraICState)              \
                                                                         \
   V(KeyedLoadIC_Initialize,         KEYED_LOAD_IC, UNINITIALIZED,       \
                                     Code::kNoExtraICState)              \
index 8cab88f..d1b00f8 100644 (file)
@@ -589,8 +589,11 @@ void Deoptimizer::DoComputeOutputFrames() {
       case Translation::CONSTRUCT_STUB_FRAME:
         DoComputeConstructStubFrame(&iterator, i);
         break;
+      case Translation::GETTER_STUB_FRAME:
+        DoComputeAccessorStubFrame(&iterator, i, false);
+        break;
       case Translation::SETTER_STUB_FRAME:
-        DoComputeSetterStubFrame(&iterator, i);
+        DoComputeAccessorStubFrame(&iterator, i, true);
         break;
       case Translation::BEGIN:
       case Translation::REGISTER:
@@ -722,6 +725,7 @@ void Deoptimizer::DoTranslateCommand(TranslationIterator* iterator,
     case Translation::JS_FRAME:
     case Translation::ARGUMENTS_ADAPTOR_FRAME:
     case Translation::CONSTRUCT_STUB_FRAME:
+    case Translation::GETTER_STUB_FRAME:
     case Translation::SETTER_STUB_FRAME:
     case Translation::DUPLICATE:
       UNREACHABLE();
@@ -1018,6 +1022,7 @@ bool Deoptimizer::DoOsrTranslateCommand(TranslationIterator* iterator,
     case Translation::JS_FRAME:
     case Translation::ARGUMENTS_ADAPTOR_FRAME:
     case Translation::CONSTRUCT_STUB_FRAME:
+    case Translation::GETTER_STUB_FRAME:
     case Translation::SETTER_STUB_FRAME:
     case Translation::DUPLICATE:
       UNREACHABLE();  // Malformed input.
@@ -1482,6 +1487,12 @@ void Translation::BeginConstructStubFrame(int literal_id, unsigned height) {
 }
 
 
+void Translation::BeginGetterStubFrame(int literal_id) {
+  buffer_->Add(GETTER_STUB_FRAME, zone());
+  buffer_->Add(literal_id, zone());
+}
+
+
 void Translation::BeginSetterStubFrame(int literal_id) {
   buffer_->Add(SETTER_STUB_FRAME, zone());
   buffer_->Add(literal_id, zone());
@@ -1574,6 +1585,7 @@ int Translation::NumberOfOperandsFor(Opcode opcode) {
     case ARGUMENTS_OBJECT:
     case DUPLICATE:
       return 0;
+    case GETTER_STUB_FRAME:
     case SETTER_STUB_FRAME:
     case REGISTER:
     case INT32_REGISTER:
@@ -1609,6 +1621,8 @@ const char* Translation::StringFor(Opcode opcode) {
       return "ARGUMENTS_ADAPTOR_FRAME";
     case CONSTRUCT_STUB_FRAME:
       return "CONSTRUCT_STUB_FRAME";
+    case GETTER_STUB_FRAME:
+      return "GETTER_STUB_FRAME";
     case SETTER_STUB_FRAME:
       return "SETTER_STUB_FRAME";
     case REGISTER:
@@ -1671,6 +1685,7 @@ SlotRef SlotRef::ComputeSlotForNextArgument(TranslationIterator* iterator,
     case Translation::JS_FRAME:
     case Translation::ARGUMENTS_ADAPTOR_FRAME:
     case Translation::CONSTRUCT_STUB_FRAME:
+    case Translation::GETTER_STUB_FRAME:
     case Translation::SETTER_STUB_FRAME:
       // Peeled off before getting here.
       break;
index c49ae86..cd33477 100644 (file)
@@ -284,8 +284,9 @@ class Deoptimizer : public Malloced {
                                       int frame_index);
   void DoComputeConstructStubFrame(TranslationIterator* iterator,
                                    int frame_index);
-  void DoComputeSetterStubFrame(TranslationIterator* iterator,
-                                int frame_index);
+  void DoComputeAccessorStubFrame(TranslationIterator* iterator,
+                                  int frame_index,
+                                  bool is_setter_stub_frame);
   void DoTranslateCommand(TranslationIterator* iterator,
                           int frame_index,
                           unsigned output_offset);
@@ -561,6 +562,7 @@ class Translation BASE_EMBEDDED {
     BEGIN,
     JS_FRAME,
     CONSTRUCT_STUB_FRAME,
+    GETTER_STUB_FRAME,
     SETTER_STUB_FRAME,
     ARGUMENTS_ADAPTOR_FRAME,
     REGISTER,
@@ -595,6 +597,7 @@ class Translation BASE_EMBEDDED {
   void BeginJSFrame(BailoutId node_id, int literal_id, unsigned height);
   void BeginArgumentsAdaptorFrame(int literal_id, unsigned height);
   void BeginConstructStubFrame(int literal_id, unsigned height);
+  void BeginGetterStubFrame(int literal_id);
   void BeginSetterStubFrame(int literal_id);
   void StoreRegister(Register reg);
   void StoreInt32Register(Register reg);
index cb4dd5b..d3ea89b 100644 (file)
@@ -213,7 +213,7 @@ DEFINE_bool(cache_optimized_code, true,
             "cache optimized code for closures")
 DEFINE_bool(inline_construct, true, "inline constructor calls")
 DEFINE_bool(inline_arguments, true, "inline functions with arguments object")
-DEFINE_bool(inline_accessors, false, "inline JavaScript accessors")
+DEFINE_bool(inline_accessors, true, "inline JavaScript accessors")
 DEFINE_int(loop_weight, 1, "loop weight for representation inference")
 
 DEFINE_bool(optimize_for_in, true,
index ba340a2..cb167d3 100644 (file)
@@ -153,6 +153,7 @@ namespace internal {
   V(StringDictionary, intrinsic_function_names, IntrinsicFunctionNames)        \
   V(Smi, arguments_adaptor_deopt_pc_offset, ArgumentsAdaptorDeoptPCOffset)     \
   V(Smi, construct_stub_deopt_pc_offset, ConstructStubDeoptPCOffset)           \
+  V(Smi, getter_stub_deopt_pc_offset, GetterStubDeoptPCOffset)                 \
   V(Smi, setter_stub_deopt_pc_offset, SetterStubDeoptPCOffset)
 
 #define ROOT_LIST(V)                                  \
@@ -1592,6 +1593,11 @@ class Heap {
     set_construct_stub_deopt_pc_offset(Smi::FromInt(pc_offset));
   }
 
+  void SetGetterStubDeoptPCOffset(int pc_offset) {
+    ASSERT(getter_stub_deopt_pc_offset() == Smi::FromInt(0));
+    set_getter_stub_deopt_pc_offset(Smi::FromInt(pc_offset));
+  }
+
   void SetSetterStubDeoptPCOffset(int pc_offset) {
     ASSERT(setter_stub_deopt_pc_offset() == Smi::FromInt(0));
     set_setter_stub_deopt_pc_offset(Smi::FromInt(pc_offset));
index 1cc0628..6c938cd 100644 (file)
@@ -1415,6 +1415,7 @@ enum InliningKind {
   NORMAL_RETURN,          // Normal function/method call and return.
   DROP_EXTRA_ON_RETURN,   // Drop an extra value from the environment on return.
   CONSTRUCT_CALL_RETURN,  // Either use allocated receiver or return value.
+  GETTER_CALL_RETURN,     // Returning from a getter, need to restore context.
   SETTER_CALL_RETURN      // Use the RHS of the assignment as the return value.
 };
 
index 02720e1..75344bb 100644 (file)
@@ -7280,7 +7280,7 @@ bool HGraphBuilder::TryInlineGetter(Handle<JSFunction> getter,
                    NULL,
                    prop->id(),
                    prop->LoadId(),
-                   NORMAL_RETURN);
+                   GETTER_CALL_RETURN);
 }
 
 
@@ -9594,6 +9594,10 @@ HEnvironment* HEnvironment::CopyForInlining(
     // actually be the constructor function, but we pass the newly allocated
     // object instead, DoComputeConstructStubFrame() relies on that.
     outer = CreateStubEnvironment(outer, target, JS_CONSTRUCT, arguments);
+  } else if (inlining_kind == GETTER_CALL_RETURN) {
+    // We need an additional StackFrame::INTERNAL frame for restoring the
+    // correct context.
+    outer = CreateStubEnvironment(outer, target, JS_GETTER, arguments);
   } else if (inlining_kind == SETTER_CALL_RETURN) {
     // We need an additional StackFrame::INTERNAL frame for temporarily saving
     // the argument of the setter, see StoreStubCompiler::CompileStoreViaSetter.
index d6d70b2..7d23ac7 100644 (file)
@@ -421,6 +421,7 @@ Zone* HBasicBlock::zone() const { return graph_->zone(); }
 enum FrameType {
   JS_FUNCTION,
   JS_CONSTRUCT,
+  JS_GETTER,
   JS_SETTER,
   ARGUMENTS_ADAPTOR
 };
index 4057ba7..f50010b 100644 (file)
@@ -694,31 +694,35 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslationIterator* iterator,
 }
 
 
-void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
-                                           int frame_index) {
-  JSFunction* setter = JSFunction::cast(ComputeLiteral(iterator->Next()));
-  // The receiver and the implicit return value are expected in registers by the
-  // StoreIC, so they don't belong to the output stack frame. This means that we
-  // have to use a height of 0.
+void Deoptimizer::DoComputeAccessorStubFrame(TranslationIterator* iterator,
+                                             int frame_index,
+                                             bool is_setter_stub_frame) {
+  JSFunction* accessor = JSFunction::cast(ComputeLiteral(iterator->Next()));
+  // The receiver (and the implicit return value, if any) are expected in
+  // registers by the LoadIC/StoreIC, so they don't belong to the output stack
+  // frame. This means that we have to use a height of 0.
   unsigned height = 0;
   unsigned height_in_bytes = height * kPointerSize;
+  const char* kind = is_setter_stub_frame ? "setter" : "getter";
   if (FLAG_trace_deopt) {
-    PrintF("  translating setter stub => height=%u\n", height_in_bytes);
+    PrintF("  translating %s stub => height=%u\n", kind, height_in_bytes);
   }
 
   // We need 1 stack entry for the return address + 4 stack entries from
   // StackFrame::INTERNAL (FP, context, frame type, code object, see
-  // MacroAssembler::EnterFrame) + 1 stack entry from setter stub (implicit
-  // return value, see StoreStubCompiler::CompileStoreViaSetter).
-  unsigned fixed_frame_size = (1 + 4 + 1) * kPointerSize;
+  // MacroAssembler::EnterFrame). For a setter stub frame we need one additional
+  // entry for the implicit return value, see
+  // StoreStubCompiler::CompileStoreViaSetter.
+  unsigned fixed_frame_entries = 1 + 4 + (is_setter_stub_frame ? 1 : 0);
+  unsigned fixed_frame_size = fixed_frame_entries * kPointerSize;
   unsigned output_frame_size = height_in_bytes + fixed_frame_size;
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, setter);
+      new(output_frame_size) FrameDescription(output_frame_size, accessor);
   output_frame->SetFrameType(StackFrame::INTERNAL);
 
-  // A frame for a setter stub can not be the topmost or bottommost one.
+  // A frame for an accessor stub can not be the topmost or bottommost one.
   ASSERT(frame_index > 0 && frame_index < output_count_ - 1);
   ASSERT(output_[frame_index] == NULL);
   output_[frame_index] = output_frame;
@@ -768,15 +772,17 @@ void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
   output_frame->SetFrameSlot(output_offset, value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08" V8PRIxPTR ": [top + %u] <- 0x%08" V8PRIxPTR
-           " ; function (setter sentinel)\n",
-           top_address + output_offset, output_offset, value);
+           " ; function (%s sentinel)\n",
+           top_address + output_offset, output_offset, value, kind);
   }
 
-  // Get Code object from setter stub.
+  // Get Code object from accessor stub.
   output_offset -= kPointerSize;
-  Code* setter_stub =
-      isolate_->builtins()->builtin(Builtins::kStoreIC_Setter_ForDeopt);
-  value = reinterpret_cast<intptr_t>(setter_stub);
+  Builtins::Name name = is_setter_stub_frame ?
+      Builtins::kStoreIC_Setter_ForDeopt :
+      Builtins::kLoadIC_Getter_ForDeopt;
+  Code* accessor_stub = isolate_->builtins()->builtin(name);
+  value = reinterpret_cast<intptr_t>(accessor_stub);
   output_frame->SetFrameSlot(output_offset, value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08" V8PRIxPTR ": [top + %u] <- 0x%08" V8PRIxPTR
@@ -789,16 +795,20 @@ void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
       static_cast<Translation::Opcode>(iterator->Next());
   iterator->Skip(Translation::NumberOfOperandsFor(opcode));
 
-  // The implicit return value was part of the artificial setter stub
-  // environment.
-  output_offset -= kPointerSize;
-  DoTranslateCommand(iterator, frame_index, output_offset);
+  if (is_setter_stub_frame) {
+    // The implicit return value was part of the artificial setter stub
+    // environment.
+    output_offset -= kPointerSize;
+    DoTranslateCommand(iterator, frame_index, output_offset);
+  }
 
   ASSERT(0 == output_offset);
 
+  Smi* offset = is_setter_stub_frame ?
+      isolate_->heap()->setter_stub_deopt_pc_offset() :
+      isolate_->heap()->getter_stub_deopt_pc_offset();
   intptr_t pc = reinterpret_cast<intptr_t>(
-      setter_stub->instruction_start() +
-      isolate_->heap()->setter_stub_deopt_pc_offset()->value());
+      accessor_stub->instruction_start() + offset->value());
   output_frame->SetPc(pc);
 }
 
index 26bebce..b645fd9 100644 (file)
@@ -423,6 +423,11 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
     case JS_CONSTRUCT:
       translation->BeginConstructStubFrame(closure_id, translation_size);
       break;
+    case JS_GETTER:
+      ASSERT(translation_size == 1);
+      ASSERT(height == 0);
+      translation->BeginGetterStubFrame(closure_id);
+      break;
     case JS_SETTER:
       ASSERT(translation_size == 2);
       ASSERT(height == 0);
index 5f1e26b..f5e2d05 100644 (file)
@@ -2996,6 +2996,43 @@ Handle<Code> LoadStubCompiler::CompileLoadCallback(
 }
 
 
+#undef __
+#define __ ACCESS_MASM(masm)
+
+
+void LoadStubCompiler::GenerateLoadViaGetter(MacroAssembler* masm,
+                                             Handle<JSFunction> getter) {
+  // ----------- S t a t e -------------
+  //  -- ecx    : name
+  //  -- edx    : receiver
+  //  -- esp[0] : return address
+  // -----------------------------------
+  {
+    FrameScope scope(masm, StackFrame::INTERNAL);
+
+    if (!getter.is_null()) {
+      // Call the JavaScript getter with the receiver on the stack.
+      __ push(edx);
+      ParameterCount actual(0);
+      __ InvokeFunction(getter, actual, CALL_FUNCTION, NullCallWrapper(),
+                        CALL_AS_METHOD);
+    } else {
+      // If we generate a global code snippet for deoptimization only, remember
+      // the place to continue after deoptimization.
+      masm->isolate()->heap()->SetGetterStubDeoptPCOffset(masm->pc_offset());
+    }
+
+    // Restore context register.
+    __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
+  }
+  __ ret(0);
+}
+
+
+#undef __
+#define __ ACCESS_MASM(masm())
+
+
 Handle<Code> LoadStubCompiler::CompileLoadViaGetter(
     Handle<String> name,
     Handle<JSObject> receiver,
@@ -3012,19 +3049,7 @@ Handle<Code> LoadStubCompiler::CompileLoadViaGetter(
   __ JumpIfSmi(edx, &miss);
   CheckPrototypes(receiver, edx, holder, ebx, eax, edi, name, &miss);
 
-  {
-    FrameScope scope(masm(), StackFrame::INTERNAL);
-
-    // Call the JavaScript getter with the receiver on the stack.
-    __ push(edx);
-    ParameterCount actual(0);
-    __ InvokeFunction(getter, actual, CALL_FUNCTION, NullCallWrapper(),
-                      CALL_AS_METHOD);
-
-    // Restore context register.
-    __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
-  }
-  __ ret(0);
+  GenerateLoadViaGetter(masm(), getter);
 
   __ bind(&miss);
   GenerateLoadMiss(masm(), Code::LOAD_IC);
index 666b8b3..371d120 100644 (file)
@@ -582,30 +582,34 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslationIterator* iterator,
 }
 
 
-void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
-                                           int frame_index) {
-  JSFunction* setter = JSFunction::cast(ComputeLiteral(iterator->Next()));
-  // The receiver and the implicit return value are expected in registers by the
-  // StoreIC, so they don't belong to the output stack frame. This means that we
-  // have to use a height of 0.
+void Deoptimizer::DoComputeAccessorStubFrame(TranslationIterator* iterator,
+                                             int frame_index,
+                                             bool is_setter_stub_frame) {
+  JSFunction* accessor = JSFunction::cast(ComputeLiteral(iterator->Next()));
+  // The receiver (and the implicit return value, if any) are expected in
+  // registers by the LoadIC/StoreIC, so they don't belong to the output stack
+  // frame. This means that we have to use a height of 0.
   unsigned height = 0;
   unsigned height_in_bytes = height * kPointerSize;
+  const char* kind = is_setter_stub_frame ? "setter" : "getter";
   if (FLAG_trace_deopt) {
-    PrintF("  translating setter stub => height=%u\n", height_in_bytes);
+    PrintF("  translating %s stub => height=%u\n", kind, height_in_bytes);
   }
 
   // We need 5 stack entries from StackFrame::INTERNAL (ra, fp, cp, frame type,
-  // code object, see MacroAssembler::EnterFrame) + 1 stack entry from setter
-  // stub (implicit return value, see StoreStubCompiler::CompileStoreViaSetter).
-  unsigned fixed_frame_size = (5 + 1) * kPointerSize;
+  // code object, see MacroAssembler::EnterFrame). For a setter stub frame we
+  // need one additional entry for the implicit return value, see
+  // StoreStubCompiler::CompileStoreViaSetter.
+  unsigned fixed_frame_entries = 5 + (is_setter_stub_frame ? 1 : 0);
+  unsigned fixed_frame_size = fixed_frame_entries * kPointerSize;
   unsigned output_frame_size = height_in_bytes + fixed_frame_size;
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, setter);
+      new(output_frame_size) FrameDescription(output_frame_size, accessor);
   output_frame->SetFrameType(StackFrame::INTERNAL);
 
-  // A frame for a setter stub can not be the topmost or bottommost one.
+  // A frame for an accessor stub can not be the topmost or bottommost one.
   ASSERT(frame_index > 0 && frame_index < output_count_ - 1);
   ASSERT(output_[frame_index] == NULL);
   output_[frame_index] = output_frame;
@@ -655,15 +659,17 @@ void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
   output_frame->SetFrameSlot(output_offset, value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08" V8PRIxPTR ": [top + %u] <- 0x%08" V8PRIxPTR
-           " ; function (setter sentinel)\n",
-           top_address + output_offset, output_offset, value);
+           " ; function (%s sentinel)\n",
+           top_address + output_offset, output_offset, value, kind);
   }
 
-  // Get Code object from setter stub.
+  // Get Code object from accessor stub.
   output_offset -= kPointerSize;
-  Code* setter_stub =
-      isolate_->builtins()->builtin(Builtins::kStoreIC_Setter_ForDeopt);
-  value = reinterpret_cast<intptr_t>(setter_stub);
+  Builtins::Name name = is_setter_stub_frame ?
+      Builtins::kStoreIC_Setter_ForDeopt :
+      Builtins::kLoadIC_Getter_ForDeopt;
+  Code* accessor_stub = isolate_->builtins()->builtin(name);
+  value = reinterpret_cast<intptr_t>(accessor_stub);
   output_frame->SetFrameSlot(output_offset, value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08" V8PRIxPTR ": [top + %u] <- 0x%08" V8PRIxPTR
@@ -676,16 +682,20 @@ void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
       static_cast<Translation::Opcode>(iterator->Next());
   iterator->Skip(Translation::NumberOfOperandsFor(opcode));
 
-  // The implicit return value was part of the artificial setter stub
-  // environment.
-  output_offset -= kPointerSize;
-  DoTranslateCommand(iterator, frame_index, output_offset);
+  if (is_setter_stub_frame) {
+    // The implicit return value was part of the artificial setter stub
+    // environment.
+    output_offset -= kPointerSize;
+    DoTranslateCommand(iterator, frame_index, output_offset);
+  }
 
   ASSERT(0 == output_offset);
 
+  Smi* offset = is_setter_stub_frame ?
+      isolate_->heap()->setter_stub_deopt_pc_offset() :
+      isolate_->heap()->getter_stub_deopt_pc_offset();
   intptr_t pc = reinterpret_cast<intptr_t>(
-      setter_stub->instruction_start() +
-      isolate_->heap()->setter_stub_deopt_pc_offset()->value());
+      accessor_stub->instruction_start() + offset->value());
   output_frame->SetPc(pc);
 }
 
index 99728b5..a00a92d 100644 (file)
@@ -452,6 +452,11 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
     case JS_CONSTRUCT:
       translation->BeginConstructStubFrame(closure_id, translation_size);
       break;
+    case JS_GETTER:
+      ASSERT(translation_size == 1);
+      ASSERT(height == 0);
+      translation->BeginGetterStubFrame(closure_id);
+      break;
     case JS_SETTER:
       ASSERT(translation_size == 2);
       ASSERT(height == 0);
index 2594f49..391f8e0 100644 (file)
@@ -2956,6 +2956,43 @@ Handle<Code> LoadStubCompiler::CompileLoadCallback(
 }
 
 
+#undef __
+#define __ ACCESS_MASM(masm)
+
+
+void LoadStubCompiler::GenerateLoadViaGetter(MacroAssembler* masm,
+                                             Handle<JSFunction> getter) {
+  // ----------- S t a t e -------------
+  //  -- a0    : receiver
+  //  -- a2    : name
+  //  -- ra    : return address
+  // -----------------------------------
+  {
+    FrameScope scope(masm, StackFrame::INTERNAL);
+
+    if (!getter.is_null()) {
+      // Call the JavaScript getter with the receiver on the stack.
+      __ push(a0);
+      ParameterCount actual(0);
+      __ InvokeFunction(getter, actual, CALL_FUNCTION, NullCallWrapper(),
+                        CALL_AS_METHOD);
+    } else {
+      // If we generate a global code snippet for deoptimization only, remember
+      // the place to continue after deoptimization.
+      masm->isolate()->heap()->SetGetterStubDeoptPCOffset(masm->pc_offset());
+    }
+
+    // Restore context register.
+    __ lw(cp, MemOperand(fp, StandardFrameConstants::kContextOffset));
+  }
+  __ Ret();
+}
+
+
+#undef __
+#define __ ACCESS_MASM(masm())
+
+
 Handle<Code> LoadStubCompiler::CompileLoadViaGetter(
     Handle<String> name,
     Handle<JSObject> receiver,
@@ -2972,19 +3009,7 @@ Handle<Code> LoadStubCompiler::CompileLoadViaGetter(
   __ JumpIfSmi(a0, &miss);
   CheckPrototypes(receiver, a0, holder, a3, t0, a1, name, &miss);
 
-  {
-    FrameScope scope(masm(), StackFrame::INTERNAL);
-
-    // Call the JavaScript getter with the receiver on the stack.
-    __ push(a0);
-    ParameterCount actual(0);
-    __ InvokeFunction(getter, actual, CALL_FUNCTION, NullCallWrapper(),
-                      CALL_AS_METHOD);
-
-    // Restore context register.
-    __ lw(cp, MemOperand(fp, StandardFrameConstants::kContextOffset));
-  }
-  __ Ret();
+  GenerateLoadViaGetter(masm(), getter);
 
   __ bind(&miss);
   GenerateLoadMiss(masm(), Code::LOAD_IC);
index f84aa3b..74af575 100644 (file)
@@ -8291,6 +8291,7 @@ void DeoptimizationInputData::DeoptimizationInputDataPrint(FILE* out) {
           break;
         }
 
+        case Translation::GETTER_STUB_FRAME:
         case Translation::SETTER_STUB_FRAME: {
           int function_id = iterator.Next();
           JSFunction* function =
index 418a1d2..005c537 100644 (file)
@@ -619,6 +619,9 @@ class LoadStubCompiler: public StubCompiler {
                                    Handle<JSObject> holder,
                                    Handle<AccessorInfo> callback);
 
+  static void GenerateLoadViaGetter(MacroAssembler* masm,
+                                    Handle<JSFunction> getter);
+
   Handle<Code> CompileLoadViaGetter(Handle<String> name,
                                     Handle<JSObject> receiver,
                                     Handle<JSObject> holder,
index a0cdf33..0502502 100644 (file)
@@ -591,31 +591,35 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslationIterator* iterator,
 }
 
 
-void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
-                                           int frame_index) {
-  JSFunction* setter = JSFunction::cast(ComputeLiteral(iterator->Next()));
-  // The receiver and the implicit return value are expected in registers by the
-  // StoreIC, so they don't belong to the output stack frame. This means that we
-  // have to use a height of 0.
+void Deoptimizer::DoComputeAccessorStubFrame(TranslationIterator* iterator,
+                                             int frame_index,
+                                             bool is_setter_stub_frame) {
+  JSFunction* accessor = JSFunction::cast(ComputeLiteral(iterator->Next()));
+  // The receiver (and the implicit return value, if any) are expected in
+  // registers by the LoadIC/StoreIC, so they don't belong to the output stack
+  // frame. This means that we have to use a height of 0.
   unsigned height = 0;
   unsigned height_in_bytes = height * kPointerSize;
+  const char* kind = is_setter_stub_frame ? "setter" : "getter";
   if (FLAG_trace_deopt) {
-    PrintF("  translating setter stub => height=%u\n", height_in_bytes);
+    PrintF("  translating %s stub => height=%u\n", kind, height_in_bytes);
   }
 
   // We need 1 stack entry for the return address + 4 stack entries from
   // StackFrame::INTERNAL (FP, context, frame type, code object, see
-  // MacroAssembler::EnterFrame) + 1 stack entry from setter stub (implicit
-  // return value, see StoreStubCompiler::CompileStoreViaSetter).
-  unsigned fixed_frame_size = (1 + 4 + 1) * kPointerSize;
+  // MacroAssembler::EnterFrame). For a setter stub frame we need one additional
+  // entry for the implicit return value, see
+  // StoreStubCompiler::CompileStoreViaSetter.
+  unsigned fixed_frame_entries = 1 + 4 + (is_setter_stub_frame ? 1 : 0);
+  unsigned fixed_frame_size = fixed_frame_entries * kPointerSize;
   unsigned output_frame_size = height_in_bytes + fixed_frame_size;
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, setter);
+      new(output_frame_size) FrameDescription(output_frame_size, accessor);
   output_frame->SetFrameType(StackFrame::INTERNAL);
 
-  // A frame for a setter stub can not be the topmost or bottommost one.
+  // A frame for an accessor stub can not be the topmost or bottommost one.
   ASSERT(frame_index > 0 && frame_index < output_count_ - 1);
   ASSERT(output_[frame_index] == NULL);
   output_[frame_index] = output_frame;
@@ -665,15 +669,17 @@ void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
   output_frame->SetFrameSlot(output_offset, value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08" V8PRIxPTR ": [top + %u] <- 0x%08" V8PRIxPTR
-           " ; function (setter sentinel)\n",
-           top_address + output_offset, output_offset, value);
+           " ; function (%s sentinel)\n",
+           top_address + output_offset, output_offset, value, kind);
   }
 
-  // Get Code object from setter stub.
+  // Get Code object from accessor stub.
   output_offset -= kPointerSize;
-  Code* setter_stub =
-      isolate_->builtins()->builtin(Builtins::kStoreIC_Setter_ForDeopt);
-  value = reinterpret_cast<intptr_t>(setter_stub);
+  Builtins::Name name = is_setter_stub_frame ?
+      Builtins::kStoreIC_Setter_ForDeopt :
+      Builtins::kLoadIC_Getter_ForDeopt;
+  Code* accessor_stub = isolate_->builtins()->builtin(name);
+  value = reinterpret_cast<intptr_t>(accessor_stub);
   output_frame->SetFrameSlot(output_offset, value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08" V8PRIxPTR ": [top + %u] <- 0x%08" V8PRIxPTR
@@ -686,16 +692,20 @@ void Deoptimizer::DoComputeSetterStubFrame(TranslationIterator* iterator,
       static_cast<Translation::Opcode>(iterator->Next());
   iterator->Skip(Translation::NumberOfOperandsFor(opcode));
 
-  // The implicit return value was part of the artificial setter stub
-  // environment.
-  output_offset -= kPointerSize;
-  DoTranslateCommand(iterator, frame_index, output_offset);
+  if (is_setter_stub_frame) {
+    // The implicit return value was part of the artificial setter stub
+    // environment.
+    output_offset -= kPointerSize;
+    DoTranslateCommand(iterator, frame_index, output_offset);
+  }
 
   ASSERT(0 == output_offset);
 
+  Smi* offset = is_setter_stub_frame ?
+      isolate_->heap()->setter_stub_deopt_pc_offset() :
+      isolate_->heap()->getter_stub_deopt_pc_offset();
   intptr_t pc = reinterpret_cast<intptr_t>(
-      setter_stub->instruction_start() +
-      isolate_->heap()->setter_stub_deopt_pc_offset()->value());
+      accessor_stub->instruction_start() + offset->value());
   output_frame->SetPc(pc);
 }
 
index 60f1644..fea7627 100644 (file)
@@ -371,6 +371,11 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
     case JS_CONSTRUCT:
       translation->BeginConstructStubFrame(closure_id, translation_size);
       break;
+    case JS_GETTER:
+      ASSERT(translation_size == 1);
+      ASSERT(height == 0);
+      translation->BeginGetterStubFrame(closure_id);
+      break;
     case JS_SETTER:
       ASSERT(translation_size == 2);
       ASSERT(height == 0);
index cb6198a..cd71086 100644 (file)
@@ -2819,6 +2819,43 @@ Handle<Code> LoadStubCompiler::CompileLoadCallback(
 }
 
 
+#undef __
+#define __ ACCESS_MASM(masm)
+
+
+void LoadStubCompiler::GenerateLoadViaGetter(MacroAssembler* masm,
+                                             Handle<JSFunction> getter) {
+  // ----------- S t a t e -------------
+  //  -- rax    : receiver
+  //  -- rcx    : name
+  //  -- rsp[0] : return address
+  // -----------------------------------
+  {
+    FrameScope scope(masm, StackFrame::INTERNAL);
+
+    if (!getter.is_null()) {
+      // Call the JavaScript getter with the receiver on the stack.
+      __ push(rax);
+      ParameterCount actual(0);
+      __ InvokeFunction(getter, actual, CALL_FUNCTION, NullCallWrapper(),
+                        CALL_AS_METHOD);
+    } else {
+      // If we generate a global code snippet for deoptimization only, remember
+      // the place to continue after deoptimization.
+      masm->isolate()->heap()->SetGetterStubDeoptPCOffset(masm->pc_offset());
+    }
+
+    // Restore context register.
+    __ movq(rsi, Operand(rbp, StandardFrameConstants::kContextOffset));
+  }
+  __ ret(0);
+}
+
+
+#undef __
+#define __ ACCESS_MASM(masm())
+
+
 Handle<Code> LoadStubCompiler::CompileLoadViaGetter(
     Handle<String> name,
     Handle<JSObject> receiver,
@@ -2835,19 +2872,7 @@ Handle<Code> LoadStubCompiler::CompileLoadViaGetter(
   __ JumpIfSmi(rax, &miss);
   CheckPrototypes(receiver, rax, holder, rbx, rdx, rdi, name, &miss);
 
-  {
-    FrameScope scope(masm(), StackFrame::INTERNAL);
-
-    // Call the JavaScript getter with the receiver on the stack.
-    __ push(rax);
-    ParameterCount actual(0);
-    __ InvokeFunction(getter, actual, CALL_FUNCTION, NullCallWrapper(),
-                      CALL_AS_METHOD);
-
-    // Restore context register.
-    __ movq(rsi, Operand(rbp, StandardFrameConstants::kContextOffset));
-  }
-  __ ret(0);
+  GenerateLoadViaGetter(masm(), getter),
 
   __ bind(&miss);
   GenerateLoadMiss(masm(), Code::LOAD_IC);
diff --git a/test/mjsunit/regress/regress-crbug-134609.js b/test/mjsunit/regress/regress-crbug-134609.js
new file mode 100644 (file)
index 0000000..da7d85d
--- /dev/null
@@ -0,0 +1,59 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax --inline-accessors
+
+var forceDeopt = {x:0};
+
+var objectWithGetterProperty = (function (value) {
+  var obj = {};
+  Object.defineProperty(obj, "getterProperty", {
+    get: function foo() {
+      forceDeopt.x;
+      return value;
+    },
+  });
+  return obj;
+})("bad");
+
+function test() {
+  var iAmContextAllocated = "good";
+  objectWithGetterProperty.getterProperty;
+  return iAmContextAllocated;
+
+  // Make sure that the local variable is context allocated.
+  function unused() { iAmContextAllocated; }
+}
+
+assertEquals("good", test());
+assertEquals("good", test());
+%OptimizeFunctionOnNextCall(test);
+assertEquals("good", test());
+
+// At this point, foo should have been inlined into test. Let's deopt...
+delete forceDeopt.x;
+assertEquals("good", test());