diff --git a/src/wasm/baseline/liftoff-compiler.cc b/src/wasm/baseline/liftoff-compiler.cc index df57e94c7f..1d97eb2bfd 100644 --- a/src/wasm/baseline/liftoff-compiler.cc +++ b/src/wasm/baseline/liftoff-compiler.cc @@ -690,30 +690,6 @@ class LiftoffCompiler { } if (FLAG_trace_wasm) TraceFunctionEntry(decoder); - - // If we are generating debug code, do check the "hook on function call" - // flag. If set, trigger a break. - if (V8_UNLIKELY(for_debugging_)) { - // If there is a breakpoint set on the first instruction (== start of the - // function), then skip the check for "hook on function call", since we - // will unconditionally break there anyway. - bool has_breakpoint = next_breakpoint_ptr_ != nullptr && - (*next_breakpoint_ptr_ == 0 || - *next_breakpoint_ptr_ == decoder->position()); - if (!has_breakpoint) { - DEBUG_CODE_COMMENT("check hook on function call"); - Register flag = __ GetUnusedRegister(kGpReg, {}).gp(); - LOAD_INSTANCE_FIELD(flag, HookOnFunctionCallAddress, - kSystemPointerSize); - Label no_break; - __ Load(LiftoffRegister{flag}, flag, no_reg, 0, LoadType::kI32Load8U, - {}); - // Unary "equal" means "equals zero". - __ emit_cond_jump(kEqual, &no_break, kWasmI32, flag); - EmitBreakpoint(decoder); - __ bind(&no_break); - } - } } void GenerateOutOfLineCode(OutOfLineCode* ool) { @@ -799,14 +775,14 @@ class LiftoffCompiler { } V8_NOINLINE void EmitDebuggingInfo(FullDecoder* decoder, WasmOpcode opcode) { - DCHECK(V8_UNLIKELY(for_debugging_)); + DCHECK(for_debugging_); + if (!WasmOpcodes::IsBreakable(opcode)) return; + bool has_breakpoint = false; if (next_breakpoint_ptr_) { if (*next_breakpoint_ptr_ == 0) { // A single breakpoint at offset 0 indicates stepping. DCHECK_EQ(next_breakpoint_ptr_ + 1, next_breakpoint_end_); - if (WasmOpcodes::IsBreakable(opcode)) { - EmitBreakpoint(decoder); - } + has_breakpoint = true; } else { while (next_breakpoint_ptr_ != next_breakpoint_end_ && *next_breakpoint_ptr_ < decoder->position()) { @@ -816,18 +792,34 @@ class LiftoffCompiler { if (next_breakpoint_ptr_ == next_breakpoint_end_) { next_breakpoint_ptr_ = next_breakpoint_end_ = nullptr; } else if (*next_breakpoint_ptr_ == decoder->position()) { - DCHECK(WasmOpcodes::IsBreakable(opcode)); - EmitBreakpoint(decoder); + has_breakpoint = true; } } } - if (dead_breakpoint_ == decoder->position()) { + if (has_breakpoint) { + EmitBreakpoint(decoder); + // Once we emitted a breakpoint, we don't need to check the "hook on + // function call" any more. + checked_hook_on_function_call_ = true; + } else if (!checked_hook_on_function_call_) { + checked_hook_on_function_call_ = true; + // Check the "hook on function call" flag. If set, trigger a break. + DEBUG_CODE_COMMENT("check hook on function call"); + Register flag = __ GetUnusedRegister(kGpReg, {}).gp(); + LOAD_INSTANCE_FIELD(flag, HookOnFunctionCallAddress, kSystemPointerSize); + Label no_break; + __ Load(LiftoffRegister{flag}, flag, no_reg, 0, LoadType::kI32Load8U, {}); + // Unary "equal" means "equals zero". + __ emit_cond_jump(kEqual, &no_break, kWasmI32, flag); + EmitBreakpoint(decoder); + __ bind(&no_break); + } else if (dead_breakpoint_ == decoder->position()) { DCHECK(!next_breakpoint_ptr_ || *next_breakpoint_ptr_ != dead_breakpoint_); // The top frame is paused at this position, but the breakpoint was - // removed. Adding a dead breakpoint here ensures that the source position - // exists, and that the offset to the return address is the same as in the - // old code. + // removed. Adding a dead breakpoint here ensures that the source + // position exists, and that the offset to the return address is the + // same as in the old code. Label cont; __ emit_jump(&cont); EmitBreakpoint(decoder); @@ -4076,6 +4068,11 @@ class LiftoffCompiler { // address in OSR is correct. int dead_breakpoint_ = 0; + // Remember whether the "hook on function call" has already been checked. + // This happens at the first breakable opcode in the function (if compiling + // for debugging). + bool checked_hook_on_function_call_ = false; + bool has_outstanding_op() const { return outstanding_op_ != kNoOutstandingOp; } diff --git a/test/inspector/debugger/wasm-step-from-non-breakable-position-expected.txt b/test/inspector/debugger/wasm-step-from-non-breakable-position-expected.txt new file mode 100644 index 0000000000..73fa06129a --- /dev/null +++ b/test/inspector/debugger/wasm-step-from-non-breakable-position-expected.txt @@ -0,0 +1,12 @@ +Step into a function that starts with a non-breakable opcode (i.e. block), then step from there. See https://crbug.com/1137710. +Setting up global instance variable. +Got wasm script: wasm://wasm/4658c40e +Setting breakpoint on offset 44 +Running main function. +Script wasm://wasm/4658c40e byte offset 44: Wasm opcode 0x10 +Debugger.stepInto called +Script wasm://wasm/4658c40e byte offset 40: Wasm opcode 0x0b +Debugger.stepInto called +Script wasm://wasm/4658c40e byte offset 41: Wasm opcode 0x0b +Debugger.resume called +exports.main returned. diff --git a/test/inspector/debugger/wasm-step-from-non-breakable-position.js b/test/inspector/debugger/wasm-step-from-non-breakable-position.js new file mode 100644 index 0000000000..2576d1364a --- /dev/null +++ b/test/inspector/debugger/wasm-step-from-non-breakable-position.js @@ -0,0 +1,54 @@ +// Copyright 2020 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. + +utils.load('test/inspector/wasm-inspector-test.js'); + +let {session, contextGroup, Protocol} = InspectorTest.start( + 'Step into a function that starts with a non-breakable opcode (i.e. ' + + 'block), then step from there. See https://crbug.com/1137710.'); +session.setupScriptMap(); + +var builder = new WasmModuleBuilder(); + +var callee = builder.addFunction('callee', kSig_v_v) + .addBody([kExprBlock, kWasmStmt, kExprEnd]) + .index; + +var main = builder.addFunction('main', kSig_v_i) + .addBody([kExprCallFunction, callee]) + .exportFunc(); + +var module_bytes = builder.toArray(); + +(async function test() { + InspectorTest.logProtocolCommandCalls('Debugger.stepInto'); + InspectorTest.logProtocolCommandCalls('Debugger.resume'); + + await Protocol.Debugger.enable(); + InspectorTest.log('Setting up global instance variable.'); + WasmInspectorTest.instantiate(module_bytes); + const [, {params: wasmScript}] = await Protocol.Debugger.onceScriptParsed(2); + + InspectorTest.log(`Got wasm script: ${wasmScript.url}`); + + // Set a breakpoint in 'main', at the call. + InspectorTest.log(`Setting breakpoint on offset ${main.body_offset}`); + await Protocol.Debugger.setBreakpoint({ + location: { + scriptId: wasmScript.scriptId, + lineNumber: 0, + columnNumber: main.body_offset + } + }); + + InspectorTest.log('Running main function.'); + Protocol.Runtime.evaluate({ expression: 'instance.exports.main()' }); + for (let action of ['stepInto', 'stepInto', 'resume']) { + const {params: {callFrames}} = await Protocol.Debugger.oncePaused(); + await session.logSourceLocation(callFrames[0].location); + Protocol.Debugger[action](); + } + InspectorTest.log('exports.main returned.'); +})().catch(reason => InspectorTest.log(`Failed: ${reason}`)) + .finally(InspectorTest.completeTest);