[Wasm GC] Do not refine types of exported globals in closed world (#5380)

Doing so can cause us to switch from a private type to a public type and error.

Also refactor export-utils to make this easy.

5 files changed, 146 insertions, 11 deletions

diff --git a/src/ir/CMakeLists.txt b/src/ir/CMakeLists.txt
index 2dadb743f..15aa74053 100644
--- a/src/ir/CMakeLists.txt
+++ b/src/ir/CMakeLists.txt
@@ -4,6 +4,7 @@ set(ir_SOURCES
   ExpressionManipulator.cpp
   drop.cpp
   eh-utils.cpp
+  export-utils.cpp
   intrinsics.cpp
   lubs.cpp
   memory-utils.cpp
diff --git a/src/ir/export-utils.cpp b/src/ir/export-utils.cpp
new file mode 100644
index 000000000..0b19b7204
--- /dev/null
+++ b/src/ir/export-utils.cpp
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2023 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 "ir/export-utils.h"
+#include "wasm.h"
+
+namespace wasm::ExportUtils {
+
+namespace {
+
+// Returns a vector of all the things that are exported of a particular kind,
+// given that kind and a way to look them up on the module.
+template<typename T, ExternalKind K, typename G>
+std::vector<T*> getExportedByKind(Module& wasm, G getModuleItem) {
+  std::vector<T*> ret;
+  for (auto& ex : wasm.exports) {
+    if (ex->kind == K) {
+      ret.push_back(getModuleItem(ex->value));
+    }
+  }
+  return ret;
+}
+
+} // anonymous namespace
+
+std::vector<Function*> getExportedFunctions(Module& wasm) {
+  return getExportedByKind<Function, ExternalKind::Function>(
+    wasm, [&](Name name) { return wasm.getFunction(name); });
+}
+
+std::vector<Global*> getExportedGlobals(Module& wasm) {
+  return getExportedByKind<Global, ExternalKind::Global>(
+    wasm, [&](Name name) { return wasm.getGlobal(name); });
+}
+
+} // namespace wasm::ExportUtils
diff --git a/src/ir/export-utils.h b/src/ir/export-utils.h
index 245365fbc..f50c0afa5 100644
--- a/src/ir/export-utils.h
+++ b/src/ir/export-utils.h
@@ -21,15 +21,8 @@
 
 namespace wasm::ExportUtils {
 
-inline std::vector<Function*> getExportedFunctions(Module& wasm) {
-  std::vector<Function*> ret;
-  for (auto& ex : wasm.exports) {
-    if (ex->kind == ExternalKind::Function) {
-      ret.push_back(wasm.getFunction(ex->value));
-    }
-  }
-  return ret;
-}
+std::vector<Function*> getExportedFunctions(Module& wasm);
+std::vector<Global*> getExportedGlobals(Module& wasm);
 
 } // namespace wasm::ExportUtils
 
diff --git a/src/passes/GlobalRefining.cpp b/src/passes/GlobalRefining.cpp
index 67a186f92..129f34283 100644
--- a/src/passes/GlobalRefining.cpp
+++ b/src/passes/GlobalRefining.cpp
@@ -18,6 +18,7 @@
 // Apply more specific subtypes to global variables where possible.
 //
 
+#include "ir/export-utils.h"
 #include "ir/find_all.h"
 #include "ir/lubs.h"
 #include "ir/module-utils.h"
@@ -63,10 +64,20 @@ struct GlobalRefining : public Pass {
       }
     }
 
+    // In closed world we cannot change the types of exports, as we might change
+    // from a public type to a private that would cause a validation error.
+    // TODO We could refine to a type that is still public, however.
+    std::unordered_set<Name> unoptimizable;
+    if (getPassOptions().closedWorld) {
+      for (auto* global : ExportUtils::getExportedGlobals(*module)) {
+        unoptimizable.insert(global->name);
+      }
+    }
+
     bool optimized = false;
 
     for (auto& global : module->globals) {
-      if (global->imported()) {
+      if (global->imported() || unoptimizable.count(global->name)) {
         continue;
       }
 
diff --git a/test/lit/passes/global-refining.wast b/test/lit/passes/global-refining.wast
index 06af50bf1..980194c91 100644
--- a/test/lit/passes/global-refining.wast
+++ b/test/lit/passes/global-refining.wast
@@ -1,20 +1,28 @@
 ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
-;; RUN: foreach %s %t wasm-opt --nominal --global-refining -all -S -o - | filecheck %s
+
+;; RUN: foreach %s %t wasm-opt --nominal --global-refining                -all -S -o - | filecheck %s
+;; RUN: foreach %s %t wasm-opt --nominal --global-refining --closed-world -all -S -o - | filecheck %s --check-prefix=CLOSD
 
 (module
   ;; Globals with no assignments aside from their initial values. The first is a
   ;; null, so we can optimize to a nullfuncref. The second is a ref.func which
   ;; lets us refine to the specific function type.
   ;; CHECK:      (type $foo_t (func))
+  ;; CLOSD:      (type $foo_t (func))
   (type $foo_t (func))
 
   ;; CHECK:      (global $func-null-init (mut nullfuncref) (ref.null nofunc))
+  ;; CLOSD:      (global $func-null-init (mut nullfuncref) (ref.null nofunc))
   (global $func-null-init (mut funcref) (ref.null $foo_t))
   ;; CHECK:      (global $func-func-init (mut (ref $foo_t)) (ref.func $foo))
+  ;; CLOSD:      (global $func-func-init (mut (ref $foo_t)) (ref.func $foo))
   (global $func-func-init (mut funcref) (ref.func $foo))
   ;; CHECK:      (func $foo (type $foo_t)
   ;; CHECK-NEXT:  (nop)
   ;; CHECK-NEXT: )
+  ;; CLOSD:      (func $foo (type $foo_t)
+  ;; CLOSD-NEXT:  (nop)
+  ;; CLOSD-NEXT: )
   (func $foo (type $foo_t))
 )
 
@@ -23,11 +31,14 @@
   ;; init will update the null to allow it to refine.
 
   ;; CHECK:      (type $foo_t (func))
+  ;; CLOSD:      (type $foo_t (func))
   (type $foo_t (func))
 
   ;; CHECK:      (global $func-null-init (mut nullfuncref) (ref.null nofunc))
+  ;; CLOSD:      (global $func-null-init (mut nullfuncref) (ref.null nofunc))
   (global $func-null-init (mut funcref) (ref.null $foo_t))
   ;; CHECK:      (global $func-func-init (mut (ref null $foo_t)) (ref.func $foo))
+  ;; CLOSD:      (global $func-func-init (mut (ref null $foo_t)) (ref.func $foo))
   (global $func-func-init (mut funcref) (ref.func $foo))
 
   ;; CHECK:      (func $foo (type $foo_t)
@@ -38,6 +49,14 @@
   ;; CHECK-NEXT:   (ref.null nofunc)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
+  ;; CLOSD:      (func $foo (type $foo_t)
+  ;; CLOSD-NEXT:  (global.set $func-null-init
+  ;; CLOSD-NEXT:   (ref.null nofunc)
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT:  (global.set $func-func-init
+  ;; CLOSD-NEXT:   (ref.null nofunc)
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT: )
   (func $foo (type $foo_t)
    (global.set $func-null-init (ref.null func))
    (global.set $func-func-init (ref.null $foo_t))
@@ -51,8 +70,12 @@
   ;; CHECK:      (type $none_=>_none (func))
 
   ;; CHECK:      (global $func-null-init (mut (ref null $none_=>_none)) (ref.null nofunc))
+  ;; CLOSD:      (type $none_=>_none (func))
+
+  ;; CLOSD:      (global $func-null-init (mut (ref null $none_=>_none)) (ref.null nofunc))
   (global $func-null-init (mut funcref) (ref.null func))
   ;; CHECK:      (global $func-func-init (mut (ref $none_=>_none)) (ref.func $foo))
+  ;; CLOSD:      (global $func-func-init (mut (ref $none_=>_none)) (ref.func $foo))
   (global $func-func-init (mut funcref) (ref.func $foo))
 
   ;; CHECK:      (elem declare func $foo)
@@ -65,6 +88,16 @@
   ;; CHECK-NEXT:   (ref.func $foo)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
+  ;; CLOSD:      (elem declare func $foo)
+
+  ;; CLOSD:      (func $foo (type $none_=>_none)
+  ;; CLOSD-NEXT:  (global.set $func-null-init
+  ;; CLOSD-NEXT:   (ref.func $foo)
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT:  (global.set $func-func-init
+  ;; CLOSD-NEXT:   (ref.func $foo)
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT: )
   (func $foo
    (global.set $func-null-init (ref.func $foo))
    (global.set $func-func-init (ref.func $foo))
@@ -78,10 +111,14 @@
   ;; CHECK:      (type $none_=>_none (func))
 
   ;; CHECK:      (type $struct (struct ))
+  ;; CLOSD:      (type $none_=>_none (func))
+
+  ;; CLOSD:      (type $struct (struct ))
   (type $struct (struct))
   (type $array (array i8))
 
   ;; CHECK:      (global $global (mut eqref) (ref.null none))
+  ;; CLOSD:      (global $global (mut eqref) (ref.null none))
   (global $global (mut anyref) (ref.null any))
 
   ;; CHECK:      (func $foo (type $none_=>_none)
@@ -103,6 +140,25 @@
   ;; CHECK-NEXT:   (ref.null none)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
+  ;; CLOSD:      (func $foo (type $none_=>_none)
+  ;; CLOSD-NEXT:  (global.set $global
+  ;; CLOSD-NEXT:   (i31.new
+  ;; CLOSD-NEXT:    (i32.const 0)
+  ;; CLOSD-NEXT:   )
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT:  (global.set $global
+  ;; CLOSD-NEXT:   (struct.new_default $struct)
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT:  (global.set $global
+  ;; CLOSD-NEXT:   (ref.null none)
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT:  (global.set $global
+  ;; CLOSD-NEXT:   (ref.null none)
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT:  (global.set $global
+  ;; CLOSD-NEXT:   (ref.null none)
+  ;; CLOSD-NEXT:  )
+  ;; CLOSD-NEXT: )
   (func $foo
    (global.set $global (i31.new (i32.const 0)))
    (global.set $global (struct.new_default $struct))
@@ -112,3 +168,28 @@
    (global.set $global (ref.null $array))
   )
 )
+
+;; We can refine here, but as it is an export we only do so in open world.
+(module
+  ;; CHECK:      (type $none_=>_none (func))
+
+  ;; CHECK:      (global $func-init (mut (ref $none_=>_none)) (ref.func $foo))
+  ;; CLOSD:      (type $none_=>_none (func))
+
+  ;; CLOSD:      (global $func-init (mut funcref) (ref.func $foo))
+  (global $func-init (mut funcref) (ref.func $foo))
+
+  ;; CHECK:      (export "global" (global $func-init))
+  ;; CLOSD:      (export "global" (global $func-init))
+  (export "global" (global $func-init))
+
+  ;; CHECK:      (func $foo (type $none_=>_none)
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT: )
+  ;; CLOSD:      (func $foo (type $none_=>_none)
+  ;; CLOSD-NEXT:  (nop)
+  ;; CLOSD-NEXT: )
+  (func $foo
+    (nop)
+  )
+)