GlobalRefining: Do not refine mutable exported globals (#7007)

A mutable exported global might be shared with another module which
writes to it using the current type, which is unsafe and the type system does
not allow, so do not refine there.

2 files changed, 40 insertions, 27 deletions

diff --git a/src/passes/GlobalRefining.cpp b/src/passes/GlobalRefining.cpp
index 1313421d5..4ef6252b5 100644
--- a/src/passes/GlobalRefining.cpp
+++ b/src/passes/GlobalRefining.cpp
@@ -67,9 +67,16 @@ 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.
+    //
+    // We are also limited in open world: in that mode we must assume that
+    // another module might import our exported globals with the current type
+    // (that type is a contract between them), and in such a case the type of
+    // mutable globals must match precisely (the same rule as for mutable struct
+    // fields in subtypes - the types must match exactly, or else a write in
+    // one place could store a type considered in valid in another place).
     std::unordered_set<Name> unoptimizable;
-    if (getPassOptions().closedWorld) {
-      for (auto* global : ExportUtils::getExportedGlobals(*module)) {
+    for (auto* global : ExportUtils::getExportedGlobals(*module)) {
+      if (getPassOptions().closedWorld || global->mutable_) {
         unoptimizable.insert(global->name);
       }
     }
diff --git a/test/lit/passes/global-refining.wast b/test/lit/passes/global-refining.wast
index f1f6048f7..1ede2009e 100644
--- a/test/lit/passes/global-refining.wast
+++ b/test/lit/passes/global-refining.wast
@@ -169,31 +169,6 @@
   )
 )
 
-;; We can refine here, but as it is an export we only do so in open world.
-(module
-  ;; CHECK:      (type $0 (func))
-
-  ;; CHECK:      (global $func-init (mut (ref $0)) (ref.func $foo))
-  ;; CLOSD:      (type $0 (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 $0)
-  ;; CHECK-NEXT:  (nop)
-  ;; CHECK-NEXT: )
-  ;; CLOSD:      (func $foo (type $0)
-  ;; CLOSD-NEXT:  (nop)
-  ;; CLOSD-NEXT: )
-  (func $foo
-    (nop)
-  )
-)
-
 ;; We can refine $a, after which we should update the global.get in the other
 ;; global, or else we'd error on validation.
 ;; TODO: we could optimize further here and refine the type of the global $b.
@@ -221,3 +196,34 @@
   (func $func (type $sub)
   )
 )
+
+;; Test all combinations of being exported and being mutable.
+;;
+;; Mutability limits our ability to optimize in open world: mutable globals that
+;; are exported cannot be refined, as they might be modified in another module
+;; using the old type. In closed world, however, we can optimize both globals
+;; here, as mutability is not a concern. As a result, we can refine the
+;; (ref null func) to nullfuncref only when not exported, and if exported, then
+;; only when immutable in open world.
+(module
+  ;; CHECK:      (global $mut (mut nullfuncref) (ref.null nofunc))
+  ;; CLOSD:      (global $mut (mut nullfuncref) (ref.null nofunc))
+  (global $mut (mut (ref null func)) (ref.null nofunc))
+  ;; CHECK:      (global $imm nullfuncref (ref.null nofunc))
+  ;; CLOSD:      (global $imm nullfuncref (ref.null nofunc))
+  (global $imm (ref null func) (ref.null nofunc))
+  ;; CHECK:      (global $mut-exp (mut funcref) (ref.null nofunc))
+  ;; CLOSD:      (global $mut-exp (mut funcref) (ref.null nofunc))
+  (global $mut-exp (mut (ref null func)) (ref.null nofunc))
+  ;; CHECK:      (global $imm-exp nullfuncref (ref.null nofunc))
+  ;; CLOSD:      (global $imm-exp funcref (ref.null nofunc))
+  (global $imm-exp (ref null func) (ref.null nofunc))
+
+  ;; CHECK:      (export "mut-exp" (global $mut-exp))
+  ;; CLOSD:      (export "mut-exp" (global $mut-exp))
+  (export "mut-exp" (global $mut-exp))
+  ;; CHECK:      (export "imm-exp" (global $imm-exp))
+  ;; CLOSD:      (export "imm-exp" (global $imm-exp))
+  (export "imm-exp" (global $imm-exp))
+)
+