[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.

Author: aheejin

scripts/fuzz_opt.py

@@ -2525,7 +2525,7 @@ def handle(self, wasm):
     TrapsNeverHappen(),
     CtorEval(),
     Merge(),
-    Split(),
+#    Split(), # Will reenable after stabilized
     RoundtripText(),
     ClusterFuzz(),
     Two(),

src/ir/module-splitting.cpp

@@ -112,13 +112,15 @@ struct TableSlotManager {
     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 @@ void TableSlotManager::addSlot(Name func, Slot slot) {
   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 @@ TableSlotManager::TableSlotManager(Module& module) : module(module) {
 }
 
 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 @@ struct ModuleSplitter {
   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();

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: )