Fix type updating for indirect call effects

src/ir/type-updating.cpp

@@ -26,6 +26,78 @@
 
 namespace wasm {
 
+namespace {
+
+// Copy over `indirectCallEffects` when types are rewritten. When rewriting a
+// type A to type B, A will lose its effects and B will gain A's effects. If the
+// destination type already existed in the program but had no effects recorded,
+// we must assume the worst (e.g. there may have been an import of type B) and
+// clear its entry in the effects map. OTOH if the destination type is a brand
+// new type, then it can only have effects from the source type. If the source
+// type didn't exist, it must have been created by a pass sometime after
+// GlobalEffects last ran. We again assume that effects are unknown.
+std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
+updateIndirectCallEffects(
+  const Module& wasm,
+  const InsertOrderedMap<HeapType, ModuleUtils::HeapTypeInfo>& typeInfo,
+  const GlobalTypeRewriter::TypeMap& typeMap) {
+
+  std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
+    newTypeEffects;
+
+  // Types that don't already appear in the module.
+  std::unordered_set<HeapType> newTypes;
+
+  std::unordered_set<HeapType> allOldTypes;
+  for (auto [oldType, _] : typeInfo) {
+    allOldTypes.insert(oldType);
+  }
+  for (auto& [oldType, _] : wasm.indirectCallEffects) {
+    allOldTypes.insert(oldType);
+  }
+
+  for (auto oldType : allOldTypes) {
+    HeapType destType;
+    {
+      auto it = typeMap.find(oldType);
+      if (it == typeMap.end()) {
+        destType = oldType;
+      } else {
+        destType = it->second;
+      }
+    }
+
+    if (newTypes.contains(destType)) {
+      continue;
+    }
+
+    const std::shared_ptr<const EffectAnalyzer>* oldEffects =
+      find_or_null(wasm.indirectCallEffects, oldType);
+
+    if (!oldEffects) {
+      // oldType has no entry, which means its effects are explicitly unknown.
+      // Why? It's a source type in `typeMap`, so it must have appeared in
+      // the module at some point, but GlobalEffects were never computed for it,
+      // or GlobalEffects intentionally ommitted its entry because it couldn't
+      // determine its effects (e.g. if an import has that type).
+      newTypes.insert(destType);
+      newTypeEffects.erase(destType);
+      continue;
+    }
+
+    auto [it, inserted] = newTypeEffects.emplace(destType, *oldEffects);
+    if (!inserted) {
+      auto merged = std::make_shared<EffectAnalyzer>(*it->second);
+      merged->mergeIn(**oldEffects);
+      it->second = std::move(merged);
+    }
+  }
+
+  return newTypeEffects;
+}
+
+} // anonymous namespace
+
 GlobalTypeRewriter::GlobalTypeRewriter(Module& wasm, WorldMode worldMode)
   : wasm(wasm), publicGroups(wasm.features) {
   // Find the heap types that are not publicly observable. Even in a closed
@@ -212,6 +284,11 @@ GlobalTypeRewriter::rebuildTypes(std::vector<HeapType> types) {
 }
 
 void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) {
+  if (!wasm.indirectCallEffects.empty()) {
+    wasm.indirectCallEffects =
+      updateIndirectCallEffects(wasm, typeInfo, oldToNewTypes);
+  }
+
   // Replace all the old types in the module with the new ones.
   struct CodeUpdater
     : public WalkerPass<
@@ -325,35 +402,6 @@ void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) {
   for (auto& tag : wasm.tags) {
     tag->type = updater.getNew(tag->type);
   }
-
-  // Update indirect call effects per type.
-  // When A is rewritten to B, B inherits the effects of A and A loses its
-  // effects.
-  std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
-    newTypeEffects;
-
-  for (const auto& [oldType, newType] : oldToNewTypes) {
-    std::shared_ptr<const EffectAnalyzer>* oldEffects =
-      find_or_null(wasm.indirectCallEffects, oldType);
-    std::shared_ptr<const EffectAnalyzer>* targetEffects =
-      find_or_null(wasm.indirectCallEffects, newType);
-
-    if (!targetEffects) {
-      // Nothing to update, we already know nothing and assume all effects.
-      continue;
-    }
-
-    if (!oldEffects) {
-      targetEffects->reset();
-      continue;
-    }
-
-    auto merged = std::make_shared<EffectAnalyzer>(**targetEffects);
-    merged->mergeIn(**oldEffects);
-    *targetEffects = std::move(merged);
-  }
-
-  wasm.indirectCallEffects = std::move(newTypeEffects);
 }
 
 void GlobalTypeRewriter::mapTypeNamesAndIndices(const TypeMap& oldToNewTypes) {

src/passes/GlobalEffects.cpp

@@ -321,9 +321,9 @@ void mergeMaybeEffects(std::shared_ptr<EffectAnalyzer>& dest,
   dest->mergeIn(*src);
 }
 
-// Propagate effects from callees to callers transitively
-// e.g. if A -> B -> C (A calls B which calls C)
-// Then B inherits effects from C and A inherits effects from both B and C.
+// Propagate effects from callees to callers transitively and populate direct
+// and indirect call effects. e.g. if A -> B -> C (A calls B which calls C),
+// then B inherits effects from C and A inherits effects from both B and C.
 //
 // Generate SCC for the call graph, then traverse it in reverse topological
 // order processing each callee before its callers. When traversing:
@@ -335,7 +335,7 @@ void propagateEffects(
   const PassOptions& passOptions,
   std::map<Function*, FuncInfo>& funcInfos,
   std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>&
-    typeEffects,
+    indirectCallEffects,
   const CallGraph& callGraph) {
   // We only care about Functions that are roots, not types.
   // A type would be a root if a function exists with that type, but no-one
@@ -435,8 +435,8 @@ void propagateEffects(
     // Assign each function's effects to its CC effects.
     for (auto node : cc) {
       std::visit(overloaded{[&](HeapType type) {
-                              if (ccEffects != UnknownEffects) {
-                                typeEffects[type] = ccEffects;
+                              if (ccEffects) {
+                                indirectCallEffects[type] = ccEffects;
                               }
                             },
                             [&](Function* f) { f->effects = ccEffects; }},
@@ -455,6 +455,7 @@ struct GenerateGlobalEffects : public Pass {
     auto callGraph = buildCallGraph(
       *module, funcInfos, referencedFuncs, getPassOptions().worldMode);
 
+    module->indirectCallEffects.clear();
     propagateEffects(*module,
                      getPassOptions(),
                      funcInfos,
@@ -468,6 +469,7 @@ struct DiscardGlobalEffects : public Pass {
     for (auto& func : module->functions) {
       func->effects.reset();
     }
+    module->indirectCallEffects.clear();
   }
 };
 

src/passes/pass.cpp

@@ -1082,6 +1082,11 @@ void PassRunner::handleAfterEffects(Pass* pass, Function* func) {
   // Binaryen IR is modified, so we may have work here.
 
   if (!func) {
+    if (pass->addsEffects()) {
+      // Indirect call effects are now under-approximating. Clear them to avoid
+      // incorrect optimizations.
+      wasm->indirectCallEffects.clear();
+    }
     // If no function is provided, then this is not a function-parallel pass,
     // and it may have operated on any of the functions in theory, so run on
     // them all.

src/wasm.h

@@ -2727,6 +2727,9 @@ class Module {
   Name name;
 
   std::unordered_map<HeapType, TypeNames> typeNames;
+
+  // The source binary's type indices. Used in some cases for preserving
+  // ordering of types.
   std::unordered_map<HeapType, Index> typeIndices;
 
   // Potential effects for bodies of indirect calls to this type. Populated by
@@ -2743,6 +2746,7 @@ class Module {
   // This data is only meaningful for indirect calls. If no indirect call
   // exists to a function, the data can be out of date (no effort is made to
   // clean up the data if e.g. all indirect calls to a function are removed).
+  //
   // TODO: Account for exactness here.
   std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
     indirectCallEffects;

src/wasm/wasm-type.cpp

@@ -2764,24 +2764,15 @@ std::unordered_set<HeapType> getIgnorablePublicTypes() {
 
 namespace HeapTypes {
 
-HeapType getMutI8Array() {
-  static HeapType i8Array = Array(Field(Field::i8, Mutable));
-  return i8Array;
-}
+HeapType getMutI8Array() { return Array(Field(Field::i8, Mutable)); }
 
-HeapType getMutI16Array() {
-  static HeapType i16Array = Array(Field(Field::i16, Mutable));
-  return i16Array;
-}
+HeapType getMutI16Array() { return Array(Field(Field::i16, Mutable)); }
 
 } // namespace HeapTypes
 
 namespace Types {
 
-Type getI64Pair() {
-  static Type i64Pair({Type::i64, Type::i64});
-  return i64Pair;
-}
+Type getI64Pair() { return Type({Type::i64, Type::i64}); }
 
 } // namespace Types
 

src/wasm/wasm.cpp

@@ -812,17 +812,15 @@ void WideIntAddSub::finalize() {
       rightHigh->type == Type::unreachable) {
     type = Type::unreachable;
   } else {
-    static Type i64Pair = Types::getI64Pair();
-    type = i64Pair;
+    type = Types::getI64Pair();
   }
 }
 
 void WideIntMul::finalize() {
   if (left->type == Type::unreachable || right->type == Type::unreachable) {
     type = Type::unreachable;
   } else {
-    static Type i64Pair = Types::getI64Pair();
-    type = i64Pair;
+    type = Types::getI64Pair();
   }
 }
 

test/gtest/CMakeLists.txt

@@ -35,6 +35,7 @@ set(unittest_SOURCES
   suffix_tree.cpp
   topological-sort.cpp
   type-builder.cpp
+  type-updating.cpp
   wat-lexer.cpp
   validator.cpp
   source-map.cpp
@@ -57,5 +58,5 @@ if(BUILD_FUZZTEST)
   target_link_libraries(binaryen-unittests PRIVATE gmock)
   gtest_discover_tests(binaryen-unittests)
 else()
-  target_link_libraries(binaryen-unittests PRIVATE gtest gtest_main)
+  target_link_libraries(binaryen-unittests PRIVATE gtest gtest_main gmock)
 endif()

test/gtest/matchers/effects.h

@@ -0,0 +1,51 @@
+/*
+ * Copyright 2026 WebAssembly Community Group participants
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Matchers for EffectAnalyzer
+
+#ifndef WASM_TEST_GTEST_MATCHERS_EFFECTS_H
+#define WASM_TEST_GTEST_MATCHERS_EFFECTS_H
+
+#include "gmock/gmock.h"
+
+namespace wasm {
+
+MATCHER(BranchesOut, "") { return arg->branchesOut; }
+MATCHER(Calls, "") { return arg->calls; }
+MATCHER(ReadsMemory, "") { return arg->readsMemory; }
+MATCHER(WritesMemory, "") { return arg->writesMemory; }
+MATCHER(ReadsSharedMemory, "") { return arg->readsSharedMemory; }
+MATCHER(WritesSharedMemory, "") { return arg->writesSharedMemory; }
+MATCHER(ReadsTable, "") { return arg->readsTable; }
+MATCHER(WritesTable, "") { return arg->writesTable; }
+MATCHER(ReadsMutableStruct, "") { return arg->readsMutableStruct; }
+MATCHER(WritesStruct, "") { return arg->writesStruct; }
+MATCHER(ReadsSharedMutableStruct, "") { return arg->readsSharedMutableStruct; }
+MATCHER(WritesSharedStruct, "") { return arg->writesSharedStruct; }
+MATCHER(ReadsMutableArray, "") { return arg->readsMutableArray; }
+MATCHER(WritesArray, "") { return arg->writesArray; }
+MATCHER(ReadsSharedMutableArray, "") { return arg->readsSharedMutableArray; }
+MATCHER(WritesSharedArray, "") { return arg->writesSharedArray; }
+MATCHER(Traps, "") { return arg->trap; }
+MATCHER(ImplicitTraps, "") { return arg->implicitTrap; }
+MATCHER(Throws, "") { return arg->throws_; }
+MATCHER(DanglingPop, "") { return arg->danglingPop; }
+MATCHER(MayNotReturn, "") { return arg->mayNotReturn; }
+MATCHER(HasReturnCallThrow, "") { return arg->hasReturnCallThrow; }
+
+} // namespace wasm
+
+#endif // WASM_TEST_GTEST_MATCHERS_EFFECTS_H