[wasm] Fix importing wasm functions which are being debugged

test/inspector/debugger/wasm-imports.js

Index: test/inspector/debugger/wasm-imports.js
diff --git a/test/inspector/debugger/wasm-imports.js b/test/inspector/debugger/wasm-imports.js
new file mode 100644
index 0000000000000000000000000000000000000000..c82ad042c103272fc5a489470161b2404ee54130
--- /dev/null
+++ b/test/inspector/debugger/wasm-imports.js
@@ -0,0 +1,123 @@
+// 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.
+
+load('test/mjsunit/wasm/wasm-constants.js');
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+// Build two modules A and B. A defines function func, which contains a
+// breakpoint. This function is then imported by B and called via main. The
+// breakpoint must be hit.
+// This failed before (http://crbug.com/v8/5971).
+
+var builder_a = new WasmModuleBuilder();
+var func_idx = builder_a.addFunction('func', kSig_v_v)
+                   .addBody([kExprNop])
+                   .exportFunc()
+                   .index;
+var module_a_bytes = builder_a.toArray();
+
+var builder_b = new WasmModuleBuilder();
+var import_idx = builder_b.addImport('imp', 'f', kSig_v_v);
+builder_b.addFunction('main', kSig_v_v)
+    .addBody([kExprCallFunction, import_idx])
+    .exportFunc();
+var module_b_bytes = builder_b.toArray();
+
+function instantiate(bytes, imp) {
+  var buffer = new ArrayBuffer(bytes.length);
+  var view = new Uint8Array(buffer);
+  for (var i = 0; i < bytes.length; ++i) {
+    view[i] = bytes[i] | 0;
+  }
+
+  var module = new WebAssembly.Module(buffer);
+  // Add to global instances array.
+  instances.push(new WebAssembly.Instance(module, imp));
+}
+
+var evalWithUrl = (code, url) => Protocol.Runtime.evaluate(
+    {'expression': code + '\n//# sourceURL=v8://test/' + url});
+
+Protocol.Debugger.onPaused(handlePaused);
+var script_a_id;
+
+Protocol.Debugger.enable()
+    .then(() => InspectorTest.log('Installing code and global variable.'))
+    .then(
+        () => evalWithUrl(
+            'var instances = [];\n' + instantiate.toString(), 'setup'))
+    .then(() => InspectorTest.log('Calling instantiate function for module A.'))
+    .then(
+        () =>
+            (evalWithUrl(
+                 'instantiate(' + JSON.stringify(module_a_bytes) + ')',
+                 'instantiateA'),
+             0))
+    .then(waitForWasmScript)
+    .then(() => InspectorTest.log('Setting breakpoint in line 1, column 2'))
+    .then(() => Protocol.Debugger.setBreakpoint({

[kozy, 2017/02/28 16:47:02]

Just wondering, is setBreakpointByUrl supported for wasm modules?

[Clemens Hammacher, 2017/03/02 14:02:35]

Yes, it is. In order to also cover it, I just changed this test to use
setBreakpointByUrl instead of setBreakpoint. That uncovered a minor bug in
wasm-translation.cc which this CL now also fixes.

+      'location':

[kozy, 2017/02/28 16:47:01]

quotes are optional: 'location' -> location

[Clemens Hammacher, 2017/03/02 14:02:35]

Done.

+          {'scriptId': script_a_id, 'lineNumber': 1, 'columnNumber': 2}
+    }))
+    .then(printFailure)
+    .then(msg => InspectorTest.logMessage(msg.result.actualLocation))

[kozy, 2017/02/28 16:47:02]

You can use here too, but please change argument type from callFrame to
location, more details about this method below.
InspectorTest.logCallFrameSourceLocation(msg.result.actualLocation)

Then you don't actually need to dump source on parsed event and simplify
waitForWasmScript a little more then below.

[Clemens Hammacher, 2017/03/02 14:02:35]

Done, but it required some refactoring.
logCallFrameSourceLocation accepted a callFrame, not a location. I renamed the
method to logSourceLocation, and added another method logSourceLocations to log
a full array.

+    .then(() => InspectorTest.log('Calling instantiate function for module B.'))
+    .then(
+        () =>
+            (evalWithUrl(
+                 'instantiate(' + JSON.stringify(module_b_bytes) +
+                     ', {imp: {f: instances[0].exports.func}})',
+                 'instantiateB'),
+             0))
+    .then(() => InspectorTest.log('Calling main function on module B.'))
+    .then(() => evalWithUrl('instances[1].exports.main()', 'runWasm'))

[kozy, 2017/02/28 16:47:02]

To handle asynchronous call in onPaused handler you can have separate chain of
promises for it instead of listener:
Protocol.Debugger.oncePaused()
  .then(handlePaused)
  .then(() => Protocol.Debugger.resume())

[Clemens Hammacher, 2017/03/02 14:02:35]

Done, also inlined the handlePaused method, which was basically just another
promise chain.

+    .then(() => InspectorTest.log('exports.main returned.'))
+    .then(() => InspectorTest.log('Finished.'))
+    .then(InspectorTest.completeTest);
+
+function printFailure(message) {
+  if (!message.result) {
+    InspectorTest.logMessage(message);
+  }
+  return message;
+}
+
+function waitForWasmScript() {

[kozy, 2017/02/28 16:47:02]

Less symbols with more promise magic:

function waitForWasmScript(msg) {
  if (!msg || !msg.params.url.startsWith('wasm://')) {
    return Protocol.Debugger.onceScriptParsed().then(waitForWasmScript);
  }
  InspectorTest.log('Got wasm script!');
  script_a_id = msg.params.scriptId;
  InspectorTest.log('Source:');
  return Protocol.Debugger.getScriptSource({scriptId: script_a_id})
      .then(msg => InspectorTest.log(msg.result.scriptSource));
}

[Clemens Hammacher, 2017/03/02 14:02:35]

Wow, that's awesome. I now even return the url from this method instead of
storing it to a global variable.

+  InspectorTest.log('Waiting for wasm script to be parsed.');
+  var got_wasm_script;
+  var promise = new Promise(fulfill => got_wasm_script = fulfill);
+  function waitForMore() {
+    Protocol.Debugger.onceScriptParsed()
+        .then(handleNewScript);
+  }
+  function handleNewScript(msg) {
+    var url = msg.params.url;
+    if (!url.startsWith('wasm://')) {
+      InspectorTest.log('Ignoring script with url ' + url);

[kozy, 2017/02/28 16:47:01]

Could potentially add flakiness if we compile somewhere for internal purpose,
let's drop this output.

[Clemens Hammacher, 2017/03/02 14:02:35]

Done.

+      waitForMore();
+      return;
+    }
+    InspectorTest.log('Got wasm script!');
+    script_a_id = msg.params.scriptId;
+    InspectorTest.log('Source:');
+    Protocol.Debugger.getScriptSource({scriptId: script_a_id})
+        .then(printFailure)
+        .then(msg => InspectorTest.log(msg.result.scriptSource))
+        .then(() => got_wasm_script(script_a_id));
+  }
+  waitForMore();
+  return promise;
+}
+
+function handlePaused(msg) {
+  var loc = msg.params.callFrames[0].location;
+  InspectorTest.log(

[kozy, 2017/02/28 16:47:02]

You can use InspectorTest.logCallFrames([msg.params.callFrames[0]]); to log call
frames - all of them or only top if needed.

And you can use:
InspectorTest.logCallFrameSourceLocation(msg.params.callFrames[0]);
then it will dump following text:
func $func
  #nop
end
where # - position of actual break - I found it's very informative. To use this
function before Debugger.enable you need to call:
InspectorTest.setupScriptMap().
Example:
https://cs.chromium.org/chromium/src/v8/test/inspector/debugger/step-into.js?...

warning - this command dump output asynchronously (first call per scriptId) -
return promise.

[Clemens Hammacher, 2017/03/02 14:02:35]

Neverending awesomeness. Done!

+      'Paused at ' + loc.lineNumber + ':' + loc.columnNumber + '.');
+  InspectorTest.log('Getting current stack trace via "new Error().stack".');
+  // TODO(clemensh): The interpreted frame should also show up on this stack

[kozy, 2017/02/28 16:47:02]

Sounds like big TODO, how stepping works in this case? Will we make step from
top interpreter frame or from top that we show to user?

[Clemens Hammacher, 2017/03/02 14:02:35]

Stepping works correctly (on the top interpreter frame). This TODO is referring
to another TODO in Isolate::CaptureSimpleStackTrace, where
WASM_INTERPRETER_ENTRY frames are just skipped currently.

+  // trace.
+  evalWithUrl('new Error().stack', 'getStack')

[kozy, 2017/02/28 16:47:01]

This command is asynchronous and it works currently because we run commands in
FIFO order but adding any new command somewhere in between could break this
test.

[kozy, 2017/02/28 19:28:26]

Oops, please disregard this comment, just change last .then to:
.then(Protocol.Debugger.resume)
otherwise you call resume synchronously.

[Clemens Hammacher, 2017/03/02 14:02:35]

Good catch! Done.

+      .then(msg => InspectorTest.log(msg.result.result.value))
+      .then(Protocol.Debugger.resume());
+}