[wasm-split] Fix table naming conflicts (#8708)
After #8688, we split module elements, including tables, earlier than
`indirectCallsToSecondaryFunctions`, which calls `getSlot`, which calls
`makeTable`. But when making a table, we only try to get a valid name
within the primary module:
https://github.com/WebAssembly/binaryen/blob/2f1f55aef6d9adfa6fdc2c25e46d202232dbf6e2/src/ir/module-splitting.cpp#L235-L238
If an existing table's name was `0` and it was moved to a secondary
module in `shareImportable` already, this will happily create an active
table with the name `0` again. And in `setupTablePatching`, because the
secondary module already has `0`, the active table will not be exported
/ imported there:
https://github.com/WebAssembly/binaryen/blob/2f1f55aef6d9adfa6fdc2c25e46d202232dbf6e2/src/ir/module-splitting.cpp#L1103-L1117
But this existing table is NOT the active table, and this table's type
may not even be `funcref`.
This fixes `makeTable` so that it makes a table name that does not
collide with any table names not only in the primary module but all
secondary modules.
This also disables `Split` fuzzer for now; I'm finding more bugs, so
I'll reenable it after it is more stabilized.
diff --git a/scripts/fuzz_opt.py b/scripts/fuzz_opt.py
index b089ec7..4173058 100755
--- a/scripts/fuzz_opt.py
+++ b/scripts/fuzz_opt.py
@@ -2525,7 +2525,7 @@
TrapsNeverHappen(),
CtorEval(),
Merge(),
- Split(),
+# Split(), # Will reenable after stabilized
RoundtripText(),
ClusterFuzz(),
Two(),
diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp
index 3385bc7..9659121 100644
--- a/src/ir/module-splitting.cpp
+++ b/src/ir/module-splitting.cpp
@@ -112,13 +112,15 @@
Expression* makeExpr(Module& module);
};
Module& module;
+ const std::vector<std::unique_ptr<Module>>& secondaries;
Table* activeTable = nullptr;
ElementSegment* activeSegment = nullptr;
Slot activeBase;
std::map<Name, Slot> funcIndices;
std::vector<ElementSegment*> activeTableSegments;
- TableSlotManager(Module& module);
+ TableSlotManager(Module& module,
+ const std::vector<std::unique_ptr<Module>>& secondaries);
Table* makeTable();
ElementSegment* makeElementSegment();
@@ -149,7 +151,9 @@
funcIndices.insert({func, slot});
}
-TableSlotManager::TableSlotManager(Module& module) : module(module) {
+TableSlotManager::TableSlotManager(
+ Module& module, const std::vector<std::unique_ptr<Module>>& secondaries)
+ : module(module), secondaries(secondaries) {
// If possible, just create a new table to manage all primary-to-secondary
// calls lazily. Do not re-use slots for functions that will already be in
// existing tables, since that is not correct in the face of table mutations.
@@ -233,8 +237,25 @@
}
Table* TableSlotManager::makeTable() {
- return module.addTable(
- Builder::makeTable(Names::getValidTableName(module, Name::fromInt(0))));
+ // Because the active table will be imported in secondary modules, its name
+ // should not collide with any existing tables in primary and secondary
+ // modules.
+ std::unordered_set<Name> secondaryTableNames;
+ for (auto& secondary : secondaries) {
+ for (auto& table : secondary->tables) {
+ secondaryTableNames.insert(table->name);
+ }
+ }
+ Name name = Names::getValidName("0", [&](Name test) {
+ if (module.getTableOrNull(test)) {
+ return false;
+ }
+ if (secondaryTableNames.contains(test)) {
+ return false;
+ }
+ return true;
+ });
+ return module.addTable(Builder::makeTable(name));
}
ElementSegment* TableSlotManager::makeElementSegment() {
@@ -347,7 +368,7 @@
void setupTablePatching();
ModuleSplitter(Module& primary, const Config& config)
- : config(config), primary(primary), tableManager(primary),
+ : config(config), primary(primary), tableManager(primary, secondaries),
exportedPrimaryFuncs(initExportedPrimaryFuncs(primary)),
exportedPrimaryItems(initExportedPrimaryItems(primary)) {
classifyFunctions();
diff --git a/test/lit/wasm-split/table-name-conflict.wast b/test/lit/wasm-split/table-name-conflict.wast
new file mode 100644
index 0000000..983234b
--- /dev/null
+++ b/test/lit/wasm-split/table-name-conflict.wast
@@ -0,0 +1,45 @@
+;; RUN: wasm-split %s -all -g -S -o1 %t.1.wast -o2 %t.2.wast --split-funcs=split
+;; RUN: cat %t.1.wast | filecheck %s --check-prefix PRIMARY
+;; RUN: cat %t.2.wast | filecheck %s --check-prefix SECONDARY
+
+;; Regression test for a bug when an existing table, which is to be split to the
+;; secondary module, has the name '0'. The newly created active table should
+;; have a different name.
+
+(module
+ (table $0 0 externref)
+ (export "split" (func $split))
+ (func $split
+ (table.set $0
+ (i32.const 0)
+ (ref.null extern)
+ )
+ )
+)
+
+;; PRIMARY: (module
+;; PRIMARY-NEXT: (type $0 (func))
+;; PRIMARY-NEXT: (import "placeholder.deferred" "0" (func $placeholder_0 (type $0)))
+;; PRIMARY-NEXT: (table $0_0 1 funcref)
+;; PRIMARY-NEXT: (elem $0 (i32.const 0) $placeholder_0)
+;; PRIMARY-NEXT: (export "split" (func $trampoline_split))
+;; PRIMARY-NEXT: (export "table" (table $0_0))
+;; PRIMARY-NEXT: (func $trampoline_split (type $0)
+;; PRIMARY-NEXT: (call_indirect $0_0 (type $0)
+;; PRIMARY-NEXT: (i32.const 0)
+;; PRIMARY-NEXT: )
+;; PRIMARY-NEXT: )
+;; PRIMARY-NEXT: )
+
+;; SECONDARY: (module
+;; SECONDARY-NEXT: (type $0 (func))
+;; SECONDARY-NEXT: (import "primary" "table" (table $0_0 1 funcref))
+;; SECONDARY-NEXT: (table $0 0 externref)
+;; SECONDARY-NEXT: (elem $0 (table $0_0) (i32.const 0) func $split)
+;; SECONDARY-NEXT: (func $split (type $0)
+;; SECONDARY-NEXT: (table.set $0
+;; SECONDARY-NEXT: (i32.const 0)
+;; SECONDARY-NEXT: (ref.null noextern)
+;; SECONDARY-NEXT: )
+;; SECONDARY-NEXT: )
+;; SECONDARY-NEXT: )