From 0e112d4c51de69620a419e1afce88e08e725e40b Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Fri, 4 Nov 2022 10:27:06 -0700
Subject: [Wasm GC] Fix GUFA on externalize/internalize (#5220)

These operations emit a completely different type than their input, so they must be
marked as roots, and not as things that flow values through them (because then
we filter everything out as the types are not compatible).

Fixes #5219
---
 src/ir/possible-contents.cpp | 25 ++++++++++++++++++++++---
 src/passes/LocalCSE.cpp      |  2 +-
 2 files changed, 23 insertions(+), 4 deletions(-)

(limited to 'src')

diff --git a/src/ir/possible-contents.cpp b/src/ir/possible-contents.cpp
index a81520cd4..9c132fd0c 100644
--- a/src/ir/possible-contents.cpp
+++ b/src/ir/possible-contents.cpp
@@ -670,7 +670,16 @@ struct InfoCollector
     receiveChildValue(curr->ref, curr);
   }
   void visitRefAs(RefAs* curr) {
-    // TODO: optimize when possible: like RefCast, not all values flow through.
+    if (curr->op == ExternExternalize || curr->op == ExternInternalize) {
+      // The external conversion ops emit something of a completely different
+      // type, which we must mark as a root.
+      addRoot(curr);
+      return;
+    }
+
+    // All other RefAs operations flow values through while refining them (the
+    // filterExpressionContents method will handle the refinement
+    // automatically).
     receiveChildValue(curr->value, curr);
   }
 
@@ -1681,8 +1690,16 @@ bool Flower::updateContents(LocationIndex locationIndex,
   }
 
   // Check if anything changed after filtering, if we did so.
-  if (filtered && contents == oldContents) {
-    return worthSendingMore;
+  if (filtered) {
+#if defined(POSSIBLE_CONTENTS_DEBUG) && POSSIBLE_CONTENTS_DEBUG >= 2
+    std::cout << "  filtered contents:\n";
+    contents.dump(std::cout, &wasm);
+    std::cout << '\n';
+#endif
+
+    if (contents == oldContents) {
+      return worthSendingMore;
+    }
   }
 
   // After filtering we should always have more precise information than "many"
@@ -2057,6 +2074,8 @@ void Flower::writeToData(Expression* ref, Expression* value, Index fieldIndex) {
 void Flower::flowRefCast(const PossibleContents& contents, RefCast* cast) {
   // RefCast only allows valid values to go through: nulls and things of the
   // cast type. Filter anything else out.
+  // TODO: Remove this method, as it just filters by the type, which we do
+  //       automatically now in filterExpressionContents.
   auto intendedCone =
     PossibleContents::fullConeType(Type(cast->intendedType, Nullable));
   PossibleContents filtered = contents;
diff --git a/src/passes/LocalCSE.cpp b/src/passes/LocalCSE.cpp
index 97d31b9d3..2b9b26f0e 100644
--- a/src/passes/LocalCSE.cpp
+++ b/src/passes/LocalCSE.cpp
@@ -23,7 +23,7 @@
 // an add operation appear twice, and the inputs must be identical in both
 // cases, then the second one requests to reuse the computed value from the
 // first. The first one to appear is the "original" expression that will remain
-// in the code; we will save it's value to a local, and get it from that local
+// in the code; we will save its value to a local, and get it from that local
 // later:
 //
 //  (i32.add (A) (B))
-- 
cgit v1.2.3