From efa443aa301ea57ee54e8ced22ce604124773d4d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 19 Aug 2025 14:50:35 -0700 Subject: [PATCH 1/3] work --- src/tools/wasm-merge.cpp | 142 ++++++++++++++++++++------------------- 1 file changed, 72 insertions(+), 70 deletions(-) diff --git a/src/tools/wasm-merge.cpp b/src/tools/wasm-merge.cpp index 045e98275d8..dda527526c1 100644 --- a/src/tools/wasm-merge.cpp +++ b/src/tools/wasm-merge.cpp @@ -418,7 +418,7 @@ void checkLimit(bool& valid, const char* kind, T* export_, T* import) { // Find pairs of matching imports and exports, and make uses of the import refer // to the exported item (which has been merged into the module). -void fuseImportsAndExports() { +void fuseImportsAndExports(const PassOptions& options) { // First, scan the exports and build a map. We build a map of [module name] to // [export name => internal name]. For example, consider this module: // @@ -466,80 +466,82 @@ void fuseImportsAndExports() { } }); - // Make sure that the export types match the import types. - bool valid = true; - ModuleUtils::iterImportedFunctions(merged, [&](Function* import) { - auto internalName = kindModuleExportMaps[ExternalKind::Function] - [import->module][import->base]; - if (internalName.is()) { - auto* export_ = merged.getFunction(internalName); - if (!HeapType::isSubType(export_->type, import->type)) { - reportTypeMismatch(valid, "function", import); - std::cerr << "type " << export_->type << " is not a subtype of " - << import->type << ".\n"; - } - } - }); - ModuleUtils::iterImportedTables(merged, [&](Table* import) { - auto internalName = - kindModuleExportMaps[ExternalKind::Table][import->module][import->base]; - if (internalName.is()) { - auto* export_ = merged.getTable(internalName); - checkLimit(valid, "table", export_, import); - if (export_->type != import->type) { - reportTypeMismatch(valid, "table", import); - std::cerr << "export type " << export_->type - << " is different from import type " << import->type << ".\n"; - } - } - }); - ModuleUtils::iterImportedMemories(merged, [&](Memory* import) { - auto internalName = - kindModuleExportMaps[ExternalKind::Memory][import->module][import->base]; - if (internalName.is()) { - auto* export_ = merged.getMemory(internalName); - if (export_->is64() != import->is64()) { - reportTypeMismatch(valid, "memory", import); - std::cerr << "index type should match.\n"; + if (options.validate) { + // Make sure that the export types match the import types. + bool valid = true; + ModuleUtils::iterImportedFunctions(merged, [&](Function* import) { + auto internalName = kindModuleExportMaps[ExternalKind::Function] + [import->module][import->base]; + if (internalName.is()) { + auto* export_ = merged.getFunction(internalName); + if (!HeapType::isSubType(export_->type, import->type)) { + reportTypeMismatch(valid, "function", import); + std::cerr << "type " << export_->type << " is not a subtype of " + << import->type << ".\n"; + } } - checkLimit(valid, "memory", export_, import); - } - }); - ModuleUtils::iterImportedGlobals(merged, [&](Global* import) { - auto internalName = - kindModuleExportMaps[ExternalKind::Global][import->module][import->base]; - if (internalName.is()) { - auto* export_ = merged.getGlobal(internalName); - if (export_->mutable_ != import->mutable_) { - reportTypeMismatch(valid, "global", import); - std::cerr << "mutability should match.\n"; + }); + ModuleUtils::iterImportedTables(merged, [&](Table* import) { + auto internalName = + kindModuleExportMaps[ExternalKind::Table][import->module][import->base]; + if (internalName.is()) { + auto* export_ = merged.getTable(internalName); + checkLimit(valid, "table", export_, import); + if (export_->type != import->type) { + reportTypeMismatch(valid, "table", import); + std::cerr << "export type " << export_->type + << " is different from import type " << import->type << ".\n"; + } } - if (export_->mutable_ && export_->type != import->type) { - reportTypeMismatch(valid, "global", import); - std::cerr << "export type " << export_->type - << " is different from import type " << import->type << ".\n"; + }); + ModuleUtils::iterImportedMemories(merged, [&](Memory* import) { + auto internalName = + kindModuleExportMaps[ExternalKind::Memory][import->module][import->base]; + if (internalName.is()) { + auto* export_ = merged.getMemory(internalName); + if (export_->is64() != import->is64()) { + reportTypeMismatch(valid, "memory", import); + std::cerr << "index type should match.\n"; + } + checkLimit(valid, "memory", export_, import); } - if (!export_->mutable_ && !Type::isSubType(export_->type, import->type)) { - reportTypeMismatch(valid, "global", import); - std::cerr << "type " << export_->type << " is not a subtype of " - << import->type << ".\n"; + }); + ModuleUtils::iterImportedGlobals(merged, [&](Global* import) { + auto internalName = + kindModuleExportMaps[ExternalKind::Global][import->module][import->base]; + if (internalName.is()) { + auto* export_ = merged.getGlobal(internalName); + if (export_->mutable_ != import->mutable_) { + reportTypeMismatch(valid, "global", import); + std::cerr << "mutability should match.\n"; + } + if (export_->mutable_ && export_->type != import->type) { + reportTypeMismatch(valid, "global", import); + std::cerr << "export type " << export_->type + << " is different from import type " << import->type << ".\n"; + } + if (!export_->mutable_ && !Type::isSubType(export_->type, import->type)) { + reportTypeMismatch(valid, "global", import); + std::cerr << "type " << export_->type << " is not a subtype of " + << import->type << ".\n"; + } } - } - }); - ModuleUtils::iterImportedTags(merged, [&](Tag* import) { - auto internalName = - kindModuleExportMaps[ExternalKind::Tag][import->module][import->base]; - if (internalName.is()) { - auto* export_ = merged.getTag(internalName); - if (export_->type != import->type) { - reportTypeMismatch(valid, "tag", import); - std::cerr << "export type " << export_->type - << " is different from import type " << import->type << ".\n"; + }); + ModuleUtils::iterImportedTags(merged, [&](Tag* import) { + auto internalName = + kindModuleExportMaps[ExternalKind::Tag][import->module][import->base]; + if (internalName.is()) { + auto* export_ = merged.getTag(internalName); + if (export_->type != import->type) { + reportTypeMismatch(valid, "tag", import); + std::cerr << "export type " << export_->type + << " is different from import type " << import->type << ".\n"; + } } + }); + if (!valid) { + Fatal() << "import/export mismatches"; } - }); - if (!valid) { - Fatal() << "import/export mismatches"; } // Update the things we found. @@ -741,7 +743,7 @@ Input source maps can be specified by adding an -ism option right after the modu // Fuse imports and exports now that everything is all together in the merged // module. - fuseImportsAndExports(); + fuseImportsAndExports(options.passOptions); { PassRunner passRunner(&merged); From b82b98ef954d4c88349bb5b40c0ee0a28ba95243 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 19 Aug 2025 14:50:53 -0700 Subject: [PATCH 2/3] format --- src/tools/wasm-merge.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/tools/wasm-merge.cpp b/src/tools/wasm-merge.cpp index dda527526c1..15e8bb9e7db 100644 --- a/src/tools/wasm-merge.cpp +++ b/src/tools/wasm-merge.cpp @@ -490,13 +490,14 @@ void fuseImportsAndExports(const PassOptions& options) { if (export_->type != import->type) { reportTypeMismatch(valid, "table", import); std::cerr << "export type " << export_->type - << " is different from import type " << import->type << ".\n"; + << " is different from import type " << import->type + << ".\n"; } } }); ModuleUtils::iterImportedMemories(merged, [&](Memory* import) { - auto internalName = - kindModuleExportMaps[ExternalKind::Memory][import->module][import->base]; + auto internalName = kindModuleExportMaps[ExternalKind::Memory] + [import->module][import->base]; if (internalName.is()) { auto* export_ = merged.getMemory(internalName); if (export_->is64() != import->is64()) { @@ -507,8 +508,8 @@ void fuseImportsAndExports(const PassOptions& options) { } }); ModuleUtils::iterImportedGlobals(merged, [&](Global* import) { - auto internalName = - kindModuleExportMaps[ExternalKind::Global][import->module][import->base]; + auto internalName = kindModuleExportMaps[ExternalKind::Global] + [import->module][import->base]; if (internalName.is()) { auto* export_ = merged.getGlobal(internalName); if (export_->mutable_ != import->mutable_) { @@ -518,9 +519,11 @@ void fuseImportsAndExports(const PassOptions& options) { if (export_->mutable_ && export_->type != import->type) { reportTypeMismatch(valid, "global", import); std::cerr << "export type " << export_->type - << " is different from import type " << import->type << ".\n"; + << " is different from import type " << import->type + << ".\n"; } - if (!export_->mutable_ && !Type::isSubType(export_->type, import->type)) { + if (!export_->mutable_ && + !Type::isSubType(export_->type, import->type)) { reportTypeMismatch(valid, "global", import); std::cerr << "type " << export_->type << " is not a subtype of " << import->type << ".\n"; @@ -535,7 +538,8 @@ void fuseImportsAndExports(const PassOptions& options) { if (export_->type != import->type) { reportTypeMismatch(valid, "tag", import); std::cerr << "export type " << export_->type - << " is different from import type " << import->type << ".\n"; + << " is different from import type " << import->type + << ".\n"; } } }); From a9d249d17a8f668f06edb051d77f941a3c8cf3e4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 19 Aug 2025 15:55:44 -0700 Subject: [PATCH 3/3] fix --- src/tools/wasm-merge.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/tools/wasm-merge.cpp b/src/tools/wasm-merge.cpp index 15e8bb9e7db..b0c92baf9e0 100644 --- a/src/tools/wasm-merge.cpp +++ b/src/tools/wasm-merge.cpp @@ -91,6 +91,11 @@ // merged, and at the end we traverse the entire merged module once to fuse // imports and exports. // +// Debugging: Set BINARYEN_PASS_DEBUG=1 in the env to get validation after each +// merging of a module (like pass-debug mode for the pass runner, this does +// expensive work after each incremental operation). This can take quadratic +// time, so we do not do it by default. +// #include "ir/module-utils.h" #include "ir/names.h" @@ -736,15 +741,26 @@ Input source maps can be specified by adding an -ism option right after the modu // This is a later module: do a full merge. mergeInto(*currModule, inputFileName); - if (options.passOptions.validate) { - if (!WasmValidator().validate(merged)) { + // Validate after each merged module, when we are in pass-debug mode + // (this can be quadratic time). + if (PassRunner::getPassDebug()) { + std::cerr << "[WasmMerge] merged : " << inputFile << '\n'; + if (options.passOptions.validate && !WasmValidator().validate(merged)) { std::cout << merged << '\n'; - Fatal() << "error in validating merged after: " << inputFile; + Fatal() << "error in validating after: " << inputFile; } } } } + // If we didn't validate after each merged module, validate once at the very + // end. This won't catch problems at the earliest point, but is still useful. + if (!PassRunner::getPassDebug() && options.passOptions.validate && + !WasmValidator().validate(merged)) { + std::cout << merged << '\n'; + Fatal() << "error in validating final merged"; + } + // Fuse imports and exports now that everything is all together in the merged // module. fuseImportsAndExports(options.passOptions);