WebAssembly: Make our calls out to JS PIC friendly
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 1 Apr 2017 02:09:51 +0000 (02:09 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 1 Apr 2017 02:09:51 +0000 (02:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170261

Reviewed by Keith Miller.

This patch removes a direct call from the module to the Wasm to JS stub.
Instead, we do an indirect call to the stub by loading the stub's executable
address off of the CodeBlock. This is to make the code we emit for comply with
requirements needed for PIC.

Adding this indirection is not ideal. Although this patch is neutral on
WasmBench, we really want to get back to a world where we have an IC
call infrastructure. This patch is obviously a regression on some
types of programs. I've filed this bug to make sure we implement a
PIC compliant Wasm to JS call IC:
https://bugs.webkit.org/show_bug.cgi?id=170375

* wasm/WasmB3IRGenerator.cpp:
* wasm/WasmFormat.h:
* wasm/WasmPlan.cpp:
(JSC::Wasm::Plan::complete):
* wasm/js/JSWebAssemblyCodeBlock.cpp:
(JSC::JSWebAssemblyCodeBlock::initialize):
* wasm/js/JSWebAssemblyCodeBlock.h:
(JSC::JSWebAssemblyCodeBlock::create):
(JSC::JSWebAssemblyCodeBlock::offsetOfImportWasmToJSStub):
(JSC::JSWebAssemblyCodeBlock::offsetOfCallees):
(JSC::JSWebAssemblyCodeBlock::allocationSize):
(JSC::JSWebAssemblyCodeBlock::importWasmToJSStub):
* wasm/js/JSWebAssemblyInstance.cpp:
(JSC::JSWebAssemblyInstance::addUnitializedCodeBlock):
* wasm/js/JSWebAssemblyInstance.h:
(JSC::JSWebAssemblyInstance::offsetOfCodeBlock):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@214711 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
Source/JavaScriptCore/wasm/WasmFormat.h
Source/JavaScriptCore/wasm/WasmPlan.cpp
Source/JavaScriptCore/wasm/js/JSWebAssemblyCodeBlock.cpp
Source/JavaScriptCore/wasm/js/JSWebAssemblyCodeBlock.h
Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.cpp
Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.h

index d266574..25b97f2 100644 (file)
@@ -1,3 +1,39 @@
+2017-03-31  Saam Barati  <sbarati@apple.com>
+
+        WebAssembly: Make our calls out to JS PIC friendly
+        https://bugs.webkit.org/show_bug.cgi?id=170261
+
+        Reviewed by Keith Miller.
+
+        This patch removes a direct call from the module to the Wasm to JS stub.
+        Instead, we do an indirect call to the stub by loading the stub's executable
+        address off of the CodeBlock. This is to make the code we emit for comply with
+        requirements needed for PIC.
+        
+        Adding this indirection is not ideal. Although this patch is neutral on
+        WasmBench, we really want to get back to a world where we have an IC
+        call infrastructure. This patch is obviously a regression on some
+        types of programs. I've filed this bug to make sure we implement a
+        PIC compliant Wasm to JS call IC:
+        https://bugs.webkit.org/show_bug.cgi?id=170375
+
+        * wasm/WasmB3IRGenerator.cpp:
+        * wasm/WasmFormat.h:
+        * wasm/WasmPlan.cpp:
+        (JSC::Wasm::Plan::complete):
+        * wasm/js/JSWebAssemblyCodeBlock.cpp:
+        (JSC::JSWebAssemblyCodeBlock::initialize):
+        * wasm/js/JSWebAssemblyCodeBlock.h:
+        (JSC::JSWebAssemblyCodeBlock::create):
+        (JSC::JSWebAssemblyCodeBlock::offsetOfImportWasmToJSStub):
+        (JSC::JSWebAssemblyCodeBlock::offsetOfCallees):
+        (JSC::JSWebAssemblyCodeBlock::allocationSize):
+        (JSC::JSWebAssemblyCodeBlock::importWasmToJSStub):
+        * wasm/js/JSWebAssemblyInstance.cpp:
+        (JSC::JSWebAssemblyInstance::addUnitializedCodeBlock):
+        * wasm/js/JSWebAssemblyInstance.h:
+        (JSC::JSWebAssemblyInstance::offsetOfCodeBlock):
+
 2017-03-31  Keith Miller  <keith_miller@apple.com>
 
         WebAssembly: webAssemblyB3OptimizationLevel should use defaultB3OptLevel by default
index 03419cf..020abd5 100644 (file)
@@ -908,24 +908,30 @@ auto B3IRGenerator::addCall(uint32_t functionIndex, const Signature& signature,
                     AllowMacroScratchRegisterUsage allowScratch(jit);
                     CCallHelpers::Call call = jit.call();
                     jit.addLinkTask([unlinkedWasmToWasmCalls, call, functionIndex] (LinkBuffer& linkBuffer) {
-                        unlinkedWasmToWasmCalls->append({ linkBuffer.locationOf(call), functionIndex, UnlinkedWasmToWasmCall::Target::ToWasm });
+                        unlinkedWasmToWasmCalls->append({ linkBuffer.locationOf(call), functionIndex });
                     });
                 });
             });
         UpsilonValue* wasmCallResultUpsilon = returnType == Void ? nullptr : isWasmBlock->appendNew<UpsilonValue>(m_proc, origin(), wasmCallResult);
         isWasmBlock->appendNewControlValue(m_proc, Jump, origin(), continuation);
 
+        // FIXME: Lets remove this indirection by creating a PIC friendly IC
+        // for calls out to JS. This shouldn't be that hard to do. We could probably
+        // implement the IC to be over Wasm::Context*.
+        // https://bugs.webkit.org/show_bug.cgi?id=170375
+        Value* codeBlock = isJSBlock->appendNew<MemoryValue>(m_proc,
+            Load, pointerType(), origin(), m_instanceValue, JSWebAssemblyInstance::offsetOfCodeBlock());
+        Value* jumpDestination = isJSBlock->appendNew<MemoryValue>(m_proc,
+            Load, pointerType(), origin(), codeBlock, JSWebAssemblyCodeBlock::offsetOfImportWasmToJSStub(m_info.internalFunctionCount(), functionIndex));
         Value* jsCallResult = wasmCallingConvention().setupCall(m_proc, isJSBlock, origin(), args, toB3Type(returnType),
-            [=] (PatchpointValue* patchpoint) {
+            [&] (PatchpointValue* patchpoint) {
                 patchpoint->effects.writesPinned = true;
                 patchpoint->effects.readsPinned = true;
+                patchpoint->append(jumpDestination, ValueRep::SomeRegister);
                 patchpoint->clobberLate(PinnedRegisterInfo::get().toSave());
-                patchpoint->setGenerator([unlinkedWasmToWasmCalls, functionIndex] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+                patchpoint->setGenerator([unlinkedWasmToWasmCalls, functionIndex, returnType] (CCallHelpers& jit, const B3::StackmapGenerationParams& params) {
                     AllowMacroScratchRegisterUsage allowScratch(jit);
-                    CCallHelpers::Call call = jit.call();
-                    jit.addLinkTask([unlinkedWasmToWasmCalls, call, functionIndex] (LinkBuffer& linkBuffer) {
-                        unlinkedWasmToWasmCalls->append({ linkBuffer.locationOf(call), functionIndex, UnlinkedWasmToWasmCall::Target::ToJs });
-                    });
+                    jit.call(params[returnType == Void ? 0 : 1].gpr());
                 });
             });
         UpsilonValue* jsCallResultUpsilon = returnType == Void ? nullptr : isJSBlock->appendNew<UpsilonValue>(m_proc, origin(), jsCallResult);
@@ -953,7 +959,7 @@ auto B3IRGenerator::addCall(uint32_t functionIndex, const Signature& signature,
                     AllowMacroScratchRegisterUsage allowScratch(jit);
                     CCallHelpers::Call call = jit.call();
                     jit.addLinkTask([unlinkedWasmToWasmCalls, call, functionIndex] (LinkBuffer& linkBuffer) {
-                        unlinkedWasmToWasmCalls->append({ linkBuffer.locationOf(call), functionIndex, UnlinkedWasmToWasmCall::Target::ToWasm });
+                        unlinkedWasmToWasmCalls->append({ linkBuffer.locationOf(call), functionIndex });
                     });
                 });
             });
index 51f77f4..e80dd7a 100644 (file)
@@ -269,10 +269,6 @@ struct ModuleInformation {
 struct UnlinkedWasmToWasmCall {
     CodeLocationCall callLocation;
     size_t functionIndex;
-    enum class Target : uint8_t {
-        ToJs,
-        ToWasm,
-    } target;
 };
 
 struct Entrypoint {
index 304e4f7..68aaf12 100644 (file)
@@ -296,13 +296,9 @@ void Plan::complete(const AbstractLocker&)
                 void* executableAddress;
                 if (m_moduleInformation->isImportedFunctionFromFunctionIndexSpace(call.functionIndex)) {
                     // FIXME imports could have been linked in B3, instead of generating a patchpoint. This condition should be replaced by a RELEASE_ASSERT. https://bugs.webkit.org/show_bug.cgi?id=166462
-                    executableAddress = call.target == UnlinkedWasmToWasmCall::Target::ToJs
-                    ? m_wasmExitStubs.at(call.functionIndex).wasmToJs.code().executableAddress()
-                    : m_wasmExitStubs.at(call.functionIndex).wasmToWasm.code().executableAddress();
-                } else {
-                    ASSERT(call.target != UnlinkedWasmToWasmCall::Target::ToJs);
+                    executableAddress = m_wasmExitStubs.at(call.functionIndex).wasmToWasm.code().executableAddress();
+                } else
                     executableAddress = m_wasmInternalFunctions.at(call.functionIndex - m_wasmExitStubs.size())->wasmEntrypoint.compilation->code().executableAddress();
-                }
                 MacroAssembler::repatchCall(call.callLocation, CodeLocationLabel(executableAddress));
             }
         }
index ae5d921..3e7f053 100644 (file)
@@ -72,6 +72,10 @@ void JSWebAssemblyCodeBlock::initialize()
     m_callLinkInfos = plan().takeCallLinkInfos();
     m_wasmExitStubs = plan().takeWasmExitStubs();
 
+    // The code a module emits to call into JS relies on us to set this up.
+    for (unsigned i = 0; i < m_wasmExitStubs.size(); i++)
+        importWasmToJSStub(m_calleeCount, i) = m_wasmExitStubs[i].wasmToJs.code().executableAddress();
+
     plan().initializeCallees([&] (unsigned calleeIndex, JSWebAssemblyCallee* jsEntrypointCallee, JSWebAssemblyCallee* wasmEntrypointCallee) {
         setJSEntrypointCallee(vm, calleeIndex, jsEntrypointCallee);
         setWasmEntrypointCallee(vm, calleeIndex, wasmEntrypointCallee);
index 08abdbc..7c6acf0 100644 (file)
@@ -49,9 +49,9 @@ public:
     typedef JSCell Base;
     static const unsigned StructureFlags = Base::StructureFlags | StructureIsImmortal;
 
-    static JSWebAssemblyCodeBlock* create(VM& vm, JSWebAssemblyModule* owner, Wasm::MemoryMode mode, Ref<Wasm::Plan>&& plan, unsigned calleeCount)
+    static JSWebAssemblyCodeBlock* create(VM& vm, JSWebAssemblyModule* owner, Wasm::MemoryMode mode, Ref<Wasm::Plan>&& plan, unsigned calleeCount, unsigned functionImportCount)
     {
-        auto* result = new (NotNull, allocateCell<JSWebAssemblyCodeBlock>(vm.heap, allocationSize(calleeCount))) JSWebAssemblyCodeBlock(vm, owner, mode, WTFMove(plan), calleeCount);
+        auto* result = new (NotNull, allocateCell<JSWebAssemblyCodeBlock>(vm.heap, allocationSize(calleeCount, functionImportCount))) JSWebAssemblyCodeBlock(vm, owner, mode, WTFMove(plan), calleeCount);
         result->finishCreation(vm);
         return result;
     }
@@ -111,6 +111,13 @@ public:
         return m_wasmExitStubs[importIndex].wasmToJs.code().executableAddress();
     }
 
+    static ptrdiff_t offsetOfImportWasmToJSStub(unsigned calleeCount, unsigned importIndex)
+    {
+        return offsetOfCallees()
+            + (sizeof(WriteBarrier<JSWebAssemblyCallee>) * calleeCount * 2)
+            + (sizeof(void*) * importIndex);
+    }
+
 private:
     JSWebAssemblyCodeBlock(VM&, JSWebAssemblyModule*, Wasm::MemoryMode, Ref<Wasm::Plan>&&, unsigned calleeCount);
     DECLARE_EXPORT_INFO;
@@ -118,14 +125,21 @@ private:
     static void destroy(JSCell*);
     static void visitChildren(JSCell*, SlotVisitor&);
 
-    static size_t offsetOfCallees()
+    static ptrdiff_t offsetOfCallees()
     {
         return WTF::roundUpToMultipleOf<sizeof(WriteBarrier<JSWebAssemblyCallee>)>(sizeof(JSWebAssemblyCodeBlock));
     }
 
-    static size_t allocationSize(unsigned numCallees)
+    static size_t allocationSize(unsigned calleeCount, unsigned functionImportCount)
+    {
+        return offsetOfCallees()
+            + (sizeof(WriteBarrier<JSWebAssemblyCallee>) * calleeCount * 2)
+            + (sizeof(void*) * functionImportCount);
+    }
+
+    void*& importWasmToJSStub(unsigned calleeCount, unsigned importIndex)
     {
-        return offsetOfCallees() + sizeof(WriteBarrier<JSWebAssemblyCallee>) * numCallees * 2;
+        return *bitwise_cast<void**>(bitwise_cast<char*>(this) + offsetOfImportWasmToJSStub(calleeCount, importIndex));
     }
 
     class UnconditionalFinalizer : public JSC::UnconditionalFinalizer {
index 30b17b1..f12d1cf 100644 (file)
@@ -96,7 +96,8 @@ void JSWebAssemblyInstance::visitChildren(JSCell* cell, SlotVisitor& visitor)
 
 void JSWebAssemblyInstance::addUnitializedCodeBlock(VM& vm, Ref<Wasm::Plan> plan)
 {
-    auto* codeBlock = JSWebAssemblyCodeBlock::create(vm, module(), memoryMode(), WTFMove(plan), module()->moduleInformation().internalFunctionCount());
+    auto* codeBlock = JSWebAssemblyCodeBlock::create(
+        vm, module(), memoryMode(), WTFMove(plan), module()->moduleInformation().internalFunctionCount(), module()->moduleInformation().importFunctionCount());
     m_codeBlock.set(vm, this, codeBlock);
     ASSERT(!codeBlock->initialized());
 }
index a275465..de6dbea 100644 (file)
@@ -79,6 +79,7 @@ public:
     static ptrdiff_t offsetOfCallee() { return OBJECT_OFFSETOF(JSWebAssemblyInstance, m_callee); }
     static ptrdiff_t offsetOfGlobals() { return OBJECT_OFFSETOF(JSWebAssemblyInstance, m_globals); }
     static ptrdiff_t offsetOfVM() { return OBJECT_OFFSETOF(JSWebAssemblyInstance, m_vm); }
+    static ptrdiff_t offsetOfCodeBlock() { return OBJECT_OFFSETOF(JSWebAssemblyInstance, m_codeBlock); }
     static size_t offsetOfImportFunctions() { return WTF::roundUpToMultipleOf<sizeof(WriteBarrier<JSCell>)>(sizeof(JSWebAssemblyInstance)); }
     static size_t offsetOfImportFunction(size_t importFunctionNum) { return offsetOfImportFunctions() + importFunctionNum * sizeof(sizeof(WriteBarrier<JSCell>)); }