From aa75904e3c2072f6b0d6ed92f75d1fb729d7cefa Mon Sep 17 00:00:00 2001 From: yangguo Date: Thu, 12 Jan 2017 06:18:45 -0800 Subject: [PATCH] [debugger] infrastructure for side-effect-free debug-evaluate. R=jgruber@chromium.org, mstarzinger@chromium.org BUG=v8:5821 Review-Url: https://codereview.chromium.org/2622863003 Cr-Commit-Position: refs/heads/master@{#42270} --- src/api-arguments-inl.h | 20 +++ src/api-arguments.cc | 14 ++ src/api-arguments.h | 2 + src/arm/macro-assembler-arm.cc | 28 ++-- src/arm/macro-assembler-arm.h | 7 +- src/arm64/macro-assembler-arm64.cc | 26 +-- src/arm64/macro-assembler-arm64.h | 8 +- src/assembler.cc | 4 + src/assembler.h | 6 +- src/builtins/arm/builtins-arm.cc | 13 +- src/builtins/arm64/builtins-arm64.cc | 11 +- src/builtins/ia32/builtins-ia32.cc | 11 +- src/builtins/mips/builtins-mips.cc | 11 +- src/builtins/mips64/builtins-mips64.cc | 11 +- src/builtins/x64/builtins-x64.cc | 12 +- src/debug/arm/debug-arm.cc | 2 +- src/debug/arm64/debug-arm64.cc | 2 +- src/debug/debug-evaluate.cc | 152 +++++++++++++++++- src/debug/debug-evaluate.h | 3 + src/debug/debug.cc | 67 +++++++- src/debug/debug.h | 42 ++++- src/debug/ia32/debug-ia32.cc | 2 +- src/debug/mips/debug-mips.cc | 2 +- src/debug/mips64/debug-mips64.cc | 2 +- src/debug/x64/debug-x64.cc | 2 +- src/external-reference-table.cc | 2 + src/flag-definitions.h | 5 + src/ia32/macro-assembler-ia32.cc | 26 ++- src/ia32/macro-assembler-ia32.h | 7 +- src/isolate.h | 2 + src/messages.h | 1 + src/mips/macro-assembler-mips.cc | 26 ++- src/mips/macro-assembler-mips.h | 7 +- src/mips64/macro-assembler-mips64.cc | 26 ++- src/mips64/macro-assembler-mips64.h | 7 +- src/objects-inl.h | 4 + src/objects.cc | 11 ++ src/objects.h | 11 ++ src/runtime/runtime-debug.cc | 16 +- src/runtime/runtime.h | 2 +- src/x64/macro-assembler-x64.cc | 28 ++-- src/x64/macro-assembler-x64.h | 7 +- .../debug/debug-evaluate-no-side-effect.js | 83 ++++++++++ 43 files changed, 570 insertions(+), 161 deletions(-) create mode 100644 test/debugger/debug/debug-evaluate-no-side-effect.js diff --git a/src/api-arguments-inl.h b/src/api-arguments-inl.h index bf72fc4e6f..91ac253396 100644 --- a/src/api-arguments-inl.h +++ b/src/api-arguments-inl.h @@ -10,6 +10,14 @@ namespace v8 { namespace internal { +#define SIDE_EFFECT_CHECK(ISOLATE, F, RETURN_TYPE) \ + do { \ + if (ISOLATE->needs_side_effect_check() && \ + !PerformSideEffectCheck(ISOLATE, FUNCTION_ADDR(F))) { \ + return Handle(); \ + } \ + } while (false) + #define FOR_EACH_CALLBACK_TABLE_MAPPING_1_NAME(F) \ F(AccessorNameGetterCallback, "get", v8::Value, Object) \ F(GenericNamedPropertyQueryCallback, "has", v8::Integer, Object) \ @@ -19,6 +27,7 @@ namespace internal { Handle PropertyCallbackArguments::Call(Function f, \ Handle name) { \ Isolate* isolate = this->isolate(); \ + SIDE_EFFECT_CHECK(isolate, f, InternalReturn); \ RuntimeCallTimerScope timer(isolate, &RuntimeCallStats::Function); \ VMState state(isolate); \ ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f)); \ @@ -43,6 +52,7 @@ FOR_EACH_CALLBACK_TABLE_MAPPING_1_NAME(WRITE_CALL_1_NAME) Handle PropertyCallbackArguments::Call(Function f, \ uint32_t index) { \ Isolate* isolate = this->isolate(); \ + SIDE_EFFECT_CHECK(isolate, f, InternalReturn); \ RuntimeCallTimerScope timer(isolate, &RuntimeCallStats::Function); \ VMState state(isolate); \ ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f)); \ @@ -62,6 +72,7 @@ Handle PropertyCallbackArguments::Call( GenericNamedPropertySetterCallback f, Handle name, Handle value) { Isolate* isolate = this->isolate(); + SIDE_EFFECT_CHECK(isolate, f, Object); RuntimeCallTimerScope timer( isolate, &RuntimeCallStats::GenericNamedPropertySetterCallback); VMState state(isolate); @@ -77,6 +88,7 @@ Handle PropertyCallbackArguments::Call( GenericNamedPropertyDefinerCallback f, Handle name, const v8::PropertyDescriptor& desc) { Isolate* isolate = this->isolate(); + SIDE_EFFECT_CHECK(isolate, f, Object); RuntimeCallTimerScope timer( isolate, &RuntimeCallStats::GenericNamedPropertyDefinerCallback); VMState state(isolate); @@ -92,6 +104,7 @@ Handle PropertyCallbackArguments::Call(IndexedPropertySetterCallback f, uint32_t index, Handle value) { Isolate* isolate = this->isolate(); + SIDE_EFFECT_CHECK(isolate, f, Object); RuntimeCallTimerScope timer(isolate, &RuntimeCallStats::IndexedPropertySetterCallback); VMState state(isolate); @@ -107,6 +120,7 @@ Handle PropertyCallbackArguments::Call( IndexedPropertyDefinerCallback f, uint32_t index, const v8::PropertyDescriptor& desc) { Isolate* isolate = this->isolate(); + SIDE_EFFECT_CHECK(isolate, f, Object); RuntimeCallTimerScope timer( isolate, &RuntimeCallStats::IndexedPropertyDefinerCallback); VMState state(isolate); @@ -121,6 +135,10 @@ Handle PropertyCallbackArguments::Call( void PropertyCallbackArguments::Call(AccessorNameSetterCallback f, Handle name, Handle value) { Isolate* isolate = this->isolate(); + if (isolate->needs_side_effect_check() && + !PerformSideEffectCheck(isolate, FUNCTION_ADDR(f))) { + return; + } RuntimeCallTimerScope timer(isolate, &RuntimeCallStats::AccessorNameSetterCallback); VMState state(isolate); @@ -131,5 +149,7 @@ void PropertyCallbackArguments::Call(AccessorNameSetterCallback f, f(v8::Utils::ToLocal(name), v8::Utils::ToLocal(value), info); } +#undef SIDE_EFFECT_CHECK + } // namespace internal } // namespace v8 diff --git a/src/api-arguments.cc b/src/api-arguments.cc index 3baf9ea365..c7c54e5de1 100644 --- a/src/api-arguments.cc +++ b/src/api-arguments.cc @@ -4,6 +4,7 @@ #include "src/api-arguments.h" +#include "src/debug/debug.h" #include "src/objects-inl.h" #include "src/tracing/trace-event.h" #include "src/vm-state-inl.h" @@ -13,6 +14,10 @@ namespace internal { Handle FunctionCallbackArguments::Call(FunctionCallback f) { Isolate* isolate = this->isolate(); + if (isolate->needs_side_effect_check() && + !isolate->debug()->PerformSideEffectCheckForCallback(FUNCTION_ADDR(f))) { + return Handle(); + } RuntimeCallTimerScope timer(isolate, &RuntimeCallStats::FunctionCallback); VMState state(isolate); ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f)); @@ -24,6 +29,10 @@ Handle FunctionCallbackArguments::Call(FunctionCallback f) { Handle PropertyCallbackArguments::Call( IndexedPropertyEnumeratorCallback f) { Isolate* isolate = this->isolate(); + if (isolate->needs_side_effect_check() && + !isolate->debug()->PerformSideEffectCheckForCallback(FUNCTION_ADDR(f))) { + return Handle(); + } RuntimeCallTimerScope timer(isolate, &RuntimeCallStats::PropertyCallback); VMState state(isolate); ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f)); @@ -32,5 +41,10 @@ Handle PropertyCallbackArguments::Call( return GetReturnValue(isolate); } +bool PropertyCallbackArguments::PerformSideEffectCheck(Isolate* isolate, + Address function) { + return isolate->debug()->PerformSideEffectCheckForCallback(function); +} + } // namespace internal } // namespace v8 diff --git a/src/api-arguments.h b/src/api-arguments.h index d6d1b951af..6c9ad7ad6b 100644 --- a/src/api-arguments.h +++ b/src/api-arguments.h @@ -136,6 +136,8 @@ class PropertyCallbackArguments inline JSObject* holder() { return JSObject::cast(this->begin()[T::kHolderIndex]); } + + bool PerformSideEffectCheck(Isolate* isolate, Address function); }; class FunctionCallbackArguments diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc index e17dab0ae5..c3d825b87b 100644 --- a/src/arm/macro-assembler-arm.cc +++ b/src/arm/macro-assembler-arm.cc @@ -1758,18 +1758,16 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, } } - -void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual) { - Label skip_flooding; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(isolate()); - STATIC_ASSERT(StepFrame > StepIn); - mov(r4, Operand(last_step_action)); +void MacroAssembler::CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual) { + Label skip_hook; + ExternalReference debug_hook_avtive = + ExternalReference::debug_hook_on_function_call_address(isolate()); + mov(r4, Operand(debug_hook_avtive)); ldrsb(r4, MemOperand(r4)); - cmp(r4, Operand(StepIn)); - b(lt, &skip_flooding); + cmp(r4, Operand(0)); + b(eq, &skip_hook); { FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL); @@ -1786,7 +1784,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, } Push(fun); Push(fun); - CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + CallRuntime(Runtime::kDebugOnFunctionCall); Pop(fun); if (new_target.is_valid()) { Pop(new_target); @@ -1800,7 +1798,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, SmiUntag(expected.reg()); } } - bind(&skip_flooding); + bind(&skip_hook); } @@ -1814,8 +1812,8 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target, DCHECK(function.is(r1)); DCHECK_IMPLIES(new_target.is_valid(), new_target.is(r3)); - if (call_wrapper.NeedsDebugStepCheck()) { - FloodFunctionIfStepping(function, new_target, expected, actual); + if (call_wrapper.NeedsDebugHookCheck()) { + CheckDebugHook(function, new_target, expected, actual); } // Clear the new.target register if not given. diff --git a/src/arm/macro-assembler-arm.h b/src/arm/macro-assembler-arm.h index 61a4723131..1f537b0157 100644 --- a/src/arm/macro-assembler-arm.h +++ b/src/arm/macro-assembler-arm.h @@ -681,9 +681,10 @@ class MacroAssembler: public Assembler { const ParameterCount& actual, InvokeFlag flag, const CallWrapper& call_wrapper); - void FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual); + // On function call, call into the debugger if necessary. + void CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual); // Invoke the JavaScript function in the given register. Changes the // current context to the context in the function before invoking. diff --git a/src/arm64/macro-assembler-arm64.cc b/src/arm64/macro-assembler-arm64.cc index 9badac13bb..896b4f9a44 100644 --- a/src/arm64/macro-assembler-arm64.cc +++ b/src/arm64/macro-assembler-arm64.cc @@ -2366,17 +2366,15 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, Bind(®ular_invoke); } - -void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual) { - Label skip_flooding; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(isolate()); - STATIC_ASSERT(StepFrame > StepIn); - Mov(x4, Operand(last_step_action)); +void MacroAssembler::CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual) { + Label skip_hook; + ExternalReference debug_hook_active = + ExternalReference::debug_hook_on_function_call_address(isolate()); + Mov(x4, Operand(debug_hook_active)); Ldrsb(x4, MemOperand(x4)); - CompareAndBranch(x4, Operand(StepIn), lt, &skip_flooding); + CompareAndBranch(x4, Operand(0), eq, &skip_hook); { FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL); @@ -2393,7 +2391,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, } Push(fun); Push(fun); - CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + CallRuntime(Runtime::kDebugOnFunctionCall); Pop(fun); if (new_target.is_valid()) { Pop(new_target); @@ -2407,7 +2405,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, SmiUntag(expected.reg()); } } - bind(&skip_flooding); + bind(&skip_hook); } @@ -2421,7 +2419,9 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target, DCHECK(function.is(x1)); DCHECK_IMPLIES(new_target.is_valid(), new_target.is(x3)); - FloodFunctionIfStepping(function, new_target, expected, actual); + if (call_wrapper.NeedsDebugHookCheck()) { + CheckDebugHook(function, new_target, expected, actual); + } // Clear the new.target register if not given. if (!new_target.is_valid()) { diff --git a/src/arm64/macro-assembler-arm64.h b/src/arm64/macro-assembler-arm64.h index f7f696ba7b..0bd5c64769 100644 --- a/src/arm64/macro-assembler-arm64.h +++ b/src/arm64/macro-assembler-arm64.h @@ -1209,9 +1209,11 @@ class MacroAssembler : public Assembler { InvokeFlag flag, bool* definitely_mismatches, const CallWrapper& call_wrapper); - void FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual); + + // On function call, call into the debugger if necessary. + void CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual); void InvokeFunctionCode(Register function, Register new_target, const ParameterCount& expected, const ParameterCount& actual, InvokeFlag flag, diff --git a/src/assembler.cc b/src/assembler.cc index f390b7443b..a4d97ec3e6 100644 --- a/src/assembler.cc +++ b/src/assembler.cc @@ -1584,6 +1584,10 @@ ExternalReference ExternalReference::debug_is_active_address( return ExternalReference(isolate->debug()->is_active_address()); } +ExternalReference ExternalReference::debug_hook_on_function_call_address( + Isolate* isolate) { + return ExternalReference(isolate->debug()->hook_on_function_call_address()); +} ExternalReference ExternalReference::debug_after_break_target_address( Isolate* isolate) { diff --git a/src/assembler.h b/src/assembler.h index a5be37eb29..cd5867689e 100644 --- a/src/assembler.h +++ b/src/assembler.h @@ -1061,6 +1061,8 @@ class ExternalReference BASE_EMBEDDED { Isolate* isolate); static ExternalReference debug_is_active_address(Isolate* isolate); + static ExternalReference debug_hook_on_function_call_address( + Isolate* isolate); static ExternalReference debug_after_break_target_address(Isolate* isolate); static ExternalReference is_profiling_address(Isolate* isolate); @@ -1167,7 +1169,7 @@ class CallWrapper { // Called just after emitting a call, i.e., at the return site for the call. virtual void AfterCall() const = 0; // Return whether call needs to check for debug stepping. - virtual bool NeedsDebugStepCheck() const { return false; } + virtual bool NeedsDebugHookCheck() const { return false; } }; @@ -1186,7 +1188,7 @@ class CheckDebugStepCallWrapper : public CallWrapper { virtual ~CheckDebugStepCallWrapper() {} virtual void BeforeCall(int call_size) const {} virtual void AfterCall() const {} - virtual bool NeedsDebugStepCheck() const { return true; } + virtual bool NeedsDebugHookCheck() const { return true; } }; diff --git a/src/builtins/arm/builtins-arm.cc b/src/builtins/arm/builtins-arm.cc index 4097de8d26..7964fa8e4b 100644 --- a/src/builtins/arm/builtins-arm.cc +++ b/src/builtins/arm/builtins-arm.cc @@ -743,13 +743,12 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { // Flood function if we are stepping. Label prepare_step_in_if_stepping, prepare_step_in_suspended_generator; Label stepping_prepared; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(masm->isolate()); - STATIC_ASSERT(StepFrame > StepIn); - __ mov(ip, Operand(last_step_action)); + ExternalReference debug_hook = + ExternalReference::debug_hook_on_function_call_address(masm->isolate()); + __ mov(ip, Operand(debug_hook)); __ ldrsb(ip, MemOperand(ip)); - __ cmp(ip, Operand(StepIn)); - __ b(ge, &prepare_step_in_if_stepping); + __ cmp(ip, Operand(0)); + __ b(ne, &prepare_step_in_if_stepping); // Flood function if we need to continue stepping in the suspended generator. ExternalReference debug_suspended_generator = @@ -817,7 +816,7 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { { FrameAndConstantPoolScope scope(masm, StackFrame::INTERNAL); __ Push(r1, r2, r4); - __ CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + __ CallRuntime(Runtime::kDebugOnFunctionCall); __ Pop(r1, r2); __ ldr(r4, FieldMemOperand(r1, JSGeneratorObject::kFunctionOffset)); } diff --git a/src/builtins/arm64/builtins-arm64.cc b/src/builtins/arm64/builtins-arm64.cc index 5b75d52eb5..74a1685dd3 100644 --- a/src/builtins/arm64/builtins-arm64.cc +++ b/src/builtins/arm64/builtins-arm64.cc @@ -750,12 +750,11 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { // Flood function if we are stepping. Label prepare_step_in_if_stepping, prepare_step_in_suspended_generator; Label stepping_prepared; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(masm->isolate()); - STATIC_ASSERT(StepFrame > StepIn); - __ Mov(x10, Operand(last_step_action)); + ExternalReference debug_hook = + ExternalReference::debug_hook_on_function_call_address(masm->isolate()); + __ Mov(x10, Operand(debug_hook)); __ Ldrsb(x10, MemOperand(x10)); - __ CompareAndBranch(x10, Operand(StepIn), ge, &prepare_step_in_if_stepping); + __ CompareAndBranch(x10, Operand(0), ne, &prepare_step_in_if_stepping); // Flood function if we need to continue stepping in the suspended generator. ExternalReference debug_suspended_generator = @@ -815,7 +814,7 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { { FrameScope scope(masm, StackFrame::INTERNAL); __ Push(x1, x2, x4); - __ CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + __ CallRuntime(Runtime::kDebugOnFunctionCall); __ Pop(x2, x1); __ Ldr(x4, FieldMemOperand(x1, JSGeneratorObject::kFunctionOffset)); } diff --git a/src/builtins/ia32/builtins-ia32.cc b/src/builtins/ia32/builtins-ia32.cc index f95e5a5563..61b547a88b 100644 --- a/src/builtins/ia32/builtins-ia32.cc +++ b/src/builtins/ia32/builtins-ia32.cc @@ -392,11 +392,10 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { // Flood function if we are stepping. Label prepare_step_in_if_stepping, prepare_step_in_suspended_generator; Label stepping_prepared; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(masm->isolate()); - STATIC_ASSERT(StepFrame > StepIn); - __ cmpb(Operand::StaticVariable(last_step_action), Immediate(StepIn)); - __ j(greater_equal, &prepare_step_in_if_stepping); + ExternalReference debug_hook = + ExternalReference::debug_hook_on_function_call_address(masm->isolate()); + __ cmpb(Operand::StaticVariable(debug_hook), Immediate(0)); + __ j(not_equal, &prepare_step_in_if_stepping); // Flood function if we need to continue stepping in the suspended generator. ExternalReference debug_suspended_generator = @@ -464,7 +463,7 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { __ Push(ebx); __ Push(edx); __ Push(edi); - __ CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + __ CallRuntime(Runtime::kDebugOnFunctionCall); __ Pop(edx); __ Pop(ebx); __ mov(edi, FieldOperand(ebx, JSGeneratorObject::kFunctionOffset)); diff --git a/src/builtins/mips/builtins-mips.cc b/src/builtins/mips/builtins-mips.cc index 2515a3e9b8..bdc2135c69 100644 --- a/src/builtins/mips/builtins-mips.cc +++ b/src/builtins/mips/builtins-mips.cc @@ -870,12 +870,11 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { // Flood function if we are stepping. Label prepare_step_in_if_stepping, prepare_step_in_suspended_generator; Label stepping_prepared; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(masm->isolate()); - STATIC_ASSERT(StepFrame > StepIn); - __ li(t1, Operand(last_step_action)); + ExternalReference debug_hook = + ExternalReference::debug_hook_on_function_call_address(masm->isolate()); + __ li(t1, Operand(debug_hook)); __ lb(t1, MemOperand(t1)); - __ Branch(&prepare_step_in_if_stepping, ge, t1, Operand(StepIn)); + __ Branch(&prepare_step_in_if_stepping, ne, t1, Operand(zero_reg)); // Flood function if we need to continue stepping in the suspended generator. ExternalReference debug_suspended_generator = @@ -942,7 +941,7 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { { FrameScope scope(masm, StackFrame::INTERNAL); __ Push(a1, a2, t0); - __ CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + __ CallRuntime(Runtime::kDebugOnFunctionCall); __ Pop(a1, a2); } __ Branch(USE_DELAY_SLOT, &stepping_prepared); diff --git a/src/builtins/mips64/builtins-mips64.cc b/src/builtins/mips64/builtins-mips64.cc index bec23d1fa5..1473d711c1 100644 --- a/src/builtins/mips64/builtins-mips64.cc +++ b/src/builtins/mips64/builtins-mips64.cc @@ -746,12 +746,11 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { // Flood function if we are stepping. Label prepare_step_in_if_stepping, prepare_step_in_suspended_generator; Label stepping_prepared; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(masm->isolate()); - STATIC_ASSERT(StepFrame > StepIn); - __ li(a5, Operand(last_step_action)); + ExternalReference debug_hook = + ExternalReference::debug_hook_on_function_call_address(masm->isolate()); + __ li(a5, Operand(debug_hook)); __ lb(a5, MemOperand(a5)); - __ Branch(&prepare_step_in_if_stepping, ge, a5, Operand(StepIn)); + __ Branch(&prepare_step_in_if_stepping, ne, a5, Operand(zero_reg)); // Flood function if we need to continue stepping in the suspended generator. ExternalReference debug_suspended_generator = @@ -817,7 +816,7 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { { FrameScope scope(masm, StackFrame::INTERNAL); __ Push(a1, a2, a4); - __ CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + __ CallRuntime(Runtime::kDebugOnFunctionCall); __ Pop(a1, a2); } __ Branch(USE_DELAY_SLOT, &stepping_prepared); diff --git a/src/builtins/x64/builtins-x64.cc b/src/builtins/x64/builtins-x64.cc index 056dcd06f3..127d13efb6 100644 --- a/src/builtins/x64/builtins-x64.cc +++ b/src/builtins/x64/builtins-x64.cc @@ -464,12 +464,12 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { // Flood function if we are stepping. Label prepare_step_in_if_stepping, prepare_step_in_suspended_generator; Label stepping_prepared; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(masm->isolate()); - Operand last_step_action_operand = masm->ExternalOperand(last_step_action); + ExternalReference debug_hook = + ExternalReference::debug_hook_on_function_call_address(masm->isolate()); + Operand debug_hook_operand = masm->ExternalOperand(debug_hook); STATIC_ASSERT(StepFrame > StepIn); - __ cmpb(last_step_action_operand, Immediate(StepIn)); - __ j(greater_equal, &prepare_step_in_if_stepping); + __ cmpb(debug_hook_operand, Immediate(0)); + __ j(not_equal, &prepare_step_in_if_stepping); // Flood function if we need to continue stepping in the suspended generator. ExternalReference debug_suspended_generator = @@ -539,7 +539,7 @@ void Builtins::Generate_ResumeGeneratorTrampoline(MacroAssembler* masm) { __ Push(rbx); __ Push(rdx); __ Push(rdi); - __ CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + __ CallRuntime(Runtime::kDebugOnFunctionCall); __ Pop(rdx); __ Pop(rbx); __ movp(rdi, FieldOperand(rbx, JSGeneratorObject::kFunctionOffset)); diff --git a/src/debug/arm/debug-arm.cc b/src/debug/arm/debug-arm.cc index d96ec31bfa..145f371f99 100644 --- a/src/debug/arm/debug-arm.cc +++ b/src/debug/arm/debug-arm.cc @@ -136,7 +136,7 @@ void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { __ LeaveFrame(StackFrame::INTERNAL); ParameterCount dummy(0); - __ FloodFunctionIfStepping(r1, no_reg, dummy, dummy); + __ CheckDebugHook(r1, no_reg, dummy, dummy); { ConstantPoolUnavailableScope constant_pool_unavailable(masm); // Load context from the function. diff --git a/src/debug/arm64/debug-arm64.cc b/src/debug/arm64/debug-arm64.cc index e344924a61..75eb2837c2 100644 --- a/src/debug/arm64/debug-arm64.cc +++ b/src/debug/arm64/debug-arm64.cc @@ -147,7 +147,7 @@ void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { __ Pop(fp, lr); // Frame, Return address. ParameterCount dummy(0); - __ FloodFunctionIfStepping(x1, no_reg, dummy, dummy); + __ CheckDebugHook(x1, no_reg, dummy, dummy); UseScratchRegisterScope temps(masm); Register scratch = temps.AcquireX(); diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc index e93b5952c8..8f068284a7 100644 --- a/src/debug/debug-evaluate.cc +++ b/src/debug/debug-evaluate.cc @@ -12,6 +12,8 @@ #include "src/debug/debug.h" #include "src/frames-inl.h" #include "src/globals.h" +#include "src/interpreter/bytecode-array-iterator.h" +#include "src/interpreter/bytecodes.h" #include "src/isolate-inl.h" namespace v8 { @@ -92,9 +94,13 @@ MaybeHandle DebugEvaluate::Evaluate( Object); Handle result; - ASSIGN_RETURN_ON_EXCEPTION( - isolate, result, Execution::Call(isolate, eval_fun, receiver, 0, NULL), - Object); + { + NoSideEffectScope no_side_effect(isolate, + FLAG_side_effect_free_debug_evaluate); + ASSIGN_RETURN_ON_EXCEPTION( + isolate, result, Execution::Call(isolate, eval_fun, receiver, 0, NULL), + Object); + } // Skip the global proxy as it has no properties and always delegates to the // real global object. @@ -249,5 +255,145 @@ void DebugEvaluate::ContextBuilder::MaterializeReceiver( JSObject::SetOwnPropertyIgnoreAttributes(target, name, recv, NONE).Check(); } +namespace { + +bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) { + DCHECK_EQ(Runtime::INLINE, Runtime::FunctionForId(id)->intrinsic_type); + switch (id) { + // Whitelist for intrinsics. + case Runtime::kInlineToObject: + return true; + default: + if (FLAG_trace_side_effect_free_debug_evaluate) { + PrintF("[debug-evaluate] intrinsic %s may cause side effect.\n", + Runtime::FunctionForId(id)->name); + } + return false; + } +} + +bool RuntimeFunctionHasNoSideEffect(Runtime::FunctionId id) { + DCHECK_EQ(Runtime::RUNTIME, Runtime::FunctionForId(id)->intrinsic_type); + switch (id) { + // Whitelist for runtime functions. + case Runtime::kToObject: + case Runtime::kLoadLookupSlotForCall: + case Runtime::kThrowReferenceError: + return true; + default: + if (FLAG_trace_side_effect_free_debug_evaluate) { + PrintF("[debug-evaluate] runtime %s may cause side effect.\n", + Runtime::FunctionForId(id)->name); + } + return false; + } +} + +bool BytecodeHasNoSideEffect(interpreter::Bytecode bytecode) { + typedef interpreter::Bytecode Bytecode; + typedef interpreter::Bytecodes Bytecodes; + if (Bytecodes::IsWithoutExternalSideEffects(bytecode)) return true; + if (Bytecodes::IsCallOrNew(bytecode)) return true; + switch (bytecode) { + // Whitelist for bytecodes. + case Bytecode::kStackCheck: + case Bytecode::kLdaLookupSlot: + case Bytecode::kLdaGlobal: + case Bytecode::kLdaNamedProperty: + case Bytecode::kLdaKeyedProperty: + case Bytecode::kAdd: + case Bytecode::kReturn: + case Bytecode::kCreateCatchContext: + case Bytecode::kSetPendingMessage: + return true; + default: + if (FLAG_trace_side_effect_free_debug_evaluate) { + PrintF("[debug-evaluate] bytecode %s may cause side effect.\n", + Bytecodes::ToString(bytecode)); + } + return false; + } +} + +bool BuiltinHasNoSideEffect(Builtins::Name id) { + switch (id) { + // Whitelist for builtins. + case Builtins::kMathSin: + return true; + default: + if (FLAG_trace_side_effect_free_debug_evaluate) { + PrintF("[debug-evaluate] built-in %s may cause side effect.\n", + Builtins::name(id)); + } + return false; + } +} + +static const Address accessors_with_no_side_effect[] = { + // Whitelist for accessors. + FUNCTION_ADDR(Accessors::StringLengthGetter), + FUNCTION_ADDR(Accessors::ArrayLengthGetter)}; + +} // anonymous namespace + +// static +bool DebugEvaluate::FunctionHasNoSideEffect(Handle info) { + if (FLAG_trace_side_effect_free_debug_evaluate) { + PrintF("[debug-evaluate] Checking function %s for side effect.\n", + info->DebugName()->ToCString().get()); + } + + DCHECK(info->is_compiled()); + + if (info->HasBytecodeArray()) { + // Check bytecodes against whitelist. + Handle bytecode_array(info->bytecode_array()); + if (FLAG_trace_side_effect_free_debug_evaluate) bytecode_array->Print(); + for (interpreter::BytecodeArrayIterator it(bytecode_array); !it.done(); + it.Advance()) { + interpreter::Bytecode bytecode = it.current_bytecode(); + + if (interpreter::Bytecodes::IsCallRuntime(bytecode)) { + if (bytecode == interpreter::Bytecode::kInvokeIntrinsic) { + Runtime::FunctionId id = it.GetIntrinsicIdOperand(0); + if (IntrinsicHasNoSideEffect(id)) continue; + } else { + Runtime::FunctionId id = it.GetRuntimeIdOperand(0); + if (RuntimeFunctionHasNoSideEffect(id)) continue; + } + return false; + } + + if (BytecodeHasNoSideEffect(bytecode)) continue; + + // Did not match whitelist. + return false; + } + return true; + } else { + // Check built-ins against whitelist. + int builtin_index = info->code()->builtin_index(); + if (builtin_index >= 0 && builtin_index < Builtins::builtin_count && + BuiltinHasNoSideEffect(static_cast(builtin_index))) { + return true; + } + } + + return false; +} + +// static +bool DebugEvaluate::CallbackHasNoSideEffect(Address function_addr) { + for (size_t i = 0; i < arraysize(accessors_with_no_side_effect); i++) { + if (function_addr == accessors_with_no_side_effect[i]) return true; + } + + if (FLAG_trace_side_effect_free_debug_evaluate) { + PrintF("[debug-evaluate] API Callback at %p may cause side effect.\n", + reinterpret_cast(function_addr)); + } + return false; +} + } // namespace internal } // namespace v8 diff --git a/src/debug/debug-evaluate.h b/src/debug/debug-evaluate.h index 6b0f48a15d..3b4d1f4640 100644 --- a/src/debug/debug-evaluate.h +++ b/src/debug/debug-evaluate.h @@ -24,6 +24,9 @@ class DebugEvaluate : public AllStatic { int inlined_jsframe_index, Handle source); + static bool FunctionHasNoSideEffect(Handle info); + static bool CallbackHasNoSideEffect(Address function_addr); + private: // This class builds a context chain for evaluation of expressions // in debugger. diff --git a/src/debug/debug.cc b/src/debug/debug.cc index aec4814b62..50c3ec8cef 100644 --- a/src/debug/debug.cc +++ b/src/debug/debug.cc @@ -14,6 +14,7 @@ #include "src/compilation-cache.h" #include "src/compiler-dispatcher/optimizing-compile-dispatcher.h" #include "src/compiler.h" +#include "src/debug/debug-evaluate.h" #include "src/debug/liveedit.h" #include "src/deoptimizer.h" #include "src/execution.h" @@ -42,6 +43,7 @@ Debug::Debug(Isolate* isolate) command_received_(0), command_queue_(isolate->logger(), kQueueInitialSize), is_active_(false), + hook_on_function_call_(false), is_suppressed_(false), live_edit_enabled_(true), // TODO(yangguo): set to false by default. break_disabled_(false), @@ -49,6 +51,7 @@ Debug::Debug(Isolate* isolate) in_debug_event_listener_(false), break_on_exception_(false), break_on_uncaught_exception_(false), + side_effect_check_failed_(false), debug_info_list_(NULL), feature_tracker_(isolate), isolate_(isolate) { @@ -406,6 +409,7 @@ void Debug::ThreadInit() { // TODO(isolates): frames_are_dropped_? base::NoBarrier_Store(&thread_local_.current_debug_scope_, static_cast(0)); + UpdateHookOnFunctionCall(); } @@ -906,16 +910,19 @@ bool Debug::IsBreakOnException(ExceptionBreakType type) { void Debug::PrepareStepIn(Handle function) { CHECK(last_step_action() >= StepIn); - if (!is_active()) return; + if (ignore_events()) return; if (in_debug_scope()) return; + if (break_disabled()) return; FloodWithOneShot(function); } void Debug::PrepareStepInSuspendedGenerator() { CHECK(has_suspended_generator()); - if (!is_active()) return; + if (ignore_events()) return; if (in_debug_scope()) return; + if (break_disabled()) return; thread_local_.last_step_action_ = StepIn; + UpdateHookOnFunctionCall(); Handle function( JSGeneratorObject::cast(thread_local_.suspended_generator_)->function()); FloodWithOneShot(function); @@ -923,9 +930,10 @@ void Debug::PrepareStepInSuspendedGenerator() { } void Debug::PrepareStepOnThrow() { - if (!is_active()) return; if (last_step_action() == StepNone) return; + if (ignore_events()) return; if (in_debug_scope()) return; + if (break_disabled()) return; ClearOneShot(); @@ -976,6 +984,7 @@ void Debug::PrepareStep(StepAction step_action) { feature_tracker()->Track(DebugFeatureTracker::kStepping); thread_local_.last_step_action_ = step_action; + UpdateHookOnFunctionCall(); // If the function on the top frame is unresolved perform step out. This will // be the case when calling unknown function and having the debugger stopped @@ -1106,6 +1115,7 @@ void Debug::ClearStepping() { thread_local_.last_statement_position_ = kNoSourcePosition; thread_local_.last_fp_ = 0; thread_local_.target_fp_ = 0; + UpdateHookOnFunctionCall(); } @@ -2136,6 +2146,13 @@ void Debug::UpdateState() { is_active_ = is_active; } +void Debug::UpdateHookOnFunctionCall() { + STATIC_ASSERT(StepFrame > StepIn); + STATIC_ASSERT(LastStepAction == StepFrame); + hook_on_function_call_ = thread_local_.last_step_action_ >= StepIn || + isolate_->needs_side_effect_check(); +} + // Calls the registered debug message handler. This callback is part of the // public API. void Debug::InvokeMessageHandler(MessageImpl message) { @@ -2333,6 +2350,50 @@ DebugScope::~DebugScope() { debug_->UpdateState(); } +bool Debug::PerformSideEffectCheck(Handle function) { + DCHECK(isolate_->needs_side_effect_check()); + DisallowJavascriptExecution no_js(isolate_); + if (!Compiler::Compile(function, Compiler::KEEP_EXCEPTION)) return false; + Deoptimizer::DeoptimizeFunction(*function); + if (!function->shared()->HasNoSideEffect()) { + if (FLAG_trace_side_effect_free_debug_evaluate) { + PrintF("[debug-evaluate] Function %s failed side effect check.\n", + function->shared()->DebugName()->ToCString().get()); + } + side_effect_check_failed_ = true; + // Throw an uncatchable termination exception. + isolate_->TerminateExecution(); + return false; + } + return true; +} + +bool Debug::PerformSideEffectCheckForCallback(Address function) { + DCHECK(isolate_->needs_side_effect_check()); + if (DebugEvaluate::CallbackHasNoSideEffect(function)) return true; + side_effect_check_failed_ = true; + // Throw an uncatchable termination exception. + isolate_->TerminateExecution(); + isolate_->OptionalRescheduleException(false); + return false; +} + +NoSideEffectScope::~NoSideEffectScope() { + if (isolate_->needs_side_effect_check() && + isolate_->debug()->side_effect_check_failed_) { + DCHECK(isolate_->has_pending_exception()); + DCHECK_EQ(isolate_->heap()->termination_exception(), + isolate_->pending_exception()); + // Convert the termination exception into a regular exception. + isolate_->CancelTerminateExecution(); + isolate_->Throw(*isolate_->factory()->NewEvalError( + MessageTemplate::kNoSideEffectDebugEvaluate)); + } + isolate_->set_needs_side_effect_check(old_needs_side_effect_check_); + isolate_->debug()->UpdateHookOnFunctionCall(); + isolate_->debug()->side_effect_check_failed_ = false; +} + MessageImpl MessageImpl::NewEvent(DebugEvent event, bool running, Handle exec_state, Handle event_data) { diff --git a/src/debug/debug.h b/src/debug/debug.h index 4f32857c8a..6eb8b58623 100644 --- a/src/debug/debug.h +++ b/src/debug/debug.h @@ -514,6 +514,9 @@ class Debug { return is_active() && !debug_context().is_null() && break_id() != 0; } + bool PerformSideEffectCheck(Handle function); + bool PerformSideEffectCheckForCallback(Address function); + // Flags and states. DebugScope* debugger_entry() { return reinterpret_cast( @@ -547,6 +550,10 @@ class Debug { return reinterpret_cast
(&is_active_); } + Address hook_on_function_call_address() { + return reinterpret_cast
(&hook_on_function_call_); + } + Address after_break_target_address() { return reinterpret_cast
(&after_break_target_); } @@ -567,6 +574,7 @@ class Debug { explicit Debug(Isolate* isolate); void UpdateState(); + void UpdateHookOnFunctionCall(); void Unload(); void SetNextBreakId() { thread_local_.break_id_ = ++thread_local_.break_count_; @@ -662,16 +670,30 @@ class Debug { base::Semaphore command_received_; // Signaled for each command received. LockingCommandMessageQueue command_queue_; + // Debugger is active, i.e. there is a debug event listener attached. bool is_active_; + // Debugger needs to be notified on every new function call. + // Used for stepping and read-only checks + bool hook_on_function_call_; + // Suppress debug events. bool is_suppressed_; + // LiveEdit is enabled. bool live_edit_enabled_; + // Do not trigger debug break events. bool break_disabled_; + // Do not break on break points. bool break_points_active_; + // Nested inside a debug event listener. bool in_debug_event_listener_; + // Trigger debug break events for all exceptions. bool break_on_exception_; + // Trigger debug break events for uncaught exceptions. bool break_on_uncaught_exception_; + // Termination exception because side effect check has failed. + bool side_effect_check_failed_; - DebugInfoListNode* debug_info_list_; // List of active debug info objects. + // List of active debug info objects. + DebugInfoListNode* debug_info_list_; // Storage location for jump when exiting debug break calls. // Note that this address is not GC safe. It should be computed immediately @@ -731,6 +753,7 @@ class Debug { friend class DisableBreak; friend class LiveEdit; friend class SuppressDebug; + friend class NoSideEffectScope; friend Handle GetDebuggedFunctions(); // In test-debug.cc friend void CheckDebuggerUnloaded(bool check_functions); // In test-debug.cc @@ -804,6 +827,23 @@ class SuppressDebug BASE_EMBEDDED { DISALLOW_COPY_AND_ASSIGN(SuppressDebug); }; +class NoSideEffectScope { + public: + NoSideEffectScope(Isolate* isolate, bool disallow_side_effects) + : isolate_(isolate), + old_needs_side_effect_check_(isolate->needs_side_effect_check()) { + isolate->set_needs_side_effect_check(old_needs_side_effect_check_ || + disallow_side_effects); + isolate->debug()->UpdateHookOnFunctionCall(); + isolate->debug()->side_effect_check_failed_ = false; + } + ~NoSideEffectScope(); + + private: + Isolate* isolate_; + bool old_needs_side_effect_check_; + DISALLOW_COPY_AND_ASSIGN(NoSideEffectScope); +}; // Code generator routines. class DebugCodegen : public AllStatic { diff --git a/src/debug/ia32/debug-ia32.cc b/src/debug/ia32/debug-ia32.cc index 47ec69ec5b..1e0ee750ca 100644 --- a/src/debug/ia32/debug-ia32.cc +++ b/src/debug/ia32/debug-ia32.cc @@ -129,7 +129,7 @@ void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { __ pop(ebp); ParameterCount dummy(0); - __ FloodFunctionIfStepping(edi, no_reg, dummy, dummy); + __ CheckDebugHook(edi, no_reg, dummy, dummy); // Load context from the function. __ mov(esi, FieldOperand(edi, JSFunction::kContextOffset)); diff --git a/src/debug/mips/debug-mips.cc b/src/debug/mips/debug-mips.cc index 4d8b54f4b6..c6daf2d226 100644 --- a/src/debug/mips/debug-mips.cc +++ b/src/debug/mips/debug-mips.cc @@ -130,7 +130,7 @@ void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { __ LeaveFrame(StackFrame::INTERNAL); ParameterCount dummy(0); - __ FloodFunctionIfStepping(a1, no_reg, dummy, dummy); + __ CheckDebugHook(a1, no_reg, dummy, dummy); // Load context from the function. __ lw(cp, FieldMemOperand(a1, JSFunction::kContextOffset)); diff --git a/src/debug/mips64/debug-mips64.cc b/src/debug/mips64/debug-mips64.cc index 2a6ce7b5cd..0230f13145 100644 --- a/src/debug/mips64/debug-mips64.cc +++ b/src/debug/mips64/debug-mips64.cc @@ -132,7 +132,7 @@ void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { __ LeaveFrame(StackFrame::INTERNAL); ParameterCount dummy(0); - __ FloodFunctionIfStepping(a1, no_reg, dummy, dummy); + __ CheckDebugHook(a1, no_reg, dummy, dummy); // Load context from the function. __ ld(cp, FieldMemOperand(a1, JSFunction::kContextOffset)); diff --git a/src/debug/x64/debug-x64.cc b/src/debug/x64/debug-x64.cc index 4f80e18c85..afdc3303a2 100644 --- a/src/debug/x64/debug-x64.cc +++ b/src/debug/x64/debug-x64.cc @@ -129,7 +129,7 @@ void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { __ popq(rbp); ParameterCount dummy(0); - __ FloodFunctionIfStepping(rdi, no_reg, dummy, dummy); + __ CheckDebugHook(rdi, no_reg, dummy, dummy); // Load context from the function. __ movp(rsi, FieldOperand(rdi, JSFunction::kContextOffset)); diff --git a/src/external-reference-table.cc b/src/external-reference-table.cc index 8d3d0aa1f9..57dc09ce3c 100644 --- a/src/external-reference-table.cc +++ b/src/external-reference-table.cc @@ -261,6 +261,8 @@ void ExternalReferenceTable::AddReferences(Isolate* isolate) { "Debug::after_break_target_address()"); Add(ExternalReference::debug_is_active_address(isolate).address(), "Debug::is_active_address()"); + Add(ExternalReference::debug_hook_on_function_call_address(isolate).address(), + "Debug::hook_on_function_call_address()"); Add(ExternalReference::debug_last_step_action_address(isolate).address(), "Debug::step_in_enabled_address()"); Add(ExternalReference::debug_suspended_generator_address(isolate).address(), diff --git a/src/flag-definitions.h b/src/flag-definitions.h index 1770a19f97..e533e86081 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -697,6 +697,11 @@ DEFINE_IMPLICATION(trace_array_abuse, trace_external_array_abuse) // debugger DEFINE_BOOL(trace_debug_json, false, "trace debugging JSON request/response") DEFINE_BOOL(enable_liveedit, true, "enable liveedit experimental feature") +DEFINE_BOOL(side_effect_free_debug_evaluate, false, + "use side-effect-free debug-evaluate for testing") +DEFINE_BOOL( + trace_side_effect_free_debug_evaluate, false, + "print debug messages for side-effect-free debug-evaluate for testing") DEFINE_BOOL(hard_abort, true, "abort by crashing") // execution.cc diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index d93552b2a9..084901783e 100644 --- a/src/ia32/macro-assembler-ia32.cc +++ b/src/ia32/macro-assembler-ia32.cc @@ -1937,16 +1937,14 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, } } - -void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual) { - Label skip_flooding; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(isolate()); - STATIC_ASSERT(StepFrame > StepIn); - cmpb(Operand::StaticVariable(last_step_action), Immediate(StepIn)); - j(less, &skip_flooding); +void MacroAssembler::CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual) { + Label skip_hook; + ExternalReference debug_hook_active = + ExternalReference::debug_hook_on_function_call_address(isolate()); + cmpb(Operand::StaticVariable(debug_hook_active), Immediate(0)); + j(equal, &skip_hook); { FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL); @@ -1963,7 +1961,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, } Push(fun); Push(fun); - CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + CallRuntime(Runtime::kDebugOnFunctionCall); Pop(fun); if (new_target.is_valid()) { Pop(new_target); @@ -1977,7 +1975,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, SmiUntag(expected.reg()); } } - bind(&skip_flooding); + bind(&skip_hook); } @@ -1991,8 +1989,8 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target, DCHECK(function.is(edi)); DCHECK_IMPLIES(new_target.is_valid(), new_target.is(edx)); - if (call_wrapper.NeedsDebugStepCheck()) { - FloodFunctionIfStepping(function, new_target, expected, actual); + if (call_wrapper.NeedsDebugHookCheck()) { + CheckDebugHook(function, new_target, expected, actual); } // Clear the new.target register if not given. diff --git a/src/ia32/macro-assembler-ia32.h b/src/ia32/macro-assembler-ia32.h index 61e8f07a1f..50ff068551 100644 --- a/src/ia32/macro-assembler-ia32.h +++ b/src/ia32/macro-assembler-ia32.h @@ -334,9 +334,10 @@ class MacroAssembler: public Assembler { const ParameterCount& actual, InvokeFlag flag, const CallWrapper& call_wrapper); - void FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual); + // On function call, call into the debugger if necessary. + void CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual); // Invoke the JavaScript function in the given register. Changes the // current context to the context in the function before invoking. diff --git a/src/isolate.h b/src/isolate.h index 63bc50f90a..fc36de3aa5 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -419,6 +419,8 @@ typedef List DebugObjectCache; V(bool, is_profiling, false) \ /* true if a trace is being formatted through Error.prepareStackTrace. */ \ V(bool, formatting_stack_trace, false) \ + /* Perform side effect checks on function call and API callbacks. */ \ + V(bool, needs_side_effect_check, false) \ ISOLATE_INIT_SIMULATOR_LIST(V) #define THREAD_LOCAL_TOP_ACCESSOR(type, name) \ diff --git a/src/messages.h b/src/messages.h index 3f3ecce134..8e38ff0de8 100644 --- a/src/messages.h +++ b/src/messages.h @@ -654,6 +654,7 @@ class ErrorUtils : public AllStatic { T(YieldInParameter, "Yield expression not allowed in formal parameter") \ /* EvalError */ \ T(CodeGenFromStrings, "%") \ + T(NoSideEffectDebugEvaluate, "Possible side-effect in debug-evaluate") \ /* URIError */ \ T(URIMalformed, "URI malformed") \ /* Wasm errors (currently Error) */ \ diff --git a/src/mips/macro-assembler-mips.cc b/src/mips/macro-assembler-mips.cc index 1491d87548..25413f9a54 100644 --- a/src/mips/macro-assembler-mips.cc +++ b/src/mips/macro-assembler-mips.cc @@ -4558,17 +4558,15 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, } } - -void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual) { - Label skip_flooding; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(isolate()); - STATIC_ASSERT(StepFrame > StepIn); - li(t0, Operand(last_step_action)); +void MacroAssembler::CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual) { + Label skip_hook; + ExternalReference debug_hook_active = + ExternalReference::debug_hook_on_function_call_address(isolate()); + li(t0, Operand(debug_hook_active)); lb(t0, MemOperand(t0)); - Branch(&skip_flooding, lt, t0, Operand(StepIn)); + Branch(&skip_hook, eq, t0, Operand(zero_reg)); { FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL); @@ -4585,7 +4583,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, } Push(fun); Push(fun); - CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + CallRuntime(Runtime::kDebugOnFunctionCall); Pop(fun); if (new_target.is_valid()) { Pop(new_target); @@ -4599,7 +4597,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, SmiUntag(expected.reg()); } } - bind(&skip_flooding); + bind(&skip_hook); } @@ -4613,8 +4611,8 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target, DCHECK(function.is(a1)); DCHECK_IMPLIES(new_target.is_valid(), new_target.is(a3)); - if (call_wrapper.NeedsDebugStepCheck()) { - FloodFunctionIfStepping(function, new_target, expected, actual); + if (call_wrapper.NeedsDebugHookCheck()) { + CheckDebugHook(function, new_target, expected, actual); } // Clear the new.target register if not given. diff --git a/src/mips/macro-assembler-mips.h b/src/mips/macro-assembler-mips.h index 591234a50d..66ac930ad2 100644 --- a/src/mips/macro-assembler-mips.h +++ b/src/mips/macro-assembler-mips.h @@ -1047,9 +1047,10 @@ class MacroAssembler: public Assembler { const ParameterCount& actual, InvokeFlag flag, const CallWrapper& call_wrapper); - void FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual); + // On function call, call into the debugger if necessary. + void CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual); // Invoke the JavaScript function in the given register. Changes the // current context to the context in the function before invoking. diff --git a/src/mips64/macro-assembler-mips64.cc b/src/mips64/macro-assembler-mips64.cc index 226e368338..480bae10f3 100644 --- a/src/mips64/macro-assembler-mips64.cc +++ b/src/mips64/macro-assembler-mips64.cc @@ -4751,17 +4751,15 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, } } - -void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual) { - Label skip_flooding; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(isolate()); - STATIC_ASSERT(StepFrame > StepIn); - li(t0, Operand(last_step_action)); +void MacroAssembler::CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual) { + Label skip_hook; + ExternalReference debug_hook_active = + ExternalReference::debug_hook_on_function_call_address(isolate()); + li(t0, Operand(debug_hook_active)); lb(t0, MemOperand(t0)); - Branch(&skip_flooding, lt, t0, Operand(StepIn)); + Branch(&skip_hook, eq, t0, Operand(zero_reg)); { FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL); @@ -4778,7 +4776,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, } Push(fun); Push(fun); - CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + CallRuntime(Runtime::kDebugOnFunctionCall); Pop(fun); if (new_target.is_valid()) { Pop(new_target); @@ -4792,7 +4790,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, SmiUntag(expected.reg()); } } - bind(&skip_flooding); + bind(&skip_hook); } @@ -4806,8 +4804,8 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target, DCHECK(function.is(a1)); DCHECK_IMPLIES(new_target.is_valid(), new_target.is(a3)); - if (call_wrapper.NeedsDebugStepCheck()) { - FloodFunctionIfStepping(function, new_target, expected, actual); + if (call_wrapper.NeedsDebugHookCheck()) { + CheckDebugHook(function, new_target, expected, actual); } // Clear the new.target register if not given. diff --git a/src/mips64/macro-assembler-mips64.h b/src/mips64/macro-assembler-mips64.h index e779cef7a0..2b5115703e 100644 --- a/src/mips64/macro-assembler-mips64.h +++ b/src/mips64/macro-assembler-mips64.h @@ -1100,9 +1100,10 @@ class MacroAssembler: public Assembler { const ParameterCount& actual, InvokeFlag flag, const CallWrapper& call_wrapper); - void FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual); + // On function call, call into the debugger if necessary. + void CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual); // Invoke the JavaScript function in the given register. Changes the // current context to the context in the function before invoking. diff --git a/src/objects-inl.h b/src/objects-inl.h index 64d0a93cc8..13584263d8 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -6252,6 +6252,10 @@ BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, must_use_ignition_turbo, BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, dont_flush, kDontFlush) BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, is_asm_wasm_broken, kIsAsmWasmBroken) +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, has_no_side_effect, + kHasNoSideEffect) +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, computed_has_no_side_effect, + kComputedHasNoSideEffect) bool Script::HasValidSource() { Object* src = this->source(); diff --git a/src/objects.cc b/src/objects.cc index bef1046fc1..bf95773d20 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -28,6 +28,7 @@ #include "src/counters-inl.h" #include "src/counters.h" #include "src/date.h" +#include "src/debug/debug-evaluate.h" #include "src/debug/debug.h" #include "src/deoptimizer.h" #include "src/elements.h" @@ -13342,6 +13343,16 @@ String* SharedFunctionInfo::DebugName() { return String::cast(n); } +bool SharedFunctionInfo::HasNoSideEffect() { + if (!computed_has_no_side_effect()) { + DisallowHeapAllocation not_handlified; + Handle info(this); + set_has_no_side_effect(DebugEvaluate::FunctionHasNoSideEffect(info)); + set_computed_has_no_side_effect(true); + } + return has_no_side_effect(); +} + // The filter is a pattern that matches function names in this way: // "*" all; the default // "-" all but the top-level function diff --git a/src/objects.h b/src/objects.h index b2c19822f6..8ef9e04a48 100644 --- a/src/objects.h +++ b/src/objects.h @@ -7359,6 +7359,9 @@ class SharedFunctionInfo: public HeapObject { // The function's name if it is non-empty, otherwise the inferred name. String* DebugName(); + // The function cannot cause any side effects. + bool HasNoSideEffect(); + // Used for flags such as --hydrogen-filter. bool PassesFilter(const char* raw_filter); @@ -7468,6 +7471,12 @@ class SharedFunctionInfo: public HeapObject { // Indicates that asm->wasm conversion failed and should not be re-attempted. DECL_BOOLEAN_ACCESSORS(is_asm_wasm_broken) + // Indicates that the function cannot cause side-effects. + DECL_BOOLEAN_ACCESSORS(has_no_side_effect) + + // Indicates that |has_no_side_effect| has been computed and set. + DECL_BOOLEAN_ACCESSORS(computed_has_no_side_effect) + inline FunctionKind kind() const; inline void set_kind(FunctionKind kind); @@ -7754,6 +7763,8 @@ class SharedFunctionInfo: public HeapObject { // byte 3 kDeserialized = kFunctionKind + 10, kIsAsmWasmBroken, + kHasNoSideEffect, + kComputedHasNoSideEffect, kCompilerHintsCount, // Pseudo entry }; // kFunctionKind has to be byte-aligned diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc index 844c64bbe6..0615c79eb5 100644 --- a/src/runtime/runtime-debug.cc +++ b/src/runtime/runtime-debug.cc @@ -5,6 +5,7 @@ #include "src/runtime/runtime-utils.h" #include "src/arguments.h" +#include "src/compiler.h" #include "src/debug/debug-evaluate.h" #include "src/debug/debug-frames.h" #include "src/debug/debug-scopes.h" @@ -1845,14 +1846,19 @@ RUNTIME_FUNCTION(Runtime_ScriptSourceLine) { return *str; } -// Set one shot breakpoints for the callback function that is passed to a -// built-in function such as Array.forEach to enable stepping into the callback, -// if we are indeed stepping and the callback is subject to debugging. -RUNTIME_FUNCTION(Runtime_DebugPrepareStepInIfStepping) { +// On function call, depending on circumstances, prepare for stepping in, +// or perform a side effect check. +RUNTIME_FUNCTION(Runtime_DebugOnFunctionCall) { HandleScope scope(isolate); DCHECK_EQ(1, args.length()); CONVERT_ARG_HANDLE_CHECKED(JSFunction, fun, 0); - isolate->debug()->PrepareStepIn(fun); + if (isolate->debug()->last_step_action() >= StepIn) { + isolate->debug()->PrepareStepIn(fun); + } + if (isolate->needs_side_effect_check() && + !isolate->debug()->PerformSideEffectCheck(fun)) { + return isolate->heap()->exception(); + } return isolate->heap()->undefined_value(); } diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index b0b65a6e7a..674fbaefd8 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -194,7 +194,7 @@ namespace internal { F(ScriptPositionInfo, 3, 1) \ F(ScriptPositionInfo2, 3, 1) \ F(ScriptSourceLine, 2, 1) \ - F(DebugPrepareStepInIfStepping, 1, 1) \ + F(DebugOnFunctionCall, 1, 1) \ F(DebugPrepareStepInSuspendedGenerator, 0, 1) \ F(DebugRecordGenerator, 1, 1) \ F(DebugPushPromise, 1, 1) \ diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc index 26a3dae1c6..39b1768055 100644 --- a/src/x64/macro-assembler-x64.cc +++ b/src/x64/macro-assembler-x64.cc @@ -4211,8 +4211,8 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target, DCHECK(function.is(rdi)); DCHECK_IMPLIES(new_target.is_valid(), new_target.is(rdx)); - if (call_wrapper.NeedsDebugStepCheck()) { - FloodFunctionIfStepping(function, new_target, expected, actual); + if (call_wrapper.NeedsDebugHookCheck()) { + CheckDebugHook(function, new_target, expected, actual); } // Clear the new.target register if not given. @@ -4312,17 +4312,15 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, } } - -void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual) { - Label skip_flooding; - ExternalReference last_step_action = - ExternalReference::debug_last_step_action_address(isolate()); - Operand last_step_action_operand = ExternalOperand(last_step_action); - STATIC_ASSERT(StepFrame > StepIn); - cmpb(last_step_action_operand, Immediate(StepIn)); - j(less, &skip_flooding); +void MacroAssembler::CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual) { + Label skip_hook; + ExternalReference debug_hook_active = + ExternalReference::debug_hook_on_function_call_address(isolate()); + Operand debug_hook_active_operand = ExternalOperand(debug_hook_active); + cmpb(debug_hook_active_operand, Immediate(0)); + j(equal, &skip_hook); { FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL); @@ -4339,7 +4337,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, } Push(fun); Push(fun); - CallRuntime(Runtime::kDebugPrepareStepInIfStepping); + CallRuntime(Runtime::kDebugOnFunctionCall); Pop(fun); if (new_target.is_valid()) { Pop(new_target); @@ -4353,7 +4351,7 @@ void MacroAssembler::FloodFunctionIfStepping(Register fun, Register new_target, SmiToInteger64(expected.reg(), expected.reg()); } } - bind(&skip_flooding); + bind(&skip_hook); } void MacroAssembler::StubPrologue(StackFrame::Type type) { diff --git a/src/x64/macro-assembler-x64.h b/src/x64/macro-assembler-x64.h index 795ba912fa..c09b07cac8 100644 --- a/src/x64/macro-assembler-x64.h +++ b/src/x64/macro-assembler-x64.h @@ -390,9 +390,10 @@ class MacroAssembler: public Assembler { const ParameterCount& actual, InvokeFlag flag, const CallWrapper& call_wrapper); - void FloodFunctionIfStepping(Register fun, Register new_target, - const ParameterCount& expected, - const ParameterCount& actual); + // On function call, call into the debugger if necessary. + void CheckDebugHook(Register fun, Register new_target, + const ParameterCount& expected, + const ParameterCount& actual); // Invoke the JavaScript function in the given register. Changes the // current context to the context in the function before invoking. diff --git a/test/debugger/debug/debug-evaluate-no-side-effect.js b/test/debugger/debug/debug-evaluate-no-side-effect.js new file mode 100644 index 0000000000..113ca3d16a --- /dev/null +++ b/test/debugger/debug/debug-evaluate-no-side-effect.js @@ -0,0 +1,83 @@ +// Copyright 2017 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: --ignition --side-effect-free-debug-evaluate + +Debug = debug.Debug + +var exception = null; +let a = 1; +var object = { property : 2, + get getter() { return 3; } + }; +var string1 = { toString() { return "x"; } }; +var string2 = { toString() { print("x"); return "x"; } }; +var array = [4, 5]; +var error = new Error(); + +function set_a() { a = 2; } + +function get_a() { return a; } + +function listener(event, exec_state, event_data, data) { + if (event != Debug.DebugEvent.Break) return; + try { + function success(expectation, source) { + assertEquals(expectation, exec_state.frame(0).evaluate(source).value()); + } + function fail(source) { + assertThrows(() => exec_state.frame(0).evaluate(source), EvalError); + } + // Simple test. + success(3, "1 + 2"); + // Dymanic load. + success(array, "array"); + // Context load. + success(1, "a"); + // Global and named property load. + success(2, "object.property"); + // Load via read-only getter. + success(3, "object.getter"); + // Implicit call to read-only toString. + success("xy", "string1 + 'y'"); + // Keyed property load. + success(5, "array[1]"); + // Call to read-only function. + success(1, "get_a()"); + // Call to read-only function within try-catch. + success(1, "try { get_a() } catch (e) {}"); + // Call to C++ built-in. + success(Math.sin(2), "Math.sin(2)"); + // Call to whitelisted get accessors. + success(3, "'abc'.length"); + success(2, "array.length"); + // Test that non-read-only code fails. + fail("exception = 1"); + // Test that calling a non-read-only function fails. + fail("set_a()"); + // Test that implicit call to a non-read-only function fails. + fail("string2 + 'y'"); + // Test that try-catch does not catch the EvalError. + fail("try { set_a() } catch (e) {}"); + // Test that call to set accessor fails. + fail("array.length = 4"); + // Test that call to non-whitelisted get accessor fails. + fail("error.stack"); + } catch (e) { + exception = e; + print(e, e.stack); + }; +}; + +// Add the debug event listener. +Debug.setListener(listener); + +function f() { + debugger; +}; + +f(); + +assertNull(exception); +assertEquals(1, a);