[Strings] Be stricter when checking builtin types (#7829)

String builtins should not only have the expected signatures, they
should also have the expected types. For example, if a builtin has a
signature that is in a nontrivial rec group or has a supertype or is not
final, that should be a validation error. Reject such incorrect types in
StringLifting.

Author: tlively

src/passes/StringLifting.cpp

@@ -131,66 +131,64 @@ struct StringLifting : public Pass {
       if (!func->imported() || func->module != WasmStringsModule) {
         continue;
       }
-      auto sig = func->type.getSignature();
+      auto type = func->type;
       if (func->base == "fromCharCodeArray") {
-        if (sig != Signature({array16, i32, i32}, refExtern)) {
-          Fatal() << "StringLifting: bad signature for fromCharCodeArray: "
-                  << sig;
+        if (type != Signature({array16, i32, i32}, refExtern)) {
+          Fatal() << "StringLifting: bad type for fromCharCodeArray: " << type;
         }
         fromCharCodeArrayImport = func->name;
         found = true;
       } else if (func->base == "fromCodePoint") {
-        if (sig != Signature(i32, refExtern)) {
-          Fatal() << "StringLifting: bad signature for fromCodePoint: " << sig;
+        if (type != Signature(i32, refExtern)) {
+          Fatal() << "StringLifting: bad type for fromCodePoint: " << type;
         }
         fromCodePointImport = func->name;
         found = true;
       } else if (func->base == "concat") {
-        if (sig != Signature({externref, externref}, refExtern)) {
-          Fatal() << "StringLifting: bad signature for concta: " << sig;
+        if (type != Signature({externref, externref}, refExtern)) {
+          Fatal() << "StringLifting: bad type for concat: " << type;
         }
         concatImport = func->name;
         found = true;
       } else if (func->base == "intoCharCodeArray") {
-        if (sig != Signature({externref, array16, i32}, i32)) {
-          Fatal() << "StringLifting: bad signature for intoCharCodeArray: "
-                  << sig;
+        if (type != Signature({externref, array16, i32}, i32)) {
+          Fatal() << "StringLifting: bad type for intoCharCodeArray: " << type;
         }
         intoCharCodeArrayImport = func->name;
         found = true;
       } else if (func->base == "equals") {
-        if (sig != Signature({externref, externref}, i32)) {
-          Fatal() << "StringLifting: bad signature for equals: " << sig;
+        if (type != Signature({externref, externref}, i32)) {
+          Fatal() << "StringLifting: bad type for equals: " << type;
         }
         equalsImport = func->name;
         found = true;
       } else if (func->base == "test") {
-        if (sig != Signature({externref}, i32)) {
-          Fatal() << "StringLifting: bad signature for test: " << sig;
+        if (type != Signature({externref}, i32)) {
+          Fatal() << "StringLifting: bad type for test: " << type;
         }
         testImport = func->name;
         found = true;
       } else if (func->base == "compare") {
-        if (sig != Signature({externref, externref}, i32)) {
-          Fatal() << "StringLifting: bad signature for compare: " << sig;
+        if (type != Signature({externref, externref}, i32)) {
+          Fatal() << "StringLifting: bad type for compare: " << type;
         }
         compareImport = func->name;
         found = true;
       } else if (func->base == "length") {
-        if (sig != Signature({externref}, i32)) {
-          Fatal() << "StringLifting: bad signature for length: " << sig;
+        if (type != Signature({externref}, i32)) {
+          Fatal() << "StringLifting: bad type for length: " << type;
         }
         lengthImport = func->name;
         found = true;
       } else if (func->base == "charCodeAt") {
-        if (sig != Signature({externref, i32}, i32)) {
-          Fatal() << "StringLifting: bad signature for charCodeAt: " << sig;
+        if (type != Signature({externref, i32}, i32)) {
+          Fatal() << "StringLifting: bad type for charCodeAt: " << type;
         }
         charCodeAtImport = func->name;
         found = true;
       } else if (func->base == "substring") {
-        if (sig != Signature({externref, i32, i32}, refExtern)) {
-          Fatal() << "StringLifting: bad signature for substring: " << sig;
+        if (type != Signature({externref, i32, i32}, refExtern)) {
+          Fatal() << "StringLifting: bad type for substring: " << type;
         }
         substringImport = func->name;
         found = true;

test/lit/passes/string-lifting-validation.wast

@@ -18,5 +18,5 @@
 )
 
 ;; RUN: not wasm-opt %s --string-lifting -all 2>&1 | filecheck %s
-;; CHECK: Fatal: StringLifting: bad signature for fromCharCodeArray: (func (param (ref null $array.0) i32 i64) (result (ref extern)))
+;; CHECK: Fatal: StringLifting: bad type for fromCharCodeArray: (type $func.0 (func (param (ref null $array.0) i32 i64) (result (ref extern))))
 

test/lit/passes/string-lifting-wrong-type.wast

@@ -0,0 +1,24 @@
+;; The imported js string method here has the correct signature, but the wrong
+;; type. We should error.
+
+(module
+  (type $array16 (array (mut i16)))
+
+  ;; The type should be final!
+  (type $bad (sub (func (param (ref null $array16) i32 i32) (result (ref extern)))))
+
+  (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $bad)))
+
+  (func $string.new.gc (param $ref (ref $array16))
+    (drop
+      (call $fromCharCodeArray
+        (local.get $ref)
+        (i32.const 7)
+        (i32.const 8)
+      )
+    )
+  )
+)
+
+;; RUN: not wasm-opt %s --string-lifting -all 2>&1 | filecheck %s
+;; CHECK: Fatal: StringLifting: bad type for fromCharCodeArray: (type $func.0 (sub (func (param (ref null $array.0) i32 i32) (result (ref extern)))))