<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[210102] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/210102">210102</a></dd>
<dt>Author</dt> <dd>sbarati@apple.com</dd>
<dt>Date</dt> <dd>2016-12-22 10:31:04 -0800 (Thu, 22 Dec 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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/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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJSTestsChangeLog">trunk/JSTests/ChangeLog</a></li>
<li><a href="#trunkJSTestswasmyaml">trunk/JSTests/wasm.yaml</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmWasmB3IRGeneratorcpp">trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmWasmExceptionTypeh">trunk/Source/JavaScriptCore/wasm/WasmExceptionType.h</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmjsWebAssemblyModuleRecordcpp">trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmjsWebAssemblyModuleRecordh">trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJSTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/ChangeLog (210101 => 210102)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/ChangeLog 2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/JSTests/ChangeLog 2016-12-22 18:31:04 UTC (rev 210102)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+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:
+
</ins><span class="cx"> 2016-12-21 Keith Miller <keith_miller@apple.com>
</span><span class="cx">
</span><span class="cx"> WebAssembly: Fix decode floating point constants in unreachable code
</span></span></pre></div>
<a id="trunkJSTestswasmyaml"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/wasm.yaml (210101 => 210102)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -170,7 +170,7 @@
</span><span class="cx"> cmd: runWebAssemblySpecTest :normal
</span><span class="cx">
</span><span class="cx"> - path: wasm/spec-tests/start.wast.js
</span><del>- cmd: runWebAssemblySpecTest :skip
</del><ins>+ cmd: runWebAssemblySpecTest :normal
</ins><span class="cx">
</span><span class="cx"> - path: wasm/spec-tests/store_retval.wast.js
</span><span class="cx"> cmd: runWebAssemblySpecTest :normal
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (210101 => 210102)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-12-22 18:31:04 UTC (rev 210102)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+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:
+
</ins><span class="cx"> 2016-12-21 Keith Miller <keith_miller@apple.com>
</span><span class="cx">
</span><span class="cx"> WebAssembly: Fix decode floating point constants in unreachable code
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmWasmB3IRGeneratorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp (210101 => 210102)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp 2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp 2016-12-22 18:31:04 UTC (rev 210102)
</span><span class="lines">@@ -325,8 +325,8 @@
</span><span class="cx"> auto B3IRGenerator::addUnreachable() -> PartialResult
</span><span class="cx"> {
</span><span class="cx"> B3::PatchpointValue* unreachable = m_currentBlock->appendNew<B3::PatchpointValue>(m_proc, B3::Void, Origin());
</span><del>- unreachable->setGenerator([=] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
- jit.breakpoint();
</del><ins>+ unreachable->setGenerator([this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+ this->emitExceptionCheck(jit, ExceptionType::Unreachable);
</ins><span class="cx"> });
</span><span class="cx"> unreachable->effects.terminal = true;
</span><span class="cx"> return { };
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmWasmExceptionTypeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/WasmExceptionType.h (210101 => 210102)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/WasmExceptionType.h 2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/JavaScriptCore/wasm/WasmExceptionType.h 2016-12-22 18:31:04 UTC (rev 210102)
</span><span class="lines">@@ -37,6 +37,7 @@
</span><span class="cx"> macro(NullTableEntry, "call_indirect to a null table entry") \
</span><span class="cx"> macro(BadSignature, "call_indirect to a signature that does not match") \
</span><span class="cx"> macro(OutOfBoundsTrunc, "Out of bounds Trunc operation") \
</span><ins>+ macro(Unreachable, "Unreachable code should not be executed") \
</ins><span class="cx">
</span><span class="cx"> enum class ExceptionType : uint32_t {
</span><span class="cx"> #define MAKE_ENUM(enumName, error) enumName,
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmjsWebAssemblyModuleRecordcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp (210101 => 210102)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp 2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp 2016-12-22 18:31:04 UTC (rev 210102)
</span><span class="lines">@@ -94,9 +94,6 @@
</span><span class="cx"> JSWebAssemblyModule* module = instance->module();
</span><span class="cx"> const Wasm::ModuleInformation& moduleInformation = module->moduleInformation();
</span><span class="cx">
</span><del>- bool hasStart = !!moduleInformation.startFunctionIndexSpace;
- auto startFunctionIndexSpace = moduleInformation.startFunctionIndexSpace.value_or(0);
-
</del><span class="cx"> SymbolTable* exportSymbolTable = module->exportSymbolTable();
</span><span class="cx"> unsigned importCount = module->importCount();
</span><span class="cx">
</span><span class="lines">@@ -125,8 +122,6 @@
</span><span class="cx"> const Wasm::Signature* signature = Wasm::SignatureInformation::get(&vm, signatureIndex);
</span><span class="cx"> WebAssemblyFunction* function = WebAssemblyFunction::create(vm, globalObject, signature->argumentCount(), exp.field.string(), instance, jsEntrypointCallee, wasmEntrypointCallee, signatureIndex);
</span><span class="cx"> exportedValue = function;
</span><del>- if (hasStart && startFunctionIndexSpace == exp.kindIndex)
- m_startFunction.set(vm, this, function);
</del><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx"> case Wasm::ExternalKind::Table: {
</span><span class="lines">@@ -177,15 +172,18 @@
</span><span class="cx"> RELEASE_ASSERT(putResult);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ bool hasStart = !!moduleInformation.startFunctionIndexSpace;
</ins><span class="cx"> if (hasStart) {
</span><ins>+ auto startFunctionIndexSpace = moduleInformation.startFunctionIndexSpace.value_or(0);
</ins><span class="cx"> Wasm::SignatureIndex signatureIndex = module->signatureForFunctionIndexSpace(startFunctionIndexSpace);
</span><span class="cx"> const Wasm::Signature* signature = Wasm::SignatureInformation::get(&vm, signatureIndex);
</span><span class="cx"> // The start function must not take any arguments or return anything. This is enforced by the parser.
</span><span class="cx"> ASSERT(!signature->argumentCount());
</span><span class="cx"> ASSERT(signature->returnType() == Wasm::Void);
</span><del>- // 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.
</del><ins>+ if (startFunctionIndexSpace < module->importCount()) {
+ JSCell* startFunction = instance->importFunction(startFunctionIndexSpace)->get();
+ m_startFunction.set(vm, this, startFunction);
+ } else {
</ins><span class="cx"> JSWebAssemblyCallee* jsEntrypointCallee = module->jsEntrypointCalleeFromFunctionIndexSpace(startFunctionIndexSpace);
</span><span class="cx"> JSWebAssemblyCallee* wasmEntrypointCallee = module->wasmEntrypointCalleeFromFunctionIndexSpace(startFunctionIndexSpace);
</span><span class="cx"> WebAssemblyFunction* function = WebAssemblyFunction::create(vm, globalObject, signature->argumentCount(), "start", instance, jsEntrypointCallee, wasmEntrypointCallee, signatureIndex);
</span><span class="lines">@@ -275,10 +273,10 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (WebAssemblyFunction* startFunction = m_startFunction.get()) {
- ProtoCallFrame protoCallFrame;
- protoCallFrame.init(nullptr, startFunction, JSValue(), 1, nullptr);
- startFunction->call(vm, &protoCallFrame);
</del><ins>+ if (JSCell* startFunction = m_startFunction.get()) {
+ CallData callData;
+ CallType callType = JSC::getCallData(startFunction, callData);
+ call(state, startFunction, callType, callData, jsUndefined(), state->emptyList());
</ins><span class="cx"> RETURN_IF_EXCEPTION(scope, { });
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmjsWebAssemblyModuleRecordh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.h (210101 => 210102)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.h 2016-12-22 17:37:50 UTC (rev 210101)
+++ trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.h 2016-12-22 18:31:04 UTC (rev 210102)
</span><span class="lines">@@ -59,7 +59,7 @@
</span><span class="cx"> static void visitChildren(JSCell*, SlotVisitor&);
</span><span class="cx">
</span><span class="cx"> WriteBarrier<JSWebAssemblyInstance> m_instance;
</span><del>- WriteBarrier<WebAssemblyFunction> m_startFunction;
</del><ins>+ WriteBarrier<JSCell> m_startFunction;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre>
</div>
</div>
</body>
</html>