WebKit Bugzilla
Attachment 371914 Details for
Bug 198770
: [JSC] Polymorphic call stub's slow path should restore callee saves before performing tail call
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-198770-20190611194248.patch (text/plain), 18.52 KB, created by
Yusuke Suzuki
on 2019-06-11 19:42:49 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Yusuke Suzuki
Created:
2019-06-11 19:42:49 PDT
Size:
18.52 KB
patch
obsolete
>Subversion Revision: 246327 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index 420e2e84a640646ddb99e1c510496d252d6f7ef6..754c36ff84a6a5726a27532c6885c6e108192ab3 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,29 @@ >+2019-06-11 Yusuke Suzuki <ysuzuki@apple.com> >+ >+ [JSC] virtual tail call thunk should restore callee saves before jump >+ https://bugs.webkit.org/show_bug.cgi?id=198770 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * bytecode/CallLinkInfo.cpp: >+ (JSC::CallLinkInfo::CallLinkInfo): >+ * bytecode/CallLinkInfo.h: >+ (JSC::CallLinkInfo::setUpCall): >+ (JSC::CallLinkInfo::calleeGPR): >+ (JSC::CallLinkInfo::setCalleeGPR): Deleted. >+ * jit/AssemblyHelpers.cpp: >+ (JSC::AssemblyHelpers::emitDumbVirtualCall): >+ * jit/Repatch.cpp: >+ (JSC::linkSlowFor): >+ (JSC::revertCall): >+ (JSC::linkVirtualFor): >+ (JSC::linkPolymorphicCall): >+ * jit/Repatch.h: >+ * jit/ThunkGenerators.cpp: >+ (JSC::virtualThunkFor): >+ (JSC::virtualSlowPathThunkFor): >+ * jit/ThunkGenerators.h: >+ > 2019-06-11 Saam Barati <sbarati@apple.com> > > Roll out PAC cage >diff --git a/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp b/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp >index ced9d4d67baddbd47fe0d6d41f46f775d09ca82d..d8dac1585fab2b998c1e5d88752ba12df6dadbde 100644 >--- a/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp >+++ b/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp >@@ -62,7 +62,6 @@ CallLinkInfo::CallLinkInfo() > , m_allowStubs(true) > , m_clearedByJettison(false) > , m_callType(None) >- , m_calleeGPR(255) > { > } > >diff --git a/Source/JavaScriptCore/bytecode/CallLinkInfo.h b/Source/JavaScriptCore/bytecode/CallLinkInfo.h >index fa0a115b5890e5978cc4eb5815cbc274dfffb007..89875da2be2370087dc01e5add63a9e81b10d6ad 100644 >--- a/Source/JavaScriptCore/bytecode/CallLinkInfo.h >+++ b/Source/JavaScriptCore/bytecode/CallLinkInfo.h >@@ -157,7 +157,7 @@ class CallLinkInfo : public PackedRawSentinelNode<CallLinkInfo> { > bool isLinked() { return m_stub || m_calleeOrCodeBlock; } > void unlink(VM&); > >- void setUpCall(CallType callType, CodeOrigin codeOrigin, unsigned calleeGPR) >+ void setUpCall(CallType callType, CodeOrigin codeOrigin, GPRReg calleeGPR) > { > m_callType = callType; > m_codeOrigin = codeOrigin; >@@ -312,12 +312,7 @@ class CallLinkInfo : public PackedRawSentinelNode<CallLinkInfo> { > return OBJECT_OFFSETOF(CallLinkInfo, m_slowPathCount); > } > >- void setCalleeGPR(unsigned calleeGPR) >- { >- m_calleeGPR = calleeGPR; >- } >- >- unsigned calleeGPR() >+ GPRReg calleeGPR() > { > return m_calleeGPR; > } >@@ -364,7 +359,7 @@ class CallLinkInfo : public PackedRawSentinelNode<CallLinkInfo> { > bool m_allowStubs : 1; > bool m_clearedByJettison : 1; > unsigned m_callType : 4; // CallType >- unsigned m_calleeGPR : 8; >+ GPRReg m_calleeGPR { InvalidGPRReg }; > uint32_t m_slowPathCount { 0 }; > }; > >diff --git a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp >index 5b700c102d4805c4aee922a7bf3d0a691bd33e67..5df597d9830c4a26ffa417cf2dd60decb0724d0a 100644 >--- a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp >+++ b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp >@@ -630,11 +630,12 @@ void AssemblyHelpers::restoreCalleeSavesFromEntryFrameCalleeSavesBuffer(EntryFra > > void AssemblyHelpers::emitDumbVirtualCall(VM& vm, CallLinkInfo* info) > { >+ ASSERT(!info->isTailCall()); > move(TrustedImmPtr(info), GPRInfo::regT2); > Call call = nearCall(); > addLinkTask( > [=, &vm] (LinkBuffer& linkBuffer) { >- MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunk = virtualThunkFor(&vm, *info); >+ MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunk = virtualSlowPathThunkFor(&vm, *info); > info->setSlowStub(createJITStubRoutine(virtualThunk, vm, nullptr, true)); > linkBuffer.link(call, CodeLocationLabel<JITStubRoutinePtrTag>(virtualThunk.code())); > }); >diff --git a/Source/JavaScriptCore/jit/Repatch.cpp b/Source/JavaScriptCore/jit/Repatch.cpp >index f07d325aaa17fc91ff8c8c23643e7329863ee836..25b53621c92045566edaa27347639c3f880655bf 100644 >--- a/Source/JavaScriptCore/jit/Repatch.cpp >+++ b/Source/JavaScriptCore/jit/Repatch.cpp >@@ -62,6 +62,7 @@ > #include "StructureStubInfo.h" > #include "SuperSampler.h" > #include "ThunkGenerators.h" >+#include "ProbeContext.h" > #include <wtf/CommaPrinter.h> > #include <wtf/ListDump.h> > #include <wtf/StringPrintStream.h> >@@ -808,7 +809,7 @@ static void linkSlowFor(VM* vm, CallLinkInfo& callLinkInfo, ThunkGenerator gener > > static void linkSlowFor(VM* vm, CallLinkInfo& callLinkInfo) > { >- MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunk = virtualThunkFor(vm, callLinkInfo); >+ MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunk = virtualSlowPathThunkFor(vm, callLinkInfo); > linkSlowFor(vm, callLinkInfo, virtualThunk); > callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk, *vm, nullptr, true)); > } >@@ -909,7 +910,7 @@ static void revertCall(VM* vm, CallLinkInfo& callLinkInfo, MacroAssemblerCodeRef > if (!callLinkInfo.clearedByJettison()) { > MacroAssembler::revertJumpReplacementToBranchPtrWithPatch( > MacroAssembler::startOfBranchPtrWithPatchOnRegister(callLinkInfo.hotPathBegin()), >- static_cast<MacroAssembler::RegisterID>(callLinkInfo.calleeGPR()), 0); >+ callLinkInfo.calleeGPR(), 0); > linkSlowFor(vm, callLinkInfo, codeRef); > MacroAssembler::repatchPointer(callLinkInfo.hotPathBegin(), nullptr); > } >@@ -930,7 +931,7 @@ void unlinkFor(VM& vm, CallLinkInfo& callLinkInfo) > revertCall(&vm, callLinkInfo, vm.getCTIStub(linkCallThunkGenerator).retagged<JITStubRoutinePtrTag>()); > } > >-void linkVirtualFor(ExecState* exec, CallLinkInfo& callLinkInfo) >+static void linkVirtualFor(ExecState* exec, CallLinkInfo& callLinkInfo) > { > CallFrame* callerFrame = exec->callerFrame(); > VM& vm = callerFrame->vm(); >@@ -939,7 +940,7 @@ void linkVirtualFor(ExecState* exec, CallLinkInfo& callLinkInfo) > if (shouldDumpDisassemblyFor(callerCodeBlock)) > dataLog("Linking virtual call at ", FullCodeOrigin(callerCodeBlock, callerFrame->codeOrigin()), "\n"); > >- MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunk = virtualThunkFor(&vm, callLinkInfo); >+ MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunk = virtualThunkFor(&vm, callLinkInfo, callerCodeBlock); > revertCall(&vm, callLinkInfo, virtualThunk); > callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk, vm, nullptr, true)); > callLinkInfo.setClearedByVirtual(); >@@ -1035,51 +1036,7 @@ void linkPolymorphicCall( > linkVirtualFor(exec, callLinkInfo); > return; > } >- >- GPRReg calleeGPR = static_cast<GPRReg>(callLinkInfo.calleeGPR()); >- >- CCallHelpers stubJit(callerCodeBlock); >- >- CCallHelpers::JumpList slowPath; >- >- std::unique_ptr<CallFrameShuffler> frameShuffler; >- if (callLinkInfo.frameShuffleData()) { >- ASSERT(callLinkInfo.isTailCall()); >- frameShuffler = std::make_unique<CallFrameShuffler>(stubJit, *callLinkInfo.frameShuffleData()); >-#if USE(JSVALUE32_64) >- // We would have already checked that the callee is a cell, and we can >- // use the additional register this buys us. >- frameShuffler->assumeCalleeIsCell(); >-#endif >- frameShuffler->lockGPR(calleeGPR); >- } >- GPRReg comparisonValueGPR; >- >- if (isClosureCall) { >- GPRReg scratchGPR; >- if (frameShuffler) >- scratchGPR = frameShuffler->acquireGPR(); >- else >- scratchGPR = AssemblyHelpers::selectScratchGPR(calleeGPR); >- // Verify that we have a function and stash the executable in scratchGPR. >- >-#if USE(JSVALUE64) >- slowPath.append(stubJit.branchIfNotCell(calleeGPR)); >-#else >- // We would have already checked that the callee is a cell. >-#endif > >- // FIXME: We could add a fast path for InternalFunction with closure call. >- slowPath.append(stubJit.branchIfNotFunction(calleeGPR)); >- >- stubJit.loadPtr( >- CCallHelpers::Address(calleeGPR, JSFunction::offsetOfExecutable()), >- scratchGPR); >- >- comparisonValueGPR = scratchGPR; >- } else >- comparisonValueGPR = calleeGPR; >- > Vector<int64_t> caseValues(callCases.size()); > Vector<CallToCodePtr> calls(callCases.size()); > UniqueArray<uint32_t> fastCounts; >@@ -1126,6 +1083,31 @@ void linkPolymorphicCall( > caseValues[i] = newCaseValue; > } > >+ GPRReg calleeGPR = callLinkInfo.calleeGPR(); >+ >+ CCallHelpers stubJit(callerCodeBlock); >+ >+ std::unique_ptr<CallFrameShuffler> frameShuffler; >+ if (callLinkInfo.frameShuffleData()) { >+ ASSERT(callLinkInfo.isTailCall()); >+ frameShuffler = std::make_unique<CallFrameShuffler>(stubJit, *callLinkInfo.frameShuffleData()); >+#if USE(JSVALUE32_64) >+ // We would have already checked that the callee is a cell, and we can >+ // use the additional register this buys us. >+ frameShuffler->assumeCalleeIsCell(); >+#endif >+ frameShuffler->lockGPR(calleeGPR); >+ } >+ >+ GPRReg comparisonValueGPR; >+ if (isClosureCall) { >+ if (frameShuffler) >+ comparisonValueGPR = frameShuffler->acquireGPR(); >+ else >+ comparisonValueGPR = AssemblyHelpers::selectScratchGPR(calleeGPR); >+ } else >+ comparisonValueGPR = calleeGPR; >+ > GPRReg fastCountsBaseGPR; > if (frameShuffler) > fastCountsBaseGPR = frameShuffler->acquireGPR(); >@@ -1134,8 +1116,30 @@ void linkPolymorphicCall( > AssemblyHelpers::selectScratchGPR(calleeGPR, comparisonValueGPR, GPRInfo::regT3); > } > stubJit.move(CCallHelpers::TrustedImmPtr(fastCounts.get()), fastCountsBaseGPR); >- if (!frameShuffler && callLinkInfo.isTailCall()) >+ if (!frameShuffler && callLinkInfo.isTailCall()) { >+ // We strongly assume that calleeGPR is not a callee save register in the slow path. > stubJit.emitRestoreCalleeSaves(); >+ } >+ >+ CCallHelpers::JumpList slowPath; >+ if (isClosureCall) { >+ // Verify that we have a function and stash the executable in scratchGPR. >+#if USE(JSVALUE64) >+ if (callLinkInfo.isTailCall()) >+ slowPath.append(stubJit.branchIfNotCell(calleeGPR, DoNotHaveTagRegisters)); >+ else >+ slowPath.append(stubJit.branchIfNotCell(calleeGPR)); >+#else >+ // We would have already checked that the callee is a cell. >+#endif >+ // FIXME: We could add a fast path for InternalFunction with closure call. >+ slowPath.append(stubJit.branchIfNotFunction(calleeGPR)); >+ >+ stubJit.loadPtr( >+ CCallHelpers::Address(calleeGPR, JSFunction::offsetOfExecutable()), >+ comparisonValueGPR); >+ } >+ > BinarySwitch binarySwitch(comparisonValueGPR, caseValues, BinarySwitch::IntPtr); > CCallHelpers::JumpList done; > while (binarySwitch.advance(stubJit)) { >diff --git a/Source/JavaScriptCore/jit/Repatch.h b/Source/JavaScriptCore/jit/Repatch.h >index 86a6c5a3dfe31bed82c4e950f3dd06fa14236413..02db1202bebbde4dd5b5e9780875b3890d7961ff 100644 >--- a/Source/JavaScriptCore/jit/Repatch.h >+++ b/Source/JavaScriptCore/jit/Repatch.h >@@ -50,7 +50,6 @@ void linkFor(ExecState*, CallLinkInfo&, CodeBlock*, JSObject* callee, MacroAssem > void linkDirectFor(ExecState*, CallLinkInfo&, CodeBlock*, MacroAssemblerCodePtr<JSEntryPtrTag>); > void linkSlowFor(ExecState*, CallLinkInfo&); > void unlinkFor(VM&, CallLinkInfo&); >-void linkVirtualFor(ExecState*, CallLinkInfo&); > void linkPolymorphicCall(ExecState*, CallLinkInfo&, CallVariant); > void resetGetByID(CodeBlock*, StructureStubInfo&, GetByIDKind); > void resetPutByID(CodeBlock*, StructureStubInfo&); >diff --git a/Source/JavaScriptCore/jit/ThunkGenerators.cpp b/Source/JavaScriptCore/jit/ThunkGenerators.cpp >index e84f00447771ca30ff2272349c4e07e7aaa3de0b..84ede2e06ee19b4fe957ec9b8ce656c61e361bf4 100644 >--- a/Source/JavaScriptCore/jit/ThunkGenerators.cpp >+++ b/Source/JavaScriptCore/jit/ThunkGenerators.cpp >@@ -162,14 +162,14 @@ MacroAssemblerCodeRef<JITThunkPtrTag> linkPolymorphicCallThunkGenerator(VM* vm) > // path virtual call so that we can enable fast tail calls for megamorphic > // virtual calls by using the shuffler. > // https://bugs.webkit.org/show_bug.cgi?id=148831 >-MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunkFor(VM* vm, CallLinkInfo& callLinkInfo) >+MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunkFor(VM* vm, CallLinkInfo& callLinkInfo, CodeBlock* callerCodeBlock) > { > // The callee is in regT0 (for JSVALUE32_64, the tag is in regT1). > // The return address is on the stack, or in the link register. We will hence > // jump to the callee, or save the return address to the call frame while we > // make a C++ function call to the appropriate JIT operation. > >- CCallHelpers jit; >+ CCallHelpers jit(callerCodeBlock); > > CCallHelpers::JumpList slowCase; > >@@ -183,16 +183,88 @@ MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunkFor(VM* vm, CallLinkInfo > // the DFG knows that the value is definitely a cell, or definitely a function. > > #if USE(JSVALUE64) >- GPRReg tagMaskRegister = GPRInfo::tagMaskRegister; > if (callLinkInfo.isTailCall()) { > // Tail calls could have clobbered the GPRInfo::tagMaskRegister because they > // restore callee saved registers before getthing here. So, let's materialize > // the TagMask in a temp register and use the temp instead. >- tagMaskRegister = GPRInfo::regT4; >- jit.move(CCallHelpers::TrustedImm64(TagMask), tagMaskRegister); >+ slowCase.append(jit.branchIfNotCell(GPRInfo::regT0, DoNotHaveTagRegisters)); >+ } else >+ slowCase.append(jit.branchIfNotCell(GPRInfo::regT0)); >+#else >+ slowCase.append(jit.branchIfNotCell(GPRInfo::regT1)); >+#endif >+ auto notJSFunction = jit.branchIfNotType(GPRInfo::regT0, JSFunctionType); >+ >+ // Now we know we have a JSFunction. >+ >+ jit.loadPtr( >+ CCallHelpers::Address(GPRInfo::regT0, JSFunction::offsetOfExecutable()), >+ GPRInfo::regT4); >+ jit.loadPtr( >+ CCallHelpers::Address( >+ GPRInfo::regT4, ExecutableBase::offsetOfJITCodeWithArityCheckFor( >+ callLinkInfo.specializationKind())), >+ GPRInfo::regT4); >+ slowCase.append(jit.branchTestPtr(CCallHelpers::Zero, GPRInfo::regT4)); >+ >+ // Now we know that we have a CodeBlock, and we're committed to making a fast >+ // call. >+ >+ // Make a tail call. This will return back to JIT code. >+ JSInterfaceJIT::Label callCode(jit.label()); >+ emitPointerValidation(jit, GPRInfo::regT4, JSEntryPtrTag); >+ if (callLinkInfo.isTailCall()) { >+ jit.preserveReturnAddressAfterCall(GPRInfo::regT0); >+ jit.prepareForTailCallSlow(GPRInfo::regT4); > } >- slowCase.append( >- jit.branchTest64(CCallHelpers::NonZero, GPRInfo::regT0, tagMaskRegister)); >+ jit.jump(GPRInfo::regT4, JSEntryPtrTag); >+ >+ notJSFunction.link(&jit); >+ slowCase.append(jit.branchIfNotType(GPRInfo::regT0, InternalFunctionType)); >+ void* executableAddress = vm->getCTIInternalFunctionTrampolineFor(callLinkInfo.specializationKind()).executableAddress(); >+ jit.move(CCallHelpers::TrustedImmPtr(executableAddress), GPRInfo::regT4); >+ jit.jump().linkTo(callCode, &jit); >+ >+ slowCase.link(&jit); >+ >+ // Here we don't know anything, so revert to the full slow path. >+ slowPathFor(jit, vm, operationVirtualCall); >+ >+ LinkBuffer patchBuffer(jit, GLOBAL_THUNK_ID); >+ return FINALIZE_CODE( >+ patchBuffer, JITStubRoutinePtrTag, >+ "Virtual %s thunk", >+ callLinkInfo.callMode() == CallMode::Regular ? "call" : callLinkInfo.callMode() == CallMode::Tail ? "tail call" : "construct"); >+} >+ >+MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualSlowPathThunkFor(VM* vm, CallLinkInfo& callLinkInfo) >+{ >+ // The callee is in regT0 (for JSVALUE32_64, the tag is in regT1). >+ // The return address is on the stack, or in the link register. We will hence >+ // jump to the callee, or save the return address to the call frame while we >+ // make a C++ function call to the appropriate JIT operation. >+ >+ CCallHelpers jit; >+ >+ CCallHelpers::JumpList slowCase; >+ >+ // This is a slow path execution, and regT2 contains the CallLinkInfo. Count the >+ // slow path execution for the profiler. >+ jit.add32( >+ CCallHelpers::TrustedImm32(1), >+ CCallHelpers::Address(GPRInfo::regT2, CallLinkInfo::offsetOfSlowPathCount())); >+ >+ // FIXME: we should have a story for eliminating these checks. In many cases, >+ // the DFG knows that the value is definitely a cell, or definitely a function. >+ >+#if USE(JSVALUE64) >+ if (callLinkInfo.isTailCall()) { >+ // Tail calls could have clobbered the GPRInfo::tagMaskRegister because they >+ // restore callee saved registers before getthing here. So, let's materialize >+ // the TagMask in a temp register and use the temp instead. >+ slowCase.append(jit.branchIfNotCell(GPRInfo::regT0, DoNotHaveTagRegisters)); >+ } else >+ slowCase.append(jit.branchIfNotCell(GPRInfo::regT0)); > #else > slowCase.append(jit.branchIfNotCell(GPRInfo::regT1)); > #endif >@@ -240,6 +312,7 @@ MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunkFor(VM* vm, CallLinkInfo > callLinkInfo.callMode() == CallMode::Regular ? "call" : callLinkInfo.callMode() == CallMode::Tail ? "tail call" : "construct"); > } > >+ > enum ThunkEntryType { EnterViaCall, EnterViaJumpWithSavedTags, EnterViaJumpWithoutSavedTags }; > enum class ThunkFunctionType { JSFunction, InternalFunction }; > >diff --git a/Source/JavaScriptCore/jit/ThunkGenerators.h b/Source/JavaScriptCore/jit/ThunkGenerators.h >index 2d4b30f179fcd72ed49d97abb3d689cfb5cf2bf3..392665523ce1d4729b0c0874e48442fbf0dda92f 100644 >--- a/Source/JavaScriptCore/jit/ThunkGenerators.h >+++ b/Source/JavaScriptCore/jit/ThunkGenerators.h >@@ -41,7 +41,8 @@ MacroAssemblerCodeRef<JITThunkPtrTag> linkCallThunk(VM*, CallLinkInfo&, CodeSpec > MacroAssemblerCodeRef<JITThunkPtrTag> linkCallThunkGenerator(VM*); > MacroAssemblerCodeRef<JITThunkPtrTag> linkPolymorphicCallThunkGenerator(VM*); > >-MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunkFor(VM*, CallLinkInfo&); >+MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunkFor(VM*, CallLinkInfo&, CodeBlock* callerCodeBlock); >+MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualSlowPathThunkFor(VM*, CallLinkInfo&); > > MacroAssemblerCodeRef<JITThunkPtrTag> nativeCallGenerator(VM*); > MacroAssemblerCodeRef<JITThunkPtrTag> nativeConstructGenerator(VM*);
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 198770
:
371914
|
371934
|
371935