[GC] Fix ConstantFieldPropagation on incompatible types (#7054)

CFP is less precise than GUFA, in particular, when it flows around types then
it does not consider what field it is flowing them to, and its core data
structure is "if a struct.get is done on this type's field, what can be read?".
To see the issue this PR fixes, assume we have

  A
 / \
B   C

Then if we see struct.set $C, we know that can be read by a struct.get $A
(we can store a reference to a C in such a local/param/etc.), so we propagate
the value of that set to A. And, in general, anything in A can appear in B
(say, if we see a copy, a struct.set of struct.get that operates on types A,
then one of the sides might be a B), so we propagate from A to B. But
now we have propagated something from C to B, which might be of an
incompatible type.

This cannot cause runtime issues, as it just means we are propagating more
than we should, and will end up with less-useful results. But it can break
validation if no other value is possible but one with an incompatible type,
as we'd replace a struct.get $B with a value that only makes sense for C.
(The qualifier "no other value is possible" was added in the previous
sentence because if another one is possible then we'd end up with too
many values to infer anything, and not optimize at all, avoiding any error.)

1 files changed, 20 insertions, 1 deletions

diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp
index 26cc8316b..a3dd6aa6f 100644
--- a/src/passes/ConstantFieldPropagation.cpp
+++ b/src/passes/ConstantFieldPropagation.cpp
@@ -179,7 +179,26 @@ struct FunctionOptimizer : public WalkerPass<PostWalker<FunctionOptimizer>> {
     auto* value = info.makeExpression(*getModule());
     auto field = GCTypeUtils::getField(type, curr->index);
     assert(field);
-    return Bits::makePackedFieldGet(value, *field, curr->signed_, *getModule());
+    // Apply packing, if needed.
+    value =
+      Bits::makePackedFieldGet(value, *field, curr->signed_, *getModule());
+    // Check if the value makes sense. The analysis below flows values around
+    // without considering where they are placed, that is, when we see a parent
+    // type can contain a value in a field then we assume a child may as well
+    // (which in general it can, e.g., using a reference to the parent, we can
+    // write that value to it, but the reference might actually point to a
+    // child instance). If we tracked the types of fields then we might avoid
+    // flowing values into places they cannot reside, like when a child field is
+    // a subtype, and so we could ignore things not refined enough for it (GUFA
+    // does a better job at this). For here, just check we do not break
+    // validation, and if we do, then we've inferred the only possible value is
+    // an impossible one, making the code unreachable.
+    if (!Type::isSubType(value->type, field->type)) {
+      Builder builder(*getModule());
+      value = builder.makeSequence(builder.makeDrop(value),
+                                   builder.makeUnreachable());
+    }
+    return value;
   }
 
   void optimizeUsingRefTest(StructGet* curr) {