wasm-merge: When fixing name collisions, do not collide in either module

src/tools/wasm-merge.cpp

@@ -304,32 +304,62 @@ void renameInputItems(Module& input) {
     }
   };
 
+  // Given a name in the input module, and lamdbdas that query for that name in
+  // both the merged and input module, return a valid name. We want a new name
+  // that is not in the merged module - we cannot collide with anything already
+  // there - but we must also be careful to not fix such collisions with names
+  // in the input module. That is, if we have $a and $a already exists in the
+  // merged module, $a_1 might be valid - but it is not valid if $a_1 is in the
+  // input module, as that means it is a valid name there, with things referring
+  // to it, which we would need to map. For simplicity, when fixing a collision,
+  // pick a totally novel name.
+  auto getValidName = [&](Name name, auto method, Index hint) {
+    return Names::getValidName(
+      name,
+      [&](Name test) {
+        // As explained above, a name is valid if it is not in the merged
+        // module, and also it is either the original name in the input module
+        // (no collision) or it does not appear there.
+        return !(merged.*method)(test) &&
+               (test == name || !(input.*method)(test));
+      },
+      hint);
+  };
+
   for (auto& curr : input.functions) {
-    auto name = Names::getValidFunctionName(merged, curr->name);
+    auto name = getValidName(
+      curr->name, &Module::getFunctionOrNull, merged.functions.size());
     maybeAdd(ModuleItemKind::Function, curr->name, name);
   }
   for (auto& curr : input.globals) {
-    auto name = Names::getValidGlobalName(merged, curr->name);
+    auto name =
+      getValidName(curr->name, &Module::getGlobalOrNull, merged.globals.size());
     maybeAdd(ModuleItemKind::Global, curr->name, name);
   }
   for (auto& curr : input.tags) {
-    auto name = Names::getValidTagName(merged, curr->name);
+    auto name =
+      getValidName(curr->name, &Module::getTagOrNull, merged.tags.size());
     maybeAdd(ModuleItemKind::Tag, curr->name, name);
   }
   for (auto& curr : input.elementSegments) {
-    auto name = Names::getValidElementSegmentName(merged, curr->name);
+    auto name = getValidName(curr->name,
+                             &Module::getElementSegmentOrNull,
+                             merged.elementSegments.size());
     maybeAdd(ModuleItemKind::ElementSegment, curr->name, name);
   }
   for (auto& curr : input.memories) {
-    auto name = Names::getValidMemoryName(merged, curr->name);
+    auto name = getValidName(
+      curr->name, &Module::getMemoryOrNull, merged.memories.size());
     maybeAdd(ModuleItemKind::Memory, curr->name, name);
   }
   for (auto& curr : input.dataSegments) {
-    auto name = Names::getValidDataSegmentName(merged, curr->name);
+    auto name = getValidName(
+      curr->name, &Module::getDataSegmentOrNull, merged.dataSegments.size());
     maybeAdd(ModuleItemKind::DataSegment, curr->name, name);
   }
   for (auto& curr : input.tables) {
-    auto name = Names::getValidTableName(merged, curr->name);
+    auto name =
+      getValidName(curr->name, &Module::getTableOrNull, merged.tables.size());
     maybeAdd(ModuleItemKind::Table, curr->name, name);
   }
 

test/lit/merge/rename-internal-conflict.wat

@@ -0,0 +1,22 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+;; RUN: wasm-merge %s first %s.second second -all -S -o - | filecheck %s
+
+;; The global and export have name collisions with the side module, but even
+;; worse, the alternative names we initially pick (adding _1) are themselves
+;; also existing names.
+
+(module
+  ;; CHECK:      (global $classVTable (mut i32) (i32.const 1))
+  (global $classVTable (mut i32) (i32.const 1))
+
+  ;; CHECK:      (global $classVTable_2 (mut i32) (i32.const 2))
+
+  ;; CHECK:      (global $classVTable_1 (mut i32) (i32.const 3))
+
+  ;; CHECK:      (export "classVTable" (global $classVTable))
+  (export "classVTable" (global $classVTable))
+)
+
+;; CHECK:      (export "classVTable_b" (global $classVTable_2))
+
+;; CHECK:      (export "classVTable_1" (global $classVTable_1))

test/lit/merge/rename-internal-conflict.wat.second

@@ -0,0 +1,7 @@
+(module
+  (global $classVTable (mut i32) (i32.const 2))
+  (global $classVTable_1 (mut i32) (i32.const 3))
+
+  (export "classVTable_b" (global $classVTable))
+  (export "classVTable_1" (global $classVTable_1))
+)