Commit ee58dde2 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[wasm][debug] Add support for conditional breakpoints.

Previously the WebAssembly debugger support completely ignored the
condition on breakpoints. With this change, we check conditions
(snippets of JavaScript) properly, which enables not only conditional
breakpoints in the front-end, but also other features like 'Never pause
here' (which simply sets `false` as condition) and log points.

Fixed: chromium:1173007
Bug: chromium:1173006
Change-Id: I02c740d383378a1f4cc08134ad571bea08e9a905
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2666690Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72485}
parent 0363eb4d
......@@ -540,6 +540,7 @@ RUNTIME_FUNCTION(Runtime_WasmDebugBreak) {
frame_finder(isolate);
auto instance = handle(frame_finder.frame()->wasm_instance(), isolate);
int position = frame_finder.frame()->position();
auto frame_id = frame_finder.frame()->id();
isolate->set_context(instance->native_context());
// Stepping can repeatedly create code, and code GC requires stack guards to
......@@ -564,7 +565,7 @@ RUNTIME_FUNCTION(Runtime_WasmDebugBreak) {
// Check whether we hit a breakpoint.
Handle<Script> script(instance->module_object().script(), isolate);
Handle<FixedArray> breakpoints;
if (WasmScript::CheckBreakPoints(isolate, script, position)
if (WasmScript::CheckBreakPoints(isolate, script, position, frame_id)
.ToHandle(&breakpoints)) {
debug_info->ClearStepping(isolate);
StepAction stepAction = isolate->debug()->last_step_action();
......
......@@ -12,6 +12,7 @@
#include "src/codegen/assembler-inl.h"
#include "src/common/assert-scope.h"
#include "src/compiler/wasm-compiler.h"
#include "src/debug/debug-evaluate.h"
#include "src/execution/frames-inl.h"
#include "src/heap/factory.h"
#include "src/wasm/baseline/liftoff-compiler.h"
......@@ -1064,10 +1065,34 @@ bool WasmScript::GetPossibleBreakpoints(
return true;
}
namespace {
bool CheckBreakPoint(Isolate* isolate, Handle<BreakPoint> break_point,
StackFrameId frame_id) {
if (break_point->condition().length() == 0) return true;
HandleScope scope(isolate);
Handle<String> condition(break_point->condition(), isolate);
Handle<Object> result;
// The Wasm engine doesn't perform any sort of inlining.
const int inlined_jsframe_index = 0;
const bool throw_on_side_effect = false;
if (!DebugEvaluate::Local(isolate, frame_id, inlined_jsframe_index, condition,
throw_on_side_effect)
.ToHandle(&result)) {
isolate->clear_pending_exception();
return false;
}
return result->BooleanValue(isolate);
}
} // namespace
// static
MaybeHandle<FixedArray> WasmScript::CheckBreakPoints(Isolate* isolate,
Handle<Script> script,
int position) {
int position,
StackFrameId frame_id) {
if (!script->has_wasm_breakpoint_infos()) return {};
Handle<FixedArray> breakpoint_infos(script->wasm_breakpoint_infos(), isolate);
......@@ -1082,14 +1107,29 @@ MaybeHandle<FixedArray> WasmScript::CheckBreakPoints(Isolate* isolate,
Handle<BreakPointInfo>::cast(maybe_breakpoint_info);
if (breakpoint_info->source_position() != position) return {};
// There is no support for conditional break points. Just assume that every
// break point always hits.
Handle<Object> break_points(breakpoint_info->break_points(), isolate);
if (break_points->IsFixedArray()) {
return Handle<FixedArray>::cast(break_points);
if (!break_points->IsFixedArray()) {
if (!CheckBreakPoint(isolate, Handle<BreakPoint>::cast(break_points),
frame_id)) {
return {};
}
Handle<FixedArray> break_points_hit = isolate->factory()->NewFixedArray(1);
break_points_hit->set(0, *break_points);
return break_points_hit;
}
Handle<FixedArray> array = Handle<FixedArray>::cast(break_points);
Handle<FixedArray> break_points_hit =
isolate->factory()->NewFixedArray(array->length());
int break_points_hit_count = 0;
for (int i = 0; i < array->length(); ++i) {
Handle<BreakPoint> break_point(BreakPoint::cast(array->get(i)), isolate);
if (CheckBreakPoint(isolate, break_point, frame_id)) {
break_points_hit->set(break_points_hit_count++, *break_point);
}
}
Handle<FixedArray> break_points_hit = isolate->factory()->NewFixedArray(1);
break_points_hit->set(0, *break_points);
if (break_points_hit_count == 0) return {};
break_points_hit->Shrink(isolate, break_points_hit_count);
return break_points_hit;
}
......
......@@ -838,7 +838,8 @@ class WasmScript : public AllStatic {
// Return an empty handle if no breakpoint is hit at that location, or a
// FixedArray with all hit breakpoint objects.
static MaybeHandle<FixedArray> CheckBreakPoints(Isolate*, Handle<Script>,
int position);
int position,
StackFrameId stack_frame_id);
private:
// Helper functions that update the breakpoint info list.
......
......@@ -40,50 +40,27 @@ Setting breakpoint at offset 46, condition "$var0==3"
breakpointId : <breakpointId>
}
}
NOTE: The conditions are being ignored currently.
Calling fib(5)
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 5
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 5
Script wasm://wasm/f00dbc56 byte offset 46: Wasm opcode 0x10 (kExprCallFunction)
$var0: 5
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 4
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 4
Script wasm://wasm/f00dbc56 byte offset 46: Wasm opcode 0x10 (kExprCallFunction)
$var0: 4
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 3
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 3
Script wasm://wasm/f00dbc56 byte offset 46: Wasm opcode 0x10 (kExprCallFunction)
$var0: 3
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 2
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 2
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 1
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 1
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 2
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 2
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 3
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 3
Script wasm://wasm/f00dbc56 byte offset 46: Wasm opcode 0x10 (kExprCallFunction)
$var0: 3
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 2
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 2
Script wasm://wasm/f00dbc56 byte offset 34: Wasm opcode 0x20 (kExprLocalGet)
$var0: 1
Script wasm://wasm/f00dbc56 byte offset 41: Wasm opcode 0x0d (kExprBrIf)
$var0: 1
fib returned!
......@@ -66,8 +66,6 @@ InspectorTest.runAsyncTestSuite([
condition: breakpoint.cond
}));
}
// TODO(1173007): Implement conditional breakpoints for wasm.
InspectorTest.log('NOTE: The conditions are being ignored currently.');
InspectorTest.log('Calling fib(5)');
await WasmInspectorTest.evalWithUrl('instance.exports.fib(5)', 'runWasm');
InspectorTest.log('fib returned!');
......
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