Merge pull request #7447 from xtuc/fix-wasm-check-for-invalid-signatures

wasm: finalizer for checking exports
webpack · Jun 4, 2018 · 19389b7 · 19389b7
2 parents bc6b5b0 + 78b3193
commit 19389b7
Show file tree

Hide file tree

Showing 20 changed files with 215 additions and 28 deletions.
diff --git a/declarations.d.ts b/declarations.d.ts
@@ -61,14 +61,14 @@ declare module "@webassemblyjs/ast" {
 	}
 	export class ModuleExport extends Node {
 		name: string;
-		descr: {
-			type: string;
-			exportType: string;
-			id?: Identifier;
-		};
+		descr: ModuleExportDescr;
+	}
+	type Index = Identifier | NumberLiteral;
+	export class ModuleExportDescr extends Node {
+		type: string;
+		exportType: string;
+		id: Index;
 	}
-	export class ModuleExportDescr extends Node {}
-	export class IndexLiteral extends Node {}
 	export class NumberLiteral extends Node {
 		value: number;
 		raw: string;
@@ -94,12 +94,13 @@ declare module "@webassemblyjs/ast" {
 		signature: Signature;
 	}
 	export class Signature {
+		type: "Signature";
 		params: FuncParam[];
 		results: string[];
 	}
 	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,
@@ -111,29 +112,33 @@ declare module "@webassemblyjs/ast" {
 	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,
 		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
 	): 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);
+	export class FuncSignature {
+		args: string[];
+		result: string[];
+	}
 }
 
 /**

diff --git 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
@@ -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/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() {
@@ -339,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);
 		}

diff --git 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/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
@@ -0,0 +1,66 @@
+/*
+	MIT License http://www.opensource.org/licenses/mit-license.php
+	Author Tobias Koppers @sokra
+	*/
+"use strict";
+
+const UnsupportedWebAssemblyFeatureError = require("../wasm/UnsupportedWebAssemblyFeatureError");
+
+class WasmFinalizeExportsPlugin {
+	apply(compiler) {
+		compiler.hooks.compilation.tap("WasmFinalizeExportsPlugin", compilation => {
+			compilation.hooks.finishModules.tap(
+				"WasmFinalizeExportsPlugin",
+				modules => {
+					for (const module of modules) {
+						// 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();
+
+									const importedNames = ref.importedNames;
+
+									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);
+											}
+										});
+									}
+								}
+							}
+						}
+					}
+				}
+			);
+		});
+	}
+}
+
+module.exports = WasmFinalizeExportsPlugin;
diff --git a/lib/wasm/WebAssemblyGenerator.js b/lib/wasm/WebAssemblyGenerator.js
@@ -131,7 +131,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");
@@ -152,7 +152,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");
@@ -309,11 +309,11 @@ 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.Instruction[]} state.additionalInitCode list of addition instructions for the init function
- * @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
@@ -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");
@@ -40,25 +43,44 @@ 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`;
 	}
 	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
+	ignoreDataSection: true,
+
+	// this will avoid having to lookup with identifiers in the ModuleContext
+	ignoreCustomNameSection: true
 };
 
 class WebAssemblyParser extends Tapable {
@@ -73,13 +95,35 @@ 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 = []);
+		const jsIncompatibleExports = (state.module.buildMeta.jsIncompatibleExports = []);
+
 		const importedGlobals = [];
-		t.traverse(ast, {
+		t.traverse(module, {
 			ModuleExport({ node }) {
+				const descriptor = node.descr;
+
+				if (descriptor.exportType === "Func") {
+					const funcidx = descriptor.id.value;
+
+					/** @type {t.FuncSignature} */
+					const funcSignature = moduleContext.getFunction(funcidx);
+
+					const incompatibleType = getJsIncompatibleTypeOfFuncSignature(
+						funcSignature
+					);
+
+					if (incompatibleType) {
+						jsIncompatibleExports[node.name] = incompatibleType;
+					}
+				}
+
 				exports.push(node.name);
 
 				if (node.descr && node.descr.exportType === "Global") {
@@ -121,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/package.json b/package.json
@@ -6,6 +6,7 @@
   "license": "MIT",
   "dependencies": {
     "@webassemblyjs/ast": "1.5.10",
+    "@webassemblyjs/helper-module-context": "1.5.10",
     "@webassemblyjs/wasm-edit": "1.5.10",
     "@webassemblyjs/wasm-opt": "1.5.10",
     "@webassemblyjs/wasm-parser": "1.5.10",

diff --git 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/env.js 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/errors.js 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
@@ -0,0 +1 @@
+export { a } from "./export-i64-param.wat";
diff --git a/test/cases/wasm/js-incompatible-type/export-i64-param.wat 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.js 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/export-i64-result.wat b/test/cases/wasm/js-incompatible-type/export-i64-result.wat
@@ -0,0 +1,5 @@
+(module
+  (func (export "a") (result i64)
+    (i64.const 1)
+  )
+)