From d38c27cff02fa0821e5c24a41e4577c2c21c7ce3 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 30 Jul 2026 12:37:20 -0700 Subject: [PATCH] Fix determinism bug when mapping type names When we rewrite module types, we must associate the old type names and indices with the new types they are being mapped to. The code for this was previously sensitive to the iteration order over the old-to-new type map because it modified the module's type names as it traversed them and because visitation order determined which name was kept in cases where multiple old types map to the same new type. Fix both problems by keeping the lesser names and indices in the case of merges and by not updating the module type names until after the new type names have been fully populated. --- src/ir/type-updating.cpp | 50 +++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index 069ccf4f9ca..d325e539a9a 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -405,17 +405,16 @@ void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) { } void GlobalTypeRewriter::mapTypeNamesAndIndices(const TypeMap& oldToNewTypes) { - // Track all the existing names to avoid creating duplicates. - std::unordered_set seenTypeNames; - for (auto& [type, info] : wasm.typeNames) { - seenTypeNames.insert(info.name); - } // Collect new and updated type names and indices. Do not mutate the module's // names and indices until the end to avoid iteration order affecting the // results in the case where oldToNewTypes maps old types to different old // types. std::unordered_map newTypeNames; std::unordered_map newTypeIndices; + + // Assign names and indices to new types. To avoid any dependencies on the + // iteration order, deterministically keep the lesser names or indices in the + // case of merges and do not mutate typeNames as we iterate over it. for (auto& [old, new_] : oldToNewTypes) { if (old == new_) { // The type is being mapped to itself; no need to rename anything. @@ -423,25 +422,38 @@ void GlobalTypeRewriter::mapTypeNamesAndIndices(const TypeMap& oldToNewTypes) { } if (auto it = wasm.typeNames.find(old); it != wasm.typeNames.end()) { auto& names = it->second; - newTypeNames[new_] = names; - // Use the existing name in the new type, as usually it completely - // replaces the old. Rename the old name in a unique way to avoid - // confusion in the case that it remains used. - auto deduped = Names::getValidName( - names.name, [&](Name test) { return !seenTypeNames.contains(test); }); - names.name = deduped; - // Use `insert` to avoid overwriting the entry for the old type if it has - // already appeared as a new type. - if (newTypeNames.insert({old, names}).second) { - seenTypeNames.insert(names.name); + auto [newIt, inserted] = newTypeNames.insert({new_, names}); + if (!inserted) { + if (names.name.view() < newIt->second.name.view()) { + newIt->second = names; + } } } if (auto it = wasm.typeIndices.find(old); it != wasm.typeIndices.end()) { - // It's ok if we end up with duplicate indices. Ties will be resolved in - // some arbitrary manner. - newTypeIndices[new_] = it->second; + auto [newIt, inserted] = newTypeIndices.insert({new_, it->second}); + if (!inserted) { + newIt->second = std::min(newIt->second, it->second); + } } } + + // Assign old types that have been mapped to other types unique deduplicated + // names in case they remain used despite the mapping. In cases where the old + // types are also mapped _to_, the mapping will take precedence when we do the + // merge below. + std::unordered_set seenTypeNames; + for (auto& [_, names] : wasm.typeNames) { + seenTypeNames.insert(names.name); + } + for (auto& [type, names] : wasm.typeNames) { + if (auto it = oldToNewTypes.find(type); + it != oldToNewTypes.end() && it->second != type) { + names.name = Names::getValidName(names.name, [&](Name name) { + return seenTypeNames.insert(name).second; + }); + } + } + newTypeNames.merge(wasm.typeNames); wasm.typeNames = std::move(newTypeNames); newTypeIndices.merge(wasm.typeIndices);