TypeRefining: Fix a bug with chains of StructGets (#5757)

If we have

(struct.get $A
  (struct.get $B

then if both types end up refined we may have a problem. If the inner one is
refined to emit nullref then the outer one no longer knows what type it is,
since it depends on the type of the ref child for that in our IR. We can't just
skip updating it, as the outside may depend on its new refined type to
validate. To avoid errors here, just make this code that is effectively
unreachable also actually unreachable.

2 files changed, 78 insertions, 1 deletions

diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp
index 87366e148..7fd039411 100644
--- a/src/passes/TypeRefining.cpp
+++ b/src/passes/TypeRefining.cpp
@@ -250,7 +250,29 @@ struct TypeRefining : public Pass {
       }
 
       void visitStructGet(StructGet* curr) {
-        if (curr->ref->type == Type::unreachable || curr->ref->type.isNull()) {
+        if (curr->ref->type == Type::unreachable) {
+          return;
+        }
+
+        if (curr->ref->type.isNull()) {
+          // This get will trap. In theory we could leave this for later
+          // optimizations to do, but we must actually handle it here, because
+          // of the situation where this get's type is refined, and the input
+          // type is the result of a refining:
+          //
+          //   (struct.get $A    ;; should be refined to something
+          //     (struct.get $B  ;; just refined to nullref
+          //
+          // If the input has become a nullref then we can't just return out of
+          // this function, as we'd be leaving a struct.get of $A with the
+          // wrong type. But we can't find the right type since in Binaryen IR
+          // we use the ref's type to see what is being read, and that just
+          // turned into nullref. To avoid that corner case, just turn this code
+          // into unreachable code now, and the later refinalize will turn all
+          // the parents unreachable, avoiding any type-checking problems.
+          Builder builder(*getModule());
+          replaceCurrent(builder.makeSequence(builder.makeDrop(curr->ref),
+                                              builder.makeUnreachable()));
           return;
         }
 
diff --git a/test/lit/passes/type-refining.wast b/test/lit/passes/type-refining.wast
index 8258a3a54..824ad43a8 100644
--- a/test/lit/passes/type-refining.wast
+++ b/test/lit/passes/type-refining.wast
@@ -1167,3 +1167,58 @@
     )
   )
 )
+
+(module
+  (rec
+    ;; CHECK:      (rec
+    ;; CHECK-NEXT:  (type $A (struct (field (mut nullref))))
+    (type $A (struct (field (mut anyref))))
+    ;; CHECK:       (type $B (struct (field (mut nullref))))
+    (type $B (struct (field (mut (ref null $A)))))
+  )
+
+  ;; CHECK:       (type $none_=>_none (func))
+
+  ;; CHECK:      (func $0 (type $none_=>_none)
+  ;; CHECK-NEXT:  (block ;; (replaces something unreachable we can't emit)
+  ;; CHECK-NEXT:   (drop
+  ;; CHECK-NEXT:    (block ;; (replaces something unreachable we can't emit)
+  ;; CHECK-NEXT:     (drop
+  ;; CHECK-NEXT:      (block
+  ;; CHECK-NEXT:       (drop
+  ;; CHECK-NEXT:        (struct.get $B 0
+  ;; CHECK-NEXT:         (struct.new_default $B)
+  ;; CHECK-NEXT:        )
+  ;; CHECK-NEXT:       )
+  ;; CHECK-NEXT:       (unreachable)
+  ;; CHECK-NEXT:      )
+  ;; CHECK-NEXT:     )
+  ;; CHECK-NEXT:     (unreachable)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (drop
+  ;; CHECK-NEXT:    (ref.null none)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (unreachable)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $0
+    (struct.set $A 0
+      (struct.new $A
+        ;; These two struct.gets will both be refined. That of $B will be
+        ;; refined to a get of a null, at which point the get of $A could get
+        ;; confused and not know what type it is reading from (since in the IR,
+        ;; we depend on the ref for that). The pass should not error here while
+        ;; it refines both the struct type's fields to nullref, after which the
+        ;; code here will be unreachable (since we do a struct.get from a
+        ;; nullref).
+        (struct.get $A 0
+          (struct.get $B 0
+            (struct.new_default $B)
+          )
+        )
+      )
+      (ref.null none)
+    )
+  )
+)