Changeset 210244 in webkit


Ignore:
Timestamp:
Jan 3, 2017 12:24:36 PM (7 years ago)
Author:
jfbastien@apple.com
Message:

WebAssembly JS API: check and test in-call / out-call values
​https://bugs.webkit.org/show_bug.cgi?id=164876
<rdar://problem/29844107>

Reviewed by Saam Barati.

JSTests:

  • wasm.yaml:
  • wasm/assert.js: add an assert for NaN comparison
  • wasm/fuzz/export-function.js: Added. Generate random wasm export

signatures, and call them with random parameters.
(const.paramExporter):
(const.setBuffer):
(const.types.generate):
(generate):

  • wasm/js-api/export-arity.js: Added.

(const.paramExporter): Test that mismatched arities when JS calls
wasm follow the defined semantics: i32 is 0, f32 / f64 are NaN.
​https://github.com/WebAssembly/design/blob/master/JS.md#exported-function-exotic-objects

  • wasm/js-api/export-void-is-undef.js: Added. Test that "void"

wasm functions return "undefined" in JS.

Source/JavaScriptCore:

  • wasm/WasmBinding.cpp:

(JSC::Wasm::wasmToJs): fix the wasm -> JS call coercions for f32 /
f64 which the assotiated tests inadvertently tripped on: the
previous code wasn't correctly performing JSValue boxing for
"double" values. This change is slightly involved because it
requires two scratch registers to materialize the
DoubleEncodeOffset value. This change therefore reorganizes the
code to first generate traps, then handle all integers (freeing
all GPRs), and then all the floating-point values.

  • wasm/js/WebAssemblyFunction.cpp:

(JSC::callWebAssemblyFunction): Implement the defined semantics
for mismatched arities when JS calls wasm:
​https://github.com/WebAssembly/design/blob/master/JS.md#exported-function-exotic-objects

  • i32 is 0, f32 / f64 are NaN.
  • wasm functions which return "void" are "undefined" in JS.
Location:
trunk
Files:
4 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r210229 r210244  
     12017-01-03  JF Bastien  <jfbastien@apple.com>
     2
     3        WebAssembly JS API: check and test in-call / out-call values
     4        https://bugs.webkit.org/show_bug.cgi?id=164876
     5        <rdar://problem/29844107>
     6
     7        Reviewed by Saam Barati.
     8
     9        * wasm.yaml:
     10        * wasm/assert.js: add an assert for NaN comparison
     11        * wasm/fuzz/export-function.js: Added. Generate random wasm export
     12        signatures, and call them with random parameters.
     13        (const.paramExporter):
     14        (const.setBuffer):
     15        (const.types.generate):
     16        (generate):
     17        * wasm/js-api/export-arity.js: Added.
     18        (const.paramExporter): Test that mismatched arities when JS calls
     19        wasm follow the defined semantics: i32 is 0, f32 / f64 are NaN.
     20        https://github.com/WebAssembly/design/blob/master/JS.md#exported-function-exotic-objects
     21        * wasm/js-api/export-void-is-undef.js: Added. Test that "void"
     22        wasm functions return "undefined" in JS.
     23
    1242017-01-02  JF Bastien  <jfbastien@apple.com>
    225

  • trunk/JSTests/wasm.yaml

    r210228 r210244  
    2727  cmd: runWebAssembly
    2828- path: wasm/function-tests
     29  cmd: runWebAssembly
     30- path: wasm/fuzz
    2931  cmd: runWebAssembly
    3032

  • trunk/JSTests/wasm/Builder.js

    r210229 r210244  
    753753            section: this._sections
    754754        };
    755         return JSON.stringify(obj);
     755        // JSON.stringify serializes -0.0 as 0.0.
     756        const replacer = (key, value) => {
     757            if (value === 0.0 && 1.0 / value === -Infinity)
     758                return "NEGATIVE_ZERO";
     759            return value;
     760        };
     761        return JSON.stringify(obj, replacer);
    756762    }
    757763    AsmJS() {

  • trunk/JSTests/wasm/assert.js

    r210073 r210244  
    7373
    7474export const eq = (lhs, rhs, msg) => {
     75    if (typeof lhs !== typeof rhs)
     76        _fail(`Not the same: "${lhs}" and "${rhs}"`, msg);
    7577    if (Array.isArray(lhs) && Array.isArray(rhs) && (lhs.length === rhs.length)) {
    7678        for (let i = 0; i !== lhs.length; ++i)
    7779            eq(lhs[i], rhs[i], msg);
    78     } else if (lhs !== rhs)
     80    } else if (lhs !== rhs) {
     81        if (typeof lhs === "number" && isNaN(lhs) && isNaN(rhs))
     82            return;
    7983        _fail(`Not the same: "${lhs}" and "${rhs}"`, msg);
     84    } else {
     85        if (typeof lhs === "number" && (1.0 / lhs !== 1.0 / rhs)) // Distinguish -0.0 from 0.0.
     86            _fail(`Not the same: "${lhs}" and "${rhs}"`, msg);
     87    }
    8088};
    8189

  • trunk/JSTests/wasm/self-test/test_BuilderJSON.js

    r209863 r210244  
    511511        assert.eq(j.section[1].data[0].code[0].arguments.length, 0);
    512512        assert.eq(j.section[1].data[0].code[0].immediates.length, 1);
    513         assert.eq(j.section[1].data[0].code[0].immediates[0], c);
     513        assert.eq(j.section[1].data[0].code[0].immediates[0] === "NEGATIVE_ZERO" ? -0.0 : j.section[1].data[0].code[0].immediates[0], c);
    514514    }
    515515})();
    … …  
    527527        assert.eq(j.section[1].data[0].code[0].arguments.length, 0);
    528528        assert.eq(j.section[1].data[0].code[0].immediates.length, 1);
    529         assert.eq(j.section[1].data[0].code[0].immediates[0], c);
     529        assert.eq(j.section[1].data[0].code[0].immediates[0] === "NEGATIVE_ZERO" ? -0.0 : j.section[1].data[0].code[0].immediates[0], c);
    530530    }
    531531})();

  • trunk/Source/JavaScriptCore/ChangeLog

    r210237 r210244  
     12017-01-03  JF Bastien  <jfbastien@apple.com>
     2
     3        WebAssembly JS API: check and test in-call / out-call values
     4        https://bugs.webkit.org/show_bug.cgi?id=164876
     5        <rdar://problem/29844107>
     6
     7        Reviewed by Saam Barati.
     8
     9        * wasm/WasmBinding.cpp:
     10        (JSC::Wasm::wasmToJs): fix the wasm -> JS call coercions for f32 /
     11        f64 which the assotiated tests inadvertently tripped on: the
     12        previous code wasn't correctly performing JSValue boxing for
     13        "double" values. This change is slightly involved because it
     14        requires two scratch registers to materialize the
     15        `DoubleEncodeOffset` value. This change therefore reorganizes the
     16        code to first generate traps, then handle all integers (freeing
     17        all GPRs), and then all the floating-point values.
     18        * wasm/js/WebAssemblyFunction.cpp:
     19        (JSC::callWebAssemblyFunction): Implement the defined semantics
     20        for mismatched arities when JS calls wasm:
     21        https://github.com/WebAssembly/design/blob/master/JS.md#exported-function-exotic-objects
     22          - i32 is 0, f32 / f64 are NaN.
     23          - wasm functions which return "void" are "undefined" in JS.
     24
    1252017-01-03  Per Arne Vollan  <pvollan@apple.com>
    226

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

    r210229 r210244  
    7575    jit.subPtr(MacroAssembler::TrustedImm32(stackOffset), MacroAssembler::stackPointerRegister);
    7676    JIT::Address calleeFrame = CCallHelpers::Address(MacroAssembler::stackPointerRegister, -static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC)));
    77 
    78     // FIXME make this a loop which switches on Signature if there are many arguments on the stack. It'll otherwise be huge for huge signatures. https://bugs.webkit.org/show_bug.cgi?id=165547
    79     unsigned marshalledGPRs = 0;
    80     unsigned marshalledFPRs = 0;
    81     unsigned calleeFrameOffset = CallFrameSlot::firstArgument * static_cast<int>(sizeof(Register));
    82     unsigned frOffset = CallFrameSlot::firstArgument * static_cast<int>(sizeof(Register));
     77   
    8378    for (unsigned argNum = 0; argNum < argCount; ++argNum) {
    8479        Type argType = signature->argument(argNum);
    … …  
    8782        case Func:
    8883        case Anyfunc:
    89         case I64:
    90             // FIXME: Figure out the correct behavior here. I suspect we want such a stub to throw an exception immediately
     84        case I64: {
     85            // FIXME: Figure out the correct behavior here. I suspect we want such a stub to throw an exception immediately.
    9186            // if called. https://bugs.webkit.org/show_bug.cgi?id=165991
    9287            jit.breakpoint();
    93             break;
    94         case I32: {
    95             GPRReg gprReg;
    96             if (marshalledGPRs < wasmCC.m_gprArgs.size())
    97                 gprReg = wasmCC.m_gprArgs[marshalledGPRs].gpr();
    98             else {
    99                 // We've already spilled all arguments, these registers are available as scratch.
    100                 gprReg = GPRInfo::argumentGPR0;
    101                 jit.load64(JIT::Address(GPRInfo::callFrameRegister, frOffset), gprReg);
    102                 frOffset += sizeof(Register);
    103             }
    104             ++marshalledGPRs;
    105             jit.boxInt32(gprReg, JSValueRegs(gprReg), DoNotHaveTagRegisters);
    106             jit.store64(gprReg, calleeFrame.withOffset(calleeFrameOffset));
    107             calleeFrameOffset += sizeof(Register);
    108             break;
     88            LinkBuffer patchBuffer(*vm, jit, GLOBAL_THUNK_ID);
     89            return FINALIZE_CODE(patchBuffer, ("WebAssembly import[%i] stub for signature %i", importIndex, signatureIndex));
    10990        }
    110         case F32: {
    111             FPRReg fprReg;
    112             if (marshalledFPRs < wasmCC.m_fprArgs.size())
    113                 fprReg = wasmCC.m_fprArgs[marshalledFPRs].fpr();
    114             else {
    115                 // We've already spilled all arguments, these registers are available as scratch.
    116                 fprReg = FPRInfo::argumentFPR0;
    117                 jit.loadFloat(JIT::Address(GPRInfo::callFrameRegister, frOffset), fprReg);
    118                 frOffset += sizeof(Register);
    119             }
    120             jit.convertFloatToDouble(fprReg, fprReg);
    121             jit.purifyNaN(fprReg);
    122             jit.storeDouble(fprReg, calleeFrame.withOffset(calleeFrameOffset));
    123             calleeFrameOffset += sizeof(Register);
    124             ++marshalledFPRs;
    125             break;
     91        case I32:
     92        case F32:
     93        case F64:
     94            continue;
    12695        }
    127         case F64: {
    128             FPRReg fprReg;
    129             if (marshalledFPRs < wasmCC.m_fprArgs.size())
    130                 fprReg = wasmCC.m_fprArgs[marshalledFPRs].fpr();
    131             else {
    132                 // We've already spilled all arguments, these registers are available as scratch.
    133                 fprReg = FPRInfo::argumentFPR0;
    134                 jit.loadDouble(JIT::Address(GPRInfo::callFrameRegister, frOffset), fprReg);
    135                 frOffset += sizeof(Register);
    136             }
    137             jit.purifyNaN(fprReg);
    138             jit.storeDouble(fprReg, calleeFrame.withOffset(calleeFrameOffset));
    139             calleeFrameOffset += sizeof(Register);
    140             ++marshalledFPRs;
    141             break;
     96    }
     97
     98    // FIXME make these loops which switch on Signature if there are many arguments on the stack. It'll otherwise be huge for huge signatures. https://bugs.webkit.org/show_bug.cgi?id=165547
     99   
     100    // First go through the integer parameters, freeing up their register for use afterwards.
     101    {
     102        unsigned marshalledGPRs = 0;
     103        unsigned marshalledFPRs = 0;
     104        unsigned calleeFrameOffset = CallFrameSlot::firstArgument * static_cast<int>(sizeof(Register));
     105        unsigned frOffset = CallFrameSlot::firstArgument * static_cast<int>(sizeof(Register));
     106        for (unsigned argNum = 0; argNum < argCount; ++argNum) {
     107            Type argType = signature->argument(argNum);
     108            switch (argType) {
     109            case Void:
     110            case Func:
     111            case Anyfunc:
     112            case I64:
     113                RELEASE_ASSERT_NOT_REACHED(); // Handled above.
     114            case I32: {
     115                GPRReg gprReg;
     116                if (marshalledGPRs < wasmCC.m_gprArgs.size())
     117                    gprReg = wasmCC.m_gprArgs[marshalledGPRs].gpr();
     118                else {
     119                    // We've already spilled all arguments, these registers are available as scratch.
     120                    gprReg = GPRInfo::argumentGPR0;
     121                    jit.load64(JIT::Address(GPRInfo::callFrameRegister, frOffset), gprReg);
     122                    frOffset += sizeof(Register);
     123                }
     124                ++marshalledGPRs;
     125                jit.boxInt32(gprReg, JSValueRegs(gprReg), DoNotHaveTagRegisters);
     126                jit.store64(gprReg, calleeFrame.withOffset(calleeFrameOffset));
     127                calleeFrameOffset += sizeof(Register);
     128                break;
     129            }
     130            case F32:
     131            case F64:
     132                // Skipped: handled below.
     133                if (marshalledFPRs >= wasmCC.m_fprArgs.size())
     134                    frOffset += sizeof(Register);
     135                ++marshalledFPRs;
     136                calleeFrameOffset += sizeof(Register);
     137                break;
     138            }
    142139        }
     140    }
     141   
     142    {
     143        // Integer registers have already been spilled, these are now available.
     144        GPRReg doubleEncodeOffsetGPRReg = GPRInfo::argumentGPR0;
     145        GPRReg scratch = GPRInfo::argumentGPR1;
     146        bool hasMaterializedDoubleEncodeOffset = false;
     147        auto materializeDoubleEncodeOffset = [&hasMaterializedDoubleEncodeOffset, &jit] (GPRReg dest) {
     148            if (!hasMaterializedDoubleEncodeOffset) {
     149                static_assert(DoubleEncodeOffset == 1ll << 48, "codegen assumes this below");
     150                jit.move(JIT::TrustedImm32(1), dest);
     151                jit.lshift64(JIT::TrustedImm32(48), dest);
     152                hasMaterializedDoubleEncodeOffset = true;
     153            }
     154        };
     155
     156        unsigned marshalledGPRs = 0;
     157        unsigned marshalledFPRs = 0;
     158        unsigned calleeFrameOffset = CallFrameSlot::firstArgument * static_cast<int>(sizeof(Register));
     159        unsigned frOffset = CallFrameSlot::firstArgument * static_cast<int>(sizeof(Register));
     160        for (unsigned argNum = 0; argNum < argCount; ++argNum) {
     161            Type argType = signature->argument(argNum);
     162            switch (argType) {
     163            case Void:
     164            case Func:
     165            case Anyfunc:
     166            case I64:
     167                RELEASE_ASSERT_NOT_REACHED(); // Handled above.
     168            case I32:
     169                // Skipped: handled above.
     170                if (marshalledGPRs < wasmCC.m_gprArgs.size())
     171                    frOffset += sizeof(Register);
     172                ++marshalledGPRs;
     173                calleeFrameOffset += sizeof(Register);
     174                break;
     175            case F32: {
     176                FPRReg fprReg;
     177                if (marshalledFPRs < wasmCC.m_fprArgs.size())
     178                    fprReg = wasmCC.m_fprArgs[marshalledFPRs].fpr();
     179                else {
     180                    // We've already spilled all arguments, these registers are available as scratch.
     181                    fprReg = FPRInfo::argumentFPR0;
     182                    jit.loadFloat(JIT::Address(GPRInfo::callFrameRegister, frOffset), fprReg);
     183                    frOffset += sizeof(Register);
     184                }
     185                jit.convertFloatToDouble(fprReg, fprReg);
     186                jit.purifyNaN(fprReg);
     187                jit.moveDoubleTo64(fprReg, scratch);
     188                materializeDoubleEncodeOffset(doubleEncodeOffsetGPRReg);
     189                jit.add64(doubleEncodeOffsetGPRReg, scratch);
     190                jit.store64(scratch, calleeFrame.withOffset(calleeFrameOffset));
     191                calleeFrameOffset += sizeof(Register);
     192                ++marshalledFPRs;
     193                break;
     194            }
     195            case F64: {
     196                FPRReg fprReg;
     197                if (marshalledFPRs < wasmCC.m_fprArgs.size())
     198                    fprReg = wasmCC.m_fprArgs[marshalledFPRs].fpr();
     199                else {
     200                    // We've already spilled all arguments, these registers are available as scratch.
     201                    fprReg = FPRInfo::argumentFPR0;
     202                    jit.loadDouble(JIT::Address(GPRInfo::callFrameRegister, frOffset), fprReg);
     203                    frOffset += sizeof(Register);
     204                }
     205                jit.purifyNaN(fprReg);
     206                jit.moveDoubleTo64(fprReg, scratch);
     207                materializeDoubleEncodeOffset(doubleEncodeOffsetGPRReg);
     208                jit.add64(doubleEncodeOffsetGPRReg, scratch);
     209                jit.store64(scratch, calleeFrame.withOffset(calleeFrameOffset));
     210                calleeFrameOffset += sizeof(Register);
     211                ++marshalledFPRs;
     212                break;
     213            }
     214            }
    143215        }
    144216    }

  • trunk/Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp

    r210229 r210244  
    5757    const Wasm::Signature* signature = Wasm::SignatureInformation::get(&vm, signatureIndex);
    5858
    59     // FIXME is this the right behavior? https://bugs.webkit.org/show_bug.cgi?id=164876
    60     if (exec->argumentCount() != signature->argumentCount())
    61         return JSValue::encode(throwException(exec, scope, createNotEnoughArgumentsError(exec, defaultSourceAppender)));
    62 
    6359    {
    6460        // Check if we have a disallowed I64 use.
    … …  
    7975    }
    8076
    81     // FIXME is this boxing correct? https://bugs.webkit.org/show_bug.cgi?id=164876
    8277    Vector<JSValue> boxedArgs;
    8378    for (unsigned argIndex = 0; argIndex < signature->argumentCount(); ++argIndex) {
    84         JSValue arg = exec->uncheckedArgument(argIndex);
     79        JSValue arg = exec->argument(argIndex);
    8580        switch (signature->argument(argIndex)) {
    8681        case Wasm::I32:
    … …  
    128123    RETURN_IF_EXCEPTION(scope, { });
    129124
    130     // FIXME is this correct? https://bugs.webkit.org/show_bug.cgi?id=164876
    131125    switch (signature->returnType()) {
    132126    case Wasm::Void:
Note: See TracChangeset for help on using the changeset viewer.