Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (216172 => 216173)
--- trunk/Source/_javascript_Core/ChangeLog 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-05-04 05:50:01 UTC (rev 216173)
@@ -1,3 +1,33 @@
+2017-05-03 Saam Barati <[email protected]>
+
+ How we build polymorphic cases is wrong when making a call from Wasm
+ https://bugs.webkit.org/show_bug.cgi?id=171527
+
+ Reviewed by JF Bastien.
+
+ This patches fixes a bug when we emit a polymorphic call IC from
+ Wasm. We were incorrectly assuming that if we made a call *from wasm*,
+ then the thing we are *calling to* does not have a CodeBlock. This
+ is obviously wrong. This patch fixes the incorrect assumption.
+
+ This patch also does two more things:
+ 1. Add a new option that makes us make calls to JS using a
+ slow path instead of using a call IC.
+ 2. Fixes a potential GC bug where we didn't populate JSWebAssemblyCodeBlock's
+ JSWebAssemblyModule pointer.
+
+ * jit/Repatch.cpp:
+ (JSC::linkPolymorphicCall):
+ * runtime/Options.h:
+ * wasm/WasmBinding.cpp:
+ (JSC::Wasm::wasmToJs):
+ * wasm/js/JSWebAssemblyCodeBlock.cpp:
+ (JSC::JSWebAssemblyCodeBlock::create):
+ (JSC::JSWebAssemblyCodeBlock::finishCreation):
+ * wasm/js/JSWebAssemblyCodeBlock.h:
+ * wasm/js/JSWebAssemblyInstance.cpp:
+ (JSC::JSWebAssemblyInstance::finalizeCreation):
+
2017-05-03 Keith Miller <[email protected]>
Array.prototype.sort should also allow a null comparator
Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (216172 => 216173)
--- trunk/Source/_javascript_Core/jit/Repatch.cpp 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp 2017-05-04 05:50:01 UTC (rev 216173)
@@ -762,7 +762,7 @@
// Figure out what our cases are.
for (CallVariant variant : list) {
CodeBlock* codeBlock;
- if (isWebAssembly || variant.executable()->isHostFunction())
+ if (variant.executable()->isHostFunction())
codeBlock = nullptr;
else {
ExecutableBase* executable = variant.executable();
Modified: trunk/Source/_javascript_Core/runtime/Options.h (216172 => 216173)
--- trunk/Source/_javascript_Core/runtime/Options.h 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Source/_javascript_Core/runtime/Options.h 2017-05-04 05:50:01 UTC (rev 216173)
@@ -446,7 +446,8 @@
v(unsigned, webAssemblyFastMemoryRedzonePages, 128, Normal, "WebAssembly fast memories use 4GiB virtual allocations, plus a redzone (counted as multiple of 64KiB WebAssembly pages) at the end to catch reg+imm accesses which exceed 32-bit, anything beyond the redzone is explicitly bounds-checked") \
v(bool, crashIfWebAssemblyCantFastMemory, false, Normal, "If true, we will crash if we can't obtain fast memory for wasm.") \
v(unsigned, webAssemblyFastMemoryPreallocateCount, 0, Normal, "WebAssembly fast memories can be pre-allocated at program startup and remain cached to avoid fragmentation leading to bounds-checked memory. This number is an upper bound on initial allocation as well as total count of fast memories. Zero means no pre-allocation, no caching, and no limit to the number of runtime allocations.") \
- v(bool, useWebAssemblyFastTLS, true, Normal, "If true, we will try to use fast thread-local storage if available on the current platform.")
+ v(bool, useWebAssemblyFastTLS, true, Normal, "If true, we will try to use fast thread-local storage if available on the current platform.") \
+ v(bool, useCallICsForWebAssemblyToJSCalls, true, Normal, "If true, we will use CallLinkInfo to inline cache Wasm to JS calls.")
enum OptionEquivalence {
Modified: trunk/Source/_javascript_Core/wasm/WasmBinding.cpp (216172 => 216173)
--- trunk/Source/_javascript_Core/wasm/WasmBinding.cpp 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Source/_javascript_Core/wasm/WasmBinding.cpp 2017-05-04 05:50:01 UTC (rev 216173)
@@ -125,6 +125,186 @@
missingCalleeSaves.exclude(jsCC.m_calleeSaveRegisters);
ASSERT(missingCalleeSaves.isEmpty());
+ if (!Options::useCallICsForWebAssemblyToJSCalls()) {
+ ScratchBuffer* scratchBuffer = vm->scratchBufferForSize(argCount * sizeof(uint64_t));
+ char* buffer = argCount ? static_cast<char*>(scratchBuffer->dataBuffer()) : nullptr;
+ unsigned marshalledGPRs = 0;
+ unsigned marshalledFPRs = 0;
+ unsigned bufferOffset = 0;
+ unsigned frOffset = CallFrame::headerSizeInRegisters * static_cast<int>(sizeof(Register));
+ const GPRReg scratchGPR = GPRInfo::regCS0;
+ jit.subPtr(MacroAssembler::TrustedImm32(WTF::roundUpToMultipleOf(stackAlignmentBytes(), sizeof(Register))), MacroAssembler::stackPointerRegister);
+ jit.storePtr(scratchGPR, MacroAssembler::Address(MacroAssembler::stackPointerRegister));
+
+ for (unsigned argNum = 0; argNum < argCount; ++argNum) {
+ Type argType = signature.argument(argNum);
+ switch (argType) {
+ case Void:
+ case Func:
+ case Anyfunc:
+ case I64:
+ RELEASE_ASSERT_NOT_REACHED();
+ case I32: {
+ GPRReg gprReg;
+ if (marshalledGPRs < wasmCC.m_gprArgs.size())
+ gprReg = wasmCC.m_gprArgs[marshalledGPRs].gpr();
+ else {
+ // We've already spilled all arguments, these registers are available as scratch.
+ gprReg = GPRInfo::argumentGPR0;
+ jit.load64(JIT::Address(GPRInfo::callFrameRegister, frOffset), gprReg);
+ frOffset += sizeof(Register);
+ }
+ jit.zeroExtend32ToPtr(gprReg, gprReg);
+ jit.store64(gprReg, buffer + bufferOffset);
+ ++marshalledGPRs;
+ break;
+ }
+ case F32: {
+ FPRReg fprReg;
+ if (marshalledFPRs < wasmCC.m_fprArgs.size())
+ fprReg = wasmCC.m_fprArgs[marshalledFPRs].fpr();
+ else {
+ // We've already spilled all arguments, these registers are available as scratch.
+ fprReg = FPRInfo::argumentFPR0;
+ jit.loadFloat(JIT::Address(GPRInfo::callFrameRegister, frOffset), fprReg);
+ frOffset += sizeof(Register);
+ }
+ jit.convertFloatToDouble(fprReg, fprReg);
+ jit.moveDoubleTo64(fprReg, scratchGPR);
+ jit.store64(scratchGPR, buffer + bufferOffset);
+ ++marshalledFPRs;
+ break;
+ }
+ case F64: {
+ FPRReg fprReg;
+ if (marshalledFPRs < wasmCC.m_fprArgs.size())
+ fprReg = wasmCC.m_fprArgs[marshalledFPRs].fpr();
+ else {
+ // We've already spilled all arguments, these registers are available as scratch.
+ fprReg = FPRInfo::argumentFPR0;
+ jit.loadDouble(JIT::Address(GPRInfo::callFrameRegister, frOffset), fprReg);
+ frOffset += sizeof(Register);
+ }
+ jit.moveDoubleTo64(fprReg, scratchGPR);
+ jit.store64(scratchGPR, buffer + bufferOffset);
+ ++marshalledFPRs;
+ break;
+ }
+ }
+
+ bufferOffset += sizeof(Register);
+ }
+ jit.loadPtr(MacroAssembler::Address(MacroAssembler::stackPointerRegister), scratchGPR);
+ if (argCount) {
+ // The GC should not look at this buffer at all, these aren't JSValues.
+ jit.move(CCallHelpers::TrustedImmPtr(scratchBuffer->activeLengthPtr()), GPRInfo::argumentGPR0);
+ jit.storePtr(CCallHelpers::TrustedImmPtr(0), GPRInfo::argumentGPR0);
+ }
+
+ uint64_t (*callFunc)(ExecState*, JSObject*, SignatureIndex, uint64_t*) =
+ [] (ExecState* exec, JSObject* callee, SignatureIndex signatureIndex, uint64_t* buffer) -> uint64_t {
+ VM* vm = &exec->vm();
+ NativeCallFrameTracer tracer(vm, exec);
+ auto throwScope = DECLARE_THROW_SCOPE(*vm);
+ const Signature& signature = SignatureInformation::get(signatureIndex);
+ MarkedArgumentBuffer args;
+ for (unsigned argNum = 0; argNum < signature.argumentCount(); ++argNum) {
+ Type argType = signature.argument(argNum);
+ JSValue arg;
+ switch (argType) {
+ case Void:
+ case Func:
+ case Anyfunc:
+ case I64:
+ RELEASE_ASSERT_NOT_REACHED();
+ case I32:
+ arg = jsNumber(static_cast<int32_t>(buffer[argNum]));
+ break;
+ case F32:
+ case F64:
+ arg = jsNumber(bitwise_cast<double>(buffer[argNum]));
+ break;
+ }
+ args.append(arg);
+ }
+
+ CallData callData;
+ CallType callType = callee->methodTable(*vm)->getCallData(callee, callData);
+ RELEASE_ASSERT(callType != CallType::None);
+ JSValue result = call(exec, callee, callType, callData, jsUndefined(), args);
+ RETURN_IF_EXCEPTION(throwScope, 0);
+
+ uint64_t realResult;
+ switch (signature.returnType()) {
+ case Func:
+ case Anyfunc:
+ case I64:
+ RELEASE_ASSERT_NOT_REACHED();
+ break;
+ case Void:
+ break;
+ case I32: {
+ realResult = static_cast<uint64_t>(static_cast<uint32_t>(result.toInt32(exec)));
+ break;
+ }
+ case F64:
+ case F32: {
+ realResult = bitwise_cast<uint64_t>(result.toNumber(exec));
+ break;
+ }
+ }
+
+ RETURN_IF_EXCEPTION(throwScope, 0);
+ return realResult;
+ };
+
+ jit.loadWasmContext(GPRInfo::argumentGPR0);
+ jit.loadPtr(CCallHelpers::Address(GPRInfo::argumentGPR0, JSWebAssemblyInstance::offsetOfCallee()), GPRInfo::argumentGPR0);
+ jit.storePtr(GPRInfo::argumentGPR0, JIT::Address(GPRInfo::callFrameRegister, CallFrameSlot::callee * static_cast<int>(sizeof(Register))));
+
+ materializeImportJSCell(jit, importIndex, GPRInfo::argumentGPR1);
+ static_assert(GPRInfo::numberOfArgumentRegisters >= 4, "We rely on this with the call below.");
+ jit.setupArgumentsWithExecState(GPRInfo::argumentGPR1, CCallHelpers::TrustedImm32(signatureIndex), CCallHelpers::TrustedImmPtr(buffer));
+ auto call = jit.call();
+ auto noException = jit.emitExceptionCheck(*vm, AssemblyHelpers::InvertedExceptionCheck);
+
+ // exception here.
+ jit.copyCalleeSavesToVMEntryFrameCalleeSavesBuffer(*vm);
+ jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+ void (*doUnwinding)(ExecState*) = [] (ExecState* exec) -> void {
+ VM* vm = &exec->vm();
+ NativeCallFrameTracer tracer(vm, exec);
+ genericUnwind(vm, exec);
+ ASSERT(!!vm->callFrameForCatch);
+ };
+ auto exceptionCall = jit.call();
+ jit.jumpToExceptionHandler(*vm);
+
+ noException.link(&jit);
+ switch (signature.returnType()) {
+ case F64: {
+ jit.move64ToDouble(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
+ break;
+ }
+ case F32: {
+ jit.move64ToDouble(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
+ jit.convertDoubleToFloat(FPRInfo::returnValueFPR, FPRInfo::returnValueFPR);
+ break;
+ }
+ default:
+ break;
+ }
+
+ jit.emitFunctionEpilogue();
+ jit.ret();
+
+ LinkBuffer linkBuffer(jit, GLOBAL_THUNK_ID);
+ linkBuffer.link(call, callFunc);
+ linkBuffer.link(exceptionCall, doUnwinding);
+
+ return FINALIZE_CODE(linkBuffer, ("WebAssembly->_javascript_ import[%i] %s", importIndex, signature.toString().ascii().data()));
+ }
+
// FIXME perform a stack check before updating SP. https://bugs.webkit.org/show_bug.cgi?id=165546
const unsigned numberOfParameters = argCount + 1; // There is a "this" argument.
Modified: trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyCodeBlock.cpp (216172 => 216173)
--- trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyCodeBlock.cpp 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyCodeBlock.cpp 2017-05-04 05:50:01 UTC (rev 216173)
@@ -41,10 +41,11 @@
const ClassInfo JSWebAssemblyCodeBlock::s_info = { "WebAssemblyCodeBlock", nullptr, 0, CREATE_METHOD_TABLE(JSWebAssemblyCodeBlock) };
-JSWebAssemblyCodeBlock* JSWebAssemblyCodeBlock::create(VM& vm, Ref<Wasm::CodeBlock> codeBlock, const Wasm::ModuleInformation& moduleInformation)
+JSWebAssemblyCodeBlock* JSWebAssemblyCodeBlock::create(VM& vm, Ref<Wasm::CodeBlock> codeBlock, JSWebAssemblyModule* module)
{
+ const Wasm::ModuleInformation& moduleInformation = module->module().moduleInformation();
auto* result = new (NotNull, allocateCell<JSWebAssemblyCodeBlock>(vm.heap, allocationSize(moduleInformation.importFunctionCount()))) JSWebAssemblyCodeBlock(vm, WTFMove(codeBlock), moduleInformation);
- result->finishCreation(vm);
+ result->finishCreation(vm, module);
return result;
}
@@ -62,6 +63,12 @@
}
}
+void JSWebAssemblyCodeBlock::finishCreation(VM& vm, JSWebAssemblyModule* module)
+{
+ Base::finishCreation(vm);
+ m_module.set(vm, this, module);
+}
+
void JSWebAssemblyCodeBlock::destroy(JSCell* cell)
{
static_cast<JSWebAssemblyCodeBlock*>(cell)->JSWebAssemblyCodeBlock::~JSWebAssemblyCodeBlock();
Modified: trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyCodeBlock.h (216172 => 216173)
--- trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyCodeBlock.h 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyCodeBlock.h 2017-05-04 05:50:01 UTC (rev 216173)
@@ -51,7 +51,7 @@
typedef JSCell Base;
static const unsigned StructureFlags = Base::StructureFlags | StructureIsImmortal;
- static JSWebAssemblyCodeBlock* create(VM&, Ref<Wasm::CodeBlock>, const Wasm::ModuleInformation&);
+ static JSWebAssemblyCodeBlock* create(VM&, Ref<Wasm::CodeBlock>, JSWebAssemblyModule*);
static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
{
return Structure::create(vm, globalObject, prototype, TypeInfo(CellType, StructureFlags), info());
@@ -62,6 +62,8 @@
bool isSafeToRun(JSWebAssemblyMemory*) const;
+ void finishCreation(VM&, JSWebAssemblyModule*);
+
// These two callee getters are only valid once the callees have been populated.
Wasm::Callee& jsEntrypointCalleeFromFunctionIndexSpace(unsigned functionIndexSpace)
Modified: trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyInstance.cpp (216172 => 216173)
--- trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyInstance.cpp 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Source/_javascript_Core/wasm/js/JSWebAssemblyInstance.cpp 2017-05-04 05:50:01 UTC (rev 216173)
@@ -112,7 +112,7 @@
ASSERT(&codeBlock->codeBlock() == wasmCodeBlock.ptr());
m_codeBlock.set(vm, this, codeBlock);
} else {
- codeBlock = JSWebAssemblyCodeBlock::create(vm, wasmCodeBlock.copyRef(), m_module->module().moduleInformation());
+ codeBlock = JSWebAssemblyCodeBlock::create(vm, wasmCodeBlock.copyRef(), m_module.get());
m_codeBlock.set(vm, this, codeBlock);
module()->setCodeBlock(vm, memoryMode(), codeBlock);
}
Modified: trunk/Tools/ChangeLog (216172 => 216173)
--- trunk/Tools/ChangeLog 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Tools/ChangeLog 2017-05-04 05:50:01 UTC (rev 216173)
@@ -1,3 +1,12 @@
+2017-05-03 Saam Barati <[email protected]>
+
+ How we build polymorphic cases is wrong when making a call from Wasm
+ https://bugs.webkit.org/show_bug.cgi?id=171527
+
+ Reviewed by JF Bastien.
+
+ * Scripts/run-jsc-stress-tests:
+
2017-05-03 Eric Carlson <[email protected]>
[MediaStream] Allow host application to enable/disable media capture
Modified: trunk/Tools/Scripts/run-jsc-stress-tests (216172 => 216173)
--- trunk/Tools/Scripts/run-jsc-stress-tests 2017-05-04 04:48:07 UTC (rev 216172)
+++ trunk/Tools/Scripts/run-jsc-stress-tests 2017-05-04 05:50:01 UTC (rev 216173)
@@ -1200,6 +1200,8 @@
prepareExtraRelativeFiles(modules.map { |f| "../" + f }, $collection)
run("default-wasm", "-m", *FTL_OPTIONS)
run("wasm-no-cjit", "-m", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
+ run("wasm-eager-jettison", "-m", "--forceCodeBlockToJettisonDueToOldAge=true", *FTL_OPTIONS)
+ run("wasm-no-call-ic", "-m", "--useCallICsForWebAssemblyToJSCalls=false", *FTL_OPTIONS)
end
def runWebAssemblyEmscripten(mode)
@@ -1213,6 +1215,8 @@
prepareExtraRelativeFiles([Pathname('..') + wasm], $collection)
run("default-wasm", *FTL_OPTIONS)
run("wasm-no-cjit", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
+ run("wasm-eager-jettison", "--forceCodeBlockToJettisonDueToOldAge=true", *FTL_OPTIONS)
+ run("wasm-no-call-ic", "--useCallICsForWebAssemblyToJSCalls=false", *FTL_OPTIONS)
end
def runWebAssemblySpecTest(mode)
@@ -1233,6 +1237,8 @@
runWithOutputHandler("default-wasm", noisyOutputHandler, "../spec-harness.js", *FTL_OPTIONS)
runWithOutputHandler("wasm-no-cjit", noisyOutputHandler, "../spec-harness.js", *(FTL_OPTIONS + NO_CJIT_OPTIONS))
+ runWithOutputHandler("wasm-eager-jettison", noisyOutputHandler, "../spec-harness.js", "--forceCodeBlockToJettisonDueToOldAge=true", *FTL_OPTIONS)
+ runWithOutputHandler("wasm-no-call-ic", noisyOutputHandler, "../spec-harness.js", "--useCallICsForWebAssemblyToJSCalls=false", *FTL_OPTIONS)
end
def runChakra(mode, exception, baselineFile, extraFiles)
