From 6656dfaf9c52b1dda3426b6d7e2a1db3ec3617e5 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Wed, 24 Feb 2021 17:54:57 +0000
Subject: 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
---
 src/ir/ExpressionAnalyzer.cpp | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

(limited to 'src')

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()); }
-- 
cgit v1.2.3