Support i8/i16 mutable arrays as public types for string interop (#5814)

Probably any array of non-reference data can be allowed to be public and sent
out of the module, as it is just data. For now, however, just special case the i8
and i16 array types which are useful already for string interop.

7 files changed, 115 insertions, 6 deletions

diff --git a/src/ir/module-utils.cpp b/src/ir/module-utils.cpp
index 2b2f51323..ed147053b 100644
--- a/src/ir/module-utils.cpp
+++ b/src/ir/module-utils.cpp
@@ -506,6 +506,11 @@ InsertOrderedSet<HeapType> getPublicTypeSet(Module& wasm) {
     WASM_UNREACHABLE("unexpected export kind");
   }
 
+  // Ignorable public types are public.
+  for (auto type : getIgnorablePublicTypes()) {
+    notePublic(type);
+  }
+
   // Find all the other public types reachable from directly publicized types.
   std::vector<HeapType> workList(publicTypes.begin(), publicTypes.end());
   while (workList.size()) {
diff --git a/src/wasm-type.h b/src/wasm-type.h
index 927e37845..d97bdeba3 100644
--- a/src/wasm-type.h
+++ b/src/wasm-type.h
@@ -661,6 +661,20 @@ struct TypeBuilder {
   void dump();
 };
 
+// We consider certain specific types to always be public, to allow closed-
+// world to operate even if they escape. Specifically, "plain old data" types
+// like array of i8 and i16, which are used to represent strings, may cross
+// the boundary in Web environments.
+//
+// These are "ignorable as public", because we do not error on them being
+// public. That is, we
+//
+//  1. Consider them public, so that passes that do not operate on public types
+//     do not in fact operate on them, and
+//  2. Are ok with them being public in the validator.
+//
+std::unordered_set<HeapType> getIgnorablePublicTypes();
+
 std::ostream& operator<<(std::ostream&, Type);
 std::ostream& operator<<(std::ostream&, Type::Printed);
 std::ostream& operator<<(std::ostream&, HeapType);
diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp
index ba38231b6..4ce72582c 100644
--- a/src/wasm/wasm-type.cpp
+++ b/src/wasm/wasm-type.cpp
@@ -2580,6 +2580,29 @@ void TypeBuilder::dump() {
   }
 }
 
+std::unordered_set<HeapType> getIgnorablePublicTypes() {
+  auto array8 = Array(Field(Field::i8, Mutable));
+  auto array16 = Array(Field(Field::i16, Mutable));
+  TypeBuilder builder(4);
+  // We handle final and non-final here, but should remove one of them
+  // eventually TODO
+  builder[0] = array8;
+  builder[0].setOpen(false);
+  builder[1] = array16;
+  builder[1].setOpen(false);
+  builder[2] = array8;
+  builder[2].setOpen(true);
+  builder[3] = array16;
+  builder[3].setOpen(true);
+  auto result = builder.build();
+  assert(result);
+  std::unordered_set<HeapType> ret;
+  for (auto type : *result) {
+    ret.insert(type);
+  }
+  return ret;
+}
+
 } // namespace wasm
 
 namespace std {
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 2d00c4b67..3baaacb16 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -3726,8 +3726,12 @@ static void validateClosedWorldInterface(Module& module, ValidationInfo& info) {
     }
   }
 
+  // Ignorable public types are public, but we can ignore them for purposes of
+  // erroring here: It is always ok that they are public.
+  auto ignorable = getIgnorablePublicTypes();
+
   for (auto type : ModuleUtils::getPublicHeapTypes(module)) {
-    if (!publicFuncTypes.count(type)) {
+    if (!publicFuncTypes.count(type) && !ignorable.count(type)) {
       auto name = type.toString();
       if (auto it = module.typeNames.find(type); it != module.typeNames.end()) {
         name = it->second.name.toString();
diff --git a/test/lit/passes/gto-mutability.wast b/test/lit/passes/gto-mutability.wast
index 28fc1bc69..ed04edccc 100644
--- a/test/lit/passes/gto-mutability.wast
+++ b/test/lit/passes/gto-mutability.wast
@@ -589,3 +589,66 @@
     (drop (struct.get $sub 0 (local.get $sub)))
   )
 )
+
+;; This pass does not refine array types yet. Verify that we do not. This is
+;; particularly important for array of i8 and i6, which we special-case as they
+;; are used for string interop even in closed world. For now, however, we do not
+;; optimize even arrays of i32.
+;;
+;; Nothing should change in these array types. They have no writes, but they'll
+;; stay mutable. Also, this test verifies that we can even validate this module
+;; in closed world, even though it contains imports of i8 and i16 arrays.
+;;
+;; The test also verifies that while we refine the mutability of a struct type,
+;; which causes us to rewrite types, that we keep the i8 and i16 arrays in
+;; their own size-1 rec groups by themselves, unmodified. The i32 array will be
+;; moved into a new big rec group, together with the struct type (that is also
+;; refined to be immutable).
+(module
+  ;; CHECK:      (type $array8 (array (mut i8)))
+  (type $array8 (array (mut i8)))
+  ;; CHECK:      (type $array16 (array (mut i16)))
+  (type $array16 (array (mut i16)))
+  ;; CHECK:      (rec
+  ;; CHECK-NEXT:  (type $struct (struct (field funcref)))
+
+  ;; CHECK:       (type $array32 (array (mut i32)))
+  (type $array32 (array (mut i32)))
+
+  (type $struct (struct (field (mut funcref))))
+
+  ;; CHECK:       (type $4 (func (param funcref)))
+
+  ;; CHECK:      (import "a" "b" (global $i8 (ref $array8)))
+  (import "a" "b" (global $i8 (ref $array8)))
+
+  ;; CHECK:      (import "a" "c" (global $i16 (ref $array16)))
+  (import "a" "c" (global $i16 (ref $array16)))
+
+  ;; CHECK:      (func $use (type $4) (param $funcref funcref)
+  ;; CHECK-NEXT:  (local $array8 (ref $array8))
+  ;; CHECK-NEXT:  (local $array16 (ref $array16))
+  ;; CHECK-NEXT:  (local $array32 (ref $array32))
+  ;; CHECK-NEXT:  (local $struct (ref $struct))
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (struct.get $struct 0
+  ;; CHECK-NEXT:    (struct.new $struct
+  ;; CHECK-NEXT:     (local.get $funcref)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $use (param $funcref funcref)
+    (local $array8 (ref $array8))
+    (local $array16 (ref $array16))
+    (local $array32 (ref $array32))
+    (local $struct (ref $struct))
+    (drop
+      (struct.get $struct 0
+        (struct.new $struct
+          (local.get $funcref)
+        )
+      )
+    )
+  )
+)
diff --git a/test/lit/passes/type-merging.wast b/test/lit/passes/type-merging.wast
index 643e28bd9..7c44fe8e2 100644
--- a/test/lit/passes/type-merging.wast
+++ b/test/lit/passes/type-merging.wast
@@ -388,17 +388,17 @@
 
   ;; CHECK:       (type $C (array i16))
 
-  ;; CHECK:       (type $B (array (mut i8)))
+  ;; CHECK:       (type $B (array (mut i32)))
 
   ;; CHECK:       (type $A (array i8))
   (type $A  (array i8))
   (type $A' (array i8))
-  (type $B  (array (mut i8)))
-  (type $B' (array (mut i8)))
   (type $C  (array i16))
   (type $C' (array i16))
   (type $D  (array i32))
   (type $D' (array i32))
+  (type $B  (array (mut i32)))
+  (type $B' (array (mut i32)))
   (type $E  (array i64))
   (type $E' (array i64))
   (type $F  (array anyref))
diff --git a/test/lit/validation/closed-world-interface.wast b/test/lit/validation/closed-world-interface.wast
index f5dd7ad46..06cf1cef7 100644
--- a/test/lit/validation/closed-world-interface.wast
+++ b/test/lit/validation/closed-world-interface.wast
@@ -9,7 +9,7 @@
 
 ;; This is pulled in by a global.
 ;; CHECK:      publicly exposed type disallowed with a closed world: $array, on
-;; CHECK-NEXT: (array (mut i8))
+;; CHECK-NEXT: (array (mut i32))
 
 ;; This is pulled in only by a global, so it is disallowed even though it is a function type.
 ;; CHECK:      publicly exposed type disallowed with a closed world: $private, on
@@ -21,7 +21,7 @@
 
 (module
   (type $struct (struct))
-  (type $array (array (mut i8)))
+  (type $array (array (mut i32)))
 
   (type $void (func))
   (type $abstract (func (param anyref)))