Title: [210102] trunk
Revision
210102
Author
[email protected]
Date
2016-12-22 10:31:04 -0800 (Thu, 22 Dec 2016)

Log Message

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.

JSTests:

* wasm.yaml:

Source/_javascript_Core:

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:

Modified Paths

Diff

Modified: trunk/JSTests/ChangeLog (210101 => 210102)


--- trunk/JSTests/ChangeLog	2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/JSTests/ChangeLog	2016-12-22 18:31:04 UTC (rev 210102)
@@ -1,3 +1,13 @@
+2016-12-22  Saam Barati  <[email protected]>
+
+        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  <[email protected]>
 
         WebAssembly: Fix decode floating point constants in unreachable code

Modified: trunk/JSTests/wasm.yaml (210101 => 210102)


--- trunk/JSTests/wasm.yaml	2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/JSTests/wasm.yaml	2016-12-22 18:31:04 UTC (rev 210102)
@@ -170,7 +170,7 @@
   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

Modified: trunk/Source/_javascript_Core/ChangeLog (210101 => 210102)


--- trunk/Source/_javascript_Core/ChangeLog	2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-12-22 18:31:04 UTC (rev 210102)
@@ -1,3 +1,32 @@
+2016-12-22  Saam Barati  <[email protected]>
+
+        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  <[email protected]>
 
         WebAssembly: Fix decode floating point constants in unreachable code

Modified: trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp (210101 => 210102)


--- trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp	2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp	2016-12-22 18:31:04 UTC (rev 210102)
@@ -325,8 +325,8 @@
 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 { };

Modified: trunk/Source/_javascript_Core/wasm/WasmExceptionType.h (210101 => 210102)


--- trunk/Source/_javascript_Core/wasm/WasmExceptionType.h	2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/_javascript_Core/wasm/WasmExceptionType.h	2016-12-22 18:31:04 UTC (rev 210102)
@@ -37,6 +37,7 @@
     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,

Modified: trunk/Source/_javascript_Core/wasm/js/WebAssemblyModuleRecord.cpp (210101 => 210102)


--- trunk/Source/_javascript_Core/wasm/js/WebAssemblyModuleRecord.cpp	2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/_javascript_Core/wasm/js/WebAssemblyModuleRecord.cpp	2016-12-22 18:31:04 UTC (rev 210102)
@@ -94,9 +94,6 @@
     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 @@
             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 @@
         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 @@
         }
     }
 
-    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, { });
     }
 

Modified: trunk/Source/_javascript_Core/wasm/js/WebAssemblyModuleRecord.h (210101 => 210102)


--- trunk/Source/_javascript_Core/wasm/js/WebAssemblyModuleRecord.h	2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/_javascript_Core/wasm/js/WebAssemblyModuleRecord.h	2016-12-22 18:31:04 UTC (rev 210102)
@@ -59,7 +59,7 @@
     static void visitChildren(JSCell*, SlotVisitor&);
 
     WriteBarrier<JSWebAssemblyInstance> m_instance;
-    WriteBarrier<WebAssemblyFunction> m_startFunction;
+    WriteBarrier<JSCell> m_startFunction;
 };
 
 } // namespace JSC
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to