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

[inspector][wasm] Remove obsolete Debugger.executeWasmEvaluator().

With https://crrev.com/c/2087396 we introduced a new CDP method
`Debugger.executeWasmEvaluator()`, which we originally intended
to use as the foundation for Debug-Evaluate on Wasm frames.

However in the process of prototyping we learned that it is too
costly and too inefficient to use WebAssembly modules here, and
we switched to regular Debug-Evaluate with JavaScript instead
(with a special debug proxy exposed that allows JavaScript to
peak into the Wasm frame), since JavaScript is better suited
for short-lived / short-running snippets and we don't need
clang and wasm-ld then to generate these snippets.

The JavaScript exposed debug proxy (as described in [1]) not
only enables more powerful and flexible Debug-Evaluate for the
DWARF C/C++ extension, but also serves as the basis for various
aspects of the Basic Wasm Developer Experience.

In order to pay down technical debt and to keep the maintenance
overhead low, we should remove the initial prototype now, also
to ensure that we don't accidentally attract other users of CDP
to rely on this unsupported API (despite it being marked as
"experimental").

[1]: https://docs.google.com/document/d/1VZOJrU2VsqOZe3IUzbwQWQQSZwgGySsm5119Ust1gUA

Fixed: chromium:1162062
Bug: chromium:1020120, chromium:1068571, chromium:1127914
Change-Id: I6dba8c906a8675ce6c29a52e3c32bb6626a27247
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2605186
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71882}
parent 76d6f06a
......@@ -3430,8 +3430,6 @@ v8_source_set("v8_base_without_compiler") {
"src/wasm/wasm-code-manager.cc",
"src/wasm/wasm-code-manager.h",
"src/wasm/wasm-constants.h",
"src/wasm/wasm-debug-evaluate.cc",
"src/wasm/wasm-debug-evaluate.h",
"src/wasm/wasm-debug.cc",
"src/wasm/wasm-engine.cc",
"src/wasm/wasm-engine.h",
......
......@@ -211,21 +211,6 @@ domain Debugger
# Exception details.
optional Runtime.ExceptionDetails exceptionDetails
# Execute a Wasm Evaluator module on a given call frame.
experimental command executeWasmEvaluator
parameters
# WebAssembly call frame identifier to evaluate on.
CallFrameId callFrameId
# Code of the evaluator module.
binary evaluator
# Terminate execution after timing out (number of milliseconds).
experimental optional Runtime.TimeDelta timeout
returns
# Object wrapper for the evaluation result.
Runtime.RemoteObject result
# Exception details.
optional Runtime.ExceptionDetails exceptionDetails
# Returns possible locations for breakpoint. scriptId in start and end range locations should be
# the same.
command getPossibleBreakpoints
......
......@@ -455,7 +455,6 @@ class V8_EXPORT_PRIVATE ScopeIterator {
class V8_EXPORT_PRIVATE StackTraceIterator {
public:
static bool SupportsWasmDebugEvaluate();
static std::unique_ptr<StackTraceIterator> Create(Isolate* isolate,
int index = 0);
StackTraceIterator() = default;
......@@ -478,8 +477,6 @@ class V8_EXPORT_PRIVATE StackTraceIterator {
virtual bool Restart() = 0;
virtual v8::MaybeLocal<v8::Value> Evaluate(v8::Local<v8::String> source,
bool throw_on_side_effect) = 0;
virtual v8::MaybeLocal<v8::String> EvaluateWasm(
internal::Vector<const internal::byte> source, int frame_index) = 0;
};
class QueryObjectPredicate {
......
......@@ -6,22 +6,14 @@
#include "src/api/api-inl.h"
#include "src/debug/debug-evaluate.h"
#include "src/debug/debug-interface.h"
#include "src/debug/debug-scope-iterator.h"
#include "src/debug/debug.h"
#include "src/debug/liveedit.h"
#include "src/execution/frames-inl.h"
#include "src/execution/frames.h"
#include "src/execution/isolate.h"
#include "src/wasm/wasm-debug-evaluate.h"
#include "src/wasm/wasm-debug.h"
namespace v8 {
bool debug::StackTraceIterator::SupportsWasmDebugEvaluate() {
return i::FLAG_wasm_expose_debug_eval;
}
std::unique_ptr<debug::StackTraceIterator> debug::StackTraceIterator::Create(
v8::Isolate* isolate, int index) {
return std::unique_ptr<debug::StackTraceIterator>(
......@@ -209,25 +201,5 @@ v8::MaybeLocal<v8::Value> DebugStackTraceIterator::Evaluate(
return Utils::ToLocal(value);
}
v8::MaybeLocal<v8::String> DebugStackTraceIterator::EvaluateWasm(
internal::Vector<const internal::byte> source, int frame_index) {
DCHECK(!Done());
if (!i::FLAG_wasm_expose_debug_eval || !iterator_.is_wasm()) {
return v8::MaybeLocal<v8::String>();
}
Handle<String> value;
i::SafeForInterruptsScope safe_for_interrupt_scope(isolate_);
FrameSummary summary = FrameSummary::Get(iterator_.frame(), 0);
const FrameSummary::WasmFrameSummary& wasmSummary = summary.AsWasm();
Handle<WasmInstanceObject> instance = wasmSummary.wasm_instance();
if (!v8::internal::wasm::DebugEvaluate(source, instance, iterator_.frame())
.ToHandle(&value)) {
isolate_->OptionalRescheduleException(false);
return v8::MaybeLocal<v8::String>();
}
return Utils::ToLocal(value);
}
} // namespace internal
} // namespace v8
......@@ -34,8 +34,6 @@ class DebugStackTraceIterator final : public debug::StackTraceIterator {
bool Restart() override;
v8::MaybeLocal<v8::Value> Evaluate(v8::Local<v8::String> source,
bool throw_on_side_effect) override;
v8::MaybeLocal<v8::String> EvaluateWasm(
internal::Vector<const internal::byte> source, int frame_index) override;
private:
Isolate* isolate_;
......
......@@ -836,9 +836,6 @@ DEFINE_BOOL(trace_wasm_memory, false,
DEFINE_INT(wasm_tier_mask_for_testing, 0,
"bitmask of functions to compile with TurboFan instead of Liftoff")
DEFINE_BOOL(wasm_expose_debug_eval, true,
"Expose wasm evaluator support on the CDP")
DEFINE_BOOL(validate_asm, true, "validate asm.js modules before compiling")
DEFINE_BOOL(suppress_asm_messages, false,
"don't emit asm.js related messages (for golden file testing)")
......
......@@ -1241,55 +1241,6 @@ Response V8DebuggerAgentImpl::evaluateOnCallFrame(
result, exceptionDetails);
}
Response V8DebuggerAgentImpl::executeWasmEvaluator(
const String16& callFrameId, const protocol::Binary& evaluator,
Maybe<double> timeout,
std::unique_ptr<protocol::Runtime::RemoteObject>* result,
Maybe<protocol::Runtime::ExceptionDetails>* exceptionDetails) {
if (!v8::debug::StackTraceIterator::SupportsWasmDebugEvaluate()) {
return Response::ServerError(
"--wasm-expose-debug-eval is required to execte evaluator modules");
}
if (!isPaused()) return Response::ServerError(kDebuggerNotPaused);
InjectedScript::CallFrameScope scope(m_session, callFrameId);
Response response = scope.initialize();
if (!response.IsSuccess()) return response;
int frameOrdinal = static_cast<int>(scope.frameOrdinal());
std::unique_ptr<v8::debug::StackTraceIterator> it =
v8::debug::StackTraceIterator::Create(m_isolate, frameOrdinal);
if (it->Done()) {
return Response::ServerError("Could not find call frame with given id");
}
if (!it->GetScript()->IsWasm()) {
return Response::ServerError(
"executeWasmEvaluator can only be called on WebAssembly frames");
}
v8::MaybeLocal<v8::Value> maybeResultValue;
{
V8InspectorImpl::EvaluateScope evaluateScope(scope);
if (timeout.isJust()) {
response = evaluateScope.setTimeout(timeout.fromJust() / 1000.0);
if (!response.IsSuccess()) return response;
}
v8::MaybeLocal<v8::String> eval_result =
it->EvaluateWasm({evaluator.data(), evaluator.size()}, frameOrdinal);
if (!eval_result.IsEmpty()) maybeResultValue = eval_result.ToLocalChecked();
}
// Re-initialize after running client's code, as it could have destroyed
// context or session.
response = scope.initialize();
if (!response.IsSuccess()) return response;
String16 object_group = "";
InjectedScript* injected_script = scope.injectedScript();
return injected_script->wrapEvaluateResult(maybeResultValue, scope.tryCatch(),
object_group, WrapMode::kNoPreview,
result, exceptionDetails);
}
Response V8DebuggerAgentImpl::setVariableValue(
int scopeNumber, const String16& variableName,
std::unique_ptr<protocol::Runtime::CallArgument> newValueArgument,
......
......@@ -118,11 +118,6 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
Maybe<double> timeout,
std::unique_ptr<protocol::Runtime::RemoteObject>* result,
Maybe<protocol::Runtime::ExceptionDetails>*) override;
Response executeWasmEvaluator(
const String16& callFrameId, const protocol::Binary& evaluator,
Maybe<double> timeout,
std::unique_ptr<protocol::Runtime::RemoteObject>* result,
Maybe<protocol::Runtime::ExceptionDetails>* exceptionDetails) override;
Response setVariableValue(
int scopeNumber, const String16& variableName,
std::unique_ptr<protocol::Runtime::CallArgument> newValue,
......
This diff is collapsed.
// 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.
#ifndef V8_WASM_WASM_DEBUG_EVALUATE_H_
#define V8_WASM_WASM_DEBUG_EVALUATE_H_
#include "src/base/macros.h"
#include "src/handles/maybe-handles.h"
#include "src/wasm/wasm-objects.h"
namespace v8 {
namespace internal {
namespace wasm {
MaybeHandle<String> V8_EXPORT_PRIVATE
DebugEvaluate(Vector<const byte> snippet,
Handle<WasmInstanceObject> debuggee_instance, CommonFrame* frame);
} // namespace wasm
} // namespace internal
} // namespace v8
#endif // V8_WASM_WASM_DEBUG_EVALUATE_H_
......@@ -317,8 +317,6 @@ v8_source_set("cctest_sources") {
"wasm/test-streaming-compilation.cc",
"wasm/test-wasm-breakpoints.cc",
"wasm/test-wasm-codegen.cc",
"wasm/test-wasm-debug-evaluate.cc",
"wasm/test-wasm-debug-evaluate.h",
"wasm/test-wasm-import-wrapper-cache.cc",
"wasm/test-wasm-metrics.cc",
"wasm/test-wasm-serialization.cc",
......
......@@ -418,7 +418,6 @@
# Liftoff is not currently supported on ppc and s390
'test-liftoff-*': [SKIP],
'test-wasm-breakpoints/*' : [SKIP],
'test-wasm-debug-evaluate/*': [SKIP],
'test-run-wasm-module/Run_WasmModule_CompilationHintsNoTiering': [SKIP],
'test-wasm-serialization/TierDownAfterDeserialization': [SKIP],
'test-gc/RunWasmLiftoff*': [SKIP],
......@@ -514,7 +513,6 @@
'test-streaming-compilation/*': [SKIP],
'test-wasm-breakpoints/*': [SKIP],
'test-wasm-codegen/*': [SKIP],
'test-wasm-debug-evaluate/*': [SKIP],
'test-wasm-import-wrapper-cache/*': [SKIP],
'test-wasm-metrics/*': [SKIP],
'test-wasm-serialization/*': [SKIP],
......
This diff is collapsed.
Tests wasm debug-evaluate
Test: TestGetMemory
Result: 2
Expected: 2
Finished!
// 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.
// Flags: --expose-wasm --wasm-expose-debug-eval
utils.load('test/inspector/wasm-inspector-test.js');
const {session, contextGroup, Protocol} =
InspectorTest.start('Tests wasm debug-evaluate');
function printFailure(message) {
if (!message.result) {
InspectorTest.logMessage(message);
}
return message;
}
async function getWasmScript() {
while (true) {
const script = await Protocol.Debugger.onceScriptParsed();
if (script.params.url.startsWith('wasm://')) return script.params;
}
}
async function handleDebuggerPaused(data, messageObject) {
const topFrameId = messageObject.params.callFrames[0].callFrameId;
const params = {callFrameId: topFrameId, evaluator: data};
try {
const evalResult = await Protocol.Debugger.executeWasmEvaluator(params);
InspectorTest.log('Result: ' + evalResult.result.result.value);
} catch (err) {
InspectorTest.log(
'Eval failed: ' + err + '\nGot: ' + JSON.stringify(evalResult));
}
await Protocol.Debugger.resume();
}
async function runTest(testName, breakLine, debuggeeBytes, snippetBytes) {
try {
await Protocol.Debugger.onPaused(
handleDebuggerPaused.bind(null, snippetBytes));
InspectorTest.log('Test: ' + testName);
const scriptListener = getWasmScript();
await WasmInspectorTest.instantiate(debuggeeBytes);
const script = await scriptListener;
const msg = await Protocol.Debugger.setBreakpoint({
'location': {
'scriptId': script.scriptId,
'lineNumber': 0,
'columnNumber': breakLine
}
});
printFailure(msg);
const eval = await Protocol.Runtime.evaluate(
{'expression': 'instance.exports.main()'});
InspectorTest.log(
'Expected: ' + String.fromCharCode(eval.result.result.value));
InspectorTest.log('Finished!');
} catch (err) {
InspectorTest.log(err.message);
}
}
// copied from v8
function encode64(data) {
const BASE =
'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/';
const PAD = '=';
var ret = '';
var leftchar = 0;
var leftbits = 0;
for (var i = 0; i < data.length; i++) {
leftchar = (leftchar << 8) | data[i];
leftbits += 8;
while (leftbits >= 6) {
const curr = (leftchar >> (leftbits - 6)) & 0x3f;
leftbits -= 6;
ret += BASE[curr];
}
}
if (leftbits == 2) {
ret += BASE[(leftchar & 3) << 4];
ret += PAD + PAD;
} else if (leftbits == 4) {
ret += BASE[(leftchar & 0xf) << 2];
ret += PAD;
}
return ret;
}
(async () => {
try {
await Protocol.Debugger.enable();
await (async function TestGetMemory() {
const debuggee_builder = new WasmModuleBuilder();
debuggee_builder.addMemory(256, 256);
const mainFunc =
debuggee_builder.addFunction('main', kSig_i_v)
.addBody([
// clang-format off
kExprI32Const, 32,
kExprI32Const, 50,
kExprI32StoreMem, 0, 0,
kExprI32Const, 32,
kExprI32LoadMem, 0, 0,
kExprReturn
// clang-format on
])
.exportAs('main');
const snippet_builder = new WasmModuleBuilder();
snippet_builder.addMemory(1, 1);
const getMemoryIdx = snippet_builder.addImport(
'env', '__getMemory', makeSig([kWasmI32, kWasmI32, kWasmI32], []));
const heapBase = 32; // Just pick some position in memory
snippet_builder.addFunction('wasm_format', kSig_i_v)
.addBody([
// clang-format off
// __getMemory(32, 4, heapBase)
kExprI32Const, 32, kExprI32Const, 4, kExprI32Const, heapBase,
kExprCallFunction, getMemoryIdx,
// return heapBase
kExprI32Const, heapBase,
kExprReturn
// clang-format on
])
.exportAs('wasm_format');
const debuggeeModule = debuggee_builder.toArray();
await runTest(
'TestGetMemory', mainFunc.body_offset + 9, debuggeeModule,
encode64(snippet_builder.toArray()));
})();
} catch (err) {
InspectorTest.log(err)
}
InspectorTest.completeTest();
})();
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