Debugger: clear ICs on activating step-in to correctly flood accessor pairs.

If we compile handlers to call accessors, Debug::HandleStepIn won't get
called. Therefore we need to clear ICs each time. This has not been
necessary before because we used to patch ICs for breaking, and restored
them with cleared ICs. This is no longer the case. We do not use ICs
for breaking anymore, so they are not implicitly cleared any longer.

R=mvstanton@chromium.org
BUG=v8:4269
LOG=N

Review URL: https://codereview.chromium.org/1212253009

Cr-Commit-Position: refs/heads/master@{#29518}

diff --git a/src/debug.cc b/src/debug.cc
index dc39b48118173aedf2cc9699fc5c52162bb40366..e952fe7ebb88a9dc07893f4e973be4fc5fc9ffab 100644 (file)
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -1351,7 +1351,7 @@ void Debug::PrepareStep(StepAction step_action,
       }
     }
 
-    ActivateStepIn(frame);
+    ActivateStepIn(function, frame);
   }
 
   // Fill the current function with one-shot break points even for step in on
@@ -1455,30 +1455,27 @@ Handle<Object> Debug::GetSourceBreakLocations(
 
 
 // Handle stepping into a function.
-void Debug::HandleStepIn(Handle<Object> function_obj, Handle<Object> holder,
-                         Address fp, bool is_constructor) {
+void Debug::HandleStepIn(Handle<Object> function_obj, bool is_constructor) {
   // Flood getter/setter if we either step in or step to another frame.
   bool step_frame = thread_local_.last_step_action_ == StepFrame;
   if (!StepInActive() && !step_frame) return;
   if (!function_obj->IsJSFunction()) return;
   Handle<JSFunction> function = Handle<JSFunction>::cast(function_obj);
   Isolate* isolate = function->GetIsolate();
-  // If the frame pointer is not supplied by the caller find it.
-  if (fp == 0) {
-    StackFrameIterator it(isolate);
+
+  StackFrameIterator it(isolate);
+  it.Advance();
+  // For constructor functions skip another frame.
+  if (is_constructor) {
+    DCHECK(it.frame()->is_construct());
     it.Advance();
-    // For constructor functions skip another frame.
-    if (is_constructor) {
-      DCHECK(it.frame()->is_construct());
-      it.Advance();
-    }
-    fp = it.frame()->fp();
   }
+  Address fp = it.frame()->fp();
 
   // Flood the function with one-shot break points if it is called from where
   // step into was requested, or when stepping into a new frame.
   if (fp == thread_local_.step_into_fp_ || step_frame) {
-    FloodWithOneShotGeneric(function, holder);
+    FloodWithOneShotGeneric(function, Handle<Object>());
   }
 }
 
@@ -1512,8 +1509,12 @@ void Debug::ClearOneShot() {
 }
 
 
-void Debug::ActivateStepIn(StackFrame* frame) {
+void Debug::ActivateStepIn(Handle<JSFunction> function, StackFrame* frame) {
   DCHECK(!StepOutActive());
+  // Make sure IC state is clean. This is so that we correct flood
+  // accessor pairs when stepping in.
+  function->code()->ClearInlineCaches();
+  function->shared()->feedback_vector()->ClearICSlots(function->shared());
   thread_local_.step_into_fp_ = frame->UnpaddedFP();
 }
 
@@ -2069,10 +2070,6 @@ bool Debug::EnsureDebugInfo(Handle<SharedFunctionInfo> shared,
     return false;
   }
 
-  // Make sure IC state is clean.
-  shared->code()->ClearInlineCaches();
-  shared->feedback_vector()->ClearICSlots(*shared);
-
   // Create the debug info object.
   Handle<DebugInfo> debug_info = isolate->factory()->NewDebugInfo(shared);
 

diff --git a/src/debug.h b/src/debug.h
index 5882ffbf2b9c1370724c02d4cd1429ff0256f69a..fd5f67d6f505d996c1c9830b2830634f4fd1debf 100644 (file)
--- a/src/debug.h
+++ b/src/debug.h
@@ -465,8 +465,7 @@ class Debug {
   bool IsStepping() { return thread_local_.step_count_ > 0; }
   bool StepNextContinue(BreakLocation* location, JavaScriptFrame* frame);
   bool StepInActive() { return thread_local_.step_into_fp_ != 0; }
-  void HandleStepIn(Handle<Object> function_obj, Handle<Object> holder,
-                    Address fp, bool is_constructor);
+  void HandleStepIn(Handle<Object> function_obj, bool is_constructor);
   bool StepOutActive() { return thread_local_.step_out_fp_ != 0; }
 
   // Purge all code objects that have no debug break slots.
@@ -625,7 +624,7 @@ class Debug {
 
   static bool CompileDebuggerScript(Isolate* isolate, int index);
   void ClearOneShot();
-  void ActivateStepIn(StackFrame* frame);
+  void ActivateStepIn(Handle<JSFunction> function, StackFrame* frame);
   void ClearStepIn();
   void ActivateStepOut(StackFrame* frame);
   void ClearStepNext();

diff --git a/src/objects.cc b/src/objects.cc
index 24b54c0d4a3710160d5cde7b57d2bb9a732c2a86..b14281a41bbbe9648a6838a292b811264596392b 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -440,9 +440,7 @@ MaybeHandle<Object> Object::GetPropertyWithDefinedGetter(
   Debug* debug = isolate->debug();
   // Handle stepping into a getter if step into is active.
   // TODO(rossberg): should this apply to getters that are function proxies?
-  if (debug->is_active()) {
-    debug->HandleStepIn(getter, Handle<Object>::null(), 0, false);
-  }
+  if (debug->is_active()) debug->HandleStepIn(getter, false);
 
   return Execution::Call(isolate, getter, receiver, 0, NULL, true);
 }
@@ -457,9 +455,7 @@ MaybeHandle<Object> Object::SetPropertyWithDefinedSetter(
   Debug* debug = isolate->debug();
   // Handle stepping into a setter if step into is active.
   // TODO(rossberg): should this apply to getters that are function proxies?
-  if (debug->is_active()) {
-    debug->HandleStepIn(setter, Handle<Object>::null(), 0, false);
-  }
+  if (debug->is_active()) debug->HandleStepIn(setter, false);
 
   Handle<Object> argv[] = { value };
   RETURN_ON_EXCEPTION(isolate, Execution::Call(isolate, setter, receiver,

diff --git a/src/runtime/runtime-classes.cc b/src/runtime/runtime-classes.cc
index 121530feab40d641210e11c559cbe6e0607b0c7c..97a19c1bc78ac84f1aadb2dcd55efee343725a7c 100644 (file)
--- a/src/runtime/runtime-classes.cc
+++ b/src/runtime/runtime-classes.cc
@@ -470,9 +470,7 @@ RUNTIME_FUNCTION(Runtime_HandleStepInForDerivedConstructors) {
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
   Debug* debug = isolate->debug();
   // Handle stepping into constructors if step into is active.
-  if (debug->StepInActive()) {
-    debug->HandleStepIn(function, Handle<Object>::null(), 0, true);
-  }
+  if (debug->StepInActive()) debug->HandleStepIn(function, true);
   return *isolate->factory()->undefined_value();
 }
 

diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
index 9aa4341b30ad30e226d4a65c6a1c3860e98b8ac0..e7aaed1f6fbed71adc8d88d5e2eb68d4b499b8f4 100644 (file)
--- a/src/runtime/runtime-debug.cc
+++ b/src/runtime/runtime-debug.cc
@@ -3212,6 +3212,17 @@ RUNTIME_FUNCTION(Runtime_DebugIsActive) {
 }
 
 
+RUNTIME_FUNCTION(Runtime_DebugHandleStepIntoAccessor) {
+  HandleScope scope(isolate);
+  DCHECK(args.length() == 2);
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
+  Debug* debug = isolate->debug();
+  // Handle stepping into constructors if step into is active.
+  if (debug->StepInActive()) debug->HandleStepIn(function, false);
+  return *isolate->factory()->undefined_value();
+}
+
+
 RUNTIME_FUNCTION(Runtime_DebugBreakInOptimizedCode) {
   UNIMPLEMENTED();
   return NULL;

diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc
index b3ac1864ba822a6a91ca7bd5a112f1b2acbf8e2d..da1ec4977be36f1965f86e60cbfd5e08c8e02fdd 100644 (file)
--- a/src/runtime/runtime-object.cc
+++ b/src/runtime/runtime-object.cc
@@ -1072,9 +1072,7 @@ static Object* Runtime_NewObjectHelper(Isolate* isolate,
 
   Debug* debug = isolate->debug();
   // Handle stepping into constructors if step into is active.
-  if (debug->StepInActive()) {
-    debug->HandleStepIn(function, Handle<Object>::null(), 0, true);
-  }
+  if (debug->StepInActive()) debug->HandleStepIn(function, true);
 
   if (function->has_initial_map()) {
     if (function->initial_map()->instance_type() == JS_FUNCTION_TYPE) {

diff --git a/test/mjsunit/debug-stepin-accessor-ic.js b/test/mjsunit/debug-stepin-accessor-ic.js
new file mode 100644 (file)
index 0000000..5f40dcb
--- /dev/null
+++ b/test/mjsunit/debug-stepin-accessor-ic.js
@@ -0,0 +1,49 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug
+
+function get() {
+  return 3;                       // Break
+}                                 // Break
+
+function set(x) {
+  this.x = x;                     // Break
+}                                 // Break
+
+var o = {};
+Object.defineProperty(o, "get", { get : get });
+Object.defineProperty(o, "set", { set : set });
+
+function f() {
+  for (var i = 0; i < 10; i++) {  // Break
+    o.get;                        // Break
+    o.set = 1;                    // Break
+  }
+}                                 // Break
+
+var break_count = 0;
+var exception = null;
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    var source_line = exec_state.frame(0).sourceLineText();
+    assertTrue(source_line.indexOf("// Break") > 0);
+    exec_state.prepareStep(Debug.StepAction.StepIn, 1);
+    break_count++;
+  } catch (e) {
+    exception = e;
+  }
+}
+
+var Debug = debug.Debug;
+Debug.setListener(listener);
+
+debugger;                         // Break
+f();                              // Break
+
+Debug.setListener(null);          // Break
+assertEquals(86, break_count);
+assertNull(exception);