Revert "Reland "Fix renaming in FixInvokeFunctionNamesWalker (#2513)""

scripts/test/generate_lld_tests.py

@@ -30,8 +30,8 @@ def files_with_extensions(path, extensions):
             yield file, ext
 
 
-def generate_wat_files(llvm_bin, emscripten_root):
-    print('\n[ building wat files from C sources... ]\n')
+def generate_wast_files(llvm_bin, emscripten_root):
+    print('\n[ building wast 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_wat_files(llvm_bin, emscripten_root):
         obj_path = os.path.join(lld_path, obj_file)
 
         wasm_file = src_file.replace(ext, '.wasm')
-        wat_file = src_file.replace(ext, '.wat')
+        wast_file = src_file.replace(ext, '.wast')
 
         obj_path = os.path.join(lld_path, obj_file)
         wasm_path = os.path.join(lld_path, wasm_file)
-        wat_path = os.path.join(lld_path, wat_file)
+        wast_path = os.path.join(lld_path, wast_file)
         is_shared = 'shared' in src_file
 
         compile_cmd = [
@@ -70,10 +70,6 @@ def generate_wat_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 'longjmp' in src_file:
-            link_cmd.append('--strip-debug')
         if is_shared:
             compile_cmd.append('-fPIC')
             compile_cmd.append('-fvisibility=default')
@@ -84,7 +80,7 @@ def generate_wat_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', wat_path])
+            support.run_command(shared.WASM_DIS + [wasm_path, '-o', wast_path])
         finally:
             # Don't need the .o or .wasm files, don't leave them around
             shared.delete_from_orbit(obj_path)
@@ -95,4 +91,4 @@ def generate_wat_files(llvm_bin, emscripten_root):
     if len(shared.options.positional_args) != 2:
         print('Usage: generate_lld_tests.py [llvm/bin/dir] [path/to/emscripten]')
         sys.exit(1)
-    generate_wat_files(*shared.options.positional_args)
+    generate_wast_files(*shared.options.positional_args)

scripts/test/shared.py

@@ -87,7 +87,6 @@ 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 = []
@@ -124,7 +123,8 @@ def warn(text):
 
 # Locate Binaryen source directory if not specified.
 if not options.binaryen_root:
-    options.binaryen_root = os.path.dirname(os.path.dirname(script_dir))
+    path_parts = os.path.abspath(__file__).split(os.path.sep)
+    options.binaryen_root = os.path.sep.join(path_parts[:-3])
 
 options.binaryen_test = os.path.join(options.binaryen_root, 'test')
 
@@ -205,7 +205,8 @@ def wrap_with_valgrind(cmd):
 
 
 def in_binaryen(*args):
-    return os.path.join(options.binaryen_root, *args)
+    __rootpath__ = os.path.abspath(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
+    return os.path.join(__rootpath__, *args)
 
 
 os.environ['BINARYEN'] = in_binaryen()

src/wasm/wasm-emscripten.cpp

@@ -994,11 +994,11 @@ struct FixInvokeFunctionNamesWalker
   : public PostWalker<FixInvokeFunctionNamesWalker> {
   Module& wasm;
   std::map<Name, Name> importRenames;
-  std::map<Name, Name> functionReplace;
+  std::vector<Name> toRemove;
+  std::set<Name> newImports;
   std::set<Signature> invokeSigs;
-  ImportInfo imports;
 
-  FixInvokeFunctionNamesWalker(Module& _wasm) : wasm(_wasm), imports(wasm) {}
+  FixInvokeFunctionNamesWalker(Module& _wasm) : wasm(_wasm) {}
 
   // Converts invoke wrapper names generated by LLVM backend to real invoke
   // wrapper names that are expected by JavaScript glue code.
@@ -1053,38 +1053,27 @@ struct FixInvokeFunctionNamesWalker
       return;
     }
 
-    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;
+    assert(importRenames.count(curr->name) == 0);
+    BYN_TRACE("renaming: " << curr->name << " -> " << newname << "\n");
+    importRenames[curr->name] = newname;
+    // Either rename or remove the existing import
+    if (wasm.getFunctionOrNull(newname) || !newImports.insert(newname).second) {
+      toRemove.push_back(curr->name);
     } else {
-      BYN_TRACE("renaming the import in place\n");
       curr->base = newname;
+      curr->name = 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 importName : toRemove) {
+      wasm.removeFunction(importName);
     }
-    // Rename all uses of the removed functions
-    ModuleUtils::renameFunctions(wasm, functionReplace);
-
-    // For imports that for renamed, update any associated GOT.func imports.
+    ModuleUtils::renameFunctions(wasm, importRenames);
+    ImportInfo imports(wasm);
     for (auto& pair : importRenames) {
-      BYN_TRACE("looking for: GOT.func." << pair.first << "\n");
+      // 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;
       }
     }