Commit 3c98a2a3 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[wasm] Prevent breakpoints on nonbreakable positions

If a breakpoint is set on a non-breakable position, the wasm interpreter
just stores the value 0xFF (kInternalBreakpoint) in the function body
(actually, a copy of the function body). This might overwrite immediates
and cause subsequent failures in the wasm interpreter.

In JavaScript, breakpoints are just forwarded to the next breakable
position. This CL implements the same for WebAssembly.
A cctest tests this behavior, and the existing
wasm-stepping-byte-offsets.js inspector test is extended to also set the
breakpoint within an i32 constant immediate.

R=leese@chromium.org, mstarzinger@chromium.org
CC=​bmeurer@chromium.org

Bug: chromium:1025184
Change-Id: Ia2706f8f1c3d686cbbe8e1e7339d9ee86247bb4a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1925152
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65070}
parent e614ca2c
......@@ -674,9 +674,12 @@ Handle<Code> WasmDebugInfo::GetCWasmEntry(Handle<WasmDebugInfo> debug_info,
namespace {
#ifdef DEBUG
bool IsBreakablePosition(wasm::NativeModule* native_module, int func_index,
int offset_in_func) {
// Return the next breakable position after {offset_in_func} in function
// {func_index}, or 0 if there is none.
// Note that 0 is never a breakable position in wasm, since the first byte
// contains the locals count for the function.
int FindNextBreakablePosition(wasm::NativeModule* native_module, int func_index,
int offset_in_func) {
AccountingAllocator alloc;
Zone tmp(&alloc, ZONE_NAME);
wasm::BodyLocalDecls locals(&tmp);
......@@ -687,13 +690,12 @@ bool IsBreakablePosition(wasm::NativeModule* native_module, int func_index,
module_start + func.code.end_offset(),
&locals);
DCHECK_LT(0, locals.encoded_size);
if (offset_in_func < 0) return 0;
for (uint32_t offset : iterator.offsets()) {
if (offset > static_cast<uint32_t>(offset_in_func)) break;
if (offset == static_cast<uint32_t>(offset_in_func)) return true;
if (offset >= static_cast<uint32_t>(offset_in_func)) return offset;
}
return false;
return 0;
}
#endif // DEBUG
} // namespace
......@@ -709,10 +711,10 @@ bool WasmScript::SetBreakPoint(Handle<Script> script, int* position,
const wasm::WasmFunction& func = module->functions[func_index];
int offset_in_func = *position - func.code.offset();
// According to the current design, we should only be called with valid
// breakable positions.
DCHECK(IsBreakablePosition(script->wasm_native_module(), func_index,
offset_in_func));
int breakable_offset = FindNextBreakablePosition(script->wasm_native_module(),
func_index, offset_in_func);
if (breakable_offset == 0) return false;
*position = func.code.offset() + breakable_offset;
// Insert new break point into break_positions of module object.
WasmScript::AddBreakpointToInfo(script, *position, break_point);
......@@ -729,7 +731,7 @@ bool WasmScript::SetBreakPoint(Handle<Script> script, int* position,
isolate);
Handle<WasmDebugInfo> debug_info =
WasmInstanceObject::GetOrCreateDebugInfo(instance);
WasmDebugInfo::SetBreakpoint(debug_info, func_index, offset_in_func);
WasmDebugInfo::SetBreakpoint(debug_info, func_index, breakable_offset);
}
}
......
......@@ -280,6 +280,13 @@ std::vector<WasmValue> wasmVec(Args... args) {
return std::vector<WasmValue>{arr.begin(), arr.end()};
}
int GetIntReturnValue(MaybeHandle<Object> retval) {
CHECK(!retval.is_null());
int result;
CHECK(retval.ToHandleChecked()->ToInt32(&result));
return result;
}
} // namespace
WASM_COMPILED_EXEC_TEST(WasmCollectPossibleBreakpoints) {
......@@ -324,10 +331,25 @@ WASM_COMPILED_EXEC_TEST(WasmSimpleBreak) {
Handle<Object> global(isolate->context().global_object(), isolate);
MaybeHandle<Object> retval =
Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
CHECK(!retval.is_null());
int result;
CHECK(retval.ToHandleChecked()->ToInt32(&result));
CHECK_EQ(14, result);
CHECK_EQ(14, GetIntReturnValue(retval));
}
WASM_COMPILED_EXEC_TEST(WasmNonBreakablePosition) {
WasmRunner<int> runner(execution_tier);
Isolate* isolate = runner.main_isolate();
BUILD(runner, WASM_RETURN1(WASM_I32V_2(1024)));
Handle<JSFunction> main_fun_wrapper =
runner.builder().WrapCode(runner.function_index());
SetBreakpoint(&runner, runner.function_index(), 2, 4);
BreakHandler count_breaks(isolate, {{4, BreakHandler::Continue}});
Handle<Object> global(isolate->context().global_object(), isolate);
MaybeHandle<Object> retval =
Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
CHECK_EQ(1024, GetIntReturnValue(retval));
}
WASM_COMPILED_EXEC_TEST(WasmSimpleStepping) {
......@@ -351,10 +373,7 @@ WASM_COMPILED_EXEC_TEST(WasmSimpleStepping) {
Handle<Object> global(isolate->context().global_object(), isolate);
MaybeHandle<Object> retval =
Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
CHECK(!retval.is_null());
int result;
CHECK(retval.ToHandleChecked()->ToInt32(&result));
CHECK_EQ(14, result);
CHECK_EQ(14, GetIntReturnValue(retval));
}
WASM_COMPILED_EXEC_TEST(WasmStepInAndOut) {
......@@ -470,10 +489,7 @@ WASM_COMPILED_EXEC_TEST(WasmRemoveBreakPoint) {
Handle<Object> global(isolate->context().global_object(), isolate);
MaybeHandle<Object> retval =
Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
CHECK(!retval.is_null());
int result;
CHECK(retval.ToHandleChecked()->ToInt32(&result));
CHECK_EQ(14, result);
CHECK_EQ(14, GetIntReturnValue(retval));
}
WASM_COMPILED_EXEC_TEST(WasmRemoveLastBreakPoint) {
......@@ -501,10 +517,7 @@ WASM_COMPILED_EXEC_TEST(WasmRemoveLastBreakPoint) {
Handle<Object> global(isolate->context().global_object(), isolate);
MaybeHandle<Object> retval =
Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
CHECK(!retval.is_null());
int result;
CHECK(retval.ToHandleChecked()->ToInt32(&result));
CHECK_EQ(14, result);
CHECK_EQ(14, GetIntReturnValue(retval));
}
WASM_COMPILED_EXEC_TEST(WasmRemoveAllBreakPoint) {
......@@ -534,10 +547,7 @@ WASM_COMPILED_EXEC_TEST(WasmRemoveAllBreakPoint) {
Handle<Object> global(isolate->context().global_object(), isolate);
MaybeHandle<Object> retval =
Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
CHECK(!retval.is_null());
int result;
CHECK(retval.ToHandleChecked()->ToInt32(&result));
CHECK_EQ(14, result);
CHECK_EQ(14, GetIntReturnValue(retval));
}
} // namespace wasm
......
Tests stepping through wasm scripts with source maps
Installing code an global variable and instantiate.
Got wasm script: wasm://wasm/18214bfe
Requesting source for wasm://wasm/18214bfe...
Tests stepping through wasm scripts by byte offsets
Setting up global instance variable.
Got wasm script: wasm://wasm/41f464ee
Requesting source for wasm://wasm/41f464ee...
Source retrieved without error: true
Setting breakpoint on offset 54 (on the setlocal before the call), url wasm://wasm/18214bfe
Result: {"id":4,"result":{"breakpointId":"4:0:54:4","actualLocation":{"scriptId":"4","lineNumber":0,"columnNumber":54}}}
Setting breakpoint on offset 59 (should be propagated to 60, the offset of the call), url wasm://wasm/41f464ee
{
columnNumber : 54
columnNumber : 60
lineNumber : 0
scriptId : <scriptId>
}
Paused at wasm://wasm/18214bfe/18214bfe-1:7:6
at wasm_B (7:6):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":4}
stack: {"0":3}
at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:8:6
at wasm_B (8:6):
Paused at wasm://wasm/41f464ee/41f464ee-1:9:6
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":3}
stack: {}
stack: {"0":1024}
at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-0:1:2
Paused at wasm://wasm/41f464ee/41f464ee-0:1:2
at wasm_A (1:2):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":1024}
stack: {}
at wasm_B (8:6):
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
......@@ -48,13 +37,14 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepOver called
Paused at wasm://wasm/18214bfe/18214bfe-0:2:2
Paused at wasm://wasm/41f464ee/41f464ee-0:2:2
at wasm_A (2:2):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":1024}
stack: {}
at wasm_B (8:6):
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
......@@ -64,8 +54,8 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepOut called
Paused at wasm://wasm/18214bfe/18214bfe-1:9:6
at wasm_B (9:6):
Paused at wasm://wasm/41f464ee/41f464ee-1:10:6
at wasm_B (10:6):
- scope (global):
-- skipped
- scope (local):
......@@ -75,19 +65,19 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepOut called
Paused at wasm://wasm/18214bfe/18214bfe-1:7:6
at wasm_B (7:6):
Paused at wasm://wasm/41f464ee/41f464ee-1:9:6
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":3}
stack: {"0":2}
locals: {"arg#0":2}
stack: {"0":1024}
at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepOver called
Paused at wasm://wasm/18214bfe/18214bfe-1:8:6
at wasm_B (8:6):
Paused at wasm://wasm/41f464ee/41f464ee-1:10:6
at wasm_B (10:6):
- scope (global):
-- skipped
- scope (local):
......@@ -96,9 +86,9 @@ at wasm_B (8:6):
at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepOver called
Paused at wasm://wasm/18214bfe/18214bfe-1:9:6
at wasm_B (9:6):
Debugger.stepInto called
Paused at wasm://wasm/41f464ee/41f464ee-1:1:2
at wasm_B (1:2):
- scope (global):
-- skipped
- scope (local):
......@@ -108,35 +98,25 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.resume called
Paused at wasm://wasm/18214bfe/18214bfe-1:7:6
at wasm_B (7:6):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":2}
stack: {"0":1}
at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:8:6
at wasm_B (8:6):
Paused at wasm://wasm/41f464ee/41f464ee-1:9:6
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":1}
stack: {}
stack: {"0":1024}
at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-0:1:2
Paused at wasm://wasm/41f464ee/41f464ee-0:1:2
at wasm_A (1:2):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":1024}
stack: {}
at wasm_B (8:6):
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
......@@ -146,8 +126,8 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepOut called
Paused at wasm://wasm/18214bfe/18214bfe-1:9:6
at wasm_B (9:6):
Paused at wasm://wasm/41f464ee/41f464ee-1:10:6
at wasm_B (10:6):
- scope (global):
-- skipped
- scope (local):
......@@ -157,7 +137,7 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:1:2
Paused at wasm://wasm/41f464ee/41f464ee-1:1:2
at wasm_B (1:2):
- scope (global):
-- skipped
......@@ -168,7 +148,7 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:2:4
Paused at wasm://wasm/41f464ee/41f464ee-1:2:4
at wasm_B (2:4):
- scope (global):
-- skipped
......@@ -179,7 +159,7 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:3:4
Paused at wasm://wasm/41f464ee/41f464ee-1:3:4
at wasm_B (3:4):
- scope (global):
-- skipped
......@@ -190,7 +170,7 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:4:6
Paused at wasm://wasm/41f464ee/41f464ee-1:4:6
at wasm_B (4:6):
- scope (global):
-- skipped
......@@ -201,7 +181,7 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:5:6
Paused at wasm://wasm/41f464ee/41f464ee-1:5:6
at wasm_B (5:6):
- scope (global):
-- skipped
......@@ -212,7 +192,7 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:6:6
Paused at wasm://wasm/41f464ee/41f464ee-1:6:6
at wasm_B (6:6):
- scope (global):
-- skipped
......@@ -223,7 +203,7 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:7:6
Paused at wasm://wasm/41f464ee/41f464ee-1:7:6
at wasm_B (7:6):
- scope (global):
-- skipped
......@@ -234,7 +214,7 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:8:6
Paused at wasm://wasm/41f464ee/41f464ee-1:8:6
at wasm_B (8:6):
- scope (global):
-- skipped
......@@ -245,13 +225,25 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-0:1:2
Paused at wasm://wasm/41f464ee/41f464ee-1:9:6
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":0}
stack: {"0":1024}
at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/41f464ee/41f464ee-0:1:2
at wasm_A (1:2):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":1024}
stack: {}
at wasm_B (8:6):
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
......@@ -261,13 +253,14 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-0:2:2
Paused at wasm://wasm/41f464ee/41f464ee-0:2:2
at wasm_A (2:2):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":1024}
stack: {}
at wasm_B (8:6):
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
......@@ -277,13 +270,14 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-0:3:0
Paused at wasm://wasm/41f464ee/41f464ee-0:3:0
at wasm_A (3:0):
- scope (global):
-- skipped
- scope (local):
locals: {"arg#0":1024}
stack: {}
at wasm_B (8:6):
at wasm_B (9:6):
- scope (global):
-- skipped
- scope (local):
......@@ -293,8 +287,8 @@ at (anonymous) (0:17):
- scope (global):
-- skipped
Debugger.stepInto called
Paused at wasm://wasm/18214bfe/18214bfe-1:9:6
at wasm_B (9:6):
Paused at wasm://wasm/41f464ee/41f464ee-1:10:6
at wasm_B (10:6):
- scope (global):
-- skipped
- scope (local):
......
// Copyright 2018 the V8 project authors. All rights reserved.
// Copyright 2019 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.
let {session, contextGroup, Protocol} =
InspectorTest.start('Tests stepping through wasm scripts with source maps');
InspectorTest.start('Tests stepping through wasm scripts by byte offsets');
utils.load('test/mjsunit/wasm/wasm-module-builder.js');
var builder = new WasmModuleBuilder();
var func_a_idx =
builder.addFunction('wasm_A', kSig_v_v).addBody([kExprNop, kExprNop]).index;
builder.addFunction('wasm_A', kSig_v_i).addBody([kExprNop, kExprNop]).index;
// wasm_B calls wasm_A <param0> times.
builder.addFunction('wasm_B', kSig_v_i)
......@@ -23,6 +23,7 @@ builder.addFunction('wasm_B', kSig_v_i)
kExprI32Const, 1, // -
kExprI32Sub, // -
kExprLocalSet, 0, // decrease <param0>
...wasmI32Const(1024), // some longer i32 const (2 byte imm)
kExprCallFunction, func_a_idx, // -
kExprBr, 1, // continue
kExprEnd, // -
......@@ -51,9 +52,10 @@ function instantiate(bytes) {
InspectorTest.logProtocolCommandCalls('Debugger.' + action);
await Protocol.Debugger.enable();
InspectorTest.log('Installing code an global variable and instantiate.');
InspectorTest.log('Setting up global instance variable.');
Protocol.Runtime.evaluate({
expression: `var instance;(${instantiate.toString()})(${JSON.stringify(module_bytes)})`
expression: `var instance;` +
`(${instantiate.toString()})(${JSON.stringify(module_bytes)})`
});
const [, {params: wasmScript}, ,] = await Protocol.Debugger.onceScriptParsed(4);
......@@ -62,39 +64,39 @@ function instantiate(bytes) {
const msg =
await Protocol.Debugger.getScriptSource({scriptId: wasmScript.scriptId});
InspectorTest.log(`Source retrieved without error: ${!msg.error}`);
// TODO: Add check that source text is empty but bytecode is present.
// TODO(leese): Add check that source text is empty but bytecode is present.
// Set the breakpoint on a non-breakable position. This should resolve to the
// next instruction.
InspectorTest.log(
`Setting breakpoint on offset 54 (on the setlocal before the call), url ${wasmScript.url}`);
`Setting breakpoint on offset 59 (should be propagated to 60, the ` +
`offset of the call), url ${wasmScript.url}`);
const bpmsg = await Protocol.Debugger.setBreakpoint({
location: {scriptId: wasmScript.scriptId, lineNumber: 0, columnNumber: 54}
location: {scriptId: wasmScript.scriptId, lineNumber: 0, columnNumber: 59}
});
InspectorTest.log(`Result: ${JSON.stringify(bpmsg)}`);
const actualLocation = bpmsg.result.actualLocation;
InspectorTest.logMessage(actualLocation);
Protocol.Runtime.evaluate({ expression: 'instance.exports.main(4)' });
await waitForPauseAndStep('stepInto'); // == stepOver, to call instruction
await waitForPauseAndStep('stepInto'); // into call to wasm_A
await waitForPauseAndStep('stepOver'); // over first nop
await waitForPauseAndStep('stepOut'); // out of wasm_A
await waitForPauseAndStep('stepOut'); // out of wasm_B, stop on breakpoint again
await waitForPauseAndStep('stepOver'); // to call
await waitForPauseAndStep('stepOut'); // out of wasm_B, stop on breakpoint
await waitForPauseAndStep('stepOver'); // over call
await waitForPauseAndStep('resume'); // to next breakpoint (third iteration)
await waitForPauseAndStep('stepInto'); // to call
await waitForPauseAndStep('stepInto'); // == stepOver br
await waitForPauseAndStep('resume'); // to next breakpoint (3rd iteration)
await waitForPauseAndStep('stepInto'); // into wasm_A
await waitForPauseAndStep('stepOut'); // out to wasm_B
// now step 9 times, until we are in wasm_A again.
for (let i = 0; i < 9; ++i) await waitForPauseAndStep('stepInto');
// Now step 10 times, until we are in wasm_A again.
for (let i = 0; i < 10; ++i) await waitForPauseAndStep('stepInto');
// 3 more times, back to wasm_B.
for (let i = 0; i < 3; ++i) await waitForPauseAndStep('stepInto');
// then just resume.
// Then just resume.
await waitForPauseAndStep('resume');
InspectorTest.log('exports.main returned!');
InspectorTest.log('Finished!');
//InspectorTest.completeTest();
})().catch(reason => InspectorTest.log(`Failed: ${reason}`)).finally(InspectorTest.completeTest);
})().catch(reason => InspectorTest.log(`Failed: ${reason}`))
.finally(InspectorTest.completeTest);
async function waitForPauseAndStep(stepAction) {
const {params: {callFrames}} = await Protocol.Debugger.oncePaused();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment