From ef41de10db404a5be0957a2546b60ce7126ecff3 Mon Sep 17 00:00:00 2001 From: arv Date: Sat, 15 Nov 2014 11:48:32 -0800 Subject: [PATCH] Classes: Add support for stepping through default constructors If a class extends another class and it doesn't provide a constructor, one is created for them. We therefore need to ensure that stepping into the constructor steps into the super class constructor. BUG=v8:3674 LOG=Y R=dslomov@chromium.org, aandrey , yurys Review URL: https://codereview.chromium.org/725983002 Cr-Commit-Position: refs/heads/master@{#25366} --- src/debug.cc | 78 +++++++------- src/debug.h | 3 + .../harmony/debug-step-into-class-extends.js | 42 ++++++++ .../mjsunit/harmony/debug-step-into-constructor.js | 113 +++++++++++++++++++++ 4 files changed, 202 insertions(+), 34 deletions(-) create mode 100644 test/mjsunit/harmony/debug-step-into-class-extends.js create mode 100644 test/mjsunit/harmony/debug-step-into-constructor.js diff --git a/src/debug.cc b/src/debug.cc index 7fe9064..3029adb 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -1225,7 +1225,48 @@ void Debug::FloodBoundFunctionWithOneShot(Handle function) { if (!bindee.is_null() && bindee->IsJSFunction() && !JSFunction::cast(*bindee)->IsFromNativeScript()) { Handle bindee_function(JSFunction::cast(*bindee)); - FloodWithOneShot(bindee_function); + FloodWithOneShotGeneric(bindee_function); + } +} + + +void Debug::FloodDefaultConstructorWithOneShot(Handle function) { + DCHECK(function->shared()->is_default_constructor()); + // Instead of stepping into the function we directly step into the super class + // constructor. + Isolate* isolate = function->GetIsolate(); + PrototypeIterator iter(isolate, function); + Handle proto = PrototypeIterator::GetCurrent(iter); + if (!proto->IsJSFunction()) return; // Object.prototype + Handle function_proto = Handle::cast(proto); + FloodWithOneShotGeneric(function_proto); +} + + +void Debug::FloodWithOneShotGeneric(Handle function, + Handle holder) { + if (function->shared()->bound()) { + FloodBoundFunctionWithOneShot(function); + } else if (function->shared()->is_default_constructor()) { + FloodDefaultConstructorWithOneShot(function); + } else if (!function->IsFromNativeScript()) { + Isolate* isolate = function->GetIsolate(); + // Don't allow step into functions in the native context. + if (function->shared()->code() == + isolate->builtins()->builtin(Builtins::kFunctionApply) || + function->shared()->code() == + isolate->builtins()->builtin(Builtins::kFunctionCall)) { + // Handle function.apply and function.call separately to flood the + // function to be called and not the code for Builtins::FunctionApply or + // Builtins::FunctionCall. The receiver of call/apply is the target + // function. + if (!holder.is_null() && holder->IsJSFunction()) { + Handle js_function = Handle::cast(holder); + FloodWithOneShotGeneric(js_function); + } + } else { + FloodWithOneShot(function); + } } } @@ -1464,13 +1505,7 @@ void Debug::PrepareStep(StepAction step_action, if (fun->IsJSFunction()) { Handle js_function(JSFunction::cast(fun)); - if (js_function->shared()->bound()) { - FloodBoundFunctionWithOneShot(js_function); - } else if (!js_function->IsFromNativeScript()) { - // Don't step into builtins. - // It will also compile target function if it's not compiled yet. - FloodWithOneShot(js_function); - } + FloodWithOneShotGeneric(js_function); } } @@ -1612,32 +1647,7 @@ void Debug::HandleStepIn(Handle function_obj, Handle holder, // 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) { - if (function->shared()->bound()) { - // Handle Function.prototype.bind - FloodBoundFunctionWithOneShot(function); - } else if (!function->IsFromNativeScript()) { - // Don't allow step into functions in the native context. - if (function->shared()->code() == - isolate->builtins()->builtin(Builtins::kFunctionApply) || - function->shared()->code() == - isolate->builtins()->builtin(Builtins::kFunctionCall)) { - // Handle function.apply and function.call separately to flood the - // function to be called and not the code for Builtins::FunctionApply or - // Builtins::FunctionCall. The receiver of call/apply is the target - // function. - if (!holder.is_null() && holder->IsJSFunction()) { - Handle js_function = Handle::cast(holder); - if (!js_function->IsFromNativeScript()) { - FloodWithOneShot(js_function); - } else if (js_function->shared()->bound()) { - // Handle Function.prototype.bind - FloodBoundFunctionWithOneShot(js_function); - } - } - } else { - FloodWithOneShot(function); - } - } + FloodWithOneShotGeneric(function, holder); } } diff --git a/src/debug.h b/src/debug.h index e486d97..b8348fd 100644 --- a/src/debug.h +++ b/src/debug.h @@ -390,6 +390,9 @@ class Debug { void FloodWithOneShot(Handle function, BreakLocatorType type = ALL_BREAK_LOCATIONS); void FloodBoundFunctionWithOneShot(Handle function); + void FloodDefaultConstructorWithOneShot(Handle function); + void FloodWithOneShotGeneric(Handle function, + Handle holder = Handle()); void FloodHandlerWithOneShot(); void ChangeBreakOnException(ExceptionBreakType type, bool enable); bool IsBreakOnException(ExceptionBreakType type); diff --git a/test/mjsunit/harmony/debug-step-into-class-extends.js b/test/mjsunit/harmony/debug-step-into-class-extends.js new file mode 100644 index 0000000..ffc6fda --- /dev/null +++ b/test/mjsunit/harmony/debug-step-into-class-extends.js @@ -0,0 +1,42 @@ +// 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 --harmony-classes + +'use strict'; + +var Debug = debug.Debug + +var done = false; +var stepCount = 0; + +function listener(event, execState, eventData, data) { + if (event == Debug.DebugEvent.Break) { + if (!done) { + execState.prepareStep(Debug.StepAction.StepInto); + var s = execState.frame().sourceLineText(); + assertTrue(s.indexOf('// ' + stepCount + '.') !== -1); + stepCount++; + } + } +}; + +Debug.setListener(listener); + +function GetBase() { + var x = 1; // 1. + var y = 2; // 2. + done = true; // 3. + return null; +} + +function f() { + class Derived extends GetBase() {} // 0. +} + +var bp = Debug.setBreakPoint(f, 0); +f(); +assertEquals(4, stepCount); + +Debug.setListener(null); diff --git a/test/mjsunit/harmony/debug-step-into-constructor.js b/test/mjsunit/harmony/debug-step-into-constructor.js new file mode 100644 index 0000000..dbef60f --- /dev/null +++ b/test/mjsunit/harmony/debug-step-into-constructor.js @@ -0,0 +1,113 @@ +// 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 --harmony-classes + +'use strict'; + +var Debug = debug.Debug +var done, stepCount; + +function listener(event, execState, eventData, data) { + if (event == Debug.DebugEvent.Break) { + if (!done) { + execState.prepareStep(Debug.StepAction.StepInto); + var s = execState.frame().sourceLineText(); + assertTrue(s.indexOf('// ' + stepCount + '.') !== -1); + stepCount++; + } + } +}; + +Debug.setListener(listener); + + +class Base { + constructor() { + var x = 1; // 1. + var y = 2; // 2. + done = true; // 3. + } +} + +class Derived extends Base {} + + +(function TestBreakPointInConstructor() { + done = false; + stepCount = 1; + var bp = Debug.setBreakPoint(Base, 0); + + new Base(); + assertEquals(4, stepCount); + + Debug.clearBreakPoint(bp); +})(); + + +(function TestDefaultConstructor() { + done = false; + stepCount = 1; + + var bp = Debug.setBreakPoint(Base, 0); + new Derived(); + assertEquals(4, stepCount); + + Debug.clearBreakPoint(bp); +})(); + + +(function TestStepInto() { + done = false; + stepCount = 0; + + function f() { + new Derived(); // 0. + } + + var bp = Debug.setBreakPoint(f, 0); + f(); + assertEquals(4, stepCount); + + Debug.clearBreakPoint(bp); +})(); + + +(function TestExtraIndirection() { + done = false; + stepCount = 0; + + class Derived2 extends Derived {} + + function f() { + new Derived2(); // 0. + } + + var bp = Debug.setBreakPoint(f, 0); + f(); + assertEquals(4, stepCount); + + Debug.clearBreakPoint(bp); +})(); + + +(function TestBoundClass() { + done = false; + stepCount = 0; + + var bound = Derived.bind(null); + + function f() { + new bound(); // 0. + } + + var bp = Debug.setBreakPoint(f, 0); + f(); + assertEquals(4, stepCount); + + Debug.clearBreakPoint(bp); +})(); + + +Debug.setListener(null); -- 2.7.4