Fix isorecursive canonicalization (#5269)

Fixes a longstanding problem with isorecursive canonicalization that only showed
up in MacOS and occasionally Windows builds. The problem was that
`RecGroupEquator` was not quite correct in the presence of self-references in
rec groups. Specifically, `RecGroupEquator` did not differentiate between
instances of the same type appearing across two rec groups where the type was a
self-reference in one group but not in the other.

The reason this only showed up occasionally on some platforms was that this bug
could only cause incorrect behavior if two groups that would incorrectly be
compared as equal were hashed into the same bucket of a hash map. Apparently the
hash map used on Linux never hashes the two problematic groups into the same
bucket.

2 files changed, 5 insertions, 6 deletions

diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp
index 4fa7ff9d5..6b11ff95e 100644
--- a/src/wasm/wasm-type.cpp
+++ b/src/wasm/wasm-type.cpp
@@ -2676,11 +2676,8 @@ bool RecGroupEquator::topLevelEq(HeapType a, HeapType b) const {
 }
 
 bool RecGroupEquator::eq(Type a, Type b) const {
-  if (a == b) {
-    return true;
-  }
   if (a.isBasic() || b.isBasic()) {
-    return false;
+    return a == b;
   }
   return eq(*getTypeInfo(a), *getTypeInfo(b));
 }
@@ -2699,7 +2696,9 @@ bool RecGroupEquator::eq(HeapType a, HeapType b) const {
   }
   auto groupA = a.getRecGroup();
   auto groupB = b.getRecGroup();
-  return groupA == groupB || (groupA == newGroup && groupB == otherGroup);
+  bool selfRefA = groupA == newGroup;
+  bool selfRefB = groupB == otherGroup;
+  return (selfRefA && selfRefB) || (!selfRefA && !selfRefB && groupA == groupB);
 }
 
 bool RecGroupEquator::eq(const TypeInfo& a, const TypeInfo& b) const {
diff --git a/test/gtest/type-builder.cpp b/test/gtest/type-builder.cpp
index fe1f19871..8ba2a39d1 100644
--- a/test/gtest/type-builder.cpp
+++ b/test/gtest/type-builder.cpp
@@ -362,7 +362,7 @@ TEST_F(IsorecursiveTest, CanonicalizeUses) {
   EXPECT_NE(built[4], built[6]);
 }
 
-TEST_F(IsorecursiveTest, DISABLED_CanonicalizeSelfReferences) {
+TEST_F(IsorecursiveTest, CanonicalizeSelfReferences) {
   TypeBuilder builder(5);
   // Single self-reference
   builder[0] = makeStruct(builder, {0});