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/dfg/DFGJITCompiler.cpp

    r209678 r209725  
    100100}
    101101
    102 void JITCompiler::compileEntry()
    103 {
    104     // This code currently matches the old JIT. In the function header we need to
    105     // save return address and call frame via the prologue and perform a fast stack check.
    106     // FIXME: https://bugs.webkit.org/show_bug.cgi?id=56292
    107     // We'll need to convert the remaining cti_ style calls (specifically the stack
    108     // check) which will be dependent on stack layout. (We'd need to account for this in
    109     // both normal return code and when jumping to an exception handler).
    110     emitFunctionPrologue();
    111     emitPutToCallFrameHeader(m_codeBlock, CallFrameSlot::codeBlock);
    112 }
    113 
    114102void JITCompiler::compileSetupRegistersForEntry()
    115103{
    … …  
    278266        JSCallRecord& record = m_jsCalls[i];
    279267        CallLinkInfo& info = *record.info;
    280         linkBuffer.link(record.slowCall, FunctionPtr(m_vm->getCTIStub(linkCallThunkGenerator).code().executableAddress()));
     268        linkBuffer.link(record.slowCall, FunctionPtr(m_vm->getJITCallThunkEntryStub(linkCallThunkGenerator).entryFor(info.argumentsLocation()).executableAddress()));
    281269        info.setCallLocations(
    282270            CodeLocationLabel(linkBuffer.locationOfNearCall(record.slowCall)),
    … …  
    288276        CallLinkInfo& info = *record.info;
    289277        linkBuffer.link(record.call, linkBuffer.locationOf(record.slowPath));
     278        if (record.hasSlowCall())
     279            linkBuffer.link(record.slowCall, FunctionPtr(m_vm->getJITCallThunkEntryStub(linkDirectCallThunkGenerator).entryFor(info.argumentsLocation()).executableAddress()));
    290280        info.setCallLocations(
    291281            CodeLocationLabel(),
    … …  
    355345void JITCompiler::compile()
    356346{
     347    Label mainEntry(this);
     348
    357349    setStartOfCode();
    358     compileEntry();
     350    emitFunctionPrologue();
     351
     352    Label entryPoint(this);
     353    emitPutToCallFrameHeader(m_codeBlock, CallFrameSlot::codeBlock);
     354
    359355    m_speculative = std::make_unique<SpeculativeJIT>(*this);
    360356
    … …  
    383379    m_speculative->callOperationWithCallFrameRollbackOnException(operationThrowStackOverflowError, m_codeBlock);
    384380
     381#if NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     382    m_stackArgsArityOKEntry = label();
     383    emitFunctionPrologue();
     384
     385    // Load argument values into argument registers
     386    loadPtr(addressFor(CallFrameSlot::callee), argumentRegisterForCallee());
     387    load32(payloadFor(CallFrameSlot::argumentCount), argumentRegisterForArgumentCount());
     388   
     389    for (unsigned argIndex = 0; argIndex < static_cast<unsigned>(m_codeBlock->numParameters()) && argIndex < NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS; argIndex++)
     390        load64(Address(GPRInfo::callFrameRegister, (CallFrameSlot::thisArgument + argIndex) * static_cast<int>(sizeof(Register))), argumentRegisterForFunctionArgument(argIndex));
     391   
     392    jump(entryPoint);
     393#endif
     394
    385395    // Generate slow path code.
    386396    m_speculative->runSlowPathGenerators(m_pcToCodeOriginMapBuilder);
    … …  
    407417
    408418    disassemble(*linkBuffer);
    409    
     419
     420    JITEntryPoints entrypoints;
     421#if NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     422    entrypoints.setEntryFor(RegisterArgsArityCheckNotRequired, linkBuffer->locationOf(mainEntry));
     423    entrypoints.setEntryFor(StackArgsArityCheckNotRequired, linkBuffer->locationOf(m_stackArgsArityOKEntry));
     424#else
     425    entrypoints.setEntryFor(StackArgsArityCheckNotRequired, linkBuffer->locationOf(mainEntry));
     426#endif
     427
    410428    m_graph.m_plan.finalizer = std::make_unique<JITFinalizer>(
    411         m_graph.m_plan, WTFMove(m_jitCode), WTFMove(linkBuffer));
     429        m_graph.m_plan, WTFMove(m_jitCode), WTFMove(linkBuffer), entrypoints);
    412430}
    413431
    … …  
    415433{
    416434    setStartOfCode();
    417     compileEntry();
     435
     436#if NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     437    unsigned numParameters = static_cast<unsigned>(m_codeBlock->numParameters());
     438    GPRReg argCountReg = argumentRegisterForArgumentCount();
     439    JumpList continueRegisterEntry;
     440    Label registerArgumentsEntrypoints[NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS + 1];
     441
     442    if (numParameters < NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS) {
     443        // Spill any extra register arguments passed to function onto the stack.
     444        for (unsigned extraRegisterArgumentIndex = NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS - 1;
     445            extraRegisterArgumentIndex >= numParameters; extraRegisterArgumentIndex--) {
     446            registerArgumentsEntrypoints[extraRegisterArgumentIndex + 1] = label();
     447            emitPutArgumentToCallFrameBeforePrologue(argumentRegisterForFunctionArgument(extraRegisterArgumentIndex), extraRegisterArgumentIndex);
     448        }
     449    }
     450    incrementCounter(this, VM::RegArgsExtra);
     451
     452    continueRegisterEntry.append(jump());
     453
     454    m_registerArgsWithArityCheck = label();
     455    incrementCounter(this, VM::RegArgsArity);
     456
     457    Label registerArgsCheckArity(this);
     458
     459    Jump registerCheckArity;
     460
     461    if (numParameters < NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS)
     462        registerCheckArity = branch32(NotEqual, argCountReg, TrustedImm32(numParameters));
     463    else {
     464        registerCheckArity = branch32(Below, argCountReg, TrustedImm32(numParameters));
     465        m_registerArgsWithPossibleExtraArgs = label();
     466    }
     467   
     468    Label registerEntryNoArity(this);
     469
     470    if (numParameters <= NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS)
     471        registerArgumentsEntrypoints[numParameters] = registerEntryNoArity;
     472
     473    incrementCounter(this, VM::RegArgsNoArity);
     474
     475    continueRegisterEntry.link(this);
     476#endif // NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     477
     478    Label mainEntry(this);
     479
     480    emitFunctionPrologue();
    418481
    419482    // === Function header code generation ===
    … …  
    422485    // so enter after this.
    423486    Label fromArityCheck(this);
     487
     488#if NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     489    storePtr(argumentRegisterForCallee(), addressFor(CallFrameSlot::callee));
     490    store32(argCountReg, payloadFor(CallFrameSlot::argumentCount));
     491
     492    Label fromStackEntry(this);
     493#endif
     494   
     495    emitPutToCallFrameHeader(m_codeBlock, CallFrameSlot::codeBlock);
     496
    424497    // Plant a check that sufficient space is available in the JSStack.
    425     addPtr(TrustedImm32(virtualRegisterForLocal(m_graph.requiredRegisterCountForExecutionAndExit() - 1).offset() * sizeof(Register)), GPRInfo::callFrameRegister, GPRInfo::regT1);
    426     Jump stackOverflow = branchPtr(Above, AbsoluteAddress(m_vm->addressOfSoftStackLimit()), GPRInfo::regT1);
     498    addPtr(TrustedImm32(virtualRegisterForLocal(m_graph.requiredRegisterCountForExecutionAndExit() - 1).offset() * sizeof(Register)), GPRInfo::callFrameRegister, GPRInfo::nonArgGPR0);
     499    Jump stackOverflow = branchPtr(Above, AbsoluteAddress(m_vm->addressOfSoftStackLimit()), GPRInfo::nonArgGPR0);
    427500
    428501    // Move the stack pointer down to accommodate locals
    … …  
    453526
    454527    m_speculative->callOperationWithCallFrameRollbackOnException(operationThrowStackOverflowError, m_codeBlock);
    455    
    456     // The fast entry point into a function does not check the correct number of arguments
    457     // have been passed to the call (we only use the fast entry point where we can statically
    458     // determine the correct number of arguments have been passed, or have already checked).
    459     // In cases where an arity check is necessary, we enter here.
    460     // FIXME: change this from a cti call to a DFG style operation (normal C calling conventions).
    461     m_arityCheck = label();
    462     compileEntry();
     528
     529    JumpList arityOK;
     530   
     531#if NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     532    jump(registerArgsCheckArity);
     533
     534    JumpList registerArityNeedsFixup;
     535    if (numParameters < NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS) {
     536        registerCheckArity.link(this);
     537        registerArityNeedsFixup.append(branch32(Below, argCountReg, TrustedImm32(m_codeBlock->numParameters())));
     538
     539        // We have extra register arguments.
     540
     541        // The fast entry point into a function does not check that the correct number of arguments
     542        // have been passed to the call (we only use the fast entry point where we can statically
     543        // determine the correct number of arguments have been passed, or have already checked).
     544        // In cases where an arity check is necessary, we enter here.
     545        m_registerArgsWithPossibleExtraArgs = label();
     546
     547        incrementCounter(this, VM::RegArgsExtra);
     548
     549        // Spill extra args passed to function
     550        for (unsigned argIndex = static_cast<unsigned>(m_codeBlock->numParameters()); argIndex < NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS; argIndex++) {
     551            branch32(MacroAssembler::BelowOrEqual, argCountReg, MacroAssembler::TrustedImm32(argIndex)).linkTo(mainEntry, this);
     552            emitPutArgumentToCallFrameBeforePrologue(argumentRegisterForFunctionArgument(argIndex), argIndex);
     553        }
     554        jump(mainEntry);
     555    }
     556
     557    // Fall through
     558    if (numParameters > 0) {
     559        // There should always be a "this" parameter.
     560        unsigned registerArgumentFixupCount = std::min(numParameters - 1, NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS);
     561        Label registerArgumentsNeedArityFixup = label();
     562
     563        for (unsigned argIndex = 1; argIndex <= registerArgumentFixupCount; argIndex++)
     564            registerArgumentsEntrypoints[argIndex] = registerArgumentsNeedArityFixup;
     565    }
     566
     567    incrementCounter(this, VM::RegArgsArity);
     568
     569    registerArityNeedsFixup.link(this);
     570
     571    if (numParameters >= NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS)
     572        registerCheckArity.link(this);
     573
     574    spillArgumentRegistersToFrameBeforePrologue();
     575
     576#if ENABLE(VM_COUNTERS)
     577    Jump continueToStackArityFixup = jump();
     578#endif
     579#endif // NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     580
     581    m_stackArgsWithArityCheck = label();
     582    incrementCounter(this, VM::StackArgsArity);
     583
     584#if ENABLE(VM_COUNTERS)
     585    continueToStackArityFixup.link(this);
     586#endif
     587
     588    emitFunctionPrologue();
    463589
    464590    load32(AssemblyHelpers::payloadFor((VirtualRegister)CallFrameSlot::argumentCount), GPRInfo::regT1);
    465     branch32(AboveOrEqual, GPRInfo::regT1, TrustedImm32(m_codeBlock->numParameters())).linkTo(fromArityCheck, this);
     591    arityOK.append(branch32(AboveOrEqual, GPRInfo::regT1, TrustedImm32(m_codeBlock->numParameters())));
     592
     593    incrementCounter(this, VM::ArityFixupRequired);
     594
    466595    emitStoreCodeOrigin(CodeOrigin(0));
    467596    if (maxFrameExtentForSlowPathCall)
    … …  
    470599    if (maxFrameExtentForSlowPathCall)
    471600        addPtr(TrustedImm32(maxFrameExtentForSlowPathCall), stackPointerRegister);
    472     branchTest32(Zero, GPRInfo::returnValueGPR).linkTo(fromArityCheck, this);
     601    arityOK.append(branchTest32(Zero, GPRInfo::returnValueGPR));
     602
    473603    emitStoreCodeOrigin(CodeOrigin(0));
    474604    move(GPRInfo::returnValueGPR, GPRInfo::argumentGPR0);
    475605    m_callArityFixup = call();
     606
     607#if NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     608    Jump toFillRegisters = jump();
     609
     610    m_stackArgsArityOKEntry = label();
     611
     612    incrementCounter(this, VM::StackArgsNoArity);
     613    emitFunctionPrologue();
     614
     615    arityOK.link(this);
     616    toFillRegisters.link(this);
     617
     618    // Load argument values into argument registers
     619    for (unsigned argIndex = 0; argIndex < static_cast<unsigned>(m_codeBlock->numParameters()) && argIndex < NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS; argIndex++)
     620        load64(Address(GPRInfo::callFrameRegister, (CallFrameSlot::thisArgument + argIndex) * static_cast<int>(sizeof(Register))), argumentRegisterForFunctionArgument(argIndex));
     621
     622    jump(fromStackEntry);
     623#else
     624    arityOK.linkTo(fromArityCheck, this);
    476625    jump(fromArityCheck);
     626#endif
    477627   
    478628    // Generate slow path code.
    … …  
    503653    disassemble(*linkBuffer);
    504654
    505     MacroAssemblerCodePtr withArityCheck = linkBuffer->locationOf(m_arityCheck);
     655    JITEntryPoints entrypoints;
     656#if NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS
     657#if ENABLE(VM_COUNTERS)
     658    MacroAssemblerCodePtr mainEntryCodePtr = linkBuffer->locationOf(registerEntryNoArity);
     659#else
     660    MacroAssemblerCodePtr mainEntryCodePtr = linkBuffer->locationOf(mainEntry);
     661#endif
     662    entrypoints.setEntryFor(RegisterArgsArityCheckNotRequired, mainEntryCodePtr);
     663    entrypoints.setEntryFor(RegisterArgsPossibleExtraArgs, linkBuffer->locationOf(m_registerArgsWithPossibleExtraArgs));
     664    entrypoints.setEntryFor(RegisterArgsMustCheckArity, linkBuffer->locationOf(m_registerArgsWithArityCheck));
     665
     666    for (unsigned argCount = 1; argCount <= NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS; argCount++) {
     667        MacroAssemblerCodePtr entry;
     668        if (argCount == numParameters)
     669            entry = mainEntryCodePtr;
     670        else if (registerArgumentsEntrypoints[argCount].isSet())
     671            entry = linkBuffer->locationOf(registerArgumentsEntrypoints[argCount]);
     672        else
     673            entry = linkBuffer->locationOf(m_registerArgsWithArityCheck);
     674        entrypoints.setEntryFor(JITEntryPoints::registerEntryTypeForArgumentCount(argCount), entry);
     675    }
     676    entrypoints.setEntryFor(StackArgsArityCheckNotRequired, linkBuffer->locationOf(m_stackArgsArityOKEntry));
     677#else
     678    entrypoints.setEntryFor(StackArgsArityCheckNotRequired, linkBuffer->locationOf(mainEntry));
     679#endif
     680    entrypoints.setEntryFor(StackArgsMustCheckArity, linkBuffer->locationOf(m_stackArgsWithArityCheck));
    506681
    507682    m_graph.m_plan.finalizer = std::make_unique<JITFinalizer>(
    508         m_graph.m_plan, WTFMove(m_jitCode), WTFMove(linkBuffer), withArityCheck);
     683        m_graph.m_plan, WTFMove(m_jitCode), WTFMove(linkBuffer), entrypoints);
    509684}
    510685
Note: See TracChangeset for help on using the changeset viewer.