Changeset 256665 in webkit


Ignore:
Timestamp:
Feb 14, 2020 6:57:49 PM (4 years ago)
Author:
Tadeu Zagallo
Message:

[WASM] Wasm interpreter's calling convention doesn't match Wasm JIT's convention.
​https://bugs.webkit.org/show_bug.cgi?id=207727

JSTests:

Reviewed by Mark Lam.

  • wasm/regress/llint-callee-saves-with-fast-memory.js: Added.
  • wasm/regress/llint-callee-saves-without-fast-memory.js: Added.

Source/JavaScriptCore:

Reviewed by Mark Lam.

The Wasm JIT has unusual calling conventions, which were further complicated by the addition
of the interpreter, and the interpreter did not correctly follow these conventions (by incorrectly
saving and restoring the callee save registers used for the memory base and size). Here's a summary
of the calling convention:

  • When entering Wasm from JS, the wrapper must:
    • Preserve the base and size when entering LLInt regardless of the mode. (Prior to this patch we only preserved the base in Signaling mode)
    • Preserve the memory base in either mode, and the size for BoundsChecking.
  • Both tiers must preserve every *other* register they use. e.g. the LLInt must preserve PB and wasmInstance, but must *not* preserve memoryBase and memorySize.
  • Changes to memoryBase and memorySize are visible to the caller. This means that:
    • Intra-module calls can assume these registers are up-to-date even if the memory was resized. The only exception here is if the LLInt calls a signaling JIT, in which case the JIT will not update the size register, since it won't be using it.
    • Inter-module and JS calls require the caller to reload these registers. These calls may result in memory changes (e.g. the callee may call memory.grow).
    • A Signaling JIT caller must be aware that the LLInt may trash the size register, since it always bounds checks.
  • llint/WebAssembly.asm:
  • wasm/WasmAirIRGenerator.cpp:

(JSC::Wasm::AirIRGenerator::addCall):

  • wasm/WasmB3IRGenerator.cpp:

(JSC::Wasm::B3IRGenerator::addCall):

  • wasm/WasmCallee.cpp:

(JSC::Wasm::LLIntCallee::calleeSaveRegisters):

  • wasm/WasmCallingConvention.h:
  • wasm/WasmLLIntPlan.cpp:

(JSC::Wasm::LLIntPlan::didCompleteCompilation):

  • wasm/WasmMemoryInformation.cpp:

(JSC::Wasm::PinnedRegisterInfo::get):
(JSC::Wasm::getPinnedRegisters): Deleted.

Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r256541 r256665  
     12020-02-14  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        [WASM] Wasm interpreter's calling convention doesn't match Wasm JIT's convention.
     4        https://bugs.webkit.org/show_bug.cgi?id=207727
     5
     6        Reviewed by Mark Lam.
     7
     8        * wasm/regress/llint-callee-saves-with-fast-memory.js: Added.
     9        * wasm/regress/llint-callee-saves-without-fast-memory.js: Added.
     10
    1112020-02-13  Caio Lima  <ticaiolima@gmail.com>
    212

  • trunk/Source/JavaScriptCore/ChangeLog

    r256579 r256665  
     12020-02-14  Tadeu Zagallo  <tzagallo@apple.com> and Michael Saboff  <msaboff@apple.com>
     2
     3        [WASM] Wasm interpreter's calling convention doesn't match Wasm JIT's convention.
     4        https://bugs.webkit.org/show_bug.cgi?id=207727
     5
     6        Reviewed by Mark Lam.
     7
     8        The Wasm JIT has unusual calling conventions, which were further complicated by the addition
     9        of the interpreter, and the interpreter did not correctly follow these conventions (by incorrectly
     10        saving and restoring the callee save registers used for the memory base and size). Here's a summary
     11        of the calling convention:
     12
     13        - When entering Wasm from JS, the wrapper must:
     14            - Preserve the base and size when entering LLInt regardless of the mode. (Prior to this
     15              patch we only preserved the base in Signaling mode)
     16            - Preserve the memory base in either mode, and the size for BoundsChecking.
     17        - Both tiers must preserve every *other* register they use. e.g. the LLInt must preserve PB
     18          and wasmInstance, but must *not* preserve memoryBase and memorySize.
     19        - Changes to memoryBase and memorySize are visible to the caller. This means that:
     20            - Intra-module calls can assume these registers are up-to-date even if the memory was
     21              resized. The only exception here is if the LLInt calls a signaling JIT, in which case
     22              the JIT will not update the size register, since it won't be using it.
     23            - Inter-module and JS calls require the caller to reload these registers. These calls may
     24              result in memory changes (e.g. the callee may call memory.grow).
     25            - A Signaling JIT caller must be aware that the LLInt may trash the size register, since
     26              it always bounds checks.
     27
     28        * llint/WebAssembly.asm:
     29        * wasm/WasmAirIRGenerator.cpp:
     30        (JSC::Wasm::AirIRGenerator::addCall):
     31        * wasm/WasmB3IRGenerator.cpp:
     32        (JSC::Wasm::B3IRGenerator::addCall):
     33        * wasm/WasmCallee.cpp:
     34        (JSC::Wasm::LLIntCallee::calleeSaveRegisters):
     35        * wasm/WasmCallingConvention.h:
     36        * wasm/WasmLLIntPlan.cpp:
     37        (JSC::Wasm::LLIntPlan::didCompleteCompilation):
     38        * wasm/WasmMemoryInformation.cpp:
     39        (JSC::Wasm::PinnedRegisterInfo::get):
     40        (JSC::Wasm::getPinnedRegisters): Deleted.
     41
    1422020-02-13  Stephan Szabo  <stephan.szabo@sony.com>
    243

  • trunk/Source/JavaScriptCore/llint/WebAssembly.asm

    r253904 r256665  
    179179
    180180macro preserveCalleeSavesUsedByWasm()
     181    # NOTE: We intentionally don't save memoryBase and memorySize here. See the comment
     182    # in restoreCalleeSavesUsedByWasm() below for why.
    181183    subp CalleeSaveSpaceStackAligned, sp
    182184    if ARM64 or ARM64E
    183         emit "stp x23, x26, [x29, #-16]"
    184         emit "stp x19, x22, [x29, #-32]"
     185        emit "stp x19, x26, [x29, #-16]"
    185186    elsif X86_64
    186         storep memorySize, -0x08[cfr]
    187         storep memoryBase, -0x10[cfr]
    188         storep PB, -0x18[cfr]
    189         storep wasmInstance, -0x20[cfr]
     187        storep PB, -0x8[cfr]
     188        storep wasmInstance, -0x10[cfr]
    190189    else
    191190        error
    … …  
    194193
    195194macro restoreCalleeSavesUsedByWasm()
     195    # NOTE: We intentionally don't restore memoryBase and memorySize here. These are saved
     196    # and restored when entering Wasm by the JSToWasm wrapper and changes to them are meant
     197    # to be observable within the same Wasm module.
    196198    if ARM64 or ARM64E
    197         emit "ldp x23, x26, [x29, #-16]"
    198         emit "ldp x19, x22, [x29, #-32]"
     199        emit "ldp x19, x26, [x29, #-16]"
    199200    elsif X86_64
    200         loadp -0x08[cfr], memorySize
    201         loadp -0x10[cfr], memoryBase
    202         loadp -0x18[cfr], PB
    203         loadp -0x20[cfr], wasmInstance
     201        loadp -0x8[cfr], PB
     202        loadp -0x10[cfr], wasmInstance
    204203    else
    205204        error

  • trunk/Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp

    r254087 r256665  
    21742174    } else {
    21752175        auto* patchpoint = emitCallPatchpoint(m_currentBlock, signature, results, args);
     2176        // We need to clobber the size register since the LLInt always bounds checks
     2177        if (m_mode == MemoryMode::Signaling)
     2178            patchpoint->clobberLate(RegisterSet { PinnedRegisterInfo::get().sizeRegister });
    21762179        patchpoint->setGenerator([unlinkedWasmToWasmCalls, functionIndex] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
    21772180            AllowMacroScratchRegisterUsage allowScratch(jit);

  • trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp

    r255406 r256665  
    17271727                patchpoint->effects.readsPinned = true;
    17281728
     1729                // We need to clobber the size register since the LLInt always bounds checks
     1730                if (m_mode == MemoryMode::Signaling)
     1731                    patchpoint->clobberLate(RegisterSet { PinnedRegisterInfo::get().sizeRegister });
    17291732                patchpoint->setGenerator([unlinkedWasmToWasmCalls, functionIndex] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
    17301733                    AllowMacroScratchRegisterUsage allowScratch(jit);

  • trunk/Source/JavaScriptCore/wasm/WasmCallee.cpp

    r253538 r256665  
    9595#error Unsupported architecture.
    9696#endif
    97         registers.set(GPRInfo::regCS3); // Memory base
    98         registers.set(GPRInfo::regCS4); // Memory size
    9997        ASSERT(registers.numberOfSetRegisters() == numberOfLLIntCalleeSaveRegisters);
    10098        calleeSaveRegisters.construct(WTFMove(registers));

  • trunk/Source/JavaScriptCore/wasm/WasmCallingConvention.h

    r251886 r256665  
    4747namespace JSC { namespace Wasm {
    4848
    49 constexpr unsigned numberOfLLIntCalleeSaveRegisters = 4;
     49constexpr unsigned numberOfLLIntCalleeSaveRegisters = 2;
    5050
    5151using ArgumentLocation = B3::ValueRep;

  • trunk/Source/JavaScriptCore/wasm/WasmLLIntPlan.cpp

    r253314 r256665  
    147147            const Signature& signature = SignatureInformation::get(signatureIndex);
    148148            CCallHelpers jit;
    149             std::unique_ptr<InternalFunction> function = createJSToWasmWrapper(jit, signature, &m_unlinkedWasmToWasmCalls[functionIndex], m_moduleInformation.get(), m_mode, functionIndex);
     149            // The LLInt always bounds checks
     150            MemoryMode mode = MemoryMode::BoundsChecking;
     151            std::unique_ptr<InternalFunction> function = createJSToWasmWrapper(jit, signature, &m_unlinkedWasmToWasmCalls[functionIndex], m_moduleInformation.get(), mode, functionIndex);
    150152
    151153            LinkBuffer linkBuffer(jit, nullptr, JITCompilationCanFail);

  • trunk/Source/JavaScriptCore/wasm/WasmMemoryInformation.cpp

    r251886 r256665  
    3636namespace JSC { namespace Wasm {
    3737
    38 static Vector<GPRReg> getPinnedRegisters(unsigned remainingPinnedRegisters)
    39 {
    40     Vector<GPRReg> registers;
    41     jsCallingConvention().calleeSaveRegisters.forEach([&] (Reg reg) {
    42         if (!reg.isGPR())
    43             return;
    44         GPRReg gpr = reg.gpr();
    45         if (!remainingPinnedRegisters || RegisterSet::stackRegisters().get(reg))
    46             return;
    47         if (RegisterSet::runtimeTagRegisters().get(reg)) {
    48             // Since we don't need to, we currently don't pick from the tag registers to allow
    49             // JS->Wasm stubs to freely use these registers.
    50             return;
    51         }
    52         --remainingPinnedRegisters;
    53         registers.append(gpr);
    54     });
    55     return registers;
    56 }
    57 
    5838const PinnedRegisterInfo& PinnedRegisterInfo::get()
    5939{
    … …  
    6444        if (!Context::useFastTLS())
    6545            ++numberOfPinnedRegisters;
    66         Vector<GPRReg> pinnedRegs = getPinnedRegisters(numberOfPinnedRegisters);
    67 
    6846        GPRReg baseMemoryPointer = GPRInfo::regCS3;
    6947        GPRReg sizeRegister = GPRInfo::regCS4;
Note: See TracChangeset for help on using the changeset viewer.