From f0a2e2c75c7bb3008f10b6edbb8dc4cfd27b7d28 Mon Sep 17 00:00:00 2001
From: Sam Clegg <sbc@chromium.org>
Date: Tue, 17 Dec 2019 10:18:31 -0800
Subject: Fix renaming in FixInvokeFunctionNamesWalker (#2513)

This fixes https://github.com/emscripten-core/emscripten/issues/9950.
The issue only shows up when debug names are not present so most of
the changes in CL come from disabling debug names in the lld tests.
We want to make sure that wasm-emscripten-finalize runs fine without
debug names so I think it makes most sense to test in this mode.

The actual bugfix is in wasm-emscripten.cpp as part of the
FixInvokeFunctionNamesWalker.  The problem was the name of the function
rather than is import name was being added to importRenames.  This means
that when debug names were present (and the two names were the same)
we didn't see the bug.
---
 src/wasm/wasm-emscripten.cpp | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

(limited to 'src')

diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp
index f069e558d..ae7e7f871 100644
--- a/src/wasm/wasm-emscripten.cpp
+++ b/src/wasm/wasm-emscripten.cpp
@@ -996,8 +996,9 @@ struct FixInvokeFunctionNamesWalker
   std::vector<Name> toRemove;
   std::set<Name> newImports;
   std::set<Signature> invokeSigs;
+  ImportInfo imports;
 
-  FixInvokeFunctionNamesWalker(Module& _wasm) : wasm(_wasm) {}
+  FixInvokeFunctionNamesWalker(Module& _wasm) : wasm(_wasm), imports(wasm) {}
 
   // Converts invoke wrapper names generated by LLVM backend to real invoke
   // wrapper names that are expected by JavaScript glue code.
@@ -1052,15 +1053,18 @@ struct FixInvokeFunctionNamesWalker
       return;
     }
 
-    assert(importRenames.count(curr->name) == 0);
-    BYN_TRACE("renaming: " << curr->name << " -> " << newname << "\n");
-    importRenames[curr->name] = newname;
+    BYN_TRACE("renaming import: " << curr->module << "." << curr->base << " ("
+                                  << curr->name << ") -> " << newname << "\n");
+    assert(importRenames.count(curr->base) == 0);
+    importRenames[curr->base] = newname;
     // Either rename or remove the existing import
-    if (wasm.getFunctionOrNull(newname) || !newImports.insert(newname).second) {
+    if (imports.getImportedFunction(curr->module, newname) ||
+        !newImports.insert(newname).second) {
+      BYN_TRACE("using existing import\n");
       toRemove.push_back(curr->name);
     } else {
+      BYN_TRACE("renaming import\n");
       curr->base = newname;
-      curr->name = newname;
     }
   }
 
@@ -1069,10 +1073,11 @@ struct FixInvokeFunctionNamesWalker
       wasm.removeFunction(importName);
     }
     ModuleUtils::renameFunctions(wasm, importRenames);
-    ImportInfo imports(wasm);
+    // Update any associated GOT.func imports.
     for (auto& pair : importRenames) {
-      // Update any associated GOT.func import.
       if (auto g = imports.getImportedGlobal("GOT.func", pair.first)) {
+        BYN_TRACE("renaming corresponding GOT entry: " << g->base << " -> "
+                                                       << pair.second << "\n");
         g->base = pair.second;
       }
     }
-- 
cgit v1.2.3