Ignore:
Timestamp:
Dec 12, 2016, 1:46:45 PM (9 years ago)
Author:
[email protected]
Message:

REGRESSION(r209653): speedometer crashes making virtual slow path tailcalls
​https://bugs.webkit.org/show_bug.cgi?id=165748

Reviewed by Filip Pizlo.

JSTests:

New regression test.

  • stress/regress-165748.js: Added.

(sum1):
(sum2):
(sum3):
(sum4):
(sum5):
(sum6):
(tailCaller):
(test):

Source/JavaScriptCore:

The virtual slow path for tailcalls always passes arguments on the stack.
The fix here is to link to the stack argument entrypoint instead of a register
argument entrypoint.

While fixing this bug, I found that we weren't clearing the code origin when
shuffling the call frame for a register argument tailcall.

Also rolling back in r209653, r209654, r209663, and r209673.

  • jit/CallFrameShuffler.cpp:

(JSC::CallFrameShuffler::prepareAny):

  • jit/ThunkGenerators.cpp:

(JSC::virtualThunkFor):

Source/WTF:

Rolling back in r209653, r209654, r209663, and r209673.

  • wtf/Platform.h:
File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/jit/Repatch.cpp

    r209678 r209725  
    541541}
    542542
    543 static void linkSlowFor(VM*, CallLinkInfo& callLinkInfo, MacroAssemblerCodeRef codeRef)
    544 {
    545     MacroAssembler::repatchNearCall(callLinkInfo.callReturnLocation(), CodeLocationLabel(codeRef.code()));
    546 }
    547 
    548 static void linkSlowFor(VM* vm, CallLinkInfo& callLinkInfo, ThunkGenerator generator)
    549 {
    550     linkSlowFor(vm, callLinkInfo, vm->getCTIStub(generator));
     543static void linkSlowFor(VM*, CallLinkInfo& callLinkInfo, JITJSCallThunkEntryPointsWithRef thunkEntryPoints)
     544{
     545    MacroAssembler::repatchNearCall(callLinkInfo.callReturnLocation(), CodeLocationLabel(thunkEntryPoints.entryFor(callLinkInfo.argumentsLocation())));
     546}
     547
     548static void linkSlowFor(VM* vm, CallLinkInfo& callLinkInfo, JITCallThunkEntryGenerator generator)
     549{
     550    linkSlowFor(vm, callLinkInfo, vm->getJITCallThunkEntryStub(generator));
    551551}
    552552
    553553static void linkSlowFor(VM* vm, CallLinkInfo& callLinkInfo)
    554554{
    555     MacroAssemblerCodeRef virtualThunk = virtualThunkFor(vm, callLinkInfo);
     555    JITJSCallThunkEntryPointsWithRef virtualThunk = virtualThunkFor(vm, callLinkInfo);
    556556    linkSlowFor(vm, callLinkInfo, virtualThunk);
    557     callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk, *vm, nullptr, true));
     557    callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk.codeRef(), *vm, nullptr, true));
    558558}
    559559
    … …  
    645645}
    646646
    647 static void revertCall(VM* vm, CallLinkInfo& callLinkInfo, MacroAssemblerCodeRef codeRef)
     647static void revertCall(VM* vm, CallLinkInfo& callLinkInfo, JITJSCallThunkEntryPointsWithRef codeRef)
    648648{
    649649    if (callLinkInfo.isDirect()) {
    … …  
    672672        dataLog("Unlinking call at ", callLinkInfo.hotPathOther(), "\n");
    673673   
    674     revertCall(&vm, callLinkInfo, vm.getCTIStub(linkCallThunkGenerator));
     674    revertCall(&vm, callLinkInfo, vm.getJITCallThunkEntryStub(linkCallThunkGenerator));
    675675}
    676676
    … …  
    684684        dataLog("Linking virtual call at ", *callerCodeBlock, " ", callerFrame->codeOrigin(), "\n");
    685685
    686     MacroAssemblerCodeRef virtualThunk = virtualThunkFor(&vm, callLinkInfo);
     686    JITJSCallThunkEntryPointsWithRef virtualThunk = virtualThunkFor(&vm, callLinkInfo);
    687687    revertCall(&vm, callLinkInfo, virtualThunk);
    688     callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk, vm, nullptr, true));
     688    callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk.codeRef(), vm, nullptr, true));
    689689}
    690690
    … …  
    741741   
    742742    Vector<PolymorphicCallCase> callCases;
     743    size_t callerArgumentCount = exec->argumentCountIncludingThis();
    743744   
    744745    // Figure out what our cases are.
    … …  
    752753            // If we cannot handle a callee, either because we don't have a CodeBlock or because arity mismatch,
    753754            // assume that it's better for this whole thing to be a virtual call.
    754             if (!codeBlock || exec->argumentCountIncludingThis() < static_cast<size_t>(codeBlock->numParameters()) || callLinkInfo.isVarargs()) {
     755            if (!codeBlock || callerArgumentCount < static_cast<size_t>(codeBlock->numParameters()) || callLinkInfo.isVarargs()) {
    755756                linkVirtualFor(exec, callLinkInfo);
    756757                return;
    … …  
    776777   
    777778    GPRReg calleeGPR = static_cast<GPRReg>(callLinkInfo.calleeGPR());
    778    
     779
     780    if (callLinkInfo.argumentsInRegisters())
     781        ASSERT(calleeGPR == argumentRegisterForCallee());
     782
    779783    CCallHelpers stubJit(&vm, callerCodeBlock);
    780784   
    … …  
    798802        if (frameShuffler)
    799803            scratchGPR = frameShuffler->acquireGPR();
     804        else if (callLinkInfo.argumentsInRegisters())
     805            scratchGPR = GPRInfo::nonArgGPR0;
    800806        else
    801807            scratchGPR = AssemblyHelpers::selectScratchGPR(calleeGPR);
    … …  
    863869    if (frameShuffler)
    864870        fastCountsBaseGPR = frameShuffler->acquireGPR();
     871    else if (callLinkInfo.argumentsInRegisters())
     872#if CPU(ARM64)
     873        fastCountsBaseGPR = GPRInfo::nonArgGPR1;
     874#else
     875        fastCountsBaseGPR = GPRInfo::regT0;
     876#endif
    865877    else {
    866878        fastCountsBaseGPR =
    867879            AssemblyHelpers::selectScratchGPR(calleeGPR, comparisonValueGPR, GPRInfo::regT3);
    868880    }
    869     stubJit.move(CCallHelpers::TrustedImmPtr(fastCounts.get()), fastCountsBaseGPR);
     881    if (fastCounts)
     882        stubJit.move(CCallHelpers::TrustedImmPtr(fastCounts.get()), fastCountsBaseGPR);
    870883    if (!frameShuffler && callLinkInfo.isTailCall())
    871884        stubJit.emitRestoreCalleeSaves();
     885
     886    incrementCounter(&stubJit, VM::PolymorphicCall);
     887
    872888    BinarySwitch binarySwitch(comparisonValueGPR, caseValues, BinarySwitch::IntPtr);
    873889    CCallHelpers::JumpList done;
    … …  
    878894       
    879895        ASSERT(variant.executable()->hasJITCodeForCall());
     896
     897        EntryPointType entryType = StackArgsArityCheckNotRequired;
     898#if NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     899        if (callLinkInfo.argumentsInRegisters()) {
     900            CodeBlock* codeBlock = callCases[caseIndex].codeBlock();
     901            if (codeBlock) {
     902                size_t calleeArgumentCount = static_cast<size_t>(codeBlock->numParameters());
     903                if (calleeArgumentCount == callerArgumentCount || calleeArgumentCount >= NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS)
     904                    entryType = RegisterArgsArityCheckNotRequired;
     905                else {
     906                    EntryPointType entryForArgCount = JITEntryPoints::registerEntryTypeForArgumentCount(callerArgumentCount);
     907                    MacroAssemblerCodePtr codePtr =
     908                        variant.executable()->generatedJITCodeForCall()->addressForCall(entryForArgCount);
     909                    if (codePtr)
     910                        entryType = entryForArgCount;
     911                    else
     912                        entryType = RegisterArgsPossibleExtraArgs;
     913                }
     914            } else
     915                entryType = RegisterArgsPossibleExtraArgs;
     916        }
     917#endif
     918
    880919        MacroAssemblerCodePtr codePtr =
    881             variant.executable()->generatedJITCodeForCall()->addressForCall(ArityCheckNotRequired);
     920            variant.executable()->generatedJITCodeForCall()->addressForCall(entryType);
     921        ASSERT(codePtr);
    882922       
    883923        if (fastCounts) {
    … …  
    887927        }
    888928        if (frameShuffler) {
    889             CallFrameShuffler(stubJit, frameShuffler->snapshot()).prepareForTailCall();
     929            CallFrameShuffler(stubJit, frameShuffler->snapshot(callLinkInfo.argumentsLocation())).prepareForTailCall();
    890930            calls[caseIndex].call = stubJit.nearTailCall();
    891931        } else if (callLinkInfo.isTailCall()) {
    … …  
    908948        frameShuffler->setCalleeJSValueRegs(JSValueRegs(GPRInfo::regT1, GPRInfo::regT0));
    909949#else
    910         frameShuffler->setCalleeJSValueRegs(JSValueRegs(GPRInfo::regT0));
     950        if (callLinkInfo.argumentsLocation() == StackArgs)
     951            frameShuffler->setCalleeJSValueRegs(JSValueRegs(argumentRegisterForCallee()));
    911952#endif
    912953        frameShuffler->prepareForSlowPath();
    913954    } else {
    914         stubJit.move(calleeGPR, GPRInfo::regT0);
    915955#if USE(JSVALUE32_64)
    916956        stubJit.move(CCallHelpers::TrustedImm32(JSValue::CellTag), GPRInfo::regT1);
    917957#endif
    918958    }
    919     stubJit.move(CCallHelpers::TrustedImmPtr(&callLinkInfo), GPRInfo::regT2);
    920     stubJit.move(CCallHelpers::TrustedImmPtr(callLinkInfo.callReturnLocation().executableAddress()), GPRInfo::regT4);
    921    
    922     stubJit.restoreReturnAddressBeforeReturn(GPRInfo::regT4);
     959    stubJit.move(CCallHelpers::TrustedImmPtr(callLinkInfo.callReturnLocation().executableAddress()), GPRInfo::nonArgGPR1);
     960    stubJit.restoreReturnAddressBeforeReturn(GPRInfo::nonArgGPR1);
     961
     962    stubJit.move(CCallHelpers::TrustedImmPtr(&callLinkInfo), GPRInfo::nonArgGPR0);
    923963    AssemblyHelpers::Jump slow = stubJit.jump();
    924964       
    … …  
    941981    else
    942982        patchBuffer.link(done, callLinkInfo.hotPathOther().labelAtOffset(0));
    943     patchBuffer.link(slow, CodeLocationLabel(vm.getCTIStub(linkPolymorphicCallThunkGenerator).code()));
     983    patchBuffer.link(slow, CodeLocationLabel(vm.getJITCallThunkEntryStub(linkPolymorphicCallThunkGenerator).entryFor(callLinkInfo.argumentsLocation())));
    944984   
    945985    auto stubRoutine = adoptRef(*new PolymorphicCallStubRoutine(
Note: See TracChangeset for help on using the changeset viewer.