Also update internal name in fixEmJsFuncsAndReturnWalker (#2782)

Without this change only the import gets renamed not the internal
name. Since the internal name is the one that ends up in the name
section this means that rename wasn't effecting the name section.

3 files changed, 30 insertions, 26 deletions

diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp
index 5a4bda0a7..2e1aac0a3 100644
--- a/src/wasm/wasm-emscripten.cpp
+++ b/src/wasm/wasm-emscripten.cpp
@@ -1072,8 +1072,9 @@ EmJsWalker fixEmJsFuncsAndReturnWalker(Module& wasm) {
 struct FixInvokeFunctionNamesWalker
   : public PostWalker<FixInvokeFunctionNamesWalker> {
   Module& wasm;
+  std::vector<Name> toRemove;
   std::map<Name, Name> importRenames;
-  std::map<Name, Name> functionReplace;
+  std::map<Name, Name> functionRenames;
   std::set<Signature> invokeSigs;
   ImportInfo imports;
 
@@ -1134,29 +1135,32 @@ struct FixInvokeFunctionNamesWalker
 
     BYN_TRACE("renaming import: " << curr->module << "." << curr->base << " ("
                                   << curr->name << ") -> " << newname << "\n");
-    assert(importRenames.count(curr->base) == 0);
-    importRenames[curr->base] = newname;
-    // Either rename the import, or replace it with an existing one
-    Function* existingFunc = imports.getImportedFunction(curr->module, newname);
-    if (existingFunc) {
-      BYN_TRACE("replacing with an existing import: " << existingFunc->name
-                                                      << "\n");
-      functionReplace[curr->name] = existingFunc->name;
+
+    if (auto* f = imports.getImportedFunction(curr->module, newname)) {
+      BYN_TRACE("remove redundant import: " << curr->base << "\n");
+      toRemove.push_back(curr->name);
+      // Make sure the existing import has the correct internal name.
+      if (f->name != newname) {
+        functionRenames[f->name] = newname;
+      }
     } else {
-      BYN_TRACE("renaming the import in place\n");
+      BYN_TRACE("rename import: " << curr->base << "\n");
       curr->base = newname;
     }
+
+    functionRenames[curr->name] = newname;
+
+    // Ensure that an imported functions of this name exists.
+    importRenames[curr->base] = newname;
   }
 
   void visitModule(Module* curr) {
-    // For each replaced function first remove the function itself then
-    // rename all uses to the point to the new function.
-    for (auto& pair : functionReplace) {
-      BYN_TRACE("removeFunction " << pair.first << "\n");
-      wasm.removeFunction(pair.first);
+    for (auto name : toRemove) {
+      wasm.removeFunction(name);
     }
-    // Rename all uses of the removed functions
-    ModuleUtils::renameFunctions(wasm, functionReplace);
+
+    // Rename all uses of the old function to the new import name
+    ModuleUtils::renameFunctions(wasm, functionRenames);
 
     // For imports that for renamed, update any associated GOT.func imports.
     for (auto& pair : importRenames) {
diff --git a/test/lld/longjmp.wat.out b/test/lld/longjmp.wat.out
index 83c4b5a83..7eaebd592 100644
--- a/test/lld/longjmp.wat.out
+++ b/test/lld/longjmp.wat.out
@@ -11,14 +11,14 @@
  (import "env" "malloc" (func $fimport$0 (param i32) (result i32)))
  (import "env" "saveSetjmp" (func $fimport$1 (param i32 i32 i32 i32) (result i32)))
  (import "env" "getTempRet0" (func $fimport$2 (result i32)))
- (import "env" "invoke_vii" (func $fimport$4 (param i32 i32 i32)))
+ (import "env" "invoke_vii" (func $invoke_vii (param i32 i32 i32)))
  (import "env" "testSetjmp" (func $fimport$5 (param i32 i32 i32) (result i32)))
  (import "env" "setTempRet0" (func $fimport$6 (param i32)))
  (import "env" "free" (func $fimport$7 (param i32)))
- (import "env" "emscripten_longjmp" (func $fimport$8 (param i32 i32)))
+ (import "env" "emscripten_longjmp" (func $emscripten_longjmp (param i32 i32)))
  (memory $0 2)
  (table $0 2 2 funcref)
- (elem (i32.const 1) $fimport$8)
+ (elem (i32.const 1) $emscripten_longjmp)
  (global $global$0 (mut i32) (i32.const 66112))
  (global $global$1 i32 (i32.const 576))
  (export "memory" (memory $0))
@@ -70,7 +70,7 @@
       (i32.const 0)
       (i32.const 0)
      )
-     (call $fimport$4
+     (call $invoke_vii
       (i32.const 1)
       (local.get $0)
       (i32.const 1)
@@ -127,7 +127,7 @@
     (i32.const 0)
    )
   )
-  (call $fimport$8
+  (call $emscripten_longjmp
    (local.get $0)
    (local.get $3)
   )
diff --git a/test/lld/shared_longjmp.wat.out b/test/lld/shared_longjmp.wat.out
index 628412ed9..a029d5338 100644
--- a/test/lld/shared_longjmp.wat.out
+++ b/test/lld/shared_longjmp.wat.out
@@ -15,11 +15,11 @@
  (import "env" "malloc" (func $fimport$4 (param i32) (result i32)))
  (import "env" "saveSetjmp" (func $fimport$5 (param i32 i32 i32 i32) (result i32)))
  (import "env" "getTempRet0" (func $fimport$6 (result i32)))
- (import "env" "invoke_vii" (func $fimport$8 (param i32 i32 i32)))
+ (import "env" "invoke_vii" (func $invoke_vii (param i32 i32 i32)))
  (import "env" "testSetjmp" (func $fimport$9 (param i32 i32 i32) (result i32)))
  (import "env" "setTempRet0" (func $fimport$10 (param i32)))
  (import "env" "free" (func $fimport$11 (param i32)))
- (import "env" "emscripten_longjmp" (func $fimport$12 (param i32 i32)))
+ (import "env" "emscripten_longjmp" (func $emscripten_longjmp (param i32 i32)))
  (import "env" "g$__THREW__" (func $g$__THREW__ (result i32)))
  (import "env" "g$__threwValue" (func $g$__threwValue (result i32)))
  (import "env" "fp$emscripten_longjmp$vii" (func $fp$emscripten_longjmp$vii (result i32)))
@@ -78,7 +78,7 @@
       )
       (i32.const 0)
      )
-     (call $fimport$8
+     (call $invoke_vii
       (global.get $gimport$14)
       (local.get $0)
       (i32.const 1)
@@ -136,7 +136,7 @@
    )
    (return)
   )
-  (call $fimport$12
+  (call $emscripten_longjmp
    (local.get $3)
    (local.get $0)
   )