Fix finalization of call_ref to handle refined target types (#5883)

Previously CallRef::finalize() would never update the type of the CallRef, even
if the type of the call target had been refined to give a more precise result
type. Besides unnecessarily losing type information, this could also lead to
validation errors, since the validator checks that the type of CallRef matches
the result type of the target signature.

Fix the bug by updating CallRef's type based on its target signature in
CallRef::finalize() and add a test that depends on this refinalization.

5 files changed, 92 insertions, 8 deletions

diff --git a/src/wasm.h b/src/wasm.h
index 39cc9fb54..90628c5bf 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -1503,7 +1503,6 @@ public:
   bool isReturn = false;
 
   void finalize();
-  void finalize(Type type_);
 };
 
 class RefTest : public SpecificExpression<Expression::RefTestId> {
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 0d405251e..684afe80c 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -6936,7 +6936,10 @@ void WasmBinaryReader::visitCallRef(CallRef* curr) {
   for (size_t i = 0; i < num; i++) {
     curr->operands[num - i - 1] = popNonVoidExpression();
   }
-  curr->finalize(sig.results);
+  // If the target has bottom type, we won't be able to infer the correct type
+  // from it, so set the type manually to be safe.
+  curr->type = sig.results;
+  curr->finalize();
 }
 
 bool WasmBinaryReader::maybeVisitI31New(Expression*& out, uint32_t code) {
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 0519afd83..4fa4981e4 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -2822,6 +2822,13 @@ Expression* SExpressionWasmBuilder::makeCallRef(Element& s, bool isReturn) {
       s.line,
       s.col);
   }
+  if (!Type::isSubType(target->type, Type(sigType, Nullable))) {
+    throw ParseException(
+      std::string(isReturn ? "return_call_ref" : "call_ref") +
+        " target should match expected type",
+      s.line,
+      s.col);
+  }
   return Builder(wasm).makeCallRef(
     target, operands, sigType.getSignature().results, isReturn);
 }
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index 74b944bb6..188cd2d6f 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -920,18 +920,23 @@ void I31Get::finalize() {
 }
 
 void CallRef::finalize() {
-  handleUnreachableOperands(this);
+  if (handleUnreachableOperands(this)) {
+    return;
+  }
   if (isReturn) {
     type = Type::unreachable;
+    return;
   }
   if (target->type == Type::unreachable) {
     type = Type::unreachable;
+    return;
   }
-}
-
-void CallRef::finalize(Type type_) {
-  type = type_;
-  finalize();
+  assert(target->type.isRef());
+  if (target->type.getHeapType().isBottom()) {
+    return;
+  }
+  assert(target->type.getHeapType().isSignature());
+  type = target->type.getHeapType().getSignature().results;
 }
 
 void RefTest::finalize() {
diff --git a/test/lit/passes/local-subtyping.wast b/test/lit/passes/local-subtyping.wast
index 2d6e2fb05..034606cb0 100644
--- a/test/lit/passes/local-subtyping.wast
+++ b/test/lit/passes/local-subtyping.wast
@@ -14,6 +14,11 @@
 
   (type $array (array_subtype i8 data))
 
+  ;; CHECK:      (type $ret-any (func (result anyref)))
+  (type $ret-any (sub (func (result anyref))))
+  ;; CHECK:      (type $ret-i31 (sub $ret-any (func (result i31ref))))
+  (type $ret-i31 (sub $ret-any (func (result i31ref))))
+
   ;; CHECK:      (import "out" "i32" (func $i32 (type $none_=>_i32) (result i32)))
   (import "out" "i32" (func $i32 (result i32)))
   ;; CHECK:      (import "out" "i64" (func $i64 (type $none_=>_i64) (result i64)))
@@ -229,6 +234,71 @@
     )
   )
 
+  ;; CHECK:      (func $multiple-iterations-refinalize-call-ref (type $none_=>_none)
+  ;; CHECK-NEXT:  (local $f (ref $ret-i31))
+  ;; CHECK-NEXT:  (local $x i31ref)
+  ;; CHECK-NEXT:  (local.set $f
+  ;; CHECK-NEXT:   (ref.func $ret-i31)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.set $x
+  ;; CHECK-NEXT:   (call_ref $ret-i31
+  ;; CHECK-NEXT:    (local.get $f)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $multiple-iterations-refinalize-call-ref
+    (local $f (ref null $ret-any))
+    (local $x (anyref))
+    (local.set $f
+      (ref.func $ret-i31)
+    )
+    (local.set $x
+      ;; After $f is refined to hold $ret-i31 and the call_ref is refinalized,
+      ;; we will be able to refine $x to i31.
+      (call_ref $ret-any
+        (local.get $f)
+      )
+    )
+  )
+
+  ;; CHECK:      (func $multiple-iterations-refinalize-call-ref-bottom (type $none_=>_none)
+  ;; CHECK-NEXT:  (local $f nullfuncref)
+  ;; CHECK-NEXT:  (local $x anyref)
+  ;; CHECK-NEXT:  (local.set $f
+  ;; CHECK-NEXT:   (ref.null nofunc)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.set $x
+  ;; CHECK-NEXT:   (block ;; (replaces something unreachable we can't emit)
+  ;; CHECK-NEXT:    (drop
+  ;; CHECK-NEXT:     (local.get $f)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (unreachable)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $multiple-iterations-refinalize-call-ref-bottom
+    (local $f (ref null $ret-any))
+    (local $x (anyref))
+    ;; Same as above, but now we refine $f to nullfuncref. Check that we don't crash.
+    (local.set $f
+      (ref.null nofunc)
+    )
+    (local.set $x
+      ;; We can no longer refine $x because there is no result type we can use
+      ;; after refining $f.
+      (call_ref $ret-any
+        (local.get $f)
+      )
+    )
+  )
+
+  ;; CHECK:      (func $ret-i31 (type $ret-i31) (result i31ref)
+  ;; CHECK-NEXT:  (unreachable)
+  ;; CHECK-NEXT: )
+  (func $ret-i31 (type $ret-i31) (result i31ref)
+    (unreachable)
+  )
+
   ;; CHECK:      (func $nondefaultable (type $none_=>_none)
   ;; CHECK-NEXT:  (local $x (funcref funcref))
   ;; CHECK-NEXT:  (local.set $x