[Wasm GC] Fix precomputing of incompatible fallthrough values (#3875)

Precompute not only computes values, but looks at the fallthrough,

(local.set 0
  (block
    ..stuff we can ignore..
    ;; the fallthrough we care about - if a value is set to local 0, it is this
    (i32.const 10)
  )
)

Normally that is fine, but the fuzzer found a case where it is not: RefCast may
return a different type than the fallthrough, even an incompatible type if we
try to do something bad like cast a function to a struct. As we may then
propagate the value to a place that expects the proper type, this can cause an
error.

To fix this, check if the precomputed value is a proper subtype. If it is not,
then do not look through into the fallthrough, but compute the entire thing.
(In the case of a bad RefCast of a func to a struct, it would then indicate a
trap happens, and we would not precompute the value.)

3 files changed, 131 insertions, 4 deletions

diff --git a/src/passes/CoalesceLocals.cpp b/src/passes/CoalesceLocals.cpp
index ad64b066c..c58a48834 100644
--- a/src/passes/CoalesceLocals.cpp
+++ b/src/passes/CoalesceLocals.cpp
@@ -34,7 +34,6 @@
 #include "pass.h"
 #include "support/learning.h"
 #include "support/permutations.h"
-#include "wasm-builder.h"
 #include "wasm.h"
 #ifdef CFG_PROFILE
 #include "support/timing.h"
diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp
index c07286ec9..3e2e7a64f 100644
--- a/src/passes/Precompute.cpp
+++ b/src/passes/Precompute.cpp
@@ -311,9 +311,27 @@ private:
         // somehow know the entire expression precomputes to a 42, then we can
         // propagate that 42 along to the users, regardless of whatever the call
         // did globally.)
-        auto values = setValues[set] =
-          precomputeValue(Properties::getFallthrough(
-            set->value, getPassOptions(), getModule()->features));
+        auto values = precomputeValue(Properties::getFallthrough(
+          set->value, getPassOptions(), getModule()->features));
+        // Fix up the value. The computation we just did was to look at the
+        // fallthrough, then precompute that; that looks through expressions
+        // that pass through the value. Normally that does not matter here,
+        // for example, (block .. (value)) returns the value unmodified.
+        // However, some things change the type, for example RefAsNonNull has
+        // a non-null type, while its input may be nullable. That does not
+        // matter either, as if we managed to precompute it then the value had
+        // the more specific (in this example, non-nullable) type. But there
+        // is a situation where this can cause an issue: RefCast. An attempt to
+        // perform a "bad" cast, say of a function to a struct, is a case where
+        // the fallthrough value's type is very different than the actually
+        // returned value's type. To handle that, if we precomputed a value and
+        // if it has the wrong type then precompute it again without looking
+        // through to the fallthrough.
+        if (values.isConcrete() &&
+            !Type::isSubType(values.getType(), set->value->type)) {
+          values = precomputeValue(set->value);
+        }
+        setValues[set] = values;
         if (values.isConcrete()) {
           for (auto* get : localGraph.setInfluences[set]) {
             work.insert(get);
diff --git a/test/lit/passes/precompute-gc.wast b/test/lit/passes/precompute-gc.wast
index 0cd6d2c2c..cb6ba9bf0 100644
--- a/test/lit/passes/precompute-gc.wast
+++ b/test/lit/passes/precompute-gc.wast
@@ -6,6 +6,12 @@
  (type $struct (struct (mut i32)))
  (type $empty (struct))
 
+ ;; two incompatible struct types
+ (type $A (struct (field (mut f32))))
+ (type $B (struct (field (mut f64))))
+
+ (type $func-return-i32 (func (result i32)))
+
  (import "fuzzing-support" "log-i32" (func $log (param i32)))
 
  ;; CHECK:      (func $test-fallthrough (result i32)
@@ -391,4 +397,108 @@
   ;; precompute GC references, and so nothing will be done.
   (local.get $tempresult)
  )
+
+ ;; CHECK:      (func $odd-cast-and-get
+ ;; CHECK-NEXT:  (local $temp (ref null $B))
+ ;; CHECK-NEXT:  (local.set $temp
+ ;; CHECK-NEXT:   (ref.null $B)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT:  (drop
+ ;; CHECK-NEXT:   (struct.get $B 0
+ ;; CHECK-NEXT:    (ref.null $B)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $odd-cast-and-get
+  (local $temp (ref null $B))
+  ;; Try to cast a null of A to B. While the types are incompatible, ref.cast
+  ;; returns a null when given a null (and the null must have the type that the
+  ;; ref.cast instruction has, that is, the value is a null of type $B). So this
+  ;; is an odd cast that "works".
+  (local.set $temp
+   (ref.cast
+    (ref.null $A)
+    (rtt.canon $B)
+   )
+  )
+  (drop
+   ;; Read from the local, which precompute should set to a null with the proper
+   ;; type.
+   (struct.get $B 0
+    (local.get $temp)
+   )
+  )
+ )
+
+ ;; CHECK:      (func $odd-cast-and-get-tuple
+ ;; CHECK-NEXT:  (local $temp ((ref null $B) i32))
+ ;; CHECK-NEXT:  (local.set $temp
+ ;; CHECK-NEXT:   (tuple.make
+ ;; CHECK-NEXT:    (ref.null $B)
+ ;; CHECK-NEXT:    (i32.const 10)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT:  (drop
+ ;; CHECK-NEXT:   (struct.get $B 0
+ ;; CHECK-NEXT:    (ref.null $B)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $odd-cast-and-get-tuple
+  (local $temp ((ref null $B) i32))
+  ;; As above, but with a tuple.
+  (local.set $temp
+   (tuple.make
+    (ref.cast
+     (ref.null $A)
+     (rtt.canon $B)
+    )
+    (i32.const 10)
+   )
+  )
+  (drop
+   (struct.get $B 0
+    (tuple.extract 0
+     (local.get $temp)
+    )
+   )
+  )
+ )
+
+ ;; CHECK:      (func $receive-f64 (param $0 f64)
+ ;; CHECK-NEXT:  (unreachable)
+ ;; CHECK-NEXT: )
+ (func $receive-f64 (param f64)
+  (unreachable)
+ )
+
+ ;; CHECK:      (func $odd-cast-and-get-non-null (param $temp (ref $func-return-i32))
+ ;; CHECK-NEXT:  (local.set $temp
+ ;; CHECK-NEXT:   (ref.cast
+ ;; CHECK-NEXT:    (ref.func $receive-f64)
+ ;; CHECK-NEXT:    (rtt.canon $func-return-i32)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT:  (drop
+ ;; CHECK-NEXT:   (call_ref
+ ;; CHECK-NEXT:    (local.get $temp)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $odd-cast-and-get-non-null (param $temp (ref $func-return-i32))
+  ;; Try to cast a function to an incompatible type.
+  (local.set $temp
+   (ref.cast
+    (ref.func $receive-f64)
+    (rtt.canon $func-return-i32)
+   )
+  )
+  (drop
+   ;; Read from the local, checking whether precompute set a value there (it
+   ;; should not, as the cast fails).
+   (call_ref
+    (local.get $temp)
+   )
+  )
+ )
 )