From 32da7b409059bbb65e7132acf8f2979809804f8a Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 23 Feb 2021 16:05:04 -0800
Subject: [PATCH 1/3] Fix hashing of a use of a name without the context/target
 for it

---
 src/ir/ExpressionAnalyzer.cpp | 29 +++++++++++++++++++++--------
 test/example/hash.cpp         | 10 ++++++++++
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/src/ir/ExpressionAnalyzer.cpp b/src/ir/ExpressionAnalyzer.cpp
index 0b0adae9e27..b4c988dda7d 100644
--- a/src/ir/ExpressionAnalyzer.cpp
+++ b/src/ir/ExpressionAnalyzer.cpp
@@ -331,15 +331,28 @@ size_t ExpressionAnalyzer::hash(Expression* curr) {
       }
     }
     void visitScopeName(Name curr) {
-      if (curr.is()) { // try's delegate target can be null
-        // Names are relative, we give the same hash for
-        // (block $x (br $x))
-        // (block $y (br $y))
-        static_assert(sizeof(Index) == sizeof(int32_t),
-                      "wasm64 will need changes here");
-        assert(internalNames.find(curr) != internalNames.end());
-        rehash(digest, internalNames[curr]);
+      // We consider 3 cases here, and prefix a hash value of 0, 1, or 2 to
+      // maximally differentiate them.
+
+      // Try's delegate target can be null.
+      if (!curr.is()) {
+        rehash(digest, 0);
+        return;
+      }
+      // Names are relative, we give the same hash for
+      //   (block $x (br $x))
+      //   (block $y (br $y))
+      // But if the name is not known to us, hash the absolute one.
+      if (!internalNames.count(curr)) {
+        rehash(digest, 1);
+        rehash(digest, curr);
+        return;
       }
+      rehash(digest, 2);
+      static_assert(sizeof(Index) == sizeof(int32_t),
+                    "wasm64 will need changes here");
+      assert(internalNames.find(curr) != internalNames.end());
+      rehash(digest, internalNames[curr]);
     }
     void visitNonScopeName(Name curr) { rehash(digest, uint64_t(curr.str)); }
     void visitType(Type curr) { rehash(digest, curr.getID()); }
diff --git a/test/example/hash.cpp b/test/example/hash.cpp
index 7c3882cc924..ba9ca24ea17 100644
--- a/test/example/hash.cpp
+++ b/test/example/hash.cpp
@@ -138,5 +138,15 @@ int main() {
     y.index = 11;
     assertNotEqual(x, y);
   }
+  {
+    // It is ok to hash something that refers to an unknown name, like a break
+    // without the outside context that it branches to. And different names
+    // should have different hashes.
+    Break x;
+    x.name = "foo";
+    Break y;
+    y.name = "bar";
+    assertNotEqual(x, y);
+  }
   std::cout << "success.\n";
 }

From 85abfed22c68bde448632392efe57afa1ba0c009 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 23 Feb 2021 16:10:13 -0800
Subject: [PATCH 2/3] remove unneeded assert

---
 src/ir/ExpressionAnalyzer.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/ir/ExpressionAnalyzer.cpp b/src/ir/ExpressionAnalyzer.cpp
index b4c988dda7d..f38bcf3839d 100644
--- a/src/ir/ExpressionAnalyzer.cpp
+++ b/src/ir/ExpressionAnalyzer.cpp
@@ -351,7 +351,6 @@ size_t ExpressionAnalyzer::hash(Expression* curr) {
       rehash(digest, 2);
       static_assert(sizeof(Index) == sizeof(int32_t),
                     "wasm64 will need changes here");
-      assert(internalNames.find(curr) != internalNames.end());
       rehash(digest, internalNames[curr]);
     }
     void visitNonScopeName(Name curr) { rehash(digest, uint64_t(curr.str)); }

From 8393fb0f4d6d7f8be1d116310dcb7d39d80dbcaf Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 23 Feb 2021 16:11:43 -0800
Subject: [PATCH 3/3] better hashing

---
 src/ir/ExpressionAnalyzer.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/ir/ExpressionAnalyzer.cpp b/src/ir/ExpressionAnalyzer.cpp
index f38bcf3839d..fe4b4e80cdf 100644
--- a/src/ir/ExpressionAnalyzer.cpp
+++ b/src/ir/ExpressionAnalyzer.cpp
@@ -345,7 +345,8 @@ size_t ExpressionAnalyzer::hash(Expression* curr) {
       // But if the name is not known to us, hash the absolute one.
       if (!internalNames.count(curr)) {
         rehash(digest, 1);
-        rehash(digest, curr);
+        // Perform the same hashing as a generic name.
+        visitNonScopeName(curr);
         return;
       }
       rehash(digest, 2);