Bug 170782

Summary: WebAssembly: don't expose any WebAssembly JS object if JIT is off
Product: WebKit Reporter: JF Bastien <jfbastien>
Component: JavaScriptCoreAssignee: JF Bastien <jfbastien>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, jfbastien, joepeck, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 159775, 170998    
Attachments:
Description Flags
patch none

Description JF Bastien 2017-04-12 11:31:40 PDT
It's kinda misleading of `typeof WebAssembly !== "undefined"` but we know that compilation will never succeed. It's also weird to have WebAssembly.Memory if JIT is off.

A WebKit1 embedding would be a weird place to expose WebAssembly JS objects.
Comment 1 JF Bastien 2017-04-19 00:07:25 PDT
Created attachment 307465 [details]
patch
Comment 2 JF Bastien 2017-04-19 08:26:08 PDT
One thing I noticed when writing this: our tests run with useFTLJIT=0! That's because of BASE_OPTIONS which runWithOutputHandler uses (and in turn run calls runWithOutputHandler).

Initially I'd disabled WebAssembly if useJIT or useFTLJIT was false, but that would require changing the tests and I'm not sure the distinction matters. B3 is really what's used by wasm, and a special blend of it, so I'm not sure useFTLJIT=false is relevant to wasm.

Thoughts?
Comment 3 Saam Barati 2017-04-19 09:52:42 PDT
(In reply to JF Bastien from comment #2)
> One thing I noticed when writing this: our tests run with useFTLJIT=0!
> That's because of BASE_OPTIONS which runWithOutputHandler uses (and in turn
> run calls runWithOutputHandler).
> 
> Initially I'd disabled WebAssembly if useJIT or useFTLJIT was false, but
> that would require changing the tests and I'm not sure the distinction
> matters. B3 is really what's used by wasm, and a special blend of it, so I'm
> not sure useFTLJIT=false is relevant to wasm.
> 
> Thoughts?

I would not tie Wasm being enabled to the ftl being enabled. That said, Wasm tests should run with the FTL enabled.
Comment 4 Keith Miller 2017-04-19 09:57:26 PDT
Does this prevent me from doing --useJIT=0 --dumpDisassembly=1 --useWebAssembly=1? I like doing that because it lets me see the wasm disassembly. I guess we could just add a dumpWebAssemblyDisassembly (wow! that's a mouthful) option.
Comment 5 JF Bastien 2017-04-19 10:02:00 PDT
(In reply to Saam Barati from comment #3)
> (In reply to JF Bastien from comment #2)
> > One thing I noticed when writing this: our tests run with useFTLJIT=0!
> > That's because of BASE_OPTIONS which runWithOutputHandler uses (and in turn
> > run calls runWithOutputHandler).
> > 
> > Initially I'd disabled WebAssembly if useJIT or useFTLJIT was false, but
> > that would require changing the tests and I'm not sure the distinction
> > matters. B3 is really what's used by wasm, and a special blend of it, so I'm
> > not sure useFTLJIT=false is relevant to wasm.
> > 
> > Thoughts?
> 
> I would not tie Wasm being enabled to the ftl being enabled. That said, Wasm
> tests should run with the FTL enabled.

Sounds good.

I filed a bug:
  https://bugs.webkit.org/show_bug.cgi?id=170998
Will address later.



(In reply to Keith Miller from comment #4)
> Does this prevent me from doing --useJIT=0 --dumpDisassembly=1
> --useWebAssembly=1? I like doing that because it lets me see the wasm
> disassembly. I guess we could just add a dumpWebAssemblyDisassembly (wow!
> that's a mouthful) option.

Correct. Without JIT it won't generate the stubs to C++ code anyways, no?
Comment 6 WebKit Commit Bot 2017-04-19 10:12:41 PDT
Comment on attachment 307465 [details]
patch

Clearing flags on attachment: 307465

Committed r215517: <http://trac.webkit.org/changeset/215517>
Comment 7 WebKit Commit Bot 2017-04-19 10:12:42 PDT
All reviewed patches have been landed.  Closing bug.