Fix debugger after inlined keyed store change.

Make sure that the IC is always hit when debugging and make sure to
restore the fast case when leaving the debugger.
Review URL: http://codereview.chromium.org/125044

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

src/arm/ic-arm.cc

@@ -503,6 +503,12 @@ bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
   return false;
 }
 
+void KeyedStoreIC::ClearInlinedVersion(Address address) {}
+void KeyedStoreIC::RestoreInlinedVersion(Address address) {}
+bool KeyedStoreIC::PatchInlinedStore(Address address, Object* map) {
+  return false;
+}
+
 
 Object* KeyedLoadIC_Miss(Arguments args);
 

src/debug.cc

@@ -382,6 +382,7 @@ void BreakLocationIterator::SetDebugBreakAtIC() {
     // the code copy and will therefore have no effect on the running code
     // keeping it from using the inlined code.
     if (code->is_keyed_load_stub()) KeyedLoadIC::ClearInlinedVersion(pc());
+    if (code->is_keyed_store_stub()) KeyedStoreIC::ClearInlinedVersion(pc());
   }
 }
 
@@ -389,6 +390,19 @@ void BreakLocationIterator::SetDebugBreakAtIC() {
 void BreakLocationIterator::ClearDebugBreakAtIC() {
   // Patch the code to the original invoke.
   rinfo()->set_target_address(original_rinfo()->target_address());
+
+  RelocInfo::Mode mode = rmode();
+  if (RelocInfo::IsCodeTarget(mode)) {
+    Address target = original_rinfo()->target_address();
+    Handle<Code> code(Code::GetCodeFromTargetAddress(target));
+
+    // Restore the inlined version of keyed stores to get back to the
+    // fast case.  We need to patch back the keyed store because no
+    // patching happens when running normally.  For keyed loads, the
+    // map check will get patched back when running normally after ICs
+    // have been cleared at GC.
+    if (code->is_keyed_store_stub()) KeyedStoreIC::RestoreInlinedVersion(pc());
+  }
 }
 
 

src/ia32/codegen-ia32.cc

@@ -5749,10 +5749,13 @@ class DeferredReferenceSetKeyedValue: public DeferredCode {
 
   virtual void Generate();
 
+  Label* patch_site() { return &patch_site_; }
+
  private:
   Register value_;
   Register key_;
   Register receiver_;
+  Label patch_site_;
 };
 
 
@@ -5766,6 +5769,15 @@ void DeferredReferenceSetKeyedValue::Generate() {
   // Call the IC stub.
   Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize));
   __ call(ic, RelocInfo::CODE_TARGET);
+  // The delta from the start of the map-compare instruction to the
+  // test instruction.  We use masm_-> directly here instead of the
+  // __ macro because the macro sometimes uses macro expansion to turn
+  // into something that can't return a value.  This is encountered
+  // when doing generated code coverage tests.
+  int delta_to_patch_site = masm_->SizeOfCodeGeneratedSince(patch_site());
+  // Here we use masm_-> instead of the __ macro because this is the
+  // instruction that gets patched and coverage code gets in the way.
+  masm_->test(eax, Immediate(-delta_to_patch_site));
   // Restore value (returned from store IC), key and receiver
   // registers.
   if (!value_.is(eax)) __ mov(value_, eax);
@@ -6122,9 +6134,15 @@ void Reference::SetValue(InitState init_state) {
         // is not a dictionary.
         __ mov(tmp.reg(),
                FieldOperand(receiver.reg(), JSObject::kElementsOffset));
+        // Bind the deferred code patch site to be able to locate the
+        // fixed array map comparison.  When debugging, we patch this
+        // comparison to always fail so that we will hit the IC call
+        // in the deferred code which will allow the debugger to
+        // break for fast case stores.
+        __ bind(deferred->patch_site());
         __ cmp(FieldOperand(tmp.reg(), HeapObject::kMapOffset),
-                           Immediate(Factory::hash_table_map()));
-        deferred->Branch(equal);
+               Immediate(Factory::fixed_array_map()));
+        deferred->Branch(not_equal);
 
         // Store the value.
         __ mov(Operand(tmp.reg(),
@@ -6141,6 +6159,11 @@ void Reference::SetValue(InitState init_state) {
         cgen_->frame()->Push(&value);
       } else {
         Result answer = cgen_->frame()->CallKeyedStoreIC();
+        // Make sure that we do not have a test instruction after the
+        // call.  A test instruction after the call is used to
+        // indicate that we have generated an inline version of the
+        // keyed store.
+        __ nop();
         cgen_->frame()->Push(&answer);
       }
       break;

src/ia32/ic-ia32.cc

@@ -747,6 +747,21 @@ void KeyedLoadIC::ClearInlinedVersion(Address address) {
 }
 
 
+void KeyedStoreIC::ClearInlinedVersion(Address address) {
+  // Insert null as the elements map to check for.  This will make
+  // sure that the elements fast-case map check fails so that control
+  // flows to the IC instead of the inlined version.
+  PatchInlinedStore(address, Heap::null_value());
+}
+
+
+void KeyedStoreIC::RestoreInlinedVersion(Address address) {
+  // Restore the fast-case elements map check so that the inlined
+  // version can be used again.
+  PatchInlinedStore(address, Heap::fixed_array_map());
+}
+
+
 bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) {
   // The address of the instruction following the call.
   Address test_instruction_address = address + 4;
@@ -774,7 +789,7 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) {
 }
 
 
-bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
+static bool PatchInlinedMapCheck(Address address, Object* map) {
   Address test_instruction_address = address + 4;  // 4 = stub address
   // The keyed load has a fast inlined case if the IC call instruction
   // is immediately followed by a test instruction.
@@ -795,6 +810,16 @@ bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
 }
 
 
+bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
+  return PatchInlinedMapCheck(address, map);
+}
+
+
+bool KeyedStoreIC::PatchInlinedStore(Address address, Object* map) {
+  return PatchInlinedMapCheck(address, map);
+}
+
+
 // Defined in ic.cc.
 Object* KeyedLoadIC_Miss(Arguments args);
 

src/ic.h

@@ -356,6 +356,12 @@ class KeyedStoreIC: public IC {
   static void GenerateGeneric(MacroAssembler* masm);
   static void GenerateExtendStorage(MacroAssembler* masm);
 
+  // Clear the inlined version so the IC is always hit.
+  static void ClearInlinedVersion(Address address);
+
+  // Restore the inlined version so the fast case can get hit.
+  static void RestoreInlinedVersion(Address address);
+
  private:
   static void Generate(MacroAssembler* masm, const ExternalReference& f);
 
@@ -378,6 +384,11 @@ class KeyedStoreIC: public IC {
   }
 
   static void Clear(Address address, Code* target);
+
+  // Support for patching the map that is checked in an inlined
+  // version of keyed store.
+  static bool PatchInlinedStore(Address address, Object* map);
+
   friend class IC;
 };
 

src/x64/ic-x64.cc

@@ -40,6 +40,14 @@ void KeyedLoadIC::ClearInlinedVersion(Address address) {
   UNIMPLEMENTED();
 }
 
+void KeyedStoreIC::ClearInlinedVersion(Address address) {
+  UNIMPLEMENTED();
+}
+
+void KeyedStoreIC::RestoreInlinedVersion(Address address) {
+  UNIMPLEMENTED();
+}
+
 void KeyedLoadIC::Generate(MacroAssembler* masm,
                            ExternalReference const& f) {
   masm->int3();  // UNIMPLEMENTED.
@@ -58,6 +66,11 @@ bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
   return false;
 }
 
+bool KeyedStoreIC::PatchInlinedStore(Address address, Object* map) {
+  UNIMPLEMENTED();
+  return false;
+}
+
 Object* KeyedLoadStubCompiler::CompileLoadArrayLength(String* name) {
   UNIMPLEMENTED();
   return NULL;

test/cctest/test-debug.cc

@@ -2237,6 +2237,52 @@ TEST(DebugStepKeyedLoadLoop) {
 }
 
 
+// Test of the stepping mechanism for keyed store in a loop.
+TEST(DebugStepKeyedStoreLoop) {
+  v8::HandleScope scope;
+  DebugLocalContext env;
+
+  // Create a function for testing stepping of keyed store. The statement 'y=1'
+  // is there to have more than one breakable statement in the loop, TODO(315).
+  v8::Local<v8::Function> foo = CompileFunction(
+      &env,
+      "function foo(a) {\n"
+      "  var len = a.length;\n"
+      "  for (var i = 0; i < len; i++) {\n"
+      "    y = 1;\n"
+      "    a[i] = 42;\n"
+      "  }\n"
+      "}\n",
+      "foo");
+
+  // Create array [0,1,2,3,4,5,6,7,8,9]
+  v8::Local<v8::Array> a = v8::Array::New(10);
+  for (int i = 0; i < 10; i++) {
+    a->Set(v8::Number::New(i), v8::Number::New(i));
+  }
+
+  // Call function without any break points to ensure inlining is in place.
+  const int kArgc = 1;
+  v8::Handle<v8::Value> args[kArgc] = { a };
+  foo->Call(env->Global(), kArgc, args);
+
+  // Register a debug event listener which steps and counts.
+  v8::Debug::SetDebugEventListener(DebugEventStep);
+
+  // Setup break point and step through the function.
+  SetBreakPoint(foo, 3);
+  step_action = StepNext;
+  break_point_hit_count = 0;
+  foo->Call(env->Global(), kArgc, args);
+
+  // With stepping all break locations are hit.
+  CHECK_EQ(22, break_point_hit_count);
+
+  v8::Debug::SetDebugEventListener(NULL);
+  CheckDebuggerUnloaded();
+}
+
+
 // Test the stepping mechanism with different ICs.
 TEST(DebugStepLinearMixedICs) {
   v8::HandleScope scope;