Fix hashing of a use of a name without the context/target for it (#3601)

Before this we would assert on hashing e.g. (br $x) by itself, without the
context so we recognized the name $x. Somehow that was not an issue
until delegate, we just happened to not hash such things. I believe I remember
that @aheejin noticed this issue before, but given we didn't have a testcase,
we deferred fixing it - now is the time, I guess, as with delegate it is easy to
get e.g. CodeFolding to hash a Try with a delegate.

Issue found by emscripten-core/emscripten#13485

2 files changed, 31 insertions, 8 deletions

diff --git a/src/ir/ExpressionAnalyzer.cpp b/src/ir/ExpressionAnalyzer.cpp
index 0b0adae9e..fe4b4e80c 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);
+        // Perform the same hashing as a generic name.
+        visitNonScopeName(curr);
+        return;
       }
+      rehash(digest, 2);
+      static_assert(sizeof(Index) == sizeof(int32_t),
+                    "wasm64 will need changes here");
+      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 7c3882cc9..ba9ca24ea 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";
 }