Bug 165150 - WebAssembly JS API: implement start function
Summary: WebAssembly JS API: implement start function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: JF Bastien
URL:
Keywords: InRadar
Depends on:
Blocks: 161709 165658 165714
  Show dependency treegraph
 
Reported: 2016-11-29 12:00 PST by JF Bastien
Modified: 2016-12-09 19:09 PST (History)
7 users (show)

See Also:


Attachments
WIP (2.15 KB, patch)
2016-12-08 22:13 PST, JF Bastien
jfbastien: commit-queue-
Details | Formatted Diff | Diff
patch (4.88 KB, patch)
2016-12-08 22:43 PST, JF Bastien
jfbastien: commit-queue-
Details | Formatted Diff | Diff
patch (18.92 KB, patch)
2016-12-09 14:13 PST, JF Bastien
sbarati: review+
sbarati: commit-queue-
Details | Formatted Diff | Diff
patch (20.10 KB, patch)
2016-12-09 14:48 PST, JF Bastien
commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch (20.83 KB, patch)
2016-12-09 14:58 PST, JF Bastien
jfbastien: commit-queue+
Details | Formatted Diff | Diff
patch (21.43 KB, patch)
2016-12-09 15:15 PST, JF Bastien
commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch (21.45 KB, patch)
2016-12-09 16:06 PST, JF Bastien
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2016-11-29 12:00:00 PST
.
Comment 1 JF Bastien 2016-12-08 22:13:22 PST
Created attachment 296641 [details]
WIP

I've started implementing this. Here are basic JSON tests.
Comment 2 JF Bastien 2016-12-08 22:43:47 PST
Created attachment 296643 [details]
patch

Feed the JSON tests from the Builder. I still have some checking to do, then the binary side.
Comment 3 JF Bastien 2016-12-09 14:13:47 PST
Created attachment 296698 [details]
patch

Patch for landing.
Comment 4 JF Bastien 2016-12-09 14:18:25 PST
rdar://problem/29599190
Comment 5 Saam Barati 2016-12-09 14:26:41 PST
Comment on attachment 296698 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=296698&action=review

> JSTests/wasm/js-api/test_Start.js:14
> +    const instance = new WebAssembly.Instance(module);

Please add a test that proves that this ran. Maybe by writing to memory.

> Source/JavaScriptCore/wasm/WasmFormat.h:128
> +    std::optional<uint32_t> startFunctionIndexSpace;

Why not just call this "startFunctionIndex"?

> Source/JavaScriptCore/wasm/WasmModuleParser.cpp:397
> +        || startFunctionIndex >= m_functionIndexSpace.size())

Please add a test for this.

> Source/JavaScriptCore/wasm/WasmModuleParser.cpp:401
> +    if (signature->arguments.size() != 0
> +        || signature->returnType != Void)

Please add a test for this.
Comment 6 JF Bastien 2016-12-09 14:39:20 PST
> > Source/JavaScriptCore/wasm/WasmFormat.h:128
> > +    std::optional<uint32_t> startFunctionIndexSpace;
> 
> Why not just call this "startFunctionIndex"?

Because the value we receive in the function index space, so it can include imports for now. Once we resolve the spec issue we'll have to change things, but it's still important to note that it isn't just the internal function index.
Comment 7 JF Bastien 2016-12-09 14:48:30 PST
Created attachment 296705 [details]
patch

Address comments.
Comment 8 WebKit Commit Bot 2016-12-09 14:49:35 PST
Comment on attachment 296705 [details]
patch

Rejecting attachment 296705 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 296705, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
unk #4 succeeded at 461 (offset 25 lines).
patching file Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp
Hunk #1 succeeded at 102 with fuzz 1 (offset 12 lines).
patching file Source/JavaScriptCore/wasm/js/WebAssemblyFunction.h
patching file Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp
patching file Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.h

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/2681933
Comment 9 JF Bastien 2016-12-09 14:58:08 PST
Created attachment 296709 [details]
patch

Fix merge conflict.
Comment 10 JF Bastien 2016-12-09 15:15:53 PST
Created attachment 296713 [details]
patch

Another merge issue.
Comment 11 WebKit Commit Bot 2016-12-09 15:41:26 PST
Comment on attachment 296713 [details]
patch

Rejecting attachment 296713 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
t -o /Volumes/Data/EWS/WebKit/WebKitBuild/Release/jsc
Undefined symbols for architecture x86_64:
  "JSC::Wasm::ModuleInformation::~ModuleInformation()", referenced from:
      functionTestWasmModuleFunctions(JSC::ExecState*) in jsc.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

** BUILD FAILED **


The following build commands failed:
	Ld /Volumes/Data/EWS/WebKit/WebKitBuild/Release/jsc normal x86_64
(1 failure)

Full output: http://webkit-queues.webkit.org/results/2682613
Comment 12 JF Bastien 2016-12-09 16:06:31 PST
Created attachment 296731 [details]
patch

Fix bot build issue which doesn't occur on local cmake build.
Comment 13 WebKit Commit Bot 2016-12-09 18:34:33 PST
Comment on attachment 296731 [details]
patch

Clearing flags on attachment: 296731

Committed r209642: <http://trac.webkit.org/changeset/209642>
Comment 14 WebKit Commit Bot 2016-12-09 18:34:37 PST
All reviewed patches have been landed.  Closing bug.