Diff
Modified: branches/safari-609.1.20.0-branch/JSTests/ChangeLog (256957 => 256958)
--- branches/safari-609.1.20.0-branch/JSTests/ChangeLog 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/JSTests/ChangeLog 2020-02-19 23:07:52 UTC (rev 256958)
@@ -1,3 +1,68 @@
+2020-02-19 Russell Epstein <repst...@apple.com>
+
+ Cherry-pick r256665. rdar://problem/59551715
+
+ [WASM] Wasm interpreter's calling convention doesn't match Wasm JIT's convention.
+ https://bugs.webkit.org/show_bug.cgi?id=207727
+
+ JSTests:
+
+ Reviewed by Mark Lam.
+
+ * wasm/regress/llint-callee-saves-with-fast-memory.js: Added.
+ * wasm/regress/llint-callee-saves-without-fast-memory.js: Added.
+
+ Source/_javascript_Core:
+
+ Reviewed by Mark Lam.
+
+ The Wasm JIT has unusual calling conventions, which were further complicated by the addition
+ of the interpreter, and the interpreter did not correctly follow these conventions (by incorrectly
+ saving and restoring the callee save registers used for the memory base and size). Here's a summary
+ of the calling convention:
+
+ - When entering Wasm from JS, the wrapper must:
+ - Preserve the base and size when entering LLInt regardless of the mode. (Prior to this
+ patch we only preserved the base in Signaling mode)
+ - Preserve the memory base in either mode, and the size for BoundsChecking.
+ - Both tiers must preserve every *other* register they use. e.g. the LLInt must preserve PB
+ and wasmInstance, but must *not* preserve memoryBase and memorySize.
+ - Changes to memoryBase and memorySize are visible to the caller. This means that:
+ - Intra-module calls can assume these registers are up-to-date even if the memory was
+ resized. The only exception here is if the LLInt calls a signaling JIT, in which case
+ the JIT will not update the size register, since it won't be using it.
+ - Inter-module and JS calls require the caller to reload these registers. These calls may
+ result in memory changes (e.g. the callee may call memory.grow).
+ - A Signaling JIT caller must be aware that the LLInt may trash the size register, since
+ it always bounds checks.
+
+ * llint/WebAssembly.asm:
+ * wasm/WasmAirIRGenerator.cpp:
+ (JSC::Wasm::AirIRGenerator::addCall):
+ * wasm/WasmB3IRGenerator.cpp:
+ (JSC::Wasm::B3IRGenerator::addCall):
+ * wasm/WasmCallee.cpp:
+ (JSC::Wasm::LLIntCallee::calleeSaveRegisters):
+ * wasm/WasmCallingConvention.h:
+ * wasm/WasmLLIntPlan.cpp:
+ (JSC::Wasm::LLIntPlan::didCompleteCompilation):
+ * wasm/WasmMemoryInformation.cpp:
+ (JSC::Wasm::PinnedRegisterInfo::get):
+ (JSC::Wasm::getPinnedRegisters): Deleted.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@256665 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-02-14 Tadeu Zagallo <tzaga...@apple.com>
+
+ [WASM] Wasm interpreter's calling convention doesn't match Wasm JIT's convention.
+ https://bugs.webkit.org/show_bug.cgi?id=207727
+
+ Reviewed by Mark Lam.
+
+ * wasm/regress/llint-callee-saves-with-fast-memory.js: Added.
+ * wasm/regress/llint-callee-saves-without-fast-memory.js: Added.
+
2020-02-03 Russell Epstein <repst...@apple.com>
Cherry-pick r255529. rdar://problem/59098310
Added: branches/safari-609.1.20.0-branch/JSTests/wasm/regress/llint-callee-saves-with-fast-memory.js (0 => 256958)
--- branches/safari-609.1.20.0-branch/JSTests/wasm/regress/llint-callee-saves-with-fast-memory.js (rev 0)
+++ branches/safari-609.1.20.0-branch/JSTests/wasm/regress/llint-callee-saves-with-fast-memory.js 2020-02-19 23:07:52 UTC (rev 256958)
@@ -0,0 +1,40 @@
+//@ skip if $architecture != "x86-64"
+//@ requireOptions("--useWebAssemblyFastMemory=true")
+// FIXME: Stop skipping when we enable fast memory for iOS. https://bugs.webkit.org/show_bug.cgi?id=170774
+
+import { instantiate } from '../wabt-wrapper.js';
+
+const instance = instantiate(`
+ (module
+
+ (memory 0)
+
+ (func $grow
+ (memory.grow (i32.const 1))
+ (drop)
+ )
+
+ (func $f (param $bail i32)
+ (br_if 0 (local.get $bail))
+ (call $grow)
+ (i32.store (i32.const 42) (i32.const 0))
+ )
+
+ (func (export "main")
+ (local $i i32)
+ (local.set $i (i32.const 100000))
+ (loop $warmup
+ (i32.sub (local.get $i) (i32.const 1))
+ (local.tee $i)
+ (call $f (i32.const 1))
+ (br_if $warmup)
+ )
+ (call $f (i32.const 0))
+ )
+
+ )
+`);
+
+
+// This should not throw an OutOfBounds exception
+instance.exports.main();
Added: branches/safari-609.1.20.0-branch/JSTests/wasm/regress/llint-callee-saves-without-fast-memory.js (0 => 256958)
--- branches/safari-609.1.20.0-branch/JSTests/wasm/regress/llint-callee-saves-without-fast-memory.js (rev 0)
+++ branches/safari-609.1.20.0-branch/JSTests/wasm/regress/llint-callee-saves-without-fast-memory.js 2020-02-19 23:07:52 UTC (rev 256958)
@@ -0,0 +1,38 @@
+//@ requireOptions("--useWebAssemblyFastMemory=false")
+
+import { instantiate } from '../wabt-wrapper.js';
+
+const instance = instantiate(`
+ (module
+
+ (memory 0)
+
+ (func $grow
+ (memory.grow (i32.const 1))
+ (drop)
+ )
+
+ (func $f (param $bail i32)
+ (br_if 0 (local.get $bail))
+ (call $grow)
+ (i32.store (i32.const 42) (i32.const 0))
+ )
+
+ (func (export "main")
+ (local $i i32)
+ (local.set $i (i32.const 100000))
+ (loop $warmup
+ (i32.sub (local.get $i) (i32.const 1))
+ (local.tee $i)
+ (call $f (i32.const 1))
+ (br_if $warmup)
+ )
+ (call $f (i32.const 0))
+ )
+
+ )
+`);
+
+
+// This should not throw an OutOfBounds exception
+instance.exports.main();
Modified: branches/safari-609.1.20.0-branch/Source/_javascript_Core/ChangeLog (256957 => 256958)
--- branches/safari-609.1.20.0-branch/Source/_javascript_Core/ChangeLog 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/Source/_javascript_Core/ChangeLog 2020-02-19 23:07:52 UTC (rev 256958)
@@ -1,5 +1,101 @@
2020-02-19 Russell Epstein <repst...@apple.com>
+ Cherry-pick r256665. rdar://problem/59551715
+
+ [WASM] Wasm interpreter's calling convention doesn't match Wasm JIT's convention.
+ https://bugs.webkit.org/show_bug.cgi?id=207727
+
+ JSTests:
+
+ Reviewed by Mark Lam.
+
+ * wasm/regress/llint-callee-saves-with-fast-memory.js: Added.
+ * wasm/regress/llint-callee-saves-without-fast-memory.js: Added.
+
+ Source/_javascript_Core:
+
+ Reviewed by Mark Lam.
+
+ The Wasm JIT has unusual calling conventions, which were further complicated by the addition
+ of the interpreter, and the interpreter did not correctly follow these conventions (by incorrectly
+ saving and restoring the callee save registers used for the memory base and size). Here's a summary
+ of the calling convention:
+
+ - When entering Wasm from JS, the wrapper must:
+ - Preserve the base and size when entering LLInt regardless of the mode. (Prior to this
+ patch we only preserved the base in Signaling mode)
+ - Preserve the memory base in either mode, and the size for BoundsChecking.
+ - Both tiers must preserve every *other* register they use. e.g. the LLInt must preserve PB
+ and wasmInstance, but must *not* preserve memoryBase and memorySize.
+ - Changes to memoryBase and memorySize are visible to the caller. This means that:
+ - Intra-module calls can assume these registers are up-to-date even if the memory was
+ resized. The only exception here is if the LLInt calls a signaling JIT, in which case
+ the JIT will not update the size register, since it won't be using it.
+ - Inter-module and JS calls require the caller to reload these registers. These calls may
+ result in memory changes (e.g. the callee may call memory.grow).
+ - A Signaling JIT caller must be aware that the LLInt may trash the size register, since
+ it always bounds checks.
+
+ * llint/WebAssembly.asm:
+ * wasm/WasmAirIRGenerator.cpp:
+ (JSC::Wasm::AirIRGenerator::addCall):
+ * wasm/WasmB3IRGenerator.cpp:
+ (JSC::Wasm::B3IRGenerator::addCall):
+ * wasm/WasmCallee.cpp:
+ (JSC::Wasm::LLIntCallee::calleeSaveRegisters):
+ * wasm/WasmCallingConvention.h:
+ * wasm/WasmLLIntPlan.cpp:
+ (JSC::Wasm::LLIntPlan::didCompleteCompilation):
+ * wasm/WasmMemoryInformation.cpp:
+ (JSC::Wasm::PinnedRegisterInfo::get):
+ (JSC::Wasm::getPinnedRegisters): Deleted.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@256665 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-02-14 Tadeu Zagallo <tzaga...@apple.com> and Michael Saboff <msab...@apple.com>
+
+ [WASM] Wasm interpreter's calling convention doesn't match Wasm JIT's convention.
+ https://bugs.webkit.org/show_bug.cgi?id=207727
+
+ Reviewed by Mark Lam.
+
+ The Wasm JIT has unusual calling conventions, which were further complicated by the addition
+ of the interpreter, and the interpreter did not correctly follow these conventions (by incorrectly
+ saving and restoring the callee save registers used for the memory base and size). Here's a summary
+ of the calling convention:
+
+ - When entering Wasm from JS, the wrapper must:
+ - Preserve the base and size when entering LLInt regardless of the mode. (Prior to this
+ patch we only preserved the base in Signaling mode)
+ - Preserve the memory base in either mode, and the size for BoundsChecking.
+ - Both tiers must preserve every *other* register they use. e.g. the LLInt must preserve PB
+ and wasmInstance, but must *not* preserve memoryBase and memorySize.
+ - Changes to memoryBase and memorySize are visible to the caller. This means that:
+ - Intra-module calls can assume these registers are up-to-date even if the memory was
+ resized. The only exception here is if the LLInt calls a signaling JIT, in which case
+ the JIT will not update the size register, since it won't be using it.
+ - Inter-module and JS calls require the caller to reload these registers. These calls may
+ result in memory changes (e.g. the callee may call memory.grow).
+ - A Signaling JIT caller must be aware that the LLInt may trash the size register, since
+ it always bounds checks.
+
+ * llint/WebAssembly.asm:
+ * wasm/WasmAirIRGenerator.cpp:
+ (JSC::Wasm::AirIRGenerator::addCall):
+ * wasm/WasmB3IRGenerator.cpp:
+ (JSC::Wasm::B3IRGenerator::addCall):
+ * wasm/WasmCallee.cpp:
+ (JSC::Wasm::LLIntCallee::calleeSaveRegisters):
+ * wasm/WasmCallingConvention.h:
+ * wasm/WasmLLIntPlan.cpp:
+ (JSC::Wasm::LLIntPlan::didCompleteCompilation):
+ * wasm/WasmMemoryInformation.cpp:
+ (JSC::Wasm::PinnedRegisterInfo::get):
+ (JSC::Wasm::getPinnedRegisters): Deleted.
+
+2020-02-19 Russell Epstein <repst...@apple.com>
+
Apply patch. rdar://problem/59447004
2020-02-19 Yusuke Suzuki <ysuz...@apple.com>
Modified: branches/safari-609.1.20.0-branch/Source/_javascript_Core/llint/WebAssembly.asm (256957 => 256958)
--- branches/safari-609.1.20.0-branch/Source/_javascript_Core/llint/WebAssembly.asm 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/Source/_javascript_Core/llint/WebAssembly.asm 2020-02-19 23:07:52 UTC (rev 256958)
@@ -178,15 +178,14 @@
# Wasm specific helpers
macro preserveCalleeSavesUsedByWasm()
+ # NOTE: We intentionally don't save memoryBase and memorySize here. See the comment
+ # in restoreCalleeSavesUsedByWasm() below for why.
subp CalleeSaveSpaceStackAligned, sp
if ARM64 or ARM64E
- emit "stp x23, x26, [x29, #-16]"
- emit "stp x19, x22, [x29, #-32]"
+ emit "stp x19, x26, [x29, #-16]"
elsif X86_64
- storep memorySize, -0x08[cfr]
- storep memoryBase, -0x10[cfr]
- storep PB, -0x18[cfr]
- storep wasmInstance, -0x20[cfr]
+ storep PB, -0x8[cfr]
+ storep wasmInstance, -0x10[cfr]
else
error
end
@@ -193,14 +192,14 @@
end
macro restoreCalleeSavesUsedByWasm()
+ # NOTE: We intentionally don't restore memoryBase and memorySize here. These are saved
+ # and restored when entering Wasm by the JSToWasm wrapper and changes to them are meant
+ # to be observable within the same Wasm module.
if ARM64 or ARM64E
- emit "ldp x23, x26, [x29, #-16]"
- emit "ldp x19, x22, [x29, #-32]"
+ emit "ldp x19, x26, [x29, #-16]"
elsif X86_64
- loadp -0x08[cfr], memorySize
- loadp -0x10[cfr], memoryBase
- loadp -0x18[cfr], PB
- loadp -0x20[cfr], wasmInstance
+ loadp -0x8[cfr], PB
+ loadp -0x10[cfr], wasmInstance
else
error
end
Modified: branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp (256957 => 256958)
--- branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp 2020-02-19 23:07:52 UTC (rev 256958)
@@ -2173,6 +2173,9 @@
restoreWebAssemblyGlobalState(RestoreCachedStackLimit::Yes, m_info.memory, currentInstance, continuation);
} else {
auto* patchpoint = emitCallPatchpoint(m_currentBlock, signature, results, args);
+ // We need to clobber the size register since the LLInt always bounds checks
+ if (m_mode == MemoryMode::Signaling)
+ patchpoint->clobberLate(RegisterSet { PinnedRegisterInfo::get().sizeRegister });
patchpoint->setGenerator([unlinkedWasmToWasmCalls, functionIndex] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
AllowMacroScratchRegisterUsage allowScratch(jit);
CCallHelpers::Call call = jit.threadSafePatchableNearCall();
Modified: branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp (256957 => 256958)
--- branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp 2020-02-19 23:07:52 UTC (rev 256958)
@@ -1726,6 +1726,9 @@
patchpoint->effects.writesPinned = true;
patchpoint->effects.readsPinned = true;
+ // We need to clobber the size register since the LLInt always bounds checks
+ if (m_mode == MemoryMode::Signaling)
+ patchpoint->clobberLate(RegisterSet { PinnedRegisterInfo::get().sizeRegister });
patchpoint->setGenerator([unlinkedWasmToWasmCalls, functionIndex] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
AllowMacroScratchRegisterUsage allowScratch(jit);
CCallHelpers::Call call = jit.threadSafePatchableNearCall();
Modified: branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmCallee.cpp (256957 => 256958)
--- branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmCallee.cpp 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmCallee.cpp 2020-02-19 23:07:52 UTC (rev 256958)
@@ -94,8 +94,6 @@
#else
#error Unsupported architecture.
#endif
- registers.set(GPRInfo::regCS3); // Memory base
- registers.set(GPRInfo::regCS4); // Memory size
ASSERT(registers.numberOfSetRegisters() == numberOfLLIntCalleeSaveRegisters);
calleeSaveRegisters.construct(WTFMove(registers));
});
Modified: branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmCallingConvention.h (256957 => 256958)
--- branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmCallingConvention.h 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmCallingConvention.h 2020-02-19 23:07:52 UTC (rev 256958)
@@ -46,7 +46,7 @@
namespace JSC { namespace Wasm {
-constexpr unsigned numberOfLLIntCalleeSaveRegisters = 4;
+constexpr unsigned numberOfLLIntCalleeSaveRegisters = 2;
using ArgumentLocation = B3::ValueRep;
enum class CallRole : uint8_t {
Modified: branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmLLIntPlan.cpp (256957 => 256958)
--- branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmLLIntPlan.cpp 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmLLIntPlan.cpp 2020-02-19 23:07:52 UTC (rev 256958)
@@ -146,7 +146,9 @@
SignatureIndex signatureIndex = m_moduleInformation->internalFunctionSignatureIndices[functionIndex];
const Signature& signature = SignatureInformation::get(signatureIndex);
CCallHelpers jit;
- std::unique_ptr<InternalFunction> function = createJSToWasmWrapper(jit, signature, &m_unlinkedWasmToWasmCalls[functionIndex], m_moduleInformation.get(), m_mode, functionIndex);
+ // The LLInt always bounds checks
+ MemoryMode mode = MemoryMode::BoundsChecking;
+ std::unique_ptr<InternalFunction> function = createJSToWasmWrapper(jit, signature, &m_unlinkedWasmToWasmCalls[functionIndex], m_moduleInformation.get(), mode, functionIndex);
LinkBuffer linkBuffer(jit, nullptr, JITCompilationCanFail);
if (UNLIKELY(linkBuffer.didFailToAllocate())) {
Modified: branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmMemoryInformation.cpp (256957 => 256958)
--- branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmMemoryInformation.cpp 2020-02-19 23:07:48 UTC (rev 256957)
+++ branches/safari-609.1.20.0-branch/Source/_javascript_Core/wasm/WasmMemoryInformation.cpp 2020-02-19 23:07:52 UTC (rev 256958)
@@ -35,26 +35,6 @@
namespace JSC { namespace Wasm {
-static Vector<GPRReg> getPinnedRegisters(unsigned remainingPinnedRegisters)
-{
- Vector<GPRReg> registers;
- jsCallingConvention().calleeSaveRegisters.forEach([&] (Reg reg) {
- if (!reg.isGPR())
- return;
- GPRReg gpr = reg.gpr();
- if (!remainingPinnedRegisters || RegisterSet::stackRegisters().get(reg))
- return;
- if (RegisterSet::runtimeTagRegisters().get(reg)) {
- // Since we don't need to, we currently don't pick from the tag registers to allow
- // JS->Wasm stubs to freely use these registers.
- return;
- }
- --remainingPinnedRegisters;
- registers.append(gpr);
- });
- return registers;
-}
-
const PinnedRegisterInfo& PinnedRegisterInfo::get()
{
static LazyNeverDestroyed<PinnedRegisterInfo> staticPinnedRegisterInfo;
@@ -63,8 +43,6 @@
unsigned numberOfPinnedRegisters = 2;
if (!Context::useFastTLS())
++numberOfPinnedRegisters;
- Vector<GPRReg> pinnedRegs = getPinnedRegisters(numberOfPinnedRegisters);
-
GPRReg baseMemoryPointer = GPRInfo::regCS3;
GPRReg sizeRegister = GPRInfo::regCS4;
GPRReg wasmContextInstancePointer = InvalidGPRReg;
