Fix validateGlobally usage in validator, and an i64-to-i32 bug hidden by it (#3253)

validateGlobally means that we can't do lookups on the module. A few places
were missing that, or had it wrong. I think the reason for the wrong usages is
that we used to have types on the module, and then removed that, so more is
now validatable actually.

This uncovered a real bug, where i64-to-32 would ignore an unreachable
parameter of a call_indirect. That's bad, since if the type is i64, we need
to replace it with two parameters. To fix that, just handle unreachability
there, using the existing logic (which skips the call_indirect entirely in
this case).

4 files changed, 42 insertions, 7 deletions

diff --git a/src/passes/I64ToI32Lowering.cpp b/src/passes/I64ToI32Lowering.cpp
index a0381ccd4..c06176e53 100644
--- a/src/passes/I64ToI32Lowering.cpp
+++ b/src/passes/I64ToI32Lowering.cpp
@@ -219,6 +219,9 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
   // returns nullptr;
   template<typename T>
   T* visitGenericCall(T* curr, BuilderFunc<T> callBuilder) {
+    if (handleUnreachable(curr)) {
+      return nullptr;
+    }
     bool fixed = false;
     std::vector<Expression*> args;
     for (auto* e : curr->operands) {
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 012cb0e36..ef6a29373 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -806,9 +806,6 @@ void FunctionValidator::visitCallIndirect(CallIndirect* curr) {
   shouldBeTrue(!curr->isReturn || getModule()->features.hasTailCall(),
                curr,
                "return_call_indirect requires tail calls to be enabled");
-  if (!info.validateGlobally) {
-    return;
-  }
   shouldBeEqualOrFirstIsUnreachable(curr->target->type,
                                     Type(Type::i32),
                                     curr,
@@ -1960,6 +1957,9 @@ void FunctionValidator::visitRefFunc(RefFunc* curr) {
   shouldBeTrue(getModule()->features.hasReferenceTypes(),
                curr,
                "ref.func requires reference-types to be enabled");
+  if (!info.validateGlobally) {
+    return;
+  }
   auto* func = getModule()->getFunctionOrNull(curr->func);
   shouldBeTrue(!!func, curr, "function argument of ref.func must exist");
 }
@@ -2010,13 +2010,13 @@ void FunctionValidator::visitThrow(Throw* curr) {
   shouldBeTrue(getModule()->features.hasExceptionHandling(),
                curr,
                "throw requires exception-handling to be enabled");
-  if (!info.validateGlobally) {
-    return;
-  }
   shouldBeEqual(curr->type,
                 Type(Type::unreachable),
                 curr,
                 "throw's type must be unreachable");
+  if (!info.validateGlobally) {
+    return;
+  }
   auto* event = getModule()->getEventOrNull(curr->event);
   if (!shouldBeTrue(!!event, curr, "throw's event must exist")) {
     return;
diff --git a/test/passes/flatten_i64-to-i32-lowering.txt b/test/passes/flatten_i64-to-i32-lowering.txt
index b115ff619..b069ff6b2 100644
--- a/test/passes/flatten_i64-to-i32-lowering.txt
+++ b/test/passes/flatten_i64-to-i32-lowering.txt
@@ -549,3 +549,24 @@
   )
  )
 )
+(module
+ (type $none_=>_none (func))
+ (table $0 37 funcref)
+ (global $i64toi32_i32$HIGH_BITS (mut i32) (i32.const 0))
+ (func $0
+  (unreachable)
+  (block
+   (unreachable)
+   (drop
+    (f64.const 1)
+   )
+   (drop
+    (i32.const -32768)
+   )
+   (drop
+    (i32.const 20)
+   )
+  )
+  (unreachable)
+ )
+)
diff --git a/test/passes/flatten_i64-to-i32-lowering.wast b/test/passes/flatten_i64-to-i32-lowering.wast
index 9b75e683f..52627c9d4 100644
--- a/test/passes/flatten_i64-to-i32-lowering.wast
+++ b/test/passes/flatten_i64-to-i32-lowering.wast
@@ -63,4 +63,15 @@
   (global.set $f (i64.const 0x1122334455667788))
  )
 )
-
+(module
+ (type $i64_f64_i32_=>_none (func (param i64 f64 i32)))
+ (table $0 37 funcref)
+ (func $0
+  (call_indirect (type $i64_f64_i32_=>_none)
+   (unreachable)
+   (f64.const 1)
+   (i32.const -32768)
+   (i32.const 20)
+  )
+ )
+)