diff options
author | Thomas Lively <tlively@google.com> | 2022-12-20 13:41:04 -0600 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-20 19:41:04 +0000 |
commit | 8c2696b78f5658888d0d480eaddd9eed045b3e7b (patch) | |
tree | 6010ca853999c110c4530af535e07ca253c9e671 /src | |
parent | 569f789622f116177c8a1e32fb62a4e5a5c9dfe0 (diff) | |
download | binaryen-8c2696b78f5658888d0d480eaddd9eed045b3e7b.tar.gz binaryen-8c2696b78f5658888d0d480eaddd9eed045b3e7b.tar.bz2 binaryen-8c2696b78f5658888d0d480eaddd9eed045b3e7b.zip |
Work around bugs with open world type optimizations (#5367)
Since #5347 public types are never updated by type optimizations, but the
optimization passes have not yet been updated to take that into account, so they
are all buggy under an open world assumption. In #5359 we worked around many
closed world validation errors in the fuzzer by treating --closed-world like a
feature flag and checking whether it was necessary for fuzzer input, but that
did not prevent the type optimization passes from running under an open world,
so it did not work around all the potential issues.
Work around the problem more thoroughly by not running any type optimization
passes in the fuzzer without --closed-world. Also add logic to those passes to
error out if they are run without --closed-world and update the tests
accordingly.
Diffstat (limited to 'src')
-rw-r--r-- | src/ir/module-utils.cpp | 5 | ||||
-rw-r--r-- | src/passes/GlobalTypeOptimization.cpp | 4 | ||||
-rw-r--r-- | src/passes/RemoveUnusedTypes.cpp | 10 | ||||
-rw-r--r-- | src/passes/SignaturePruning.cpp | 4 | ||||
-rw-r--r-- | src/passes/TypeMerging.cpp | 4 | ||||
-rw-r--r-- | src/passes/TypeRefining.cpp | 6 |
6 files changed, 27 insertions, 6 deletions
diff --git a/src/ir/module-utils.cpp b/src/ir/module-utils.cpp index 8e6ac0d1a..f7ba7d776 100644 --- a/src/ir/module-utils.cpp +++ b/src/ir/module-utils.cpp @@ -317,11 +317,10 @@ std::vector<HeapType> getPublicHeapTypes(Module& wasm) { } std::vector<HeapType> getPrivateHeapTypes(Module& wasm) { - auto allTypes = getHeapTypeCounts(wasm, true); + auto usedTypes = getHeapTypeCounts(wasm, true); auto publicTypes = getPublicTypeSet(wasm); std::vector<HeapType> types; - types.reserve(allTypes.size() - publicTypes.size()); - for (auto& [type, _] : allTypes) { + for (auto& [type, _] : usedTypes) { if (!publicTypes.count(type)) { types.push_back(type); } diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 9015f3ab5..8ef6a3b89 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -118,6 +118,10 @@ struct GlobalTypeOptimization : public Pass { return; } + if (!getPassOptions().closedWorld) { + Fatal() << "GTO requires --closed-world"; + } + // Find and analyze struct operations inside each function. StructUtils::FunctionStructValuesMap<FieldInfo> functionNewInfos(*module), functionSetGetInfos(*module); diff --git a/src/passes/RemoveUnusedTypes.cpp b/src/passes/RemoveUnusedTypes.cpp index d346bc585..1ea65fb0f 100644 --- a/src/passes/RemoveUnusedTypes.cpp +++ b/src/passes/RemoveUnusedTypes.cpp @@ -34,6 +34,16 @@ struct RemoveUnusedTypes : Pass { // This pass only does anything with GC types. return; } + + // Consider (rec $A $unused), where anyrefs received from the outside are + // cast to `$A`. In an open world we cannot remove $unused because that + // would change the identity of $A. Currently we would incorrectly remove + // $unused. To fix that, we need to fix our collection of public types to + // consider $A (and $unused) public in an open world. + if (!getPassOptions().closedWorld) { + Fatal() << "RemoveUnusedTypes requires --closed-world"; + } + // We're not changing the contents of any of the types, so we just round // trip them throgh GlobalTypeRewriter, which will put all the private types // in a single new rec group and leave out all the unused types. diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp index 3244ee103..168ef8454 100644 --- a/src/passes/SignaturePruning.cpp +++ b/src/passes/SignaturePruning.cpp @@ -53,6 +53,10 @@ struct SignaturePruning : public Pass { return; } + if (!getPassOptions().closedWorld) { + Fatal() << "SignaturePruning requires --closed-world"; + } + if (!module->tables.empty()) { // When there are tables we must also take their types into account, which // would require us to take call_indirect, element segments, etc. into diff --git a/src/passes/TypeMerging.cpp b/src/passes/TypeMerging.cpp index d873152f2..c5028ee20 100644 --- a/src/passes/TypeMerging.cpp +++ b/src/passes/TypeMerging.cpp @@ -109,6 +109,10 @@ struct TypeMerging : public Pass { return; } + if (!getPassOptions().closedWorld) { + Fatal() << "TypeMerging requires --closed-world"; + } + // First, find all the cast types. ModuleUtils::ParallelFunctionAnalysis<ReferredTypes> analysis( diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp index 6e2c96fce..0938f6d8e 100644 --- a/src/passes/TypeRefining.cpp +++ b/src/passes/TypeRefining.cpp @@ -103,9 +103,9 @@ struct TypeRefining : public Pass { if (!module->features.hasGC()) { return; } - if (getTypeSystem() != TypeSystem::Nominal && - getTypeSystem() != TypeSystem::Isorecursive) { - Fatal() << "TypeRefining requires nominal/hybrid typing"; + + if (!getPassOptions().closedWorld) { + Fatal() << "TypeRefining requires --closed-world"; } // Find and analyze struct operations inside each function. |