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.

5 files changed, 73 insertions, 65 deletions

diff --git a/scripts/test/generate_lld_tests.py b/scripts/test/generate_lld_tests.py
index 64fcff2ac..005e09f15 100755
--- a/scripts/test/generate_lld_tests.py
+++ b/scripts/test/generate_lld_tests.py
@@ -30,8 +30,8 @@ def files_with_extensions(path, extensions):
             yield file, ext
 
 
-def generate_wast_files(llvm_bin, emscripten_root):
-    print('\n[ building wast files from C sources... ]\n')
+def generate_wat_files(llvm_bin, emscripten_root):
+    print('\n[ building wat files from C sources... ]\n')
 
     lld_path = os.path.join(shared.options.binaryen_test, 'lld')
     for src_file, ext in files_with_extensions(lld_path, ['.c', '.cpp']):
@@ -42,11 +42,11 @@ def generate_wast_files(llvm_bin, emscripten_root):
         obj_path = os.path.join(lld_path, obj_file)
 
         wasm_file = src_file.replace(ext, '.wasm')
-        wast_file = src_file.replace(ext, '.wast')
+        wat_file = src_file.replace(ext, '.wat')
 
         obj_path = os.path.join(lld_path, obj_file)
         wasm_path = os.path.join(lld_path, wasm_file)
-        wast_path = os.path.join(lld_path, wast_file)
+        wat_path = os.path.join(lld_path, wat_file)
         is_shared = 'shared' in src_file
 
         compile_cmd = [
@@ -70,6 +70,10 @@ def generate_wast_files(llvm_bin, emscripten_root):
             '--export', '__data_end',
             '--global-base=568',
         ]
+        # We had a regression where this test only worked if debug names
+        # were included.
+        if 'shared_longjmp' in src_file:
+            link_cmd.append('--strip-debug')
         if is_shared:
             compile_cmd.append('-fPIC')
             compile_cmd.append('-fvisibility=default')
@@ -80,7 +84,7 @@ def generate_wast_files(llvm_bin, emscripten_root):
         try:
             support.run_command(compile_cmd)
             support.run_command(link_cmd)
-            support.run_command(shared.WASM_DIS + [wasm_path, '-o', wast_path])
+            support.run_command(shared.WASM_DIS + [wasm_path, '-o', wat_path])
         finally:
             # Don't need the .o or .wasm files, don't leave them around
             shared.delete_from_orbit(obj_path)
@@ -91,4 +95,4 @@ if __name__ == '__main__':
     if len(shared.options.positional_args) != 2:
         print('Usage: generate_lld_tests.py [llvm/bin/dir] [path/to/emscripten]')
         sys.exit(1)
-    generate_wast_files(*shared.options.positional_args)
+    generate_wat_files(*shared.options.positional_args)
diff --git a/scripts/test/shared.py b/scripts/test/shared.py
index 1ec42bc59..44a8dcc52 100644
--- a/scripts/test/shared.py
+++ b/scripts/test/shared.py
@@ -87,6 +87,7 @@ def parse_args(args):
 
 options = parse_args(sys.argv[1:])
 requested = options.positional_args
+script_dir = os.path.dirname(os.path.abspath(__file__))
 
 num_failures = 0
 warnings = []
@@ -123,8 +124,7 @@ if not any(os.path.isfile(os.path.join(options.binaryen_bin, f))
 
 # Locate Binaryen source directory if not specified.
 if not options.binaryen_root:
-    path_parts = os.path.abspath(__file__).split(os.path.sep)
-    options.binaryen_root = os.path.sep.join(path_parts[:-3])
+    options.binaryen_root = os.path.dirname(os.path.dirname(script_dir))
 
 options.binaryen_test = os.path.join(options.binaryen_root, 'test')
 
@@ -207,8 +207,7 @@ if options.valgrind:
 
 
 def in_binaryen(*args):
-    __rootpath__ = os.path.abspath(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
-    return os.path.join(__rootpath__, *args)
+    return os.path.join(options.binaryen_root, *args)
 
 
 os.environ['BINARYEN'] = in_binaryen()
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;
       }
     }
diff --git a/test/lld/shared_longjmp.wat b/test/lld/shared_longjmp.wat
index 235e1fadf..b5179676c 100644
--- a/test/lld/shared_longjmp.wat
+++ b/test/lld/shared_longjmp.wat
@@ -15,41 +15,41 @@
  (import "GOT.mem" "__THREW__" (global $gimport$13 (mut i32)))
  (import "GOT.func" "emscripten_longjmp_jmpbuf" (global $gimport$14 (mut i32)))
  (import "GOT.mem" "__threwValue" (global $gimport$15 (mut i32)))
- (import "env" "malloc" (func $malloc (param i32) (result i32)))
- (import "env" "saveSetjmp" (func $saveSetjmp (param i32 i32 i32 i32) (result i32)))
- (import "env" "getTempRet0" (func $getTempRet0 (result i32)))
- (import "env" "emscripten_longjmp_jmpbuf" (func $emscripten_longjmp_jmpbuf (param i32 i32)))
- (import "env" "__invoke_void_i32_i32" (func $__invoke_void_i32_i32 (param i32 i32 i32)))
- (import "env" "testSetjmp" (func $testSetjmp (param i32 i32 i32) (result i32)))
- (import "env" "setTempRet0" (func $setTempRet0 (param i32)))
- (import "env" "free" (func $free (param i32)))
- (import "env" "emscripten_longjmp" (func $emscripten_longjmp (param i32 i32)))
+ (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" "emscripten_longjmp_jmpbuf" (func $fimport$7 (param i32 i32)))
+ (import "env" "__invoke_void_i32_i32" (func $fimport$8 (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)))
  (global $global$0 i32 (i32.const 0))
  (global $global$1 i32 (i32.const 4))
- (export "__wasm_call_ctors" (func $__wasm_call_ctors))
- (export "_start" (func $_start))
+ (export "__wasm_call_ctors" (func $0))
+ (export "_start" (func $2))
  (export "__THREW__" (global $global$0))
  (export "__threwValue" (global $global$1))
- (func $__wasm_call_ctors (; 9 ;) (type $7)
-  (call $__wasm_apply_relocs)
+ (func $0 (; 9 ;) (type $7)
+  (call $1)
  )
- (func $__wasm_apply_relocs (; 10 ;) (type $7)
+ (func $1 (; 10 ;) (type $7)
  )
- (func $_start (; 11 ;) (type $7)
+ (func $2 (; 11 ;) (type $7)
   (local $0 i32)
   (local $1 i32)
   (local $2 i32)
   (local $3 i32)
   (i32.store
    (local.tee $0
-    (call $malloc
+    (call $fimport$4
      (i32.const 40)
     )
    )
    (i32.const 0)
   )
   (local.set $1
-   (call $saveSetjmp
+   (call $fimport$5
     (local.get $0)
     (i32.const 1)
     (local.get $0)
@@ -57,7 +57,7 @@
    )
   )
   (local.set $2
-   (call $getTempRet0)
+   (call $fimport$6)
   )
   (local.set $0
    (i32.const 0)
@@ -74,7 +74,7 @@
       )
       (i32.const 0)
      )
-     (call $__invoke_void_i32_i32
+     (call $fimport$8
       (global.get $gimport$14)
       (local.get $0)
       (i32.const 1)
@@ -108,7 +108,7 @@
       )
       (br_if $label$1
        (i32.eqz
-        (call $testSetjmp
+        (call $fimport$9
          (i32.load
           (local.get $3)
          )
@@ -117,22 +117,22 @@
         )
        )
       )
-      (call $setTempRet0
+      (call $fimport$10
        (local.get $0)
       )
      )
      (local.set $0
-      (call $getTempRet0)
+      (call $fimport$6)
      )
      (br $label$3)
     )
    )
-   (call $free
+   (call $fimport$11
     (local.get $1)
    )
    (return)
   )
-  (call $emscripten_longjmp
+  (call $fimport$12
    (local.get $3)
    (local.get $0)
   )
diff --git a/test/lld/shared_longjmp.wat.out b/test/lld/shared_longjmp.wat.out
index 083fcb3cc..871e26b27 100644
--- a/test/lld/shared_longjmp.wat.out
+++ b/test/lld/shared_longjmp.wat.out
@@ -12,14 +12,14 @@
  (import "env" "table" (table $0 0 funcref))
  (import "env" "__memory_base" (global $gimport$2 i32))
  (import "env" "__table_base" (global $gimport$3 i32))
- (import "env" "malloc" (func $malloc (param i32) (result i32)))
- (import "env" "saveSetjmp" (func $saveSetjmp (param i32 i32 i32 i32) (result i32)))
- (import "env" "getTempRet0" (func $getTempRet0 (result i32)))
- (import "env" "invoke_vii" (func $invoke_vii (param i32 i32 i32)))
- (import "env" "testSetjmp" (func $testSetjmp (param i32 i32 i32) (result i32)))
- (import "env" "setTempRet0" (func $setTempRet0 (param i32)))
- (import "env" "free" (func $free (param i32)))
- (import "env" "emscripten_longjmp" (func $emscripten_longjmp (param i32 i32)))
+ (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" "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" "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)))
@@ -28,32 +28,32 @@
  (global $gimport$15 (mut i32) (i32.const 0))
  (global $global$0 i32 (i32.const 0))
  (global $global$1 i32 (i32.const 4))
- (export "_start" (func $_start))
+ (export "_start" (func $2))
  (export "__THREW__" (global $global$0))
  (export "__threwValue" (global $global$1))
  (export "dynCall_vii" (func $dynCall_vii))
  (export "__post_instantiate" (func $__post_instantiate))
- (func $__wasm_call_ctors (; 11 ;)
-  (call $__wasm_apply_relocs)
+ (func $0 (; 11 ;)
+  (call $1)
  )
- (func $__wasm_apply_relocs (; 12 ;)
+ (func $1 (; 12 ;)
   (nop)
  )
- (func $_start (; 13 ;)
+ (func $2 (; 13 ;)
   (local $0 i32)
   (local $1 i32)
   (local $2 i32)
   (local $3 i32)
   (i32.store
    (local.tee $0
-    (call $malloc
+    (call $fimport$4
      (i32.const 40)
     )
    )
    (i32.const 0)
   )
   (local.set $1
-   (call $saveSetjmp
+   (call $fimport$5
     (local.get $0)
     (i32.const 1)
     (local.get $0)
@@ -61,7 +61,7 @@
    )
   )
   (local.set $2
-   (call $getTempRet0)
+   (call $fimport$6)
   )
   (local.set $0
    (i32.const 0)
@@ -78,7 +78,7 @@
       )
       (i32.const 0)
      )
-     (call $invoke_vii
+     (call $fimport$8
       (global.get $gimport$14)
       (local.get $0)
       (i32.const 1)
@@ -112,7 +112,7 @@
       )
       (br_if $label$1
        (i32.eqz
-        (call $testSetjmp
+        (call $fimport$9
          (i32.load
           (local.get $3)
          )
@@ -121,22 +121,22 @@
         )
        )
       )
-      (call $setTempRet0
+      (call $fimport$10
        (local.get $0)
       )
      )
      (local.set $0
-      (call $getTempRet0)
+      (call $fimport$6)
      )
      (br $label$3)
     )
    )
-   (call $free
+   (call $fimport$11
     (local.get $1)
    )
    (return)
   )
-  (call $emscripten_longjmp
+  (call $fimport$12
    (local.get $3)
    (local.get $0)
   )
@@ -151,7 +151,7 @@
  )
  (func $__post_instantiate (; 15 ;)
   (call $__assign_got_enties)
-  (call $__wasm_call_ctors)
+  (call $0)
  )
  (func $__assign_got_enties (; 16 ;)
   (global.set $gimport$13