DWARF debug line updating

src/passes/DWARF.cpp

@@ -35,14 +35,6 @@ struct DWARFDump : public Pass {
   }
 };
 
-struct DWARFUpdate : public Pass {
-  void run(PassRunner* runner, Module* module) override {
-    Debug::writeDWARFSections(*module);
-  }
-};
-
 Pass* createDWARFDumpPass() { return new DWARFDump(); }
 
-Pass* createDWARFUpdatePass() { return new DWARFUpdate(); }
-
 } // namespace wasm

src/passes/Print.cpp

@@ -1427,7 +1427,8 @@ struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
         auto iter = currFunction->binaryLocations.find(curr);
         if (iter != currFunction->binaryLocations.end()) {
           Colors::grey(o);
-          o << ";; code offset: 0x" << iter->second << '\n';
+          o << ";; code offset: 0x" << std::hex << iter->second << std::dec
+            << '\n';
           restoreNormalColor(o);
           doIndent(o, indent);
         }

src/passes/RoundTrip.cpp

@@ -57,7 +57,7 @@ struct RoundTrip : public Pass {
     // Read
     ModuleUtils::clearModule(*module);
     ModuleReader reader;
-    // TODO: enable debug info when relevant
+    reader.setDWARF(runner->options.debugInfo);
     reader.read(tempName, *module);
     // Clean up
     std::remove(tempName.c_str());

src/passes/pass.cpp

@@ -110,8 +110,6 @@ void PassRegistry::registerPasses() {
   registerPass("dwarfdump",
                "dump DWARF debug info sections from the read binary",
                createDWARFDumpPass);
-  registerPass(
-    "dwarfupdate", "update DWARF debug info sections", createDWARFUpdatePass);
   registerPass("duplicate-import-elimination",
                "removes duplicate imports",
                createDuplicateImportEliminationPass);

src/passes/passes.h

@@ -36,7 +36,6 @@ Pass* createDataFlowOptsPass();
 Pass* createDeadCodeEliminationPass();
 Pass* createDirectizePass();
 Pass* createDWARFDumpPass();
-Pass* createDWARFUpdatePass();
 Pass* createDuplicateImportEliminationPass();
 Pass* createDuplicateFunctionEliminationPass();
 Pass* createEmitTargetFeaturesPass();

src/wasm-binary.h

@@ -1044,6 +1044,19 @@ class WasmBinaryWriter {
 
   std::unique_ptr<ImportInfo> importInfo;
 
+  // General debugging info: map every instruction to its original position in
+  // the binary, relative to the beginning of the code section. This is similar
+  // to binaryLocations on Function objects, which are filled as we load the
+  // functions from the binary. Here we track them as we write, and then
+  // the combination of the two can be used to update DWARF info for the new
+  // locations of things.
+  BinaryLocationsMap binaryLocations;
+  size_t binaryLocationsSizeAtSectionStart;
+  // Track the expressions that we added for the current function being
+  // written, so that we can update those specific binary locations when
+  // the function is written out.
+  std::vector<Expression*> binaryLocationTrackedExpressionsForFunc;
+
   void prepare();
 };
 

src/wasm-debug.h

@@ -37,7 +37,7 @@ bool hasDWARFSections(const Module& wasm);
 void dumpDWARF(const Module& wasm);
 
 // Update the DWARF sections.
-void writeDWARFSections(Module& wasm);
+void writeDWARFSections(Module& wasm, const BinaryLocationsMap& newLocations);
 
 } // namespace Debug
 

src/wasm-stack.h

@@ -83,8 +83,16 @@ class BinaryInstWriter : public OverriddenVisitor<BinaryInstWriter> {
 public:
   BinaryInstWriter(WasmBinaryWriter& parent,
                    BufferWithRandomAccess& o,
-                   Function* func)
-    : parent(parent), o(o), func(func) {}
+                   Function* func,
+                   bool sourceMap)
+    : parent(parent), o(o), func(func), sourceMap(sourceMap) {}
+
+  void visit(Expression* curr) {
+    if (func && !sourceMap) {
+      parent.writeDebugLocation(curr, func);
+    }
+    OverriddenVisitor<BinaryInstWriter>::visit(curr);
+  }
 
   void visitBlock(Block* curr);
   void visitIf(If* curr);
@@ -144,6 +152,8 @@ class BinaryInstWriter : public OverriddenVisitor<BinaryInstWriter> {
   WasmBinaryWriter& parent;
   BufferWithRandomAccess& o;
   Function* func = nullptr;
+  bool sourceMap;
+
   std::vector<Name> breakStack;
 
   // type => number of locals of that type in the compact form
@@ -758,7 +768,7 @@ class BinaryenIRToBinaryWriter
                            Function* func = nullptr,
                            bool sourceMap = false)
     : BinaryenIRWriter<BinaryenIRToBinaryWriter>(func), parent(parent),
-      writer(parent, o, func), sourceMap(sourceMap) {}
+      writer(parent, o, func, sourceMap), sourceMap(sourceMap) {}
 
   void visit(Expression* curr) {
     BinaryenIRWriter<BinaryenIRToBinaryWriter>::visit(curr);
@@ -833,7 +843,7 @@ class StackIRToBinaryWriter {
   StackIRToBinaryWriter(WasmBinaryWriter& parent,
                         BufferWithRandomAccess& o,
                         Function* func)
-    : writer(parent, o, func), func(func) {}
+    : writer(parent, o, func, false /* sourceMap */), func(func) {}
 
   void write();
 

src/wasm.h

@@ -1129,7 +1129,10 @@ struct Importable {
 // Stack IR is a secondary IR to the main IR defined in this file (Binaryen
 // IR). See wasm-stack.h.
 class StackInst;
-typedef std::vector<StackInst*> StackIR;
+
+using StackIR = std::vector<StackInst*>;
+
+using BinaryLocationsMap = std::unordered_map<Expression*, uint32_t>;
 
 class Function : public Importable {
 public:
@@ -1178,7 +1181,7 @@ class Function : public Importable {
 
   // General debugging info: map every instruction to its original position in
   // the binary, relative to the beginning of the code section.
-  std::unordered_map<Expression*, uint32_t> binaryLocations;
+  BinaryLocationsMap binaryLocations;
 
   size_t getNumParams();
   size_t getNumVars();

src/wasm/wasm-binary.cpp

@@ -72,6 +72,14 @@ void WasmBinaryWriter::write() {
     writeSourceMapEpilog();
   }
 
+#ifdef BUILD_LLVM_DWARF
+  // Update DWARF user sections after writing the data referred to by them
+  // (function bodies), and before writing the user sections themselves.
+  if (Debug::hasDWARFSections(*wasm)) {
+    Debug::writeDWARFSections(*wasm, binaryLocations);
+  }
+#endif
+
   writeLateUserSections();
   writeFeaturesSection();
 
@@ -109,29 +117,52 @@ template<typename T> int32_t WasmBinaryWriter::startSection(T code) {
   if (sourceMap) {
     sourceMapLocationsSizeAtSectionStart = sourceMapLocations.size();
   }
+  binaryLocationsSizeAtSectionStart = binaryLocations.size();
   return writeU32LEBPlaceholder(); // section size to be filled in later
 }
 
 void WasmBinaryWriter::finishSection(int32_t start) {
   // section size does not include the reserved bytes of the size field itself
   int32_t size = o.size() - start - MaxLEB32Bytes;
   auto sizeFieldSize = o.writeAt(start, U32LEB(size));
-  if (sizeFieldSize != MaxLEB32Bytes) {
+  // We can move things back if the actual LEB for the size doesn't use the
+  // maximum 5 bytes. In that case we need to adjust offsets after we move
+  // things backwards.
+  auto adjustmentForLEBShrinking = MaxLEB32Bytes - sizeFieldSize;
+  if (adjustmentForLEBShrinking) {
     // we can save some room, nice
     assert(sizeFieldSize < MaxLEB32Bytes);
     std::move(&o[start] + MaxLEB32Bytes,
               &o[start] + MaxLEB32Bytes + size,
               &o[start] + sizeFieldSize);
-    auto adjustment = MaxLEB32Bytes - sizeFieldSize;
-    o.resize(o.size() - adjustment);
+    o.resize(o.size() - adjustmentForLEBShrinking);
     if (sourceMap) {
       for (auto i = sourceMapLocationsSizeAtSectionStart;
            i < sourceMapLocations.size();
            ++i) {
-        sourceMapLocations[i].first -= adjustment;
+        sourceMapLocations[i].first -= adjustmentForLEBShrinking;
       }
     }
   }
+
+  if (binaryLocationsSizeAtSectionStart != binaryLocations.size()) {
+    // We added the binary locations, adjust them: they must be relative
+    // to the code section.
+    assert(binaryLocationsSizeAtSectionStart == 0);
+    // The section type byte is right before the LEB for the size; we want
+    // offsets that are relative to the body, which is after that section type
+    // byte and the the size LEB.
+    auto body = start + sizeFieldSize;
+    for (auto& pair : binaryLocations) {
+      // Offsets are relative to the body of the code section: after the
+      // section type byte and the size.
+      // Everything was moved by the adjustment, track that. After this,
+      // we are at the right absolute address.
+      pair.second -= adjustmentForLEBShrinking;
+      // We are relative to the section start.
+      pair.second -= body;
+    }
+  }
 }
 
 int32_t
@@ -266,6 +297,7 @@ void WasmBinaryWriter::writeFunctions() {
   auto start = startSection(BinaryConsts::Section::Code);
   o << U32LEB(importInfo->getNumDefinedFunctions());
   ModuleUtils::iterDefinedFunctions(*wasm, [&](Function* func) {
+    assert(binaryLocationTrackedExpressionsForFunc.empty());
     size_t sourceMapLocationsSizeAtFunctionStart = sourceMapLocations.size();
     BYN_TRACE("write one at" << o.size() << std::endl);
     size_t sizePos = writeU32LEBPlaceholder();
@@ -284,22 +316,31 @@ void WasmBinaryWriter::writeFunctions() {
     BYN_TRACE("body size: " << size << ", writing at " << sizePos
                             << ", next starts at " << o.size() << "\n");
     auto sizeFieldSize = o.writeAt(sizePos, U32LEB(size));
-    if (sizeFieldSize != MaxLEB32Bytes) {
+    // We can move things back if the actual LEB for the size doesn't use the
+    // maximum 5 bytes. In that case we need to adjust offsets after we move
+    // things backwards.
+    auto adjustmentForLEBShrinking = MaxLEB32Bytes - sizeFieldSize;
+    if (adjustmentForLEBShrinking) {
       // we can save some room, nice
       assert(sizeFieldSize < MaxLEB32Bytes);
       std::move(&o[start], &o[start] + size, &o[sizePos] + sizeFieldSize);
-      auto adjustment = MaxLEB32Bytes - sizeFieldSize;
-      o.resize(o.size() - adjustment);
+      o.resize(o.size() - adjustmentForLEBShrinking);
       if (sourceMap) {
         for (auto i = sourceMapLocationsSizeAtFunctionStart;
              i < sourceMapLocations.size();
              ++i) {
-          sourceMapLocations[i].first -= adjustment;
+          sourceMapLocations[i].first -= adjustmentForLEBShrinking;
         }
       }
+      for (auto* curr : binaryLocationTrackedExpressionsForFunc) {
+        // We added the binary locations, adjust them: they must be relative
+        // to the code section.
+        binaryLocations[curr] -= adjustmentForLEBShrinking;
+      }
     }
     tableOfContents.functionBodies.emplace_back(
       func->name, sizePos + sizeFieldSize, size);
+    binaryLocationTrackedExpressionsForFunc.clear();
   });
   finishSection(start);
 }
@@ -649,10 +690,19 @@ void WasmBinaryWriter::writeDebugLocation(const Function::DebugLocation& loc) {
 }
 
 void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
-  auto& debugLocations = func->debugLocations;
-  auto iter = debugLocations.find(curr);
-  if (iter != debugLocations.end()) {
-    writeDebugLocation(iter->second);
+  if (sourceMap) {
+    auto& debugLocations = func->debugLocations;
+    auto iter = debugLocations.find(curr);
+    if (iter != debugLocations.end()) {
+      writeDebugLocation(iter->second);
+    }
+  }
+  // TODO: remove source map debugging support and refactor this method
+  // to something that directly thinks about DWARF, instead of indirectly
+  // looking at func->binaryLocations as a proxy for that etc.
+  if (func && !func->binaryLocations.empty()) {
+    binaryLocations[curr] = o.size();
+    binaryLocationTrackedExpressionsForFunc.push_back(curr);
   }
 }
 
@@ -809,6 +859,9 @@ void WasmBinaryBuilder::read() {
         readFunctionSignatures();
         break;
       case BinaryConsts::Section::Code:
+        if (DWARF) {
+          codeSectionLocation = pos;
+        }
         readFunctions();
         break;
       case BinaryConsts::Section::Export:
@@ -1288,9 +1341,6 @@ void WasmBinaryBuilder::readFunctionSignatures() {
 
 void WasmBinaryBuilder::readFunctions() {
   BYN_TRACE("== readFunctions\n");
-  if (DWARF) {
-    codeSectionLocation = pos;
-  }
   size_t total = getU32LEB();
   if (total != functionSignatures.size()) {
     throwError("invalid function section size, must equal types");