SimplifyLocals fuzz fix: Don't be confused by subtype assigns. (#3267)

We checked if the type matches when deciding if two locals are equivalent,
but if the type didn't match, we forgot to reset any previously equivalent
things. So we thought something was equivalent when it wasn't, see the
reduced testcase.

Fixes #3266

3 files changed, 49 insertions, 4 deletions

diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp
index 9c831a551..963575c5f 100644
--- a/src/passes/SimplifyLocals.cpp
+++ b/src/passes/SimplifyLocals.cpp
@@ -964,16 +964,23 @@ struct SimplifyLocals
               }
               anotherCycle = true;
             }
+            // Nothing more to do, ignore the copy.
+            return;
           } else if (func->getLocalType(curr->index) ==
                      func->getLocalType(get->index)) {
-            // There is a new equivalence now.
+            // There is a new equivalence now. Remove all the old ones, and add
+            // the new one.
+            // Note that we ignore the case of subtyping here, to keep this
+            // optimization simple by assuming all equivalent indexes also have
+            // the same type. TODO: consider optimizing this.
             equivalences.reset(curr->index);
             equivalences.add(curr->index, get->index);
+            return;
           }
-        } else {
-          // A new value is assigned here.
-          equivalences.reset(curr->index);
         }
+        // A new value of some kind is assigned here, and it's not something we
+        // could handle earlier, so remove all the old equivalent ones.
+        equivalences.reset(curr->index);
       }
 
       void visitLocalGet(LocalGet* curr) {
diff --git a/test/passes/simplify-locals_all-features.txt b/test/passes/simplify-locals_all-features.txt
index 81fca48ef..a9f056136 100644
--- a/test/passes/simplify-locals_all-features.txt
+++ b/test/passes/simplify-locals_all-features.txt
@@ -2040,3 +2040,21 @@
   )
  )
 )
+(module
+ (type $eqref_i31ref_=>_i32 (func (param eqref i31ref) (result i32)))
+ (export "test" (func $0))
+ (func $0 (param $0 eqref) (param $1 i31ref) (result i32)
+  (local $2 eqref)
+  (local $3 i31ref)
+  (local.set $2
+   (local.get $0)
+  )
+  (local.set $0
+   (local.get $3)
+  )
+  (ref.eq
+   (local.get $2)
+   (local.get $1)
+  )
+ )
+)
diff --git a/test/passes/simplify-locals_all-features.wast b/test/passes/simplify-locals_all-features.wast
index 177b09688..61150edc1 100644
--- a/test/passes/simplify-locals_all-features.wast
+++ b/test/passes/simplify-locals_all-features.wast
@@ -1800,3 +1800,23 @@
   )
  )
 )
+;; do not be confused by subtyping: when an index is set, even to another type,
+;; it is no longer equivalent
+;; (see https://github.com/WebAssembly/binaryen/issues/3266)
+(module
+ (func "test" (param $0 eqref) (param $1 i31ref) (result i32)
+  (local $2 eqref)
+  (local $3 i31ref)
+  (local.set $2
+   (local.get $0) ;; $0 and $2 are equivalent
+  )
+  (local.set $0   ;; set $0 to something with another type
+   (local.get $3)
+  )
+  ;; compares a null eqref and a zero i31ref - should be false
+  (ref.eq
+   (local.get $2)
+   (local.get $1)
+  )
+ )
+)