Source map fixes (#6550)

* Keep debug locations at function start

The `fn_prolog_epilog.debugInfo` test is failing otherwise, since there
was debug information associated to the nop instruction at the beginning
of the function.

* Do not clear the debug information when reaching the end of the source map

The last segment should extend to the end of the function.

* Propagate debug location from the function prolog to its first instruction

* Fix printing of epilogue location

The text parser no longer propagates locations to the epilogue, so we
should always print the location if there is one.

* Fix debug location smearing

The debug location of the last instruction should not smear into the
function epilogue, and a debug location from a previous function should
not smear into the prologue of the current function.

Author: vouillon

src/passes/DebugLocationPropagation.cpp

@@ -64,6 +64,10 @@ struct DebugLocationPropagation
         if (auto it = locs.find(previous); it != locs.end()) {
           locs[curr] = it->second;
         }
+      } else if (self->getFunction()->prologLocation.size()) {
+        // Instructions may inherit their locations from the function
+        // prolog.
+        locs[curr] = *self->getFunction()->prologLocation.begin();
       }
     }
     expressionStack.push_back(curr);

src/passes/Print.cpp

@@ -3025,8 +3025,7 @@ void PrintSExpression::visitDefinedFunction(Function* curr) {
     // Print the stack IR.
     printStackIR(curr->stackIR.get(), *this);
   }
-  if (currFunction->epilogLocation.size() &&
-      lastPrintedLocation != *currFunction->epilogLocation.begin()) {
+  if (currFunction->epilogLocation.size()) {
     // Print last debug location: mix of decIndent and printDebugLocation
     // logic.
     doIndent(o, indent);

src/wasm-binary.h

@@ -1348,6 +1348,7 @@ class WasmBinaryWriter {
   void writeSourceMapProlog();
   void writeSourceMapEpilog();
   void writeDebugLocation(const Function::DebugLocation& loc);
+  void writeNoDebugLocation();
   void writeDebugLocation(Expression* curr, Function* func);
   void writeDebugLocationEnd(Expression* curr, Function* func);
   void writeExtraDebugLocation(Expression* curr, Function* func, size_t id);

src/wasm-stack.h

@@ -443,8 +443,13 @@ class BinaryenIRToBinaryWriter
   void emitDelegate(Try* curr) { writer.emitDelegate(curr); }
   void emitScopeEnd(Expression* curr) { writer.emitScopeEnd(curr); }
   void emitFunctionEnd() {
+    // Indicate the debug location corresponding to the end opcode
+    // that terminates the function code.
     if (func->epilogLocation.size()) {
       parent.writeDebugLocation(*func->epilogLocation.begin());
+    } else {
+      // The end opcode has no debug location.
+      parent.writeNoDebugLocation();
     }
     writer.emitFunctionEnd();
   }

src/wasm/wasm-binary.cpp

@@ -398,8 +398,10 @@ void WasmBinaryWriter::writeFunctions() {
   bool DWARF = Debug::hasDWARFSections(*getModule());
   ModuleUtils::iterDefinedFunctions(*wasm, [&](Function* func) {
     assert(binaryLocationTrackedExpressionsForFunc.empty());
-    size_t sourceMapLocationsSizeAtFunctionStart = sourceMapLocations.size();
     BYN_TRACE("write one at" << o.size() << std::endl);
+    // Do not smear any debug location from the previous function.
+    writeNoDebugLocation();
+    size_t sourceMapLocationsSizeAtFunctionStart = sourceMapLocations.size();
     size_t sizePos = writeU32LEBPlaceholder();
     size_t start = o.size();
     BYN_TRACE("writing" << func->name << std::endl);
@@ -1373,6 +1375,28 @@ void WasmBinaryWriter::writeDebugLocation(const Function::DebugLocation& loc) {
   lastDebugLocation = loc;
 }
 
+void WasmBinaryWriter::writeNoDebugLocation() {
+  // Emit an indication that there is no debug location there (so that
+  // we do not get "smeared" with debug info from anything before or
+  // after us).
+  //
+  // We don't need to write repeated "no debug info" indications, as a
+  // single one is enough to make it clear that the debug information
+  // before us is valid no longer. We also don't need to write one if
+  // there is nothing before us.
+  if (!sourceMapLocations.empty() &&
+      sourceMapLocations.back().second != nullptr) {
+    sourceMapLocations.emplace_back(o.size(), nullptr);
+
+    // Initialize the state of debug info to indicate there is no current
+    // debug info relevant. This sets |lastDebugLocation| to a dummy value,
+    // so that later places with debug info can see that they differ from
+    // it (without this, if we had some debug info, then a nullptr for none,
+    // and then the same debug info, we could get confused).
+    initializeDebugInfo();
+  }
+}
+
 void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
   if (sourceMap) {
     auto& debugLocations = func->debugLocations;
@@ -1381,25 +1405,8 @@ void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
       // There is debug information here, write it out.
       writeDebugLocation(iter->second);
     } else {
-      // This expression has no debug location. We need to emit an indication
-      // of that (so that we do not get "smeared" with debug info from anything
-      // before or after us).
-      //
-      // We don't need to write repeated "no debug info" indications, as a
-      // single one is enough to make it clear that the debug information before
-      // us is valid no longer. We also don't need to write one if there is
-      // nothing before us.
-      if (!sourceMapLocations.empty() &&
-          sourceMapLocations.back().second != nullptr) {
-        sourceMapLocations.emplace_back(o.size(), nullptr);
-
-        // Initialize the state of debug info to indicate there is no current
-        // debug info relevant. This sets |lastDebugLocation| to a dummy value,
-        // so that later places with debug info can see that they differ from
-        // it (without this, if we had some debug info, then a nullptr for none,
-        // and then the same debug info, we could get confused).
-        initializeDebugInfo();
-      }
+      // This expression has no debug location.
+      writeNoDebugLocation();
     }
   }
   // If this is an instruction in a function, and if the original wasm had
@@ -2687,12 +2694,11 @@ void WasmBinaryReader::readFunctions() {
 
     readVars();
 
-    std::swap(func->prologLocation, debugLocation);
+    func->prologLocation = debugLocation;
     {
       // process the function body
       BYN_TRACE("processing function: " << i << std::endl);
       nextLabel = 0;
-      debugLocation.clear();
       willBeIgnored = false;
       // process body
       assert(breakStack.empty());
@@ -2931,7 +2937,6 @@ void WasmBinaryReader::readNextDebugLocation() {
 
   if (nextDebugPos == 0) {
     // We reached the end of the source map; nothing left to read.
-    debugLocation.clear();
     return;
   }
 

src/wasm/wasm-stack.cpp

@@ -2839,8 +2839,13 @@ void StackIRToBinaryWriter::write() {
         WASM_UNREACHABLE("unexpected op");
     }
   }
+  // Indicate the debug location corresponding to the end opcode that
+  // terminates the function code.
   if (func->epilogLocation.size()) {
     parent.writeDebugLocation(*func->epilogLocation.begin());
+  } else {
+    // The end opcode has no debug location.
+    parent.writeNoDebugLocation();
   }
   writer.emitFunctionEnd();
 }

test/binaryen.js/sourcemap.js.txt

@@ -5,4 +5,4 @@ module.c
 020: u  r  c  e  M  a  p  p  i  n  g  U  R  L  0f m  
 030: o  d  u  l  e  .  w  a  s  m  .  m  a  p 
 
-{"version":3,"sources":["module.c"],"names":[],"mappings":"wBAAE"}
+{"version":3,"sources":["module.c"],"names":[],"mappings":"wBAAE,E"}

test/fib-dbg.wasm.fromBinary

@@ -209,11 +209,12 @@
      (br $label$4)
     )
    )
+   ;;@ fib.c:8:0
    (return
-    ;;@ fib.c:8:0
     (local.get $4)
    )
   )
+  ;;@ fib.c:8:0
  )
  (func $runPostSets
   (local $0 i32)

test/lit/debug/source-map-smearing.wast

@@ -0,0 +1,22 @@
+;; RUN: wasm-opt %s -g -o %t.wasm -osm %t.wasm.map
+;; RUN: echo >> %t.wasm.map
+;; RUN: cat %t.wasm.map | filecheck %s
+
+;; Also test with StackIR, which should have identical results.
+;;
+;; RUN: wasm-opt %s --generate-stack-ir -o %t.wasm -osm %t.map -g -q
+;; RUN: echo >> %t.wasm.map
+;; RUN: cat %t.wasm.map | filecheck %s
+
+;; Check that the debug locations do not smear beyond a function
+;; epilogue to the next function. The encoded segment 'C' means that
+;; the previous segment is indeed one-byte long.
+;; CHECK: {"version":3,"sources":["foo"],"names":[],"mappings":"yBAAC,C,GACC"}
+(module
+  (func $0
+    ;;@ foo:1:1
+  )
+  (func $1
+    ;;@ foo:2:2
+  )
+)

test/lit/debug/source-map-stop.wast

@@ -3,6 +3,11 @@
 ;; RUN: wasm-opt %s -g -o %t.wasm -osm %t.wasm.map
 ;; RUN: wasm-opt %t.wasm -ism %t.wasm.map -S -o - | filecheck %s
 
+;; Also test with StackIR, which should have identical results.
+;;
+;; RUN: wasm-opt %s --generate-stack-ir -o %t.wasm -osm %t.map -g -q
+;; RUN: wasm-opt %t.wasm -ism %t.map -q -o - -S | filecheck %s
+
 ;; Verify that writing to a source map and reading it back does not "smear"
 ;; debug info across adjacent instructions. The debug info in the output should
 ;; be identical to the input.