diff --git a/src/ir/module-utils.h b/src/ir/module-utils.h index f232b76e025..ad2b3a523e0 100644 --- a/src/ir/module-utils.h +++ b/src/ir/module-utils.h @@ -76,6 +76,12 @@ void renameFunction(Module& wasm, Name oldName, Name newName); // Convenient iteration over imported/non-imported module elements +inline void iterTypes(const Module& wasm, auto&& visitor) { + for (const auto& [type, _] : wasm.typeIndices) { + visitor(type); + } +} + inline void iterImportedMemories(const Module& wasm, auto&& visitor) { for (auto& import : wasm.memories) { if (import->imported()) { diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index ee7cf7d3e33..9e1a1ffb736 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -26,6 +26,56 @@ namespace wasm { +namespace { + +std::unordered_map> +updateIndirectCallEffects(const Module& wasm, + const std::vector& oldTypes, + const GlobalTypeRewriter::TypeMap& oldToNewTypes) { + std::unordered_map> + newTypeEffects; + + std::unordered_set unknownNewTypes; + + for (HeapType oldType : oldTypes) { + HeapType newType; + { + auto it = oldToNewTypes.find(oldType); + if (it == oldToNewTypes.end()) { + newType = oldType; + } else { + newType = it->second; + } + } + + if (unknownNewTypes.count(newType)) { + continue; + } + + const std::shared_ptr* oldEffects = + find_or_null(wasm.indirectCallEffects, oldType); + + if (!oldEffects) { + // oldType is an existing type and has no entry, which means its + // effects are explicitly UNKNOWN. + unknownNewTypes.insert(newType); + newTypeEffects.erase(newType); + continue; + } + + auto [it, inserted] = newTypeEffects.emplace(newType, *oldEffects); + if (!inserted) { + auto merged = std::make_shared(*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 +262,15 @@ GlobalTypeRewriter::rebuildTypes(std::vector types) { } void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) { + if (!wasm.indirectCallEffects.empty()) { + // Update indirect call effects per type. + // When A is rewritten to B, B inherits the effects of A and A loses its + // effects. + std::vector oldTypes = ModuleUtils::collectHeapTypes(wasm); + wasm.indirectCallEffects = + updateIndirectCallEffects(wasm, oldTypes, oldToNewTypes); + } + // Replace all the old types in the module with the new ones. struct CodeUpdater : public WalkerPass< @@ -325,35 +384,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> - newTypeEffects; - - for (const auto& [oldType, newType] : oldToNewTypes) { - std::shared_ptr* oldEffects = - find_or_null(wasm.indirectCallEffects, oldType); - std::shared_ptr* 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(**targetEffects); - merged->mergeIn(**oldEffects); - *targetEffects = std::move(merged); - } - - wasm.indirectCallEffects = std::move(newTypeEffects); } void GlobalTypeRewriter::mapTypeNamesAndIndices(const TypeMap& oldToNewTypes) { diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp index efcc45eadd7..f3313700e8e 100644 --- a/src/passes/GlobalEffects.cpp +++ b/src/passes/GlobalEffects.cpp @@ -321,9 +321,9 @@ void mergeMaybeEffects(std::shared_ptr& 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& funcInfos, std::unordered_map>& - 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(); } }; diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 06339ca5a6b..178b9605cc8 100644 --- a/src/passes/pass.cpp +++ b/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. diff --git a/src/wasm.h b/src/wasm.h index f5cade544e6..e7b32ba60c2 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2727,6 +2727,9 @@ class Module { Name name; std::unordered_map typeNames; + + // The source binary's type indicies. Used in some cases for preserving + // ordering of types std::unordered_map typeIndices; // Potential effects for bodies of indirect calls to this type. Populated by @@ -2743,6 +2746,13 @@ 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). + // + // A missing key in the map means that the type's effects are explicitly + // unknown. When rewriting types, we distinguish new types from existing + // types by checking whether they existed before the update. If a new type + // is created, it inherits the effects of the old types that map to it. If + // an existing type with unknown effects is mapped, the resulting type will + // also have explicitly unknown effects. // TODO: Account for exactness here. std::unordered_map> indirectCallEffects; diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index ae4983daaff..4951892abac 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -2764,24 +2764,15 @@ std::unordered_set 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 diff --git a/test/gtest/CMakeLists.txt b/test/gtest/CMakeLists.txt index 0953df45247..18c9464dadf 100644 --- a/test/gtest/CMakeLists.txt +++ b/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() diff --git a/test/gtest/matchers/effects.h b/test/gtest/matchers/effects.h new file mode 100644 index 00000000000..34d4544c18e --- /dev/null +++ b/test/gtest/matchers/effects.h @@ -0,0 +1,49 @@ +/* + * 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. + */ + +#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 diff --git a/test/gtest/type-updating.cpp b/test/gtest/type-updating.cpp new file mode 100644 index 00000000000..025217e9d3d --- /dev/null +++ b/test/gtest/type-updating.cpp @@ -0,0 +1,182 @@ +/* + * 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. + */ + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "ir/effects.h" +#include "ir/type-updating.h" +#include "matchers/effects.h" +#include "parser/wat-parser.h" +#include "support/result.h" +#include "wasm.h" + +using namespace wasm; +using namespace testing; + +namespace { + +class IndirectCallEffectsTest : public Test { +protected: + Module wasm; + PassOptions options; + + void SetUp() override { + auto moduleText = R"wasm( + (module + (rec + (type $A (func)) + (type $B (func)) + (type $C (func)) + (type $D (func)) + ) + (func $f_a (type $A)) + (func $f_b (type $B)) + (func $f_c (type $C)) + (func $f_d (type $D)) + ) + )wasm"; + + if (auto err = wasm::WATParser::parseModule(wasm, moduleText).getErr()) { + Fatal() << err->msg; + } + } + + std::unordered_map> + updateEffects( + const std::unordered_map>& oldEffects, + const std::unordered_map& typeMap) { + + std::unordered_map types; + std::unordered_map typeNames; + for (const auto& [type, name] : wasm.typeNames) { + types[name.name.toString()] = type; + typeNames[type] = name.name.toString(); + } + + // Type that previously didn't exist in the module. + // This is a new type created by GlobalTypeRewriter. + { + TypeBuilder builder(1); + builder.setHeapType(0, Signature(Type::i32, Type::i32)); + HeapType newType = (*builder.build())[0]; + types["new type"] = newType; + typeNames[newType] = "new type"; + } + + for (const auto& [name, effects] : oldEffects) { + wasm.indirectCallEffects[types.at(name)] = effects; + } + + GlobalTypeRewriter::TypeMap map; + for (const auto& [oldName, newName] : typeMap) { + HeapType oldType = types.at(oldName); + map[oldType] = types.at(newName); + } + + GlobalTypeRewriter rewriter(wasm, WorldMode::Open); + rewriter.mapTypes(map); + + std::unordered_map> + result; + for (const auto& [type, effects] : wasm.indirectCallEffects) { + auto name = typeNames.at(type); + result[name] = effects; + } + return result; + } +}; + +} // anonymous namespace + +TEST_F(IndirectCallEffectsTest, SrcHasUnknownEffects) { + auto effectsB = std::make_shared(options, wasm); + effectsB->writesMemory = true; + + auto merged = + updateEffects(/*oldEffects=*/{{"B", effectsB}}, /*typeMap=*/{{"A", "B"}}); + + EXPECT_THAT(merged, IsEmpty()); +} + +TEST_F(IndirectCallEffectsTest, DestHasUnknownEffects) { + auto effectsA = std::make_shared(options, wasm); + effectsA->calls = true; + + auto merged = + updateEffects(/*oldEffects=*/{{"A", effectsA}}, /*typeMap=*/{{"A", "B"}}); + + EXPECT_THAT(merged, IsEmpty()); +} + +TEST_F(IndirectCallEffectsTest, BothHaveEffects) { + auto effectsA = std::make_shared(options, wasm); + effectsA->calls = true; + auto effectsB = std::make_shared(options, wasm); + effectsB->writesMemory = true; + + auto merged = updateEffects(/*oldEffects=*/{{"A", effectsA}, {"B", effectsB}}, + /*typeMap=*/{{"A", "B"}}); + + EXPECT_THAT(merged, + UnorderedElementsAre(Pair("B", AllOf(Calls(), WritesMemory())))); +} + +TEST_F(IndirectCallEffectsTest, MapToNewType) { + auto effectsA = std::make_shared(options, wasm); + effectsA->calls = true; + + auto merged = updateEffects(/*oldEffects=*/{{"A", effectsA}}, + /*typeMap=*/{{"A", "new type"}}); + + // Pointer comparison + EXPECT_THAT(merged, UnorderedElementsAre(Pair("new type", effectsA))); +} + +TEST_F(IndirectCallEffectsTest, MapToNewTypeNoEffects) { + auto merged = updateEffects(/*oldEffects=*/{}, + /*typeMap=*/{{"A", "new type"}}); + + EXPECT_THAT(merged, IsEmpty()); +} + +TEST_F(IndirectCallEffectsTest, MergeNewTypeAndExisting) { + auto effectsB = std::make_shared(options, wasm); + effectsB->writesMemory = true; + + auto merged = + updateEffects(/*oldEffects=*/{{"B", effectsB}}, + /*typeMap=*/{{"A", "new type"}, {"B", "new type"}}); + + // Type A already had unknown effects, so the new type remains unknown. + EXPECT_THAT(merged, IsEmpty()); +} + +TEST_F(IndirectCallEffectsTest, MergeNewTypeAndExistingWithEffects) { + auto effectsA = std::make_shared(options, wasm); + effectsA->calls = true; + auto effectsB = std::make_shared(options, wasm); + effectsB->writesMemory = true; + + auto merged = + updateEffects(/*oldEffects=*/{{"A", effectsA}, {"B", effectsB}}, + /*typeMap=*/{{"A", "new type"}, {"B", "new type"}}); + + EXPECT_THAT( + merged, + UnorderedElementsAre(Pair("new type", AllOf(Calls(), WritesMemory())))); +} diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt index 5c77330e4fa..d704d2c7d68 100644 --- a/third_party/CMakeLists.txt +++ b/third_party/CMakeLists.txt @@ -4,11 +4,17 @@ elseif(BUILD_TESTS) # fuzztest includes gtest, but if we're not building fuzztest, build gtest ourselves. add_library(gtest STATIC googletest/googletest/src/gtest-all.cc) add_library(gtest_main STATIC googletest/googletest/src/gtest_main.cc) + add_library(gmock STATIC googletest/googlemock/src/gmock-all.cc) + add_library(gmock_main STATIC googletest/googlemock/src/gmock_main.cc) target_compile_options(gtest PRIVATE "-fno-rtti") target_compile_options(gtest_main PRIVATE "-fno-rtti") + target_compile_options(gmock PRIVATE "-fno-rtti") + target_compile_options(gmock_main PRIVATE "-fno-rtti") include_directories( googletest/googletest googletest/googletest/include + googletest/googlemock + googletest/googlemock/include ) endif()