[debug] implement debug break for FunctionTemplate accessors.

We need to bypass shortcuts when executing accessors defined via FunctionTemplate
if we have break points at function entry.

R=ishell@chromium.org, jgruber@chromium.org

Bug: v8:7596
Change-Id: I0e1bdbbba0f7dcd0fb7fe90d35b18234d073fe94
Reviewed-on: https://chromium-review.googlesource.com/980316
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52276}

src/builtins/builtins-api.cc

@@ -179,6 +179,25 @@ MaybeHandle<Object> Builtins::InvokeApiFunction(Isolate* isolate,
     }
   }
 
+  if (function->IsFunctionTemplateInfo()) {
+    Handle<FunctionTemplateInfo> info =
+        Handle<FunctionTemplateInfo>::cast(function);
+    // If we need to break at function entry, go the long way. Instantiate the
+    // function, use the DebugBreakTrampoline, and call it through JS.
+    if (info->BreakAtEntry()) {
+      DCHECK(!is_construct);
+      DCHECK(new_target->IsUndefined(isolate));
+      Handle<JSFunction> function;
+      ASSIGN_RETURN_ON_EXCEPTION(isolate, function,
+                                 ApiNatives::InstantiateFunction(
+                                     info, MaybeHandle<v8::internal::Name>()),
+                                 Object);
+      Handle<Code> trampoline = BUILTIN_CODE(isolate, DebugBreakTrampoline);
+      function->set_code(*trampoline);
+      return Execution::Call(isolate, function, receiver, argc, args);
+    }
+  }
+
   Handle<FunctionTemplateInfo> fun_data =
       function->IsFunctionTemplateInfo()
           ? Handle<FunctionTemplateInfo>::cast(function)

src/debug/debug.cc

@@ -1189,14 +1189,32 @@ void Debug::InstallDebugBreakTrampoline() {
   // be installed. If that's the case, iterate the heap for functions to rewire
   // to the trampoline.
   HandleScope scope(isolate_);
-  Handle<Code> trampoline = BUILTIN_CODE(isolate_, DebugBreakTrampoline);
-  std::vector<Handle<JSFunction>> needs_compile;
+  // If there is a breakpoint at function entry, we need to install trampoline.
+  bool needs_to_use_trampoline = false;
+  // If there we break at entry to an api callback, we need to clear ICs.
+  bool needs_to_clear_ic = false;
   for (DebugInfoListNode* current = debug_info_list_; current != nullptr;
        current = current->next()) {
     if (current->debug_info()->CanBreakAtEntry()) {
+      needs_to_use_trampoline = true;
+      if (current->debug_info()->shared()->IsApiFunction()) {
+        needs_to_clear_ic = true;
+        break;
+      }
+    }
+  }
+
+  if (!needs_to_use_trampoline) return;
+
+  Handle<Code> trampoline = BUILTIN_CODE(isolate_, DebugBreakTrampoline);
+  std::vector<Handle<JSFunction>> needs_compile;
+  {
     HeapIterator iterator(isolate_->heap());
     while (HeapObject* obj = iterator.next()) {
-        if (!obj->IsJSFunction()) continue;
+      if (needs_to_clear_ic && obj->IsFeedbackVector()) {
+        FeedbackVector::cast(obj)->ClearSlots(isolate_);
+        continue;
+      } else if (obj->IsJSFunction()) {
         JSFunction* fun = JSFunction::cast(obj);
         SharedFunctionInfo* shared = fun->shared();
         if (!shared->HasDebugInfo()) continue;
@@ -1207,7 +1225,6 @@ void Debug::InstallDebugBreakTrampoline() {
           fun->set_code(*trampoline);
         }
       }
-      break;
     }
   }
   // By overwriting the function code with DebugBreakTrampoline, which tailcalls

src/execution.cc

@@ -74,11 +74,13 @@ MUST_USE_RESULT MaybeHandle<Object> Invoke(
   }
 #endif
 
-  // api callbacks can be called directly.
+  // api callbacks can be called directly, unless we want to take the detour
+  // through JS to set up a frame for break-at-entry.
   if (target->IsJSFunction()) {
     Handle<JSFunction> function = Handle<JSFunction>::cast(target);
     if ((!is_construct || function->IsConstructor()) &&
-        function->shared()->IsApiFunction()) {
+        function->shared()->IsApiFunction() &&
+        !function->shared()->BreakAtEntry()) {
       SaveContext save(isolate);
       isolate->set_context(function->context());
       DCHECK(function->context()->global_object()->IsJSGlobalObject());

src/ic/ic.cc

@@ -820,15 +820,16 @@ Handle<Object> LoadIC::ComputeHandler(LookupIterator* lookup) {
           return ComputeHandler(lookup);
         }
 
-        // When debugging we need to go the slow path to flood the accessor.
-        if (GetHostFunction()->shared()->HasBreakInfo()) {
+        Handle<Object> getter(AccessorPair::cast(*accessors)->getter(),
+                              isolate());
+        if (!getter->IsJSFunction() && !getter->IsFunctionTemplateInfo()) {
           TRACE_HANDLER_STATS(isolate(), LoadIC_SlowStub);
           return slow_stub();
         }
 
-        Handle<Object> getter(AccessorPair::cast(*accessors)->getter(),
-                              isolate());
-        if (!getter->IsJSFunction() && !getter->IsFunctionTemplateInfo()) {
+        if (getter->IsFunctionTemplateInfo() &&
+            FunctionTemplateInfo::cast(*getter)->BreakAtEntry()) {
+          // Do not install an IC if the api function has a breakpoint.
           TRACE_HANDLER_STATS(isolate(), LoadIC_SlowStub);
           return slow_stub();
         }
@@ -1554,6 +1555,14 @@ Handle<Object> StoreIC::ComputeHandler(LookupIterator* lookup) {
           TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
           return slow_stub();
         }
+
+        if (setter->IsFunctionTemplateInfo() &&
+            FunctionTemplateInfo::cast(*setter)->BreakAtEntry()) {
+          // Do not install an IC if the api function has a breakpoint.
+          TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
+          return slow_stub();
+        }
+
         CallOptimization call_optimization(setter);
         if (call_optimization.is_simple_api_call()) {
           if (call_optimization.IsCompatibleReceiver(receiver, holder)) {

src/objects-inl.h

@@ -2327,6 +2327,15 @@ bool FunctionTemplateInfo::instantiated() {
   return shared_function_info()->IsSharedFunctionInfo();
 }
 
+bool FunctionTemplateInfo::BreakAtEntry() {
+  Object* maybe_shared = shared_function_info();
+  if (maybe_shared->IsSharedFunctionInfo()) {
+    SharedFunctionInfo* shared = SharedFunctionInfo::cast(maybe_shared);
+    return shared->BreakAtEntry();
+  }
+  return false;
+}
+
 FunctionTemplateInfo* FunctionTemplateInfo::GetParent(Isolate* isolate) {
   Object* parent = parent_template();
   return parent->IsUndefined(isolate) ? nullptr

src/objects.cc

@@ -13547,6 +13547,13 @@ bool SharedFunctionInfo::HasBreakInfo() const {
   return has_break_info;
 }
 
+bool SharedFunctionInfo::BreakAtEntry() const {
+  if (!HasDebugInfo()) return false;
+  DebugInfo* info = DebugInfo::cast(debug_info());
+  bool break_at_entry = info->BreakAtEntry();
+  return break_at_entry;
+}
+
 bool SharedFunctionInfo::HasCoverageInfo() const {
   if (!HasDebugInfo()) return false;
   DebugInfo* info = DebugInfo::cast(debug_info());

src/objects.h

@@ -4801,6 +4801,8 @@ class FunctionTemplateInfo: public TemplateInfo {
   bool IsTemplateFor(Map* map);
   inline bool instantiated();
 
+  inline bool BreakAtEntry();
+
   // Helper function for cached accessors.
   static MaybeHandle<Name> TryGetCachedPropertyName(Isolate* isolate,
                                                     Handle<Object> getter);

src/objects/shared-function-info.h

@@ -207,6 +207,7 @@ class SharedFunctionInfo : public HeapObject {
   // Break infos are contained in DebugInfo, this is a convenience method
   // to simplify access.
   bool HasBreakInfo() const;
+  bool BreakAtEntry() const;
 
   // Coverage infos are contained in DebugInfo, this is a convenience method
   // to simplify access.

src/runtime/runtime-debug.cc

@@ -86,7 +86,7 @@ RUNTIME_FUNCTION(Runtime_DebugBreakAtEntry) {
   // Get the top-most JavaScript frame.
   JavaScriptFrameIterator it(isolate);
   DCHECK_EQ(*function, it.frame()->function());
-  isolate->debug()->Break(it.frame(), handle(it.frame()->function()));
+  isolate->debug()->Break(it.frame(), function);
 
   return isolate->heap()->undefined_value();
 }

test/cctest/test-debug.cc

@@ -1488,6 +1488,192 @@ TEST(BreakPointApiFunction) {
   CheckDebuggerUnloaded();
 }
 
+void GetWrapperCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  args.GetReturnValue().Set(
+      args[0]
+          .As<v8::Object>()
+          ->Get(args.GetIsolate()->GetCurrentContext(), args[1])
+          .ToLocalChecked());
+}
+
+TEST(BreakPointApiGetter) {
+  DebugLocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+
+  SetDebugEventListener(env->GetIsolate(), DebugEventBreakPointHitCount);
+
+  i::Handle<i::BreakPoint> bp;
+
+  v8::Local<v8::FunctionTemplate> function_template =
+      v8::FunctionTemplate::New(env->GetIsolate(), NoOpFunctionCallback);
+  v8::Local<v8::FunctionTemplate> get_template =
+      v8::FunctionTemplate::New(env->GetIsolate(), GetWrapperCallback);
+
+  v8::Local<v8::Function> function =
+      function_template->GetFunction(env.context()).ToLocalChecked();
+  v8::Local<v8::Function> get =
+      get_template->GetFunction(env.context()).ToLocalChecked();
+
+  env->Global()->Set(env.context(), v8_str("f"), function).ToChecked();
+  env->Global()->Set(env.context(), v8_str("get_wrapper"), get).ToChecked();
+  CompileRun(
+      "var o = {};"
+      "Object.defineProperty(o, 'f', { get: f, enumerable: true });");
+
+  // === Test API builtin as getter ===
+  break_point_hit_count = 0;
+
+  // Run with breakpoint.
+  bp = SetBreakPoint(function, 0);
+  CompileRun("get_wrapper(o, 'f')");
+  CHECK_EQ(1, break_point_hit_count);
+
+  CompileRun("o.f");
+  CHECK_EQ(2, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  CompileRun("get_wrapper(o, 'f', 2)");
+  CHECK_EQ(2, break_point_hit_count);
+
+  SetDebugEventListener(env->GetIsolate(), nullptr);
+  CheckDebuggerUnloaded();
+}
+
+void SetWrapperCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  CHECK(args[0]
+            .As<v8::Object>()
+            ->Set(args.GetIsolate()->GetCurrentContext(), args[1], args[2])
+            .FromJust());
+}
+
+TEST(BreakPointApiSetter) {
+  DebugLocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+
+  SetDebugEventListener(env->GetIsolate(), DebugEventBreakPointHitCount);
+
+  i::Handle<i::BreakPoint> bp;
+
+  v8::Local<v8::FunctionTemplate> function_template =
+      v8::FunctionTemplate::New(env->GetIsolate(), NoOpFunctionCallback);
+  v8::Local<v8::FunctionTemplate> set_template =
+      v8::FunctionTemplate::New(env->GetIsolate(), SetWrapperCallback);
+
+  v8::Local<v8::Function> function =
+      function_template->GetFunction(env.context()).ToLocalChecked();
+  v8::Local<v8::Function> set =
+      set_template->GetFunction(env.context()).ToLocalChecked();
+
+  env->Global()->Set(env.context(), v8_str("f"), function).ToChecked();
+  env->Global()->Set(env.context(), v8_str("set_wrapper"), set).ToChecked();
+
+  CompileRun(
+      "var o = {};"
+      "Object.defineProperty(o, 'f', { set: f, enumerable: true });");
+
+  // === Test API builtin as setter ===
+  break_point_hit_count = 0;
+
+  // Run with breakpoint.
+  bp = SetBreakPoint(function, 0);
+
+  CompileRun("o.f = 3");
+  CHECK_EQ(1, break_point_hit_count);
+
+  CompileRun("set_wrapper(o, 'f', 2)");
+  CHECK_EQ(2, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  CompileRun("o.f = 3");
+  CHECK_EQ(2, break_point_hit_count);
+
+  // === Test API builtin as setter, with condition ===
+  break_point_hit_count = 0;
+
+  // Run with breakpoint.
+  bp = SetBreakPoint(function, 0, "arguments[0] == 3");
+  CompileRun("set_wrapper(o, 'f', 2)");
+  CHECK_EQ(0, break_point_hit_count);
+
+  CompileRun("set_wrapper(o, 'f', 3)");
+  CHECK_EQ(1, break_point_hit_count);
+
+  CompileRun("o.f = 3");
+  CHECK_EQ(2, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  CompileRun("set_wrapper(o, 'f', 2)");
+  CHECK_EQ(2, break_point_hit_count);
+
+  SetDebugEventListener(env->GetIsolate(), nullptr);
+  CheckDebuggerUnloaded();
+}
+
+TEST(BreakPointApiAccessor) {
+  DebugLocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+
+  SetDebugEventListener(env->GetIsolate(), DebugEventBreakPointHitCount);
+
+  i::Handle<i::BreakPoint> bp;
+
+  // Create 'foo' class, with a hidden property.
+  v8::Local<v8::ObjectTemplate> obj_template =
+      v8::ObjectTemplate::New(env->GetIsolate());
+  v8::Local<v8::FunctionTemplate> accessor_template =
+      v8::FunctionTemplate::New(env->GetIsolate(), NoOpFunctionCallback);
+  obj_template->SetAccessorProperty(v8_str("f"), accessor_template,
+                                    accessor_template);
+  v8::Local<v8::Object> obj =
+      obj_template->NewInstance(env.context()).ToLocalChecked();
+  env->Global()->Set(env.context(), v8_str("o"), obj).ToChecked();
+
+  v8::Local<v8::Function> function =
+      CompileRun("Object.getOwnPropertyDescriptor(o, 'f').set")
+          .As<v8::Function>();
+
+  // === Test API accessor ===
+  break_point_hit_count = 0;
+
+  CompileRun("function get_loop() { for (var i = 0; i < 10; i++) o.f }");
+  CompileRun("function set_loop() { for (var i = 0; i < 10; i++) o.f = 2 }");
+
+  CompileRun("get_loop(); set_loop();");  // Initialize ICs.
+
+  // Run with breakpoint.
+  bp = SetBreakPoint(function, 0);
+
+  CompileRun("o.f = 3");
+  CHECK_EQ(1, break_point_hit_count);
+
+  CompileRun("o.f");
+  CHECK_EQ(2, break_point_hit_count);
+
+  CompileRun("for (var i = 0; i < 10; i++) o.f");
+  CHECK_EQ(12, break_point_hit_count);
+
+  CompileRun("get_loop();");
+  CHECK_EQ(22, break_point_hit_count);
+
+  CompileRun("for (var i = 0; i < 10; i++) o.f = 2");
+  CHECK_EQ(32, break_point_hit_count);
+
+  CompileRun("set_loop();");
+  CHECK_EQ(42, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  CompileRun("o.f = 3");
+  CompileRun("o.f");
+  CHECK_EQ(42, break_point_hit_count);
+
+  SetDebugEventListener(env->GetIsolate(), nullptr);
+  CheckDebuggerUnloaded();
+}
+
 TEST(BreakPointInlineApiFunction) {
   i::FLAG_allow_natives_syntax = true;
   DebugLocalContext env;