Skip to content

Commit 3cd49e3

Browse files
authored
Handle module name collisions in MinifyImportsAndExports using a new output format (#8550)
Before we just minified the basename, ignoring the module name entirely. That meant we errored if two modules had the same basename, ```wat (import "A" "foo" ..) (import "B" "foo" ..) ``` Fix this by tracking module names alongside base names. The output format must change to support this, as we need to print out the module name as well. While making a breaking change here, change the output to JSON which will handle escaping properly, and is more structured and explicit. Fixes #8203
1 parent 6474c70 commit 3cd49e3

File tree

6 files changed

+10104
-10041
lines changed

6 files changed

+10104
-10041
lines changed

CHANGELOG.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,13 @@ Current Trunk
2222
- Convert `module.getMemorySegmentInfo` to take a data segment reference instead of a name, and return the name as part of the info.
2323

2424
- Add support for non-nullable table types and initialization expressions for
25-
tables. This comes with a breaking change to C API: `BinaryenAddTable` takes
26-
an additional `BinaryenExpressionRef` parameter to provide an initialization
27-
expression. This may be set to NULL for tables without an initializer. In JS
28-
this parameter is optional and so is not breaking. (#8405)
25+
tables. This comes with a breaking change to C API: `BinaryenAddTable` takes
26+
an additional `BinaryenExpressionRef` parameter to provide an initialization
27+
expression. This may be set to NULL for tables without an initializer. In JS
28+
this parameter is optional and so is not breaking. (#8405)
29+
- MinifyImportsAndExports now has a new output format using JSON. This was
30+
changed while fixing bugs with colliding module names (to avoid two breaking
31+
changes to the output). (#8550)
2932

3033
v128
3134
----

src/passes/MinifyImportsAndExports.cpp

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include <ir/names.h>
4242
#include <pass.h>
4343
#include <shared-constants.h>
44+
#include <support/string.h>
4445
#include <wasm.h>
4546

4647
namespace wasm {
@@ -62,14 +63,19 @@ struct MinifyImportsAndExports : public Pass {
6263
void run(Module* module) override {
6364
// Minify the imported names.
6465
Names::MinifiedNameGenerator names;
65-
std::map<Name, Name> oldToNew;
66-
std::map<Name, Name> newToOld;
67-
auto process = [&](Name& name) {
68-
auto iter = oldToNew.find(name);
66+
// Use a key of (module, base) for the old values, to handle colliding
67+
// basenames between modules.
68+
std::map<std::pair<Name, Name>, Name> oldToNew;
69+
std::map<Name, std::pair<Name, Name>> newToOld;
70+
// Process a name. This can be the basename of an import, or the full name
71+
// of an export. The module name is only used for imports.
72+
auto process = [&](Name& name, Name module = Name()) {
73+
std::pair<Name, Name> key(module, name);
74+
auto iter = oldToNew.find(key);
6975
if (iter == oldToNew.end()) {
7076
auto newName = names.getName();
71-
oldToNew[name] = newName;
72-
newToOld[newName] = name;
77+
oldToNew[key] = newName;
78+
newToOld[newName] = key;
7379
name = newName;
7480
} else {
7581
name = iter->second;
@@ -82,7 +88,7 @@ struct MinifyImportsAndExports : public Pass {
8288
// and wasi, but not custom user things.
8389
if (minifyModules || curr->module == ENV ||
8490
curr->module.startsWith("wasi_")) {
85-
process(curr->base);
91+
process(curr->base, curr->module);
8692
}
8793
});
8894

@@ -93,10 +99,40 @@ struct MinifyImportsAndExports : public Pass {
9399
}
94100
}
95101
module->updateMaps();
102+
96103
// Emit the mapping.
97-
for (auto& [new_, old] : newToOld) {
98-
std::cout << old.str << " => " << new_.str << '\n';
104+
std::cout << "{\n";
105+
std::cout << " \"imports\": [";
106+
bool first = true;
107+
for (auto& [new_, key] : newToOld) {
108+
if (key.first) {
109+
if (first) {
110+
first = false;
111+
} else {
112+
std::cout << ',';
113+
}
114+
std::cout << "\n [";
115+
String::printEscaped(std::cout, key.first.str) << ", ";
116+
String::printEscaped(std::cout, key.second.str) << ", ";
117+
String::printEscaped(std::cout, new_.str) << "]";
118+
}
119+
}
120+
std::cout << "\n ],\n\"exports\": [";
121+
first = true;
122+
for (auto& [new_, key] : newToOld) {
123+
if (!key.first) {
124+
if (first) {
125+
first = false;
126+
} else {
127+
std::cout << ',';
128+
}
129+
std::cout << "\n [";
130+
String::printEscaped(std::cout, key.second.str) << ", ";
131+
String::printEscaped(std::cout, new_.str) << "]";
132+
}
99133
}
134+
std::cout << "\n ]\n";
135+
std::cout << "}\n";
100136

101137
if (minifyModules) {
102138
doMinifyModules(module);
Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,23 @@
1-
memory => a
2-
table => b
3-
longname1 => c
4-
longname2 => d
5-
longname3 => e
6-
longname4 => f
1+
{
2+
"imports": [
3+
["env", "memory", "a"],
4+
["env", "table", "b"],
5+
["env", "longname1", "c"],
6+
["env", "longname2", "d"],
7+
["env", "longname3", "e"],
8+
["other", "longname3", "f"],
9+
["other", "longname4", "g"]
10+
],
11+
"exports": [
12+
]
13+
}
714
(module
815
(type $0 (func))
916
(import "a" "a" (memory $0 256 256))
1017
(import "a" "b" (table $0 4 funcref))
1118
(import "a" "c" (func $internal1))
1219
(import "a" "d" (func $internal2))
1320
(import "a" "e" (func $internal3))
14-
(import "a" "f" (func $internal4))
21+
(import "a" "f" (func $internal3b))
22+
(import "a" "g" (func $internal4))
1523
)
Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
(module
2-
(import "env" "memory" (memory $0 256 256))
3-
(import "env" "table" (table $0 4 funcref))
4-
(import "env" "longname1" (func $internal1))
5-
(import "env" "longname2" (func $internal2))
6-
(import "env" "longname3" (func $internal3))
2+
(import "env" "memory" (memory $0 256 256))
3+
(import "env" "table" (table $0 4 funcref))
4+
(import "env" "longname1" (func $internal1))
5+
(import "env" "longname2" (func $internal2))
6+
7+
(import "env" "longname3" (func $internal3)) ;; \ note overlap
8+
(import "other" "longname3" (func $internal3b)) ;; / in basename
9+
710
(import "other" "longname4" (func $internal4))
811
)

0 commit comments

Comments
 (0)