wasi: update start() behavior to match spec

_start() and _initialize() shouldn't be called from the same function, as they have different behavior. Furthermore, Node should throw if both are provided. This commit updates the implementation, docs, and tests accordingly. PR-URL: #33073 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
nodejs · May 7, 2020 · 49df7fa · 49df7fa
1 parent dd80022
commit 49df7fa
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 77 deletions.
diff --git a/doc/api/wasi.md b/doc/api/wasi.md
@@ -70,14 +70,15 @@ added: v12.16.0
 
 * `instance` {WebAssembly.Instance}
 
-Attempt to begin execution of `instance` by invoking its `_start()` export.
-If `instance` does not contain a `_start()` export, then `start()` attempts to
-invoke the `_initialize()` export. If neither of those exports is present on
-`instance`, then `start()` does nothing.
+Attempt to begin execution of `instance` as a WASI command by invoking its
+`_start()` export. If `instance` does not contain a `_start()` export, or if
+`instance` contains an `_initialize()` export, then an exception is thrown.
 
 `start()` requires that `instance` exports a [`WebAssembly.Memory`][] named
 `memory`. If `instance` does not have a `memory` export an exception is thrown.
 
+If `start()` is called more than once, an exception is thrown.
+
 ### `wasi.wasiImport`
 <!-- YAML
 added: v12.16.0

diff --git a/lib/wasi.js b/lib/wasi.js
@@ -79,7 +79,17 @@ class WASI {
 
     validateObject(exports, 'instance.exports');
 
-    const { memory } = exports;
+    const { _initialize, _start, memory } = exports;
+
+    if (typeof _start !== 'function') {
+      throw new ERR_INVALID_ARG_TYPE(
+        'instance.exports._start', 'function', _start);
+    }
+
+    if (_initialize !== undefined) {
+      throw new ERR_INVALID_ARG_TYPE(
+        'instance.exports._initialize', 'undefined', _initialize);
+    }
 
     if (!(memory instanceof WebAssembly.Memory)) {
       throw new ERR_INVALID_ARG_TYPE(
@@ -94,10 +104,7 @@ class WASI {
     this[kSetMemory](memory);
 
     try {
-      if (exports._start)
-        exports._start();
-      else if (exports._initialize)
-        exports._initialize();
+      exports._start();
     } catch (err) {
       if (err !== kExitCode) {
         throw err;

diff --git a/test/wasi/test-wasi-start-validation.js b/test/wasi/test-wasi-start-validation.js
@@ -8,85 +8,111 @@ const { WASI } = require('wasi');
 const fixtures = require('../common/fixtures');
 const bufferSource = fixtures.readSync('simple.wasm');
 
-{
-  const wasi = new WASI();
-  assert.throws(
-    () => {
-      wasi.start();
-    },
-    { code: 'ERR_INVALID_ARG_TYPE', message: /\bWebAssembly\.Instance\b/ }
-  );
-}
-
 (async () => {
-  const wasi = new WASI({});
-  const wasm = await WebAssembly.compile(bufferSource);
-  const instance = await WebAssembly.instantiate(wasm);
+  {
+    // Verify that a WebAssembly.Instance is passed in.
+    const wasi = new WASI();
 
-  assert.throws(
-    () => { wasi.start(instance); },
-    { code: 'ERR_INVALID_ARG_TYPE', message: /\bWebAssembly\.Memory\b/ }
-  );
-})();
+    assert.throws(
+      () => { wasi.start(); },
+      { code: 'ERR_INVALID_ARG_TYPE', message: /\bWebAssembly\.Instance\b/ }
+    );
+  }
 
-(async () => {
-  const wasi = new WASI();
-  const wasm = await WebAssembly.compile(bufferSource);
-  const instance = await WebAssembly.instantiate(wasm);
-  const values = [undefined, null, 'foo', 42, true, false, () => {}];
-  let cnt = 0;
+  {
+    // Verify that the passed instance has an exports objects.
+    const wasi = new WASI({});
+    const wasm = await WebAssembly.compile(bufferSource);
+    const instance = await WebAssembly.instantiate(wasm);
+
+    Object.defineProperty(instance, 'exports', { get() { return null; } });
+    assert.throws(
+      () => { wasi.start(instance); },
+      {
+        code: 'ERR_INVALID_ARG_TYPE',
+        message: /"instance\.exports" property must be of type object/
+      }
+    );
+  }
 
-  // Mock instance.exports to trigger start() validation.
-  Object.defineProperty(instance, 'exports', {
-    get() { return values[cnt++]; }
-  });
+  {
+    // Verify that a _start() export was passed.
+    const wasi = new WASI({});
+    const wasm = await WebAssembly.compile(bufferSource);
+    const instance = await WebAssembly.instantiate(wasm);
 
-  values.forEach((val) => {
+    Object.defineProperty(instance, 'exports', { get() { return {}; } });
     assert.throws(
       () => { wasi.start(instance); },
-      { code: 'ERR_INVALID_ARG_TYPE', message: /\binstance\.exports\b/ }
+      {
+        code: 'ERR_INVALID_ARG_TYPE',
+        message: /"instance\.exports\._start" property must be of type function/
+      }
     );
-  });
-})();
+  }
 
-(async () => {
-  const wasi = new WASI();
-  const wasm = await WebAssembly.compile(bufferSource);
-  const instance = await WebAssembly.instantiate(wasm);
+  {
+    // Verify that an _initialize export was not passed.
+    const wasi = new WASI({});
+    const wasm = await WebAssembly.compile(bufferSource);
+    const instance = await WebAssembly.instantiate(wasm);
 
-  // Mock instance.exports.memory to bypass start() validation.
-  Object.defineProperty(instance, 'exports', {
-    get() {
-      return {
-        memory: new WebAssembly.Memory({ initial: 1 })
-      };
-    }
-  });
+    Object.defineProperty(instance, 'exports', {
+      get() {
+        return {
+          _start() {},
+          _initialize() {}
+        };
+      }
+    });
+    assert.throws(
+      () => { wasi.start(instance); },
+      {
+        code: 'ERR_INVALID_ARG_TYPE',
+        message: /"instance\.exports\._initialize" property must be undefined/
+      }
+    );
+  }
 
-  wasi.start(instance);
-  assert.throws(
-    () => { wasi.start(instance); },
-    {
-      code: 'ERR_WASI_ALREADY_STARTED',
-      message: /^WASI instance has already started$/
-    }
-  );
-})();
+  {
+    // Verify that a memory export was passed.
+    const wasi = new WASI({});
+    const wasm = await WebAssembly.compile(bufferSource);
+    const instance = await WebAssembly.instantiate(wasm);
 
-(async () => {
-  const wasi = new WASI();
-  const wasm = await WebAssembly.compile(bufferSource);
-  const instance = await WebAssembly.instantiate(wasm);
+    Object.defineProperty(instance, 'exports', {
+      get() { return { _start() {} }; }
+    });
+    assert.throws(
+      () => { wasi.start(instance); },
+      {
+        code: 'ERR_INVALID_ARG_TYPE',
+        message: /"instance\.exports\.memory" property .+ WebAssembly\.Memory/
+      }
+    );
+  }
 
-  // Mock instance.exports to bypass start() validation.
-  Object.defineProperty(instance, 'exports', {
-    get() {
-      return {
-        memory: new WebAssembly.Memory({ initial: 1 }),
-        _initialize: common.mustCall()
-      };
-    }
-  });
+  {
+    // Verify that start() can only be called once.
+    const wasi = new WASI({});
+    const wasm = await WebAssembly.compile(bufferSource);
+    const instance = await WebAssembly.instantiate(wasm);
 
-  wasi.start(instance);
-})();
+    Object.defineProperty(instance, 'exports', {
+      get() {
+        return {
+          _start() {},
+          memory: new WebAssembly.Memory({ initial: 1 })
+        };
+      }
+    });
+    wasi.start(instance);
+    assert.throws(
+      () => { wasi.start(instance); },
+      {
+        code: 'ERR_WASI_ALREADY_STARTED',
+        message: /^WASI instance has already started$/
+      }
+    );
+  }
+})().then(common.mustCall());