From 8c2696b78f5658888d0d480eaddd9eed045b3e7b Mon Sep 17 00:00:00 2001
From: Thomas Lively <tlively@google.com>
Date: Tue, 20 Dec 2022 13:41:04 -0600
Subject: 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.
---
 src/ir/module-utils.cpp               |  5 ++---
 src/passes/GlobalTypeOptimization.cpp |  4 ++++
 src/passes/RemoveUnusedTypes.cpp      | 10 ++++++++++
 src/passes/SignaturePruning.cpp       |  4 ++++
 src/passes/TypeMerging.cpp            |  4 ++++
 src/passes/TypeRefining.cpp           |  6 +++---
 6 files changed, 27 insertions(+), 6 deletions(-)

(limited to 'src')

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.
-- 
cgit v1.2.3