Bug 168694

Summary: WebAssembly: disable when --useJIT=0
Product: WebKit Reporter: JF Bastien <jfbastien>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: fpizlo, jfbastien, keith_miller, mark.lam, msaboff, sbarati
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 159775    

Description JF Bastien 2017-02-21 17:00:03 PST
We get an assertion in repatchNearCall because there's an immediate that doesn't fit. The problem is that without JIT the wasm code can't be linked to trampolines because they don't exist.

Simple repro:
  (cd ./JSTests/wasm/ && lldb ../../current-debug/bin/jsc -- -m --useWebAssembly=1 ./js-api/wasm-to-wasm.js --useConcurrentJIT=0 --useJIT=0)

We probably want to JIT just those stubs when WebAssembly is used, even if the JIT is disabled.

Backtrace:

1   0x1013d3e9d WTFCrash
2   0x100300839 JSC::X86Assembler::setRel32(void*, void*)
3   0x1009cf43d JSC::X86Assembler::relinkCall(void*, void*)
4   0x100e37731 JSC::AbstractMacroAssembler<JSC::X86Assembler, JSC::MacroAssemblerX86Common>::repatchNearCall(JSC::CodeLocationNearCall, JSC::CodeLocationLabel)
5   0x100e3753f JSC::linkFor(JSC::ExecState*, JSC::CallLinkInfo&, JSC::CodeBlock*, JSC::JSFunction*, JSC::MacroAssemblerCodePtr)
6   0x100e02ff6 operationLinkCall
...

frame #1: 0x0000000100300839 JavaScriptCore`JSC::X86Assembler::setRel32(from=0x000039c74ec011be, to=0x0000000100e58c6e) + 89 at X86Assembler.h:3123
   3120	    static void setRel32(void* from, void* to)
   3121	    {
   3122	        intptr_t offset = reinterpret_cast<intptr_t>(to) - reinterpret_cast<intptr_t>(from);
-> 3123	        ASSERT(offset == static_cast<int32_t>(offset));
   3124	
   3125	        setInt32(from, offset);
   3126	    }
(lldb) p/x from
(void *) $0 = 0x000039c74ec011be
(lldb) p/x to
(void *) $1 = 0x0000000100e58c6e
(lldb) p/x offset
(intptr_t) $2 = 0xffffc639b2257ab0
Comment 1 Saam Barati 2017-02-21 18:10:56 PST
(In reply to comment #0)
> We get an assertion in repatchNearCall because there's an immediate that
> doesn't fit. The problem is that without JIT the wasm code can't be linked
> to trampolines because they don't exist.
> 
> Simple repro:
>   (cd ./JSTests/wasm/ && lldb ../../current-debug/bin/jsc -- -m
> --useWebAssembly=1 ./js-api/wasm-to-wasm.js --useConcurrentJIT=0 --useJIT=0)
> 
> We probably want to JIT just those stubs when WebAssembly is used, even if
> the JIT is disabled.
> 
> Backtrace:
> 
> 1   0x1013d3e9d WTFCrash
> 2   0x100300839 JSC::X86Assembler::setRel32(void*, void*)
> 3   0x1009cf43d JSC::X86Assembler::relinkCall(void*, void*)
> 4   0x100e37731 JSC::AbstractMacroAssembler<JSC::X86Assembler,
> JSC::MacroAssemblerX86Common>::repatchNearCall(JSC::CodeLocationNearCall,
> JSC::CodeLocationLabel)
> 5   0x100e3753f JSC::linkFor(JSC::ExecState*, JSC::CallLinkInfo&,
> JSC::CodeBlock*, JSC::JSFunction*, JSC::MacroAssemblerCodePtr)
> 6   0x100e02ff6 operationLinkCall
> ...
> 
> frame #1: 0x0000000100300839
> JavaScriptCore`JSC::X86Assembler::setRel32(from=0x000039c74ec011be,
> to=0x0000000100e58c6e) + 89 at X86Assembler.h:3123
>    3120	    static void setRel32(void* from, void* to)
>    3121	    {
>    3122	        intptr_t offset = reinterpret_cast<intptr_t>(to) -
> reinterpret_cast<intptr_t>(from);
> -> 3123	        ASSERT(offset == static_cast<int32_t>(offset));
>    3124	
>    3125	        setInt32(from, offset);
>    3126	    }
> (lldb) p/x from
> (void *) $0 = 0x000039c74ec011be
> (lldb) p/x to
> (void *) $1 = 0x0000000100e58c6e
> (lldb) p/x offset
> (intptr_t) $2 = 0xffffc639b2257ab0

Why would useJIT()=0 ever be valid with Wasm code? Seems like we should never allow running in that configuration since we're already relying on JITing for that call IC.
Comment 2 JF Bastien 2017-02-21 18:16:00 PST
> Why would useJIT()=0 ever be valid with Wasm code? Seems like we should
> never allow running in that configuration since we're already relying on
> JITing for that call IC.

It would be nice to be able to say "compile wasm, but don't JIT JS". I'm not sure what we would call that option. Regardless of what we do, it would:

  - AOT wasm (as always)
  - Compile the wasm thunks at the same time
  - Compile the trampolines that are required for ICs when we compile wasm code (because usually baseline does this?)

I think we might have a super unlikely race in current wasm where it's possible to always be in the interpreter, compile some wasm, and then not have those trampolines either? I'm not sure.
Comment 3 Filip Pizlo 2017-02-21 19:19:32 PST
(In reply to comment #2)
> > Why would useJIT()=0 ever be valid with Wasm code? Seems like we should
> > never allow running in that configuration since we're already relying on
> > JITing for that call IC.
> 
> It would be nice to be able to say "compile wasm, but don't JIT JS". I'm not
> sure what we would call that option. Regardless of what we do, it would:
> 
>   - AOT wasm (as always)
>   - Compile the wasm thunks at the same time
>   - Compile the trampolines that are required for ICs when we compile wasm
> code (because usually baseline does this?)
> 
> I think we might have a super unlikely race in current wasm where it's
> possible to always be in the interpreter, compile some wasm, and then not
> have those trampolines either? I'm not sure.

I don't think useJIT means what you want it to mean.  useJIT determines whether JSC is allowed to JIT.  If it's not allowed to JIT, then it's not allowed to wasm.  The only time we set it to false is to simulate environments where we don't have the JIT entitlement, and so cannot JIT at all.

So I think this is wontfix.
Comment 4 JF Bastien 2017-02-22 09:44:22 PST
> I don't think useJIT means what you want it to mean.  useJIT determines
> whether JSC is allowed to JIT.  If it's not allowed to JIT, then it's not
> allowed to wasm.  The only time we set it to false is to simulate
> environments where we don't have the JIT entitlement, and so cannot JIT at
> all.
> 
> So I think this is wontfix.

That's fine. In that case we should throw when users try to use WebAssembly and useJIT is false. Uses of WebAssembly.Module / WebAssembly.compile / WebAssembly.instantiate should throw. Or maybe the WebAssembly object just shouldn't be there if useJIT == false. I think that's better.

I can get what I want by disabling baseline+DFG+FTL instead. I'd also update the description of useJIT, which is currently "allows the baseline JIT to be used if true".

So I'll reopen this bug, change its description, and implement it when I feel like doing something simple :)
Comment 5 JF Bastien 2017-04-19 16:26:27 PDT
Fixed separately.