- Revision
- 249069
- Author
- justin_mich...@apple.com
- Date
- 2019-08-23 14:33:08 -0700 (Fri, 23 Aug 2019)
Log Message
[WASM-References] Do not overwrite argument registers in jsCallEntrypoint
https://bugs.webkit.org/show_bug.cgi?id=200952
Reviewed by Saam Barati.
JSTests:
* wasm/references/func_ref.js:
(assert.throws):
Source/_javascript_Core:
The c call that we emitted was incorrect. If we had an int argument that was supposed to be placed in GPR0 by this loop,
we would clobber it while making the call (among many other possible registers). To fix this, we just inline the call
to isWebassemblyHostFunction.
* wasm/js/WebAssemblyFunction.cpp:
(JSC::WebAssemblyFunction::jsCallEntrypointSlow):
Modified Paths
Diff
Modified: trunk/JSTests/ChangeLog (249068 => 249069)
--- trunk/JSTests/ChangeLog 2019-08-23 21:21:51 UTC (rev 249068)
+++ trunk/JSTests/ChangeLog 2019-08-23 21:33:08 UTC (rev 249069)
@@ -1,3 +1,13 @@
+2019-08-23 Justin Michaud <justin_mich...@apple.com>
+
+ [WASM-References] Do not overwrite argument registers in jsCallEntrypoint
+ https://bugs.webkit.org/show_bug.cgi?id=200952
+
+ Reviewed by Saam Barati.
+
+ * wasm/references/func_ref.js:
+ (assert.throws):
+
2019-08-22 Justin Michaud <justin_mich...@apple.com>
Add missing exception check in canonicalizeLocaleList
Modified: trunk/JSTests/wasm/references/func_ref.js (249068 => 249069)
--- trunk/JSTests/wasm/references/func_ref.js 2019-08-23 21:21:51 UTC (rev 249068)
+++ trunk/JSTests/wasm/references/func_ref.js 2019-08-23 21:33:08 UTC (rev 249069)
@@ -448,3 +448,62 @@
}
}
}
+
+{
+ const $1 = new WebAssembly.Instance(new WebAssembly.Module((new Builder())
+ .Type().End()
+ .Import().End()
+ .Function().End()
+ .Export()
+ .Function("test")
+ .End()
+ .Code()
+ .Function("test", { params: ["i32", "funcref"], ret: "i32" })
+ .GetLocal(0)
+ .End()
+ .End().WebAssembly().get()), { });
+
+ const myfun = makeExportedFunction(1337);
+ assert.eq(myfun(), 1337)
+ assert.eq(42, $1.exports.test(42, myfun))
+ assert.throws(() => $1.exports.test(42, () => 5), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
+}
+
+{
+ const $1 = new WebAssembly.Instance(new WebAssembly.Module((new Builder())
+ .Type().End()
+ .Import().End()
+ .Function().End()
+ .Export()
+ .Function("test")
+ .End()
+ .Code()
+ .Function("test", { params: ["i32", "funcref"], ret: "i32" })
+ .GetLocal(0)
+ .If("i32")
+ .Block("i32", (b) =>
+ b.GetLocal(0)
+ .GetLocal(1)
+ .Call(0)
+ )
+ .Else()
+ .Block("i32", (b) =>
+ b.GetLocal(0)
+ )
+ .End()
+ .End()
+ .End().WebAssembly().get()), { });
+
+ const myfun = makeExportedFunction(1337);
+ function foo(b) {
+ $1.exports.test(b, myfun)
+ }
+ noInline(foo);
+
+ for (let i = 0; i < 100; ++i)
+ foo(0);
+
+ assert.throws(() => $1.exports.test(42, () => 5), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
+ assert.throws(() => $1.exports.test(42, myfun), RangeError, "Maximum call stack size exceeded.")
+ assert.throws(() => foo(1), RangeError, "Maximum call stack size exceeded.")
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (249068 => 249069)
--- trunk/Source/_javascript_Core/ChangeLog 2019-08-23 21:21:51 UTC (rev 249068)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-08-23 21:33:08 UTC (rev 249069)
@@ -1,3 +1,17 @@
+2019-08-23 Justin Michaud <justin_mich...@apple.com>
+
+ [WASM-References] Do not overwrite argument registers in jsCallEntrypoint
+ https://bugs.webkit.org/show_bug.cgi?id=200952
+
+ Reviewed by Saam Barati.
+
+ The c call that we emitted was incorrect. If we had an int argument that was supposed to be placed in GPR0 by this loop,
+ we would clobber it while making the call (among many other possible registers). To fix this, we just inline the call
+ to isWebassemblyHostFunction.
+
+ * wasm/js/WebAssemblyFunction.cpp:
+ (JSC::WebAssemblyFunction::jsCallEntrypointSlow):
+
2019-08-23 Ross Kirsling <ross.kirsl...@sony.com>
JSC should have public API for unhandled promise rejections
Modified: trunk/Source/_javascript_Core/wasm/js/WebAssemblyFunction.cpp (249068 => 249069)
--- trunk/Source/_javascript_Core/wasm/js/WebAssemblyFunction.cpp 2019-08-23 21:21:51 UTC (rev 249068)
+++ trunk/Source/_javascript_Core/wasm/js/WebAssemblyFunction.cpp 2019-08-23 21:33:08 UTC (rev 249069)
@@ -266,12 +266,13 @@
}
GPRReg scratchGPR = Wasm::wasmCallingConventionAir().prologueScratch(1);
- GPRReg scratch2GPR = Wasm::wasmCallingConventionAir().prologueScratch(0);
- jit.loadPtr(vm.addressOfSoftStackLimit(), scratch2GPR);
+ bool stackLimitGPRIsClobbered = false;
+ GPRReg stackLimitGPR = Wasm::wasmCallingConventionAir().prologueScratch(0);
+ jit.loadPtr(vm.addressOfSoftStackLimit(), stackLimitGPR);
CCallHelpers::JumpList slowPath;
slowPath.append(jit.branchPtr(CCallHelpers::Above, MacroAssembler::stackPointerRegister, GPRInfo::callFrameRegister));
- slowPath.append(jit.branchPtr(CCallHelpers::Below, MacroAssembler::stackPointerRegister, scratch2GPR));
+ slowPath.append(jit.branchPtr(CCallHelpers::Below, MacroAssembler::stackPointerRegister, stackLimitGPR));
// Ensure:
// argCountPlusThis - 1 >= signature.argumentCount()
@@ -310,24 +311,23 @@
}
break;
case Wasm::Funcref: {
- // FIXME: Emit this inline <https://bugs.webkit.org/show_bug.cgi?id=198506>.
- bool (*shouldThrow)(Wasm::Instance*, JSValue) = [] (Wasm::Instance* wasmInstance, JSValue arg) -> bool {
- JSWebAssemblyInstance* instance = wasmInstance->owner<JSWebAssemblyInstance>();
- JSGlobalObject* globalObject = instance->globalObject();
- VM& vm = globalObject->vm();
- return !isWebAssemblyHostFunction(vm, arg) && !arg.isNull();
- };
- jit.move(CCallHelpers::TrustedImmPtr(&instance()->instance()), GPRInfo::argumentGPR0);
- jit.load64(CCallHelpers::Address(GPRInfo::callFrameRegister, jsOffset), GPRInfo::argumentGPR1);
- jit.setupArguments<decltype(shouldThrow)>(GPRInfo::argumentGPR0, GPRInfo::argumentGPR1);
- auto call = jit.call(OperationPtrTag);
+ // Ensure we have a WASM exported function.
+ jit.load64(CCallHelpers::Address(GPRInfo::callFrameRegister, jsOffset), scratchGPR);
+ auto isNull = jit.branchIfNull(scratchGPR);
+ slowPath.append(jit.branchIfNotCell(scratchGPR));
- jit.addLinkTask([=] (LinkBuffer& linkBuffer) {
- linkBuffer.link(call, FunctionPtr<OperationPtrTag>(shouldThrow));
- });
+ stackLimitGPRIsClobbered = true;
+ jit.emitLoadStructure(vm, scratchGPR, scratchGPR, stackLimitGPR);
+ jit.loadPtr(CCallHelpers::Address(scratchGPR, Structure::classInfoOffset()), scratchGPR);
- slowPath.append(jit.branchTest32(CCallHelpers::NonZero, GPRInfo::returnValueGPR));
+ static_assert(std::is_final<WebAssemblyFunction>::value, "We do not check for subtypes below");
+ static_assert(std::is_final<WebAssemblyWrapperFunction>::value, "We do not check for subtypes below");
+ auto isWasmFunction = jit.branchPtr(CCallHelpers::Equal, scratchGPR, CCallHelpers::TrustedImmPtr(WebAssemblyFunction::info()));
+ slowPath.append(jit.branchPtr(CCallHelpers::NotEqual, scratchGPR, CCallHelpers::TrustedImmPtr(WebAssemblyWrapperFunction::info())));
+
+ isWasmFunction.link(&jit);
+ isNull.link(&jit);
FALLTHROUGH;
}
case Wasm::Anyref: {
@@ -432,12 +432,13 @@
jit.move(scratchGPR, pinnedRegs.wasmContextInstancePointer);
jit.storePtr(scratchGPR, vm.wasmContext.pointerToInstance());
}
- // This contains the cached stack limit still.
- jit.storePtr(scratch2GPR, CCallHelpers::Address(scratchGPR, Wasm::Instance::offsetOfCachedStackLimit()));
+ if (stackLimitGPRIsClobbered)
+ jit.loadPtr(vm.addressOfSoftStackLimit(), stackLimitGPR);
+ jit.storePtr(stackLimitGPR, CCallHelpers::Address(scratchGPR, Wasm::Instance::offsetOfCachedStackLimit()));
if (!!moduleInformation.memory) {
GPRReg baseMemory = pinnedRegs.baseMemoryPointer;
- GPRReg scratchOrSize = scratch2GPR;
+ GPRReg scratchOrSize = stackLimitGPR;
auto mode = instance()->memoryMode();
if (isARM64E()) {
