Changeset 211195 in webkit


Ignore:
Timestamp:
Jan 25, 2017 6:38:41 PM (7 years ago)
Author:
sbarati@apple.com
Message:

WebAssembly JS API: coerce return values from imports
​https://bugs.webkit.org/show_bug.cgi?id=165480
<rdar://problem/29760318>

Reviewed by Yusuke Suzuki.

JSTests:

  • wasm/function-tests/function-import-return-value.js: Added.

(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.const.tests.x.assert.eq):
(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.const.tests.Math.fround):
(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.let.type.of.Reflect.ownKeys):
(test.1):
(assert.truthy):
(assert.throws):

Source/JavaScriptCore:

This patch does proper coercion for all possible
JSValue return types from an imported function.

It also adds the spec-compliant code to throw an exception
when calling an import that has an i64 parameter or return
value.

  • jit/AssemblyHelpers.cpp:

(JSC::AssemblyHelpers::emitJumpIfException):

  • jit/AssemblyHelpers.h:
  • wasm/WasmB3IRGenerator.cpp:
  • wasm/WasmBinding.cpp:

(JSC::Wasm::wasmToJs):

Location:
trunk
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r211194 r211195  
     12017-01-25  Saam Barati  <sbarati@apple.com>
     2
     3        WebAssembly JS API: coerce return values from imports
     4        https://bugs.webkit.org/show_bug.cgi?id=165480
     5        <rdar://problem/29760318>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        * wasm/function-tests/function-import-return-value.js: Added.
     10        (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.const.tests.x.assert.eq):
     11        (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.const.tests.Math.fround):
     12        (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.let.type.of.Reflect.ownKeys):
     13        (test.1):
     14        (assert.truthy):
     15        (assert.throws):
     16
    1172017-01-25  Filip Pizlo  <fpizlo@apple.com>
    218

  • trunk/Source/JavaScriptCore/ChangeLog

    r211194 r211195  
     12017-01-25  Saam Barati  <sbarati@apple.com>
     2
     3        WebAssembly JS API: coerce return values from imports
     4        https://bugs.webkit.org/show_bug.cgi?id=165480
     5        <rdar://problem/29760318>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        This patch does proper coercion for all possible
     10        JSValue return types from an imported function.
     11       
     12        It also adds the spec-compliant code to throw an exception
     13        when calling an import that has an i64 parameter or return
     14        value.
     15
     16        * jit/AssemblyHelpers.cpp:
     17        (JSC::AssemblyHelpers::emitJumpIfException):
     18        * jit/AssemblyHelpers.h:
     19        * wasm/WasmB3IRGenerator.cpp:
     20        * wasm/WasmBinding.cpp:
     21        (JSC::Wasm::wasmToJs):
     22
    1232017-01-25  Filip Pizlo  <fpizlo@apple.com>
    224

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

    r210695 r211195  
    353353#endif
    354354    }
     355}
     356
     357AssemblyHelpers::Jump AssemblyHelpers::emitJumpIfException()
     358{
     359    return emitExceptionCheck(NormalExceptionCheck);
    355360}
    356361

  • trunk/Source/JavaScriptCore/jit/AssemblyHelpers.h

    r210844 r211195  
    11861186        ExceptionCheckKind = NormalExceptionCheck, ExceptionJumpWidth = NormalJumpWidth);
    11871187    JS_EXPORT_PRIVATE Jump emitNonPatchableExceptionCheck();
     1188    Jump emitJumpIfException();
    11881189
    11891190#if ENABLE(SAMPLING_COUNTERS)

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

    r210259 r211195  
    977977
    978978            patchpoint->setGenerator([=] (CCallHelpers& jit, const B3::StackmapGenerationParams& params) {
     979                AllowMacroScratchRegisterUsage allowScratch(jit);
    979980                jit.call(params[returnType == Void ? 0 : 1].gpr());
    980981            });

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

    r210244 r211195  
    2929#if ENABLE(WEBASSEMBLY)
    3030
    31 #include "AssemblyHelpers.h"
    32 #include "JSCJSValueInlines.h"
     31#include "CCallHelpers.h"
     32#include "FrameTracers.h"
     33#include "JITExceptions.h"
     34#include "JSCInlines.h"
    3335#include "JSWebAssemblyInstance.h"
    3436#include "LinkBuffer.h"
     37#include "NativeErrorConstructor.h"
    3538#include "WasmCallingConvention.h"
     39#include "WasmExceptionType.h"
    3640
    3741namespace JSC { namespace Wasm {
    3842
    39 typedef AssemblyHelpers JIT;
     43typedef CCallHelpers JIT;
    4044
    4145static void materializeImportJSCell(VM* vm, JIT& jit, unsigned importIndex, GPRReg result)
    … …  
    6266    jit.storePtr(JIT::TrustedImmPtr(vm->webAssemblyToJSCallee.get()), JIT::Address(GPRInfo::callFrameRegister, CallFrameSlot::callee * static_cast<int>(sizeof(Register))));
    6367
     68
     69    {
     70        bool hasBadI64Use = false;
     71        hasBadI64Use |= signature->returnType() == I64;
     72        for (unsigned argNum = 0; argNum < argCount && !hasBadI64Use; ++argNum) {
     73            Type argType = signature->argument(argNum);
     74            switch (argType) {
     75            case Void:
     76            case Func:
     77            case Anyfunc:
     78                RELEASE_ASSERT_NOT_REACHED();
     79
     80            case I64: {
     81                hasBadI64Use = true;
     82                break;
     83            }
     84
     85            default:
     86                break;
     87            }
     88        }
     89
     90        if (hasBadI64Use) {
     91            jit.copyCalleeSavesToVMEntryFrameCalleeSavesBuffer();
     92            jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
     93            auto call = jit.call();
     94            jit.jumpToExceptionHandler();
     95
     96            void (*throwBadI64)(ExecState*) = [] (ExecState* exec) -> void {
     97                VM* vm = &exec->vm();
     98                NativeCallFrameTracer tracer(vm, exec);
     99
     100                {
     101                    auto throwScope = DECLARE_THROW_SCOPE(*vm);
     102                    JSGlobalObject* globalObject = vm->topJSWebAssemblyInstance->globalObject();
     103                    auto* error = ErrorInstance::create(exec, *vm, globalObject->typeErrorConstructor()->errorStructure(), ASCIILiteral("i64 not allowed as return type or argument to an imported function"));
     104                    throwException(exec, throwScope, error);
     105                }
     106
     107                genericUnwind(vm, exec);
     108                ASSERT(!!vm->callFrameForCatch);
     109            };
     110
     111            LinkBuffer linkBuffer(*vm, jit, GLOBAL_THUNK_ID);
     112            linkBuffer.link(call, throwBadI64);
     113            return FINALIZE_CODE(linkBuffer, ("WebAssembly->JavaScript invalid i64 use in import[%i]", importIndex));
     114        }
     115    }
     116
    64117    // Here we assume that the JS calling convention saves at least all the wasm callee saved. We therefore don't need to save and restore more registers since the wasm callee already took care of this.
    65118    RegisterSet missingCalleeSaves = wasmCC.m_calleeSaveRegisters;
    … …  
    75128    jit.subPtr(MacroAssembler::TrustedImm32(stackOffset), MacroAssembler::stackPointerRegister);
    76129    JIT::Address calleeFrame = CCallHelpers::Address(MacroAssembler::stackPointerRegister, -static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC)));
    77    
    78     for (unsigned argNum = 0; argNum < argCount; ++argNum) {
    79         Type argType = signature->argument(argNum);
    80         switch (argType) {
    81         case Void:
    82         case Func:
    83         case Anyfunc:
    84         case I64: {
    85             // FIXME: Figure out the correct behavior here. I suspect we want such a stub to throw an exception immediately.
    86             // if called. https://bugs.webkit.org/show_bug.cgi?id=165991
    87             jit.breakpoint();
    88             LinkBuffer patchBuffer(*vm, jit, GLOBAL_THUNK_ID);
    89             return FINALIZE_CODE(patchBuffer, ("WebAssembly import[%i] stub for signature %i", importIndex, signatureIndex));
    90         }
    91         case I32:
    92         case F32:
    93         case F64:
    94             continue;
    95         }
    96     }
    97130
    98131    // 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
    … …  
    221254    materializeImportJSCell(vm, jit, importIndex, importJSCellGPRReg);
    222255
    223     uint64_t thisArgument = ValueUndefined; // FIXME what does the WebAssembly spec say this should be? https://bugs.webkit.org/show_bug.cgi?id=165471
    224256    jit.store64(importJSCellGPRReg, calleeFrame.withOffset(CallFrameSlot::callee * static_cast<int>(sizeof(Register))));
    225257    jit.store32(JIT::TrustedImm32(numberOfParameters), calleeFrame.withOffset(CallFrameSlot::argumentCount * static_cast<int>(sizeof(Register)) + PayloadOffset));
    226     jit.store64(JIT::TrustedImm64(thisArgument), calleeFrame.withOffset(CallFrameSlot::thisArgument * static_cast<int>(sizeof(Register))));
     258    jit.store64(JIT::TrustedImm64(ValueUndefined), calleeFrame.withOffset(CallFrameSlot::thisArgument * static_cast<int>(sizeof(Register))));
    227259
    228260    // FIXME Tail call if the wasm return type is void and no registers were spilled. https://bugs.webkit.org/show_bug.cgi?id=165488
    … …  
    241273    done.link(&jit);
    242274
     275    CCallHelpers::JumpList exceptionChecks;
     276
    243277    switch (signature->returnType()) {
    244278    case Void:
    … …  
    250284        RELEASE_ASSERT_NOT_REACHED();
    251285        break;
     286    case I64: {
     287        RELEASE_ASSERT_NOT_REACHED(); // Handled above.
     288    }
    252289    case I32: {
     290        CCallHelpers::JumpList done;
     291        CCallHelpers::JumpList slowPath;
     292
     293        slowPath.append(jit.branchIfNotNumber(GPRInfo::returnValueGPR, DoNotHaveTagRegisters));
     294        slowPath.append(jit.branchIfNotInt32(JSValueRegs(GPRInfo::returnValueGPR), DoNotHaveTagRegisters));
     295        jit.zeroExtend32ToPtr(GPRInfo::returnValueGPR, GPRInfo::returnValueGPR);
     296        done.append(jit.jump());
     297
     298        slowPath.link(&jit);
     299        jit.setupArgumentsWithExecState(GPRInfo::returnValueGPR);
     300        auto call = jit.call();
     301        exceptionChecks.append(jit.emitJumpIfException());
     302
     303        int32_t (*convertToI32)(ExecState*, JSValue) = [] (ExecState* exec, JSValue v) -> int32_t {
     304            VM* vm = &exec->vm();
     305            NativeCallFrameTracer tracer(vm, exec);
     306            return v.toInt32(exec);
     307        };
     308        jit.addLinkTask([=] (LinkBuffer& linkBuffer) {
     309            linkBuffer.link(call, convertToI32);
     310        });
     311
     312        done.link(&jit);
     313        break;
     314    }
     315    case F32: {
     316        CCallHelpers::JumpList done;
     317        auto notANumber = jit.branchIfNotNumber(GPRInfo::returnValueGPR, DoNotHaveTagRegisters);
     318        auto isDouble = jit.branchIfNotInt32(JSValueRegs(GPRInfo::returnValueGPR), DoNotHaveTagRegisters);
     319        // We're an int32
     320        jit.signExtend32ToPtr(GPRInfo::returnValueGPR, GPRInfo::returnValueGPR);
     321        jit.convertInt64ToFloat(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
     322        done.append(jit.jump());
     323
     324        isDouble.link(&jit);
    253325        jit.move(JIT::TrustedImm64(TagTypeNumber), GPRInfo::returnValueGPR2);
    254         JIT::Jump checkJSInt32 = jit.branch64(JIT::AboveOrEqual, GPRInfo::returnValueGPR, GPRInfo::returnValueGPR2);
    255         jit.move64ToDouble(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
    256         jit.truncateDoubleToInt32(FPRInfo::returnValueFPR, GPRInfo::returnValueGPR);
    257         JIT::Jump checkJSNumber = jit.branchTest64(JIT::NonZero, GPRInfo::returnValueGPR, GPRInfo::returnValueGPR2);
    258         jit.abortWithReason(AHIsNotJSNumber); // FIXME Coerce when the values aren't what we expect, instead of aborting. https://bugs.webkit.org/show_bug.cgi?id=165480
    259         checkJSInt32.link(&jit);
    260         jit.zeroExtend32ToPtr(GPRInfo::returnValueGPR, GPRInfo::returnValueGPR);
    261         checkJSNumber.link(&jit);
    262         break;
    263     }
    264     case I64: {
    265         jit.move(JIT::TrustedImm64(TagTypeNumber), GPRInfo::returnValueGPR2);
    266         JIT::Jump checkJSInt32 = jit.branch64(JIT::AboveOrEqual, GPRInfo::returnValueGPR, GPRInfo::returnValueGPR2);
    267         jit.move64ToDouble(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
    268         jit.truncateDoubleToInt64(FPRInfo::returnValueFPR, GPRInfo::returnValueGPR);
    269         JIT::Jump checkJSNumber = jit.branchTest64(JIT::NonZero, GPRInfo::returnValueGPR, GPRInfo::returnValueGPR2);
    270         jit.abortWithReason(AHIsNotJSNumber); // FIXME Coerce when the values aren't what we expect, instead of aborting. https://bugs.webkit.org/show_bug.cgi?id=165480
    271         checkJSInt32.link(&jit);
    272         jit.zeroExtend32ToPtr(GPRInfo::returnValueGPR, GPRInfo::returnValueGPR);
    273         checkJSNumber.link(&jit);
    274         break;
    275     }
    276     case F32: {
    277         jit.move(JIT::TrustedImm64(TagTypeNumber), GPRInfo::returnValueGPR2);
     326        jit.add64(GPRInfo::returnValueGPR2, GPRInfo::returnValueGPR);
    278327        jit.move64ToDouble(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
    279328        jit.convertDoubleToFloat(FPRInfo::returnValueFPR, FPRInfo::returnValueFPR);
    280         JIT::Jump checkJSInt32 = jit.branch64(JIT::AboveOrEqual, GPRInfo::returnValueGPR, GPRInfo::returnValueGPR2);
    281         JIT::Jump checkJSNumber = jit.branchTest64(JIT::NonZero, GPRInfo::returnValueGPR, GPRInfo::returnValueGPR2);
    282         jit.abortWithReason(AHIsNotJSNumber); // FIXME Coerce when the values aren't what we expect, instead of aborting. https://bugs.webkit.org/show_bug.cgi?id=165480
    283         checkJSInt32.link(&jit);
    284         jit.zeroExtend32ToPtr(GPRInfo::returnValueGPR, GPRInfo::returnValueGPR);
    285         jit.convertInt64ToFloat(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
    286         checkJSNumber.link(&jit);
     329        done.append(jit.jump());
     330
     331        notANumber.link(&jit);
     332        jit.setupArgumentsWithExecState(GPRInfo::returnValueGPR);
     333        auto call = jit.call();
     334        exceptionChecks.append(jit.emitJumpIfException());
     335
     336        float (*convertToF32)(ExecState*, JSValue) = [] (ExecState* exec, JSValue v) -> float {
     337            VM* vm = &exec->vm();
     338            NativeCallFrameTracer tracer(vm, exec);
     339            return static_cast<float>(v.toNumber(exec));
     340        };
     341        jit.addLinkTask([=] (LinkBuffer& linkBuffer) {
     342            linkBuffer.link(call, convertToF32);
     343        });
     344
     345        done.link(&jit);
    287346        break;
    288347    }
    289348    case F64: {
     349        CCallHelpers::JumpList done;
     350        auto notANumber = jit.branchIfNotNumber(GPRInfo::returnValueGPR, DoNotHaveTagRegisters);
     351        auto isDouble = jit.branchIfNotInt32(JSValueRegs(GPRInfo::returnValueGPR), DoNotHaveTagRegisters);
     352        // We're an int32
     353        jit.signExtend32ToPtr(GPRInfo::returnValueGPR, GPRInfo::returnValueGPR);
     354        jit.convertInt64ToDouble(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
     355        done.append(jit.jump());
     356
     357        isDouble.link(&jit);
    290358        jit.move(JIT::TrustedImm64(TagTypeNumber), GPRInfo::returnValueGPR2);
     359        jit.add64(GPRInfo::returnValueGPR2, GPRInfo::returnValueGPR);
    291360        jit.move64ToDouble(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
    292         JIT::Jump checkJSInt32 = jit.branch64(JIT::AboveOrEqual, GPRInfo::returnValueGPR, GPRInfo::returnValueGPR2);
    293         JIT::Jump checkJSNumber = jit.branchTest64(JIT::NonZero, GPRInfo::returnValueGPR, GPRInfo::returnValueGPR2);
    294         jit.abortWithReason(AHIsNotJSNumber); // FIXME Coerce when the values aren't what we expect, instead of aborting. https://bugs.webkit.org/show_bug.cgi?id=165480
    295         checkJSInt32.link(&jit);
    296         jit.zeroExtend32ToPtr(GPRInfo::returnValueGPR, GPRInfo::returnValueGPR);
    297         jit.convertInt64ToDouble(GPRInfo::returnValueGPR, FPRInfo::returnValueFPR);
    298         checkJSNumber.link(&jit);
     361        done.append(jit.jump());
     362
     363        notANumber.link(&jit);
     364        jit.setupArgumentsWithExecState(GPRInfo::returnValueGPR);
     365        auto call = jit.call();
     366        exceptionChecks.append(jit.emitJumpIfException());
     367
     368        double (*convertToF64)(ExecState*, JSValue) = [] (ExecState* exec, JSValue v) -> double {
     369            VM* vm = &exec->vm();
     370            NativeCallFrameTracer tracer(vm, exec);
     371            return v.toNumber(exec);
     372        };
     373        jit.addLinkTask([=] (LinkBuffer& linkBuffer) {
     374            linkBuffer.link(call, convertToF64);
     375        });
     376
     377        done.link(&jit);
    299378        break;
    300379    }
    … …  
    303382    jit.emitFunctionEpilogue();
    304383    jit.ret();
     384
     385    if (!exceptionChecks.empty()) {
     386        exceptionChecks.link(&jit);
     387        jit.copyCalleeSavesToVMEntryFrameCalleeSavesBuffer();
     388        jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
     389        auto call = jit.call();
     390        jit.jumpToExceptionHandler();
     391
     392        void (*doUnwinding)(ExecState*) = [] (ExecState* exec) -> void {
     393            VM* vm = &exec->vm();
     394            NativeCallFrameTracer tracer(vm, exec);
     395            genericUnwind(vm, exec);
     396            ASSERT(!!vm->callFrameForCatch);
     397        };
     398
     399        jit.addLinkTask([=] (LinkBuffer& linkBuffer) {
     400            linkBuffer.link(call, doUnwinding);
     401        });
     402    }
    305403
    306404    LinkBuffer patchBuffer(*vm, jit, GLOBAL_THUNK_ID);
    … …  
    310408    CodeLocationNearCall hotPathOther = patchBuffer.locationOfNearCall(fastCall);
    311409    callLinkInfo->setCallLocations(callReturnLocation, hotPathBegin, hotPathOther);
     410#if !defined(NDEBUG)
    312411    String signatureDescription = SignatureInformation::get(vm, signatureIndex)->toString();
     412#else
     413    String signatureDescription;
     414#endif
    313415    return FINALIZE_CODE(patchBuffer, ("WebAssembly->JavaScript import[%i] %s", importIndex, signatureDescription.ascii().data()));
    314416}
Note: See TracChangeset for help on using the changeset viewer.