Updates after review

src/ir/module-utils.cpp

@@ -24,26 +24,28 @@
 
 namespace wasm::ModuleUtils {
 
+// Update the file name indices when moving a set of debug locations from one
+// module to another.
 static void updateLocationSet(std::set<Function::DebugLocation>& locations,
-                              std::vector<Index>* indexMap) {
+                              std::vector<Index>& fileIndexMap) {
   std::set<Function::DebugLocation> updatedLocations;
 
   for (auto iter : locations) {
-    iter.fileIndex = (*indexMap)[iter.fileIndex];
+    iter.fileIndex = fileIndexMap[iter.fileIndex];
     updatedLocations.insert(iter);
   }
   locations.clear();
   std::swap(locations, updatedLocations);
 }
 
 // Copies a function into a module. If newName is provided it is used as the
-// name of the function (otherwise the original name is copied). If indexMap is
-// specified, it is used to rename source map filename indices when copying the
-// function from one module to another one.
+// name of the function (otherwise the original name is copied). If fileIndexMap
+// is specified, it is used to rename source map filename indices when copying
+// the function from one module to another one.
 Function* copyFunction(Function* func,
                        Module& out,
                        Name newName,
-                       std::vector<Index>* indexMap) {
+                       std::optional<std::vector<Index>> fileIndexMap) {
   auto ret = std::make_unique<Function>();
   ret->name = newName.is() ? newName : func->name;
   ret->type = func->type;
@@ -57,12 +59,12 @@ Function* copyFunction(Function* func,
   ret->prologLocation = func->prologLocation;
   ret->epilogLocation = func->epilogLocation;
   // Update file indices if needed
-  if (indexMap) {
+  if (fileIndexMap) {
     for (auto& iter : ret->debugLocations) {
-      iter.second.fileIndex = (*indexMap)[iter.second.fileIndex];
+      iter.second.fileIndex = (*fileIndexMap)[iter.second.fileIndex];
     }
-    updateLocationSet(ret->prologLocation, indexMap);
-    updateLocationSet(ret->epilogLocation, indexMap);
+    updateLocationSet(ret->prologLocation, *fileIndexMap);
+    updateLocationSet(ret->epilogLocation, *fileIndexMap);
   }
   ret->module = func->module;
   ret->base = func->base;
@@ -165,13 +167,27 @@ DataSegment* copyDataSegment(const DataSegment* segment, Module& out) {
 
 // Copies named toplevel module items (things of kind ModuleItemKind). See
 // copyModule() for something that also copies exports, the start function, etc.
-// The indexMap is used to update source map information when copying functions
-// from one module to another.
-void copyModuleItems(const Module& in,
-                     Module& out,
-                     std::vector<Index>* indexMap) {
+// The fileIndexMap is used to update source map information when copying
+// functions from one module to another.
+void copyModuleItems(const Module& in, Module& out) {
+  std::vector<Index> fileIndexMap;
+  std::unordered_map<std::string, Index> debugInfoFileIndices;
+  for (Index i = 0; i < out.debugInfoFileNames.size(); i++) {
+    debugInfoFileIndices[out.debugInfoFileNames[i]] = i;
+  }
+  for (Index i = 0; i < in.debugInfoFileNames.size(); i++) {
+    std::string file = in.debugInfoFileNames[i];
+    auto iter = debugInfoFileIndices.find(file);
+    if (iter == debugInfoFileIndices.end()) {
+      Index index = out.debugInfoFileNames.size();
+      out.debugInfoFileNames.push_back(file);
+      debugInfoFileIndices[file] = index;
+    }
+    fileIndexMap.push_back(debugInfoFileIndices[file]);
+  }
+
   for (auto& curr : in.functions) {
-    copyFunction(curr.get(), out, Name(), indexMap);
+    copyFunction(curr.get(), out, Name(), fileIndexMap);
   }
   for (auto& curr : in.globals) {
     copyGlobal(curr.get(), out);

src/ir/module-utils.h

@@ -24,13 +24,13 @@
 namespace wasm::ModuleUtils {
 
 // Copies a function into a module. If newName is provided it is used as the
-// name of the function (otherwise the original name is copied). If indexMap is
+// name of the function (otherwise the original name is copied). If fileIndexMap is
 // specified, it is used to rename source map filename indices when copying the
 // function from one module to another one.
 Function* copyFunction(Function* func,
                        Module& out,
                        Name newName = Name(),
-                       std::vector<Index>* indexMap = 0);
+                       std::optional<std::vector<Index>> fileIndexMap = std::nullopt);
 
 Global* copyGlobal(Global* global, Module& out);
 
@@ -46,11 +46,7 @@ DataSegment* copyDataSegment(const DataSegment* segment, Module& out);
 
 // Copies named toplevel module items (things of kind ModuleItemKind). See
 // copyModule() for something that also copies exports, the start function, etc.
-// The indexMap is used to update source map information when copying functions
-// from one module to another.
-void copyModuleItems(const Module& in,
-                     Module& out,
-                     std::vector<Index>* indexMap = 0);
+void copyModuleItems(const Module& in, Module& out);
 
 void copyModule(const Module& in, Module& out);
 

src/tools/wasm-merge.cpp

@@ -324,26 +324,9 @@ void renameInputItems(Module& input) {
 }
 
 void copyModuleContents(Module& input, Name inputName) {
-  // Source map filename index mapping
-  std::vector<Index> indexMap;
-  std::unordered_map<std::string, Index> debugInfoFileIndices;
-  for (Index i = 0; i < merged.debugInfoFileNames.size(); i++) {
-    debugInfoFileIndices[merged.debugInfoFileNames[i]] = i;
-  }
-  for (Index i = 0; i < input.debugInfoFileNames.size(); i++) {
-    std::string file = input.debugInfoFileNames[i];
-    auto iter = debugInfoFileIndices.find(file);
-    if (iter == debugInfoFileIndices.end()) {
-      Index index = merged.debugInfoFileNames.size();
-      merged.debugInfoFileNames.push_back(file);
-      debugInfoFileIndices[file] = index;
-    }
-    indexMap.push_back(debugInfoFileIndices[file]);
-  }
-
-  // Copy the regular module items (functions, globals) etc. which we
+  // First, copy the regular module items (functions, globals) etc. which we
   // have proper names for, and can just copy.
-  ModuleUtils::copyModuleItems(input, merged, &indexMap);
+  ModuleUtils::copyModuleItems(input, merged);
 
   // We must handle exports in a special way, as we need to note their origin
   // module as we copy them in (also, they are not importable or exportable, so

src/tools/wasm-metadce.cpp

@@ -497,7 +497,7 @@ int main(int argc, const char* argv[]) {
     ModuleReader reader;
     reader.setDWARF(debugInfo);
     try {
-      reader.read(options.extra["infile"], wasm, input);
+      reader.read(options.extra["infile"], wasm, inputSourceMapFilename);
     } catch (ParseException& p) {
       p.dump(std::cerr);
       Fatal() << "error in parsing wasm input";

test/lit/merge/sourcemap.wat

@@ -1,6 +1,10 @@
 ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
 
-;; RUN: wasm-merge %s first %s.second second -S -o - | filecheck %s
+;; RUN: wasm-merge %s first %s.second second -S -o - | filecheck %s --check-prefix=CHECK-TEXT
+;; RUN: wasm-as %s -o %t.wasm --source-map %t.map
+;; RUN: wasm-as %s.second -o %t.second.wasm --source-map %t.second.map
+;; RUN: wasm-merge %t.wasm first --input-source-map %t.map  %t.second.wasm second --input-source-map %t.second.map -o %t.merged.wasm --output-source-map %t.merged.map
+;; RUN: wasm-dis %t.merged.wasm --source-map %t.merged.map -o - | filecheck %s --check-prefix=CHECK-BIN
 
 ;; Test that sourcemap information is preserved
 
@@ -12,22 +16,42 @@
      ;;@ a:3:1
   )
 )
-;; CHECK:      (type $0 (func))
+;; CHECK-TEXT:      (type $0 (func))
 
-;; CHECK:      (export "f" (func $0))
+;; CHECK-TEXT:      (export "f" (func $0))
 
-;; CHECK:      (export "g" (func $0_1))
+;; CHECK-TEXT:      (export "g" (func $0_1))
 
-;; CHECK:      ;;@ a:1:1
-;; CHECK-NEXT: (func $0
-;; CHECK-NEXT:  ;;@ a:2:1
-;; CHECK-NEXT:  (nop)
-;; CHECK-NEXT:  ;;@ a:3:1
-;; CHECK-NEXT: )
+;; CHECK-TEXT:      ;;@ a:1:1
+;; CHECK-TEXT-NEXT: (func $0
+;; CHECK-TEXT-NEXT:  ;;@ a:2:1
+;; CHECK-TEXT-NEXT:  (nop)
+;; CHECK-TEXT-NEXT:  ;;@ a:3:1
+;; CHECK-TEXT-NEXT: )
 
-;; CHECK:      ;;@ b:1:2
-;; CHECK-NEXT: (func $0_1
-;; CHECK-NEXT:  ;;@ b:2:2
-;; CHECK-NEXT:  (nop)
-;; CHECK-NEXT:  ;;@ b:3:2
-;; CHECK-NEXT: )
+;; CHECK-TEXT:      ;;@ b:1:2
+;; CHECK-TEXT-NEXT: (func $0_1
+;; CHECK-TEXT-NEXT:  ;;@ b:2:2
+;; CHECK-TEXT-NEXT:  (nop)
+;; CHECK-TEXT-NEXT:  ;;@ b:3:2
+;; CHECK-TEXT-NEXT: )
+
+;; CHECK-BIN:      (type $0 (func))
+
+;; CHECK-BIN:      (export "f" (func $0))
+
+;; CHECK-BIN:      (export "g" (func $1))
+
+;; CHECK-BIN:      ;;@ a:1:1
+;; CHECK-BIN-NEXT: (func $0
+;; CHECK-BIN-NEXT:  ;;@ a:2:1
+;; CHECK-BIN-NEXT:  (nop)
+;; CHECK-BIN-NEXT:  ;;@ a:3:1
+;; CHECK-BIN-NEXT: )
+
+;; CHECK-BIN:      ;;@ b:1:2
+;; CHECK-BIN-NEXT: (func $1
+;; CHECK-BIN-NEXT:  ;;@ b:2:2
+;; CHECK-BIN-NEXT:  (nop)
+;; CHECK-BIN-NEXT:  ;;@ b:3:2
+;; CHECK-BIN-NEXT: )

test/lit/metadce/sourcemap.wat

@@ -0,0 +1,26 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
+;; RUN: wasm-metadce %s --graph-file %s.json -S -o - | filecheck %s
+;; RUN: wasm-as %s -o %t.wasm --source-map %t.map
+;; RUN: wasm-metadce %t.wasm --input-source-map %t.map --graph-file %s.json -o %t.out.wasm --output-source-map %t.out.map
+;; RUN: wasm-dis %t.out.wasm --source-map %t.out.map -o - | filecheck %s
+
+;; Test that sourcemap information is preserved
+
+(module
+  ;;@ a:1:1
+  (func (export "f")
+     ;;@ a:2:1
+     (nop)
+     ;;@ a:3:1
+  )
+)
+;; CHECK:      (type $0 (func))
+
+;; CHECK:      (export "f" (func $0))
+
+;; CHECK:      ;;@ a:1:1
+;; CHECK-NEXT: (func $0
+;; CHECK-NEXT:  ;;@ a:2:1
+;; CHECK-NEXT:  (nop)
+;; CHECK-NEXT:  ;;@ a:3:1
+;; CHECK-NEXT: )

test/lit/metadce/sourcemap.wat.json

@@ -0,0 +1,11 @@
+[
+  {
+    "name": "root",
+    "reaches": ["f"],
+    "root": true
+  },
+  {
+    "name": "f",
+    "export": "f"
+  }
+]