From fbd9d0d3a8ed947d49551e9cc2c3fc663420ad91 Mon Sep 17 00:00:00 2001 From: Sven SAULEAU Date: Wed, 30 May 2018 12:45:05 +0200 Subject: [PATCH 1/7] feat(wasm): add finalizer for checking exports --- declarations.d.ts | 22 ++++--- lib/WebpackOptionsApply.js | 6 ++ lib/wasm/WasmFinalizeExportsPlugin.js | 57 +++++++++++++++++++ lib/wasm/WebAssemblyGenerator.js | 10 ++-- lib/wasm/WebAssemblyParser.js | 36 +++++++++++- package.json | 1 + test/cases/wasm/js-incompatible-type/env.js | 1 + .../js-incompatible-type/export-i64-param.wat | 3 + .../export-i64-result.wat | 5 ++ .../wasm/js-incompatible-type/import-i64.wat | 3 + test/cases/wasm/js-incompatible-type/index.js | 11 ++++ .../wasm/js-incompatible-type/test.filter.js | 5 ++ yarn.lock | 2 +- 13 files changed, 145 insertions(+), 17 deletions(-) create mode 100644 lib/wasm/WasmFinalizeExportsPlugin.js create mode 100644 test/cases/wasm/js-incompatible-type/env.js create mode 100644 test/cases/wasm/js-incompatible-type/export-i64-param.wat create mode 100644 test/cases/wasm/js-incompatible-type/export-i64-result.wat create mode 100644 test/cases/wasm/js-incompatible-type/import-i64.wat create mode 100644 test/cases/wasm/js-incompatible-type/index.js create mode 100644 test/cases/wasm/js-incompatible-type/test.filter.js diff --git a/declarations.d.ts b/declarations.d.ts index bd64715943c..e48b9c4212c 100644 --- a/declarations.d.ts +++ b/declarations.d.ts @@ -60,10 +60,16 @@ declare module "@webassemblyjs/ast" { } export class ModuleExport extends Node { name: string; + descr: ModuleExportDescr; + } + type Index = Identifier | NumberLiteral; + export class ModuleExportDescr extends Node { + exportType: string; + id: Index; + } + export class NumberLiteral extends Node { + value: number; } - export class ModuleExportDescr extends Node {} - export class IndexLiteral extends Node {} - export class NumberLiteral extends Node {} export class FloatLiteral extends Node {} export class Global extends Node {} export class FuncParam extends Node { @@ -81,14 +87,14 @@ declare module "@webassemblyjs/ast" { } export class TypeInstruction extends Node {} export class IndexInFuncSection extends Node {} - export function indexLiteral(index: number): IndexLiteral; + export function indexLiteral(index: number): Index; export function numberLiteralFromRaw(num: number): NumberLiteral; export function floatLiteral(value: number, nan?: boolean, inf?: boolean, raw?: string): FloatLiteral; export function global(globalType: string, nodes: Node[]): Global; export function identifier(indentifier: string): Identifier; export function funcParam(valType: string, id: Identifier): FuncParam; export function instruction(inst: string, args: Node[]): Instruction; - export function callInstruction(funcIndex: IndexLiteral): CallInstruction; + export function callInstruction(funcIndex: Index): CallInstruction; export function objectInstruction( kind: string, type: string, @@ -97,15 +103,15 @@ declare module "@webassemblyjs/ast" { export function signature(params: FuncParam[], results: string[]): Signature; export function func(initFuncId, Signature, funcBody): Func; export function typeInstruction(id: Identifier, functype: Signature): TypeInstruction; - export function indexInFuncSection(index: IndexLiteral): IndexInFuncSection; + export function indexInFuncSection(index: Index): IndexInFuncSection; export function moduleExport( identifier: string, descr: ModuleExportDescr ): ModuleExport; export function moduleExportDescr( type: string, - index: ModuleExportDescr - ): ModuleExport; + index: Index + ): ModuleExportDescr; export function getSectionMetadata(ast: any, section: string); } diff --git a/lib/WebpackOptionsApply.js b/lib/WebpackOptionsApply.js index 0b9ba6c80ea..e3445add7d3 100644 --- a/lib/WebpackOptionsApply.js +++ b/lib/WebpackOptionsApply.js @@ -58,6 +58,7 @@ const NamedModulesPlugin = require("./NamedModulesPlugin"); const NamedChunksPlugin = require("./NamedChunksPlugin"); const DefinePlugin = require("./DefinePlugin"); const SizeLimitsPlugin = require("./performance/SizeLimitsPlugin"); +const WasmFinalizeExportsPlugin = require("./wasm/WasmFinalizeExportsPlugin"); class WebpackOptionsApply extends OptionsApply { constructor() { @@ -345,6 +346,11 @@ class WebpackOptionsApply extends OptionsApply { new SizeLimitsPlugin(options.performance).apply(compiler); } + // FIXME(sven): this should be conditional + // if (options.optimization.jsIncompatibleExports) { + new WasmFinalizeExportsPlugin().apply(compiler); + // } + new TemplatedPathPlugin().apply(compiler); new RecordIdsPlugin({ diff --git a/lib/wasm/WasmFinalizeExportsPlugin.js b/lib/wasm/WasmFinalizeExportsPlugin.js new file mode 100644 index 00000000000..942f53ca887 --- /dev/null +++ b/lib/wasm/WasmFinalizeExportsPlugin.js @@ -0,0 +1,57 @@ +/* + MIT License http://www.opensource.org/licenses/mit-license.php + Author Tobias Koppers @sokra + */ +"use strict"; + +const Queue = require("../util/Queue"); +const WebAssemblyImportDependency = require("../dependencies/WebAssemblyImportDependency"); +const UnsupportedWebAssemblyFeatureError = require("../wasm/UnsupportedWebAssemblyFeatureError"); + +class WasmFinalizeExportsPlugin { + apply(compiler) { + compiler.hooks.compilation.tap("WasmFinalizeExportsPlugin", compilation => { + compilation.hooks.finishModules.tap( + "WasmFinalizeExportsPlugin", + modules => { + const queue = new Queue(); + + let module; + let jsIncompatibleExports = []; + + for (const module of modules) { + if (module.buildMeta.jsIncompatibleExports) { + jsIncompatibleExports.push( + ...module.buildMeta.jsIncompatibleExports + ); + } + + queue.enqueue(module); + } + + while (queue.length > 0) { + module = queue.dequeue(); + + // 1. if a non WebAssembly module + if (module.type.startsWith("webassembly") === false) { + for (const dep of module.dependencies) { + // 2. imports a WebAssembly module + // FIXME(sven): pseudo code from here + if (dep.type === "webassembly") { + // 3. if the used import is flaged as invalid + if (jsIncompatibleExports.indexOf(dep.usedName)) { + throw new UnsupportedWebAssemblyFeatureError( + "JavaScript modules can not use WebAssembly export with an incompatible type signature" + ); + } + } + } + } + } + } + ); + }); + } +} + +module.exports = WasmFinalizeExportsPlugin; diff --git a/lib/wasm/WebAssemblyGenerator.js b/lib/wasm/WebAssemblyGenerator.js index 08a9ed8d64d..af38595b6ec 100644 --- a/lib/wasm/WebAssemblyGenerator.js +++ b/lib/wasm/WebAssemblyGenerator.js @@ -129,7 +129,7 @@ function getCountImportedFunc(ast) { * Get next type index * * @param {Object} ast - Module's AST - * @returns {t.IndexLiteral} - index + * @returns {t.Index} - index */ function getNextTypeIndex(ast) { const typeSectionMetadata = t.getSectionMetadata(ast, "type"); @@ -150,7 +150,7 @@ function getNextTypeIndex(ast) { * * @param {Object} ast - Module's AST * @param {Number} countImportedFunc - number of imported funcs - * @returns {t.IndexLiteral} - index + * @returns {t.Index} - index */ function getNextFuncIndex(ast, countImportedFunc) { const funcSectionMetadata = t.getSectionMetadata(ast, "func"); @@ -272,10 +272,10 @@ const rewriteImports = ({ ast, usedDependencyMap }) => bin => { * @param {Object} state transformation state * @param {Object} state.ast - Module's ast * @param {t.Identifier} state.initFuncId identifier of the init function - * @param {t.IndexLiteral} state.startAtFuncIndex index of the start function + * @param {t.Index} state.startAtFuncIndex index of the start function * @param {t.ModuleImport[]} state.importedGlobals list of imported globals - * @param {t.IndexLiteral} state.nextFuncIndex index of the next function - * @param {t.IndexLiteral} state.nextTypeIndex index of the next type + * @param {t.Index} state.nextFuncIndex index of the next function + * @param {t.Index} state.nextTypeIndex index of the next type * @returns {ArrayBufferTransform} transform */ const addInitFunction = ({ diff --git a/lib/wasm/WebAssemblyParser.js b/lib/wasm/WebAssemblyParser.js index 16103074491..650d53da64d 100644 --- a/lib/wasm/WebAssemblyParser.js +++ b/lib/wasm/WebAssemblyParser.js @@ -6,6 +6,9 @@ const t = require("@webassemblyjs/ast"); const { decode } = require("@webassemblyjs/wasm-parser"); +const { + moduleContextFromModuleAST +} = require("@webassemblyjs/helper-module-context"); const { Tapable } = require("tapable"); const WebAssemblyImportDependency = require("../dependencies/WebAssemblyImportDependency"); @@ -45,7 +48,10 @@ const getJsIncompatibleType = moduleImport => { const decoderOpts = { ignoreCodeSection: true, - ignoreDataSection: true + ignoreDataSection: true, + + // this will avoid having to lookup with identifiers in the ModuleContext + ignoreCustomNameSection: true }; class WebAssemblyParser extends Tapable { @@ -60,12 +66,36 @@ class WebAssemblyParser extends Tapable { state.module.buildMeta.exportsType = "namespace"; // parse it - const ast = decode(binary, decoderOpts); + const program = decode(binary, decoderOpts); + const module = program.body[0]; + + const moduleContext = moduleContextFromModuleAST(module); // extract imports and exports const exports = (state.module.buildMeta.providedExports = []); - t.traverse(ast, { + const jsIncompatibleExports = (state.module.buildMeta.jsIncompatibleExports = []); + + t.traverse(module, { ModuleExport({ node }) { + const descriptor = node.descr; + + if (descriptor.exportType === "Func") { + const funcidx = descriptor.id.value; + + const funcSignature = moduleContext.getFunction(funcidx); + + const hasIncompatibleArg = funcSignature.args.some( + t => !JS_COMPAT_TYPES.has(t) + ); + const hasIncompatibleResult = funcSignature.result.some( + t => !JS_COMPAT_TYPES.has(t) + ); + + if (hasIncompatibleArg === true || hasIncompatibleResult === true) { + jsIncompatibleExports.push(node.name); + } + } + exports.push(node.name); }, diff --git a/package.json b/package.json index b3ddc8fdb21..1dd4157db96 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,7 @@ "license": "MIT", "dependencies": { "@webassemblyjs/ast": "1.5.9", + "@webassemblyjs/helper-module-context": "^1.5.9", "@webassemblyjs/wasm-edit": "1.5.9", "@webassemblyjs/wasm-opt": "1.5.9", "@webassemblyjs/wasm-parser": "1.5.9", diff --git a/test/cases/wasm/js-incompatible-type/env.js b/test/cases/wasm/js-incompatible-type/env.js new file mode 100644 index 00000000000..39a36559da0 --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/env.js @@ -0,0 +1 @@ +export const n = 1; diff --git a/test/cases/wasm/js-incompatible-type/export-i64-param.wat b/test/cases/wasm/js-incompatible-type/export-i64-param.wat new file mode 100644 index 00000000000..f22fc6f5793 --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/export-i64-param.wat @@ -0,0 +1,3 @@ +(module + (func (export "a") (param i64) (nop)) +) diff --git a/test/cases/wasm/js-incompatible-type/export-i64-result.wat b/test/cases/wasm/js-incompatible-type/export-i64-result.wat new file mode 100644 index 00000000000..1aada93dbd6 --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/export-i64-result.wat @@ -0,0 +1,5 @@ +(module + (func (export "a") (result i64) + (i64.const 1) + ) +) diff --git a/test/cases/wasm/js-incompatible-type/import-i64.wat b/test/cases/wasm/js-incompatible-type/import-i64.wat new file mode 100644 index 00000000000..5be32abf044 --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/import-i64.wat @@ -0,0 +1,3 @@ +(module + (import "./env.js" "n" (global i64)) +) diff --git a/test/cases/wasm/js-incompatible-type/index.js b/test/cases/wasm/js-incompatible-type/index.js new file mode 100644 index 00000000000..ded1dfce2e7 --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/index.js @@ -0,0 +1,11 @@ +it("should disallow exporting a func signature with result i64", function() { + return expect(import("./export-i64-result.wat")).rejects.toThrow(/invalid type/); +}); + +it("should disallow exporting a func signature with param i64", function() { + return expect(import("./export-i64-param.wat")).rejects.toThrow(/invalid type/); +}); + +it("should disallow importing a value type of i64", function() { + return expect(import("./import-i64.wat")).rejects.toThrow(/invalid type/); +}); diff --git a/test/cases/wasm/js-incompatible-type/test.filter.js b/test/cases/wasm/js-incompatible-type/test.filter.js new file mode 100644 index 00000000000..23177349638 --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/test.filter.js @@ -0,0 +1,5 @@ +var supportsWebAssembly = require("../../../helpers/supportsWebAssembly"); + +module.exports = function(config) { + return supportsWebAssembly(); +}; diff --git a/yarn.lock b/yarn.lock index 0325908cd1e..10f84490c1c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -68,7 +68,7 @@ version "1.5.9" resolved "https://registry.yarnpkg.com/@webassemblyjs/helper-fsm/-/helper-fsm-1.5.9.tgz#8f996268eb07ee6728130a9e97fa3aac32772454" -"@webassemblyjs/helper-module-context@1.5.9": +"@webassemblyjs/helper-module-context@1.5.9", "@webassemblyjs/helper-module-context@^1.5.9": version "1.5.9" resolved "https://registry.yarnpkg.com/@webassemblyjs/helper-module-context/-/helper-module-context-1.5.9.tgz#69e2eea310f755a0b750b84f8af59f890f2046ac" From 24a95745f12878bc7a67f0bb7a92ee1ad91b6b8b Mon Sep 17 00:00:00 2001 From: Sven SAULEAU Date: Wed, 30 May 2018 14:20:35 +0200 Subject: [PATCH 2/7] chore: auto fix linting --- declarations.d.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/declarations.d.ts b/declarations.d.ts index e48b9c4212c..96ae3527e28 100644 --- a/declarations.d.ts +++ b/declarations.d.ts @@ -62,7 +62,7 @@ declare module "@webassemblyjs/ast" { name: string; descr: ModuleExportDescr; } - type Index = Identifier | NumberLiteral; + type Index = Identifier | NumberLiteral; export class ModuleExportDescr extends Node { exportType: string; id: Index; @@ -89,7 +89,12 @@ declare module "@webassemblyjs/ast" { export class IndexInFuncSection extends Node {} export function indexLiteral(index: number): Index; export function numberLiteralFromRaw(num: number): NumberLiteral; - export function floatLiteral(value: number, nan?: boolean, inf?: boolean, raw?: string): FloatLiteral; + export function floatLiteral( + value: number, + nan?: boolean, + inf?: boolean, + raw?: string + ): FloatLiteral; export function global(globalType: string, nodes: Node[]): Global; export function identifier(indentifier: string): Identifier; export function funcParam(valType: string, id: Identifier): FuncParam; @@ -102,7 +107,10 @@ declare module "@webassemblyjs/ast" { ): ObjectInstruction; export function signature(params: FuncParam[], results: string[]): Signature; export function func(initFuncId, Signature, funcBody): Func; - export function typeInstruction(id: Identifier, functype: Signature): TypeInstruction; + export function typeInstruction( + id: Identifier, + functype: Signature + ): TypeInstruction; export function indexInFuncSection(index: Index): IndexInFuncSection; export function moduleExport( identifier: string, From b34ed237cf374d534b8ee6113ff2395778436559 Mon Sep 17 00:00:00 2001 From: Sven SAULEAU Date: Wed, 30 May 2018 15:27:42 +0200 Subject: [PATCH 3/7] fix: address some comments --- lib/wasm/WasmFinalizeExportsPlugin.js | 57 ++++++++++--------- test/cases/wasm/js-incompatible-type/index.js | 10 +++- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/lib/wasm/WasmFinalizeExportsPlugin.js b/lib/wasm/WasmFinalizeExportsPlugin.js index 942f53ca887..cca46c4b662 100644 --- a/lib/wasm/WasmFinalizeExportsPlugin.js +++ b/lib/wasm/WasmFinalizeExportsPlugin.js @@ -4,46 +4,47 @@ */ "use strict"; -const Queue = require("../util/Queue"); -const WebAssemblyImportDependency = require("../dependencies/WebAssemblyImportDependency"); const UnsupportedWebAssemblyFeatureError = require("../wasm/UnsupportedWebAssemblyFeatureError"); +const error = new UnsupportedWebAssemblyFeatureError( + "JavaScript modules can not use a WebAssembly export with an incompatible type signature" +); + class WasmFinalizeExportsPlugin { apply(compiler) { compiler.hooks.compilation.tap("WasmFinalizeExportsPlugin", compilation => { compilation.hooks.finishModules.tap( "WasmFinalizeExportsPlugin", modules => { - const queue = new Queue(); - - let module; - let jsIncompatibleExports = []; - for (const module of modules) { - if (module.buildMeta.jsIncompatibleExports) { - jsIncompatibleExports.push( - ...module.buildMeta.jsIncompatibleExports - ); + const jsIncompatibleExports = + module.buildMeta.jsIncompatibleExports; + + if ( + typeof jsIncompatibleExports === "undefined" || + jsIncompatibleExports.length === 0 + ) { + continue; } - queue.enqueue(module); - } + // 1. if a WebAssembly module + if (module.type.startsWith("webassembly") === true) { + for (const reason of module.reasons) { + // 2. is referenced by a non-WebAssembly module + if (reason.module.type.startsWith("webassembly") === false) { + // const ref = reason.dependency.getReference(); + + // ref.importedNames // returns true? + + const names = []; - while (queue.length > 0) { - module = queue.dequeue(); - - // 1. if a non WebAssembly module - if (module.type.startsWith("webassembly") === false) { - for (const dep of module.dependencies) { - // 2. imports a WebAssembly module - // FIXME(sven): pseudo code from here - if (dep.type === "webassembly") { - // 3. if the used import is flaged as invalid - if (jsIncompatibleExports.indexOf(dep.usedName)) { - throw new UnsupportedWebAssemblyFeatureError( - "JavaScript modules can not use WebAssembly export with an incompatible type signature" - ); - } + names.forEach(name => { + // 3. and uses a func with an incompatible JS signature + if (jsIncompatibleExports.indexOf(name) !== -1) { + // 4. error + compilation.errors.push(error); + } + }); } } } diff --git a/test/cases/wasm/js-incompatible-type/index.js b/test/cases/wasm/js-incompatible-type/index.js index ded1dfce2e7..15c3274cb51 100644 --- a/test/cases/wasm/js-incompatible-type/index.js +++ b/test/cases/wasm/js-incompatible-type/index.js @@ -1,11 +1,15 @@ it("should disallow exporting a func signature with result i64", function() { - return expect(import("./export-i64-result.wat")).rejects.toThrow(/invalid type/); + return import("./export-i64-result.wat").then(({a}) => { + expect(a).toThrow(/invalid type/); + }); }); it("should disallow exporting a func signature with param i64", function() { - return expect(import("./export-i64-param.wat")).rejects.toThrow(/invalid type/); + return import("./export-i64-param.wat").then(({a}) => { + expect(a).toThrow(/invalid type/); + }); }); it("should disallow importing a value type of i64", function() { - return expect(import("./import-i64.wat")).rejects.toThrow(/invalid type/); + return expect(import("./import-i64.wat")).rejects.toThrow(/invalid type/); }); From 76fc11c505b27a0356ac8fbd9fe62b146e25fdcf Mon Sep 17 00:00:00 2001 From: Sven SAULEAU Date: Wed, 30 May 2018 15:53:23 +0200 Subject: [PATCH 4/7] fix: update test msg --- test/cases/wasm/js-incompatible-type/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cases/wasm/js-incompatible-type/index.js b/test/cases/wasm/js-incompatible-type/index.js index 15c3274cb51..5240a6e5632 100644 --- a/test/cases/wasm/js-incompatible-type/index.js +++ b/test/cases/wasm/js-incompatible-type/index.js @@ -1,12 +1,12 @@ it("should disallow exporting a func signature with result i64", function() { return import("./export-i64-result.wat").then(({a}) => { - expect(a).toThrow(/invalid type/); + expect(a).toThrow(/incompatible type signature/); }); }); it("should disallow exporting a func signature with param i64", function() { return import("./export-i64-param.wat").then(({a}) => { - expect(a).toThrow(/invalid type/); + expect(a).toThrow(/incompatible type signature/); }); }); From 5ecf74917f191def22ca14ce472dd35ca1004545 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 4 Jun 2018 16:23:41 +0200 Subject: [PATCH 5/7] finish plugin and tests --- declarations.d.ts | 7 ++- lib/ModuleDependencyError.js | 1 + lib/ModuleDependencyWarning.js | 1 + .../WebAssemblyImportDependency.js | 2 +- lib/wasm/WasmFinalizeExportsPlugin.js | 58 +++++++++++-------- lib/wasm/WebAssemblyParser.js | 45 +++++++++----- .../cases/wasm/js-incompatible-type/errors.js | 17 ++++++ .../js-incompatible-type/export-i64-param.js | 1 + .../js-incompatible-type/export-i64-result.js | 1 + test/cases/wasm/js-incompatible-type/index.js | 8 +-- 10 files changed, 97 insertions(+), 44 deletions(-) create mode 100644 test/cases/wasm/js-incompatible-type/errors.js create mode 100644 test/cases/wasm/js-incompatible-type/export-i64-param.js create mode 100644 test/cases/wasm/js-incompatible-type/export-i64-result.js diff --git a/declarations.d.ts b/declarations.d.ts index 44e59fe897e..794a87e1674 100644 --- a/declarations.d.ts +++ b/declarations.d.ts @@ -94,6 +94,7 @@ declare module "@webassemblyjs/ast" { signature: Signature; } export class Signature { + type: "Signature"; params: FuncParam[]; results: string[]; } @@ -118,7 +119,7 @@ declare module "@webassemblyjs/ast" { init: Node[] ): ObjectInstruction; export function signature(params: FuncParam[], results: string[]): Signature; - export function func(initFuncId, Signature, funcBody): Func; + export function func(initFuncId, signature: Signature, funcBody): Func; export function typeInstruction( id: Identifier, functype: Signature @@ -134,6 +135,10 @@ declare module "@webassemblyjs/ast" { ): ModuleExportDescr; export function getSectionMetadata(ast: any, section: string); + export class FuncSignature { + args: string[]; + result: string[]; + } } /** diff --git a/lib/ModuleDependencyError.js b/lib/ModuleDependencyError.js index 1cf6142e6aa..cb16cc34a1a 100644 --- a/lib/ModuleDependencyError.js +++ b/lib/ModuleDependencyError.js @@ -26,6 +26,7 @@ class ModuleDependencyError extends WebpackError { this.module = module; this.loc = loc; this.error = err; + this.origin = module.issuer; Error.captureStackTrace(this, this.constructor); } diff --git a/lib/ModuleDependencyWarning.js b/lib/ModuleDependencyWarning.js index 6c5245055f8..be62791cbdc 100644 --- a/lib/ModuleDependencyWarning.js +++ b/lib/ModuleDependencyWarning.js @@ -18,6 +18,7 @@ module.exports = class ModuleDependencyWarning extends WebpackError { this.module = module; this.loc = loc; this.error = err; + this.origin = module.issuer; Error.captureStackTrace(this, this.constructor); } diff --git a/lib/dependencies/WebAssemblyImportDependency.js b/lib/dependencies/WebAssemblyImportDependency.js index ee1e60755ac..64e97159711 100644 --- a/lib/dependencies/WebAssemblyImportDependency.js +++ b/lib/dependencies/WebAssemblyImportDependency.js @@ -32,7 +32,7 @@ class WebAssemblyImportDependency extends ModuleDependency { ) { return [ new UnsupportedWebAssemblyFeatureError( - `Import with ${ + `Import "${this.name}" from "${this.request}" with ${ this.onlyDirectImport } can only be used for direct wasm to wasm dependencies` ) diff --git a/lib/wasm/WasmFinalizeExportsPlugin.js b/lib/wasm/WasmFinalizeExportsPlugin.js index cca46c4b662..5187dd9f124 100644 --- a/lib/wasm/WasmFinalizeExportsPlugin.js +++ b/lib/wasm/WasmFinalizeExportsPlugin.js @@ -6,10 +6,6 @@ const UnsupportedWebAssemblyFeatureError = require("../wasm/UnsupportedWebAssemblyFeatureError"); -const error = new UnsupportedWebAssemblyFeatureError( - "JavaScript modules can not use a WebAssembly export with an incompatible type signature" -); - class WasmFinalizeExportsPlugin { apply(compiler) { compiler.hooks.compilation.tap("WasmFinalizeExportsPlugin", compilation => { @@ -17,34 +13,46 @@ class WasmFinalizeExportsPlugin { "WasmFinalizeExportsPlugin", modules => { for (const module of modules) { - const jsIncompatibleExports = - module.buildMeta.jsIncompatibleExports; - - if ( - typeof jsIncompatibleExports === "undefined" || - jsIncompatibleExports.length === 0 - ) { - continue; - } - // 1. if a WebAssembly module if (module.type.startsWith("webassembly") === true) { + const jsIncompatibleExports = + module.buildMeta.jsIncompatibleExports; + + if (jsIncompatibleExports === undefined) { + continue; + } + for (const reason of module.reasons) { // 2. is referenced by a non-WebAssembly module if (reason.module.type.startsWith("webassembly") === false) { - // const ref = reason.dependency.getReference(); - - // ref.importedNames // returns true? + const ref = reason.dependency.getReference(); - const names = []; + const importedNames = ref.importedNames; - names.forEach(name => { - // 3. and uses a func with an incompatible JS signature - if (jsIncompatibleExports.indexOf(name) !== -1) { - // 4. error - compilation.errors.push(error); - } - }); + if (Array.isArray(importedNames)) { + importedNames.forEach(name => { + // 3. and uses a func with an incompatible JS signature + if ( + Object.prototype.hasOwnProperty.call( + jsIncompatibleExports, + name + ) + ) { + // 4. error + /** @type {any} */ + const error = new UnsupportedWebAssemblyFeatureError( + `Export "${name}" with ${ + jsIncompatibleExports[name] + } can only be used for direct wasm to wasm dependencies` + ); + error.module = module; + error.origin = reason.module; + error.originLoc = reason.dependency.loc; + error.dependencies = [reason.dependency]; + compilation.errors.push(error); + } + }); + } } } } diff --git a/lib/wasm/WebAssemblyParser.js b/lib/wasm/WebAssemblyParser.js index ca45a69ab07..0df2008a12d 100644 --- a/lib/wasm/WebAssemblyParser.js +++ b/lib/wasm/WebAssemblyParser.js @@ -43,15 +43,14 @@ const isGlobalImport = n => n.descr.type === "GlobalType"; const JS_COMPAT_TYPES = new Set(["i32", "f32", "f64"]); /** - * @param {t.ModuleImport} moduleImport the import + * @param {t.Signature} signature the func signature * @returns {null | string} the type incompatible with js types */ -const getJsIncompatibleType = moduleImport => { - if (moduleImport.descr.type !== "FuncImportDescr") return null; - const signature = moduleImport.descr.signature; +const getJsIncompatibleType = signature => { for (const param of signature.params) { - if (!JS_COMPAT_TYPES.has(param.valtype)) + if (!JS_COMPAT_TYPES.has(param.valtype)) { return `${param.valtype} as parameter`; + } } for (const type of signature.results) { if (!JS_COMPAT_TYPES.has(type)) return `${type} as result`; @@ -59,6 +58,23 @@ const getJsIncompatibleType = moduleImport => { return null; }; +/** + * TODO why are there two different Signature types? + * @param {t.FuncSignature} signature the func signature + * @returns {null | string} the type incompatible with js types + */ +const getJsIncompatibleTypeOfFuncSignature = signature => { + for (const param of signature.args) { + if (!JS_COMPAT_TYPES.has(param)) { + return `${param} as parameter`; + } + } + for (const type of signature.result) { + if (!JS_COMPAT_TYPES.has(type)) return `${type} as result`; + } + return null; +}; + const decoderOpts = { ignoreCodeSection: true, ignoreDataSection: true, @@ -96,17 +112,15 @@ class WebAssemblyParser extends Tapable { if (descriptor.exportType === "Func") { const funcidx = descriptor.id.value; + /** @type {t.FuncSignature} */ const funcSignature = moduleContext.getFunction(funcidx); - const hasIncompatibleArg = funcSignature.args.some( - t => !JS_COMPAT_TYPES.has(t) - ); - const hasIncompatibleResult = funcSignature.result.some( - t => !JS_COMPAT_TYPES.has(t) + const incompatibleType = getJsIncompatibleTypeOfFuncSignature( + funcSignature ); - if (hasIncompatibleArg === true || hasIncompatibleResult === true) { - jsIncompatibleExports.push(node.name); + if (incompatibleType) { + jsIncompatibleExports[node.name] = incompatibleType; } } @@ -151,10 +165,15 @@ class WebAssemblyParser extends Tapable { } else if (isTableImport(node) === true) { onlyDirectImport = "Table"; } else if (isFuncImport(node) === true) { - const incompatibleType = getJsIncompatibleType(node); + const incompatibleType = getJsIncompatibleType(node.descr.signature); if (incompatibleType) { onlyDirectImport = `Non-JS-compatible Func Sigurature (${incompatibleType})`; } + } else if (isGlobalImport(node) === true) { + const type = node.descr.valtype; + if (!JS_COMPAT_TYPES.has(type)) { + onlyDirectImport = `Non-JS-compatible Global Type (${type})`; + } } const dep = new WebAssemblyImportDependency( diff --git a/test/cases/wasm/js-incompatible-type/errors.js b/test/cases/wasm/js-incompatible-type/errors.js new file mode 100644 index 00000000000..e06824329ce --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/errors.js @@ -0,0 +1,17 @@ +module.exports = [ + [ + /export-i64-param\.wat/, + /Export "a" with i64 as parameter can only be used for direct wasm to wasm dependencies/, + /export-i64-param\.js/ + ], + [ + /export-i64-result\.wat/, + /Export "a" with i64 as result can only be used for direct wasm to wasm dependencies/, + /export-i64-result\.js/ + ], + [ + /import-i64\.wat/, + /Import "n" from "\.\/env.js" with Non-JS-compatible Global Type \(i64\) can only be used for direct wasm to wasm dependencies/, + /index\.js/ + ] +] diff --git a/test/cases/wasm/js-incompatible-type/export-i64-param.js b/test/cases/wasm/js-incompatible-type/export-i64-param.js new file mode 100644 index 00000000000..db1be78bea6 --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/export-i64-param.js @@ -0,0 +1 @@ +export { a } from "./export-i64-param.wat"; diff --git a/test/cases/wasm/js-incompatible-type/export-i64-result.js b/test/cases/wasm/js-incompatible-type/export-i64-result.js new file mode 100644 index 00000000000..96a1241ee02 --- /dev/null +++ b/test/cases/wasm/js-incompatible-type/export-i64-result.js @@ -0,0 +1 @@ +export { a } from "./export-i64-result.wat"; diff --git a/test/cases/wasm/js-incompatible-type/index.js b/test/cases/wasm/js-incompatible-type/index.js index 5240a6e5632..436fadded37 100644 --- a/test/cases/wasm/js-incompatible-type/index.js +++ b/test/cases/wasm/js-incompatible-type/index.js @@ -1,12 +1,12 @@ it("should disallow exporting a func signature with result i64", function() { - return import("./export-i64-result.wat").then(({a}) => { - expect(a).toThrow(/incompatible type signature/); + return import("./export-i64-result").then(({a}) => { + expect(() => a()).toThrow(/invalid type/); }); }); it("should disallow exporting a func signature with param i64", function() { - return import("./export-i64-param.wat").then(({a}) => { - expect(a).toThrow(/incompatible type signature/); + return import("./export-i64-param").then(({a}) => { + expect(() => a()).toThrow(/invalid type/); }); }); From 591521b310d909f08cec99d32cb2cc44ef602054 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 4 Jun 2018 18:27:19 +0200 Subject: [PATCH 6/7] support node.js 10 and 8 --- test/cases/wasm/js-incompatible-type/index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/cases/wasm/js-incompatible-type/index.js b/test/cases/wasm/js-incompatible-type/index.js index 436fadded37..f5258dc4277 100644 --- a/test/cases/wasm/js-incompatible-type/index.js +++ b/test/cases/wasm/js-incompatible-type/index.js @@ -1,15 +1,17 @@ +const errorRegex = /wasm function signature contains illegal type|invalid type/; + it("should disallow exporting a func signature with result i64", function() { return import("./export-i64-result").then(({a}) => { - expect(() => a()).toThrow(/invalid type/); + expect(() => a()).toThrow(errorRegex); }); }); it("should disallow exporting a func signature with param i64", function() { return import("./export-i64-param").then(({a}) => { - expect(() => a()).toThrow(/invalid type/); + expect(() => a()).toThrow(errorRegex); }); }); it("should disallow importing a value type of i64", function() { - return expect(import("./import-i64.wat")).rejects.toThrow(/invalid type/); + return expect(import("./import-i64.wat")).rejects.toThrow(errorRegex); }); From 78b31936c3baf7dc0e71f10189ddf415d22c2762 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 4 Jun 2018 20:14:08 +0200 Subject: [PATCH 7/7] add checkWasmTypes flag, enabled it only in production --- lib/WebpackOptionsApply.js | 8 +++----- lib/WebpackOptionsDefaulter.js | 3 +++ schemas/WebpackOptions.json | 4 ++++ test/cases/wasm/js-incompatible-type/test.filter.js | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/WebpackOptionsApply.js b/lib/WebpackOptionsApply.js index 8887759b189..c4cc617bd74 100644 --- a/lib/WebpackOptionsApply.js +++ b/lib/WebpackOptionsApply.js @@ -340,6 +340,9 @@ class WebpackOptionsApply extends OptionsApply { if (options.optimization.noEmitOnErrors) { new NoEmitOnErrorsPlugin().apply(compiler); } + if (options.optimization.checkWasmTypes) { + new WasmFinalizeExportsPlugin().apply(compiler); + } if (options.optimization.namedModules) { new NamedModulesPlugin().apply(compiler); } @@ -361,11 +364,6 @@ class WebpackOptionsApply extends OptionsApply { new SizeLimitsPlugin(options.performance).apply(compiler); } - // FIXME(sven): this should be conditional - // if (options.optimization.jsIncompatibleExports) { - new WasmFinalizeExportsPlugin().apply(compiler); - // } - new TemplatedPathPlugin().apply(compiler); new RecordIdsPlugin({ diff --git a/lib/WebpackOptionsDefaulter.js b/lib/WebpackOptionsDefaulter.js index 70678c867ba..27a434654f9 100644 --- a/lib/WebpackOptionsDefaulter.js +++ b/lib/WebpackOptionsDefaulter.js @@ -254,6 +254,9 @@ class WebpackOptionsDefaulter extends OptionsDefaulter { this.set("optimization.noEmitOnErrors", "make", options => isProductionLikeMode(options) ); + this.set("optimization.checkWasmTypes", "make", options => + isProductionLikeMode(options) + ); this.set( "optimization.namedModules", "make", diff --git a/schemas/WebpackOptions.json b/schemas/WebpackOptions.json index f8d94d68320..b07b349c928 100644 --- a/schemas/WebpackOptions.json +++ b/schemas/WebpackOptions.json @@ -1542,6 +1542,10 @@ "description": "Avoid emitting assets when errors occur", "type": "boolean" }, + "checkWasmTypes": { + "description": "Check for incompatible wasm types when importing/exporting from/to ESM", + "type": "boolean" + }, "namedModules": { "description": "Use readable module identifiers for better debugging", "type": "boolean" diff --git a/test/cases/wasm/js-incompatible-type/test.filter.js b/test/cases/wasm/js-incompatible-type/test.filter.js index 23177349638..bd31021b996 100644 --- a/test/cases/wasm/js-incompatible-type/test.filter.js +++ b/test/cases/wasm/js-incompatible-type/test.filter.js @@ -1,5 +1,5 @@ var supportsWebAssembly = require("../../../helpers/supportsWebAssembly"); module.exports = function(config) { - return supportsWebAssembly(); + return supportsWebAssembly() && config.mode === "production"; };