WebAssembly: Make the spec-tests/start.wast.js test pass
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Dec 2016 18:31:04 +0000 (18:31 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Dec 2016 18:31:04 +0000 (18:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=166416
<rdar://problem/29784532>

Reviewed by Yusuke Suzuki.

JSTests:

* wasm.yaml:

Source/JavaScriptCore:

To make the test run, I had to fix two bugs:

1. We weren't properly finding the start function. There was code
that would try to find the start function from the list of *exported*
functions. This is wrong; the start function is an index into the
function index space, which is the space for *imports* and *local*
functions. So the code was just wrong in this respect, and I've
fixed it do the right thing. We weren't sure if this was originally
allowed or not in the spec, but it has been decided that it is allowed
and the spec-tests test for it: https://github.com/WebAssembly/design/issues/896

2. We were emitting a breakpoint for Unreachable. Instead of crashing,
this opcode needs to throw an exception when executing.

* wasm/WasmB3IRGenerator.cpp:
* wasm/WasmExceptionType.h:
* wasm/js/WebAssemblyModuleRecord.cpp:
(JSC::WebAssemblyModuleRecord::link):
(JSC::WebAssemblyModuleRecord::evaluate):
* wasm/js/WebAssemblyModuleRecord.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@210102 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/wasm.yaml
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
Source/JavaScriptCore/wasm/WasmExceptionType.h
Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp
Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.h

index 3a180f2..b5ed6d2 100644 (file)
@@ -1,3 +1,13 @@
+2016-12-22  Saam Barati  <sbarati@apple.com>
+
+        WebAssembly: Make the spec-tests/start.wast.js test pass
+        https://bugs.webkit.org/show_bug.cgi?id=166416
+        <rdar://problem/29784532>
+
+        Reviewed by Yusuke Suzuki.
+
+        * wasm.yaml:
+
 2016-12-21  Keith Miller  <keith_miller@apple.com>
 
         WebAssembly: Fix decode floating point constants in unreachable code
index 2b80796..1fc1b0b 100644 (file)
   cmd: runWebAssemblySpecTest :normal
 
 - path: wasm/spec-tests/start.wast.js
-  cmd: runWebAssemblySpecTest :skip
+  cmd: runWebAssemblySpecTest :normal
 
 - path: wasm/spec-tests/store_retval.wast.js
   cmd: runWebAssemblySpecTest :normal
index b79f064..a5f5725 100644 (file)
@@ -1,3 +1,32 @@
+2016-12-22  Saam Barati  <sbarati@apple.com>
+
+        WebAssembly: Make the spec-tests/start.wast.js test pass
+        https://bugs.webkit.org/show_bug.cgi?id=166416
+        <rdar://problem/29784532>
+
+        Reviewed by Yusuke Suzuki.
+
+        To make the test run, I had to fix two bugs:
+        
+        1. We weren't properly finding the start function. There was code
+        that would try to find the start function from the list of *exported*
+        functions. This is wrong; the start function is an index into the
+        function index space, which is the space for *imports* and *local*
+        functions. So the code was just wrong in this respect, and I've
+        fixed it do the right thing. We weren't sure if this was originally
+        allowed or not in the spec, but it has been decided that it is allowed
+        and the spec-tests test for it: https://github.com/WebAssembly/design/issues/896
+        
+        2. We were emitting a breakpoint for Unreachable. Instead of crashing,
+        this opcode needs to throw an exception when executing.
+
+        * wasm/WasmB3IRGenerator.cpp:
+        * wasm/WasmExceptionType.h:
+        * wasm/js/WebAssemblyModuleRecord.cpp:
+        (JSC::WebAssemblyModuleRecord::link):
+        (JSC::WebAssemblyModuleRecord::evaluate):
+        * wasm/js/WebAssemblyModuleRecord.h:
+
 2016-12-21  Keith Miller  <keith_miller@apple.com>
 
         WebAssembly: Fix decode floating point constants in unreachable code
index 73ca024..22c6e85 100644 (file)
@@ -325,8 +325,8 @@ auto B3IRGenerator::getLocal(uint32_t index, ExpressionType& result) -> PartialR
 auto B3IRGenerator::addUnreachable() -> PartialResult
 {
     B3::PatchpointValue* unreachable = m_currentBlock->appendNew<B3::PatchpointValue>(m_proc, B3::Void, Origin());
-    unreachable->setGenerator([=] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
-        jit.breakpoint();
+    unreachable->setGenerator([this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+        this->emitExceptionCheck(jit, ExceptionType::Unreachable);
     });
     unreachable->effects.terminal = true;
     return { };
index c5979c0..1a67eaf 100644 (file)
@@ -37,6 +37,7 @@ namespace Wasm {
     macro(NullTableEntry,  "call_indirect to a null table entry") \
     macro(BadSignature, "call_indirect to a signature that does not match") \
     macro(OutOfBoundsTrunc, "Out of bounds Trunc operation") \
+    macro(Unreachable, "Unreachable code should not be executed") \
 
 enum class ExceptionType : uint32_t {
 #define MAKE_ENUM(enumName, error) enumName,
index ead6716..b4cb055 100644 (file)
@@ -94,9 +94,6 @@ void WebAssemblyModuleRecord::link(ExecState* state, JSWebAssemblyInstance* inst
     JSWebAssemblyModule* module = instance->module();
     const Wasm::ModuleInformation& moduleInformation = module->moduleInformation();
 
-    bool hasStart = !!moduleInformation.startFunctionIndexSpace;
-    auto startFunctionIndexSpace = moduleInformation.startFunctionIndexSpace.value_or(0);
-
     SymbolTable* exportSymbolTable = module->exportSymbolTable();
     unsigned importCount = module->importCount();
 
@@ -125,8 +122,6 @@ void WebAssemblyModuleRecord::link(ExecState* state, JSWebAssemblyInstance* inst
             const Wasm::Signature* signature = Wasm::SignatureInformation::get(&vm, signatureIndex);
             WebAssemblyFunction* function = WebAssemblyFunction::create(vm, globalObject, signature->argumentCount(), exp.field.string(), instance, jsEntrypointCallee, wasmEntrypointCallee, signatureIndex);
             exportedValue = function;
-            if (hasStart && startFunctionIndexSpace == exp.kindIndex)
-                m_startFunction.set(vm, this, function);
             break;
         }
         case Wasm::ExternalKind::Table: {
@@ -177,15 +172,18 @@ void WebAssemblyModuleRecord::link(ExecState* state, JSWebAssemblyInstance* inst
         RELEASE_ASSERT(putResult);
     }
 
+    bool hasStart = !!moduleInformation.startFunctionIndexSpace;
     if (hasStart) {
+        auto startFunctionIndexSpace = moduleInformation.startFunctionIndexSpace.value_or(0);
         Wasm::SignatureIndex signatureIndex = module->signatureForFunctionIndexSpace(startFunctionIndexSpace);
         const Wasm::Signature* signature = Wasm::SignatureInformation::get(&vm, signatureIndex);
         // The start function must not take any arguments or return anything. This is enforced by the parser.
         ASSERT(!signature->argumentCount());
         ASSERT(signature->returnType() == Wasm::Void);
-        // FIXME can start call imports / tables? This assumes not. https://github.com/WebAssembly/design/issues/896
-        if (!m_startFunction.get()) {
-            // The start function wasn't added above. It must be a purely internal function.
+        if (startFunctionIndexSpace < module->importCount()) {
+            JSCell* startFunction = instance->importFunction(startFunctionIndexSpace)->get();
+            m_startFunction.set(vm, this, startFunction);
+        } else {
             JSWebAssemblyCallee* jsEntrypointCallee = module->jsEntrypointCalleeFromFunctionIndexSpace(startFunctionIndexSpace);
             JSWebAssemblyCallee* wasmEntrypointCallee = module->wasmEntrypointCalleeFromFunctionIndexSpace(startFunctionIndexSpace);
             WebAssemblyFunction* function = WebAssemblyFunction::create(vm, globalObject, signature->argumentCount(), "start", instance, jsEntrypointCallee, wasmEntrypointCallee, signatureIndex);
@@ -275,10 +273,10 @@ JSValue WebAssemblyModuleRecord::evaluate(ExecState* state)
         }
     }
 
-    if (WebAssemblyFunction* startFunction = m_startFunction.get()) {
-        ProtoCallFrame protoCallFrame;
-        protoCallFrame.init(nullptr, startFunction, JSValue(), 1, nullptr);
-        startFunction->call(vm, &protoCallFrame);
+    if (JSCell* startFunction = m_startFunction.get()) {
+        CallData callData;
+        CallType callType = JSC::getCallData(startFunction, callData);
+        call(state, startFunction, callType, callData, jsUndefined(), state->emptyList());
         RETURN_IF_EXCEPTION(scope, { });
     }
 
index 4e70e79..e126670 100644 (file)
@@ -59,7 +59,7 @@ private:
     static void visitChildren(JSCell*, SlotVisitor&);
 
     WriteBarrier<JSWebAssemblyInstance> m_instance;
-    WriteBarrier<WebAssemblyFunction> m_startFunction;
+    WriteBarrier<JSCell> m_startFunction;
 };
 
 } // namespace JSC