From 2caae6abc5f2de05479de67bcf1d10e9b390bff9 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Mon, 28 Aug 2023 17:10:05 -0700
Subject: [PATCH 01/15] yolo

---
 src/wasm/wasm-binary.cpp            | 43 +++++++++++++++++++++++++----
 test/lit/debug/source-map-stop.wast | 37 +++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 5 deletions(-)
 create mode 100644 test/lit/debug/source-map-stop.wast

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 556f52157d8..be1dfbbc7d5 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -1186,12 +1186,16 @@ void WasmBinaryWriter::writeSourceMapEpilog() {
       *sourceMap << ",";
     }
     writeBase64VLQ(*sourceMap, int32_t(offset - lastOffset));
-    writeBase64VLQ(*sourceMap, int32_t(loc->fileIndex - lastLoc.fileIndex));
-    writeBase64VLQ(*sourceMap, int32_t(loc->lineNumber - lastLoc.lineNumber));
-    writeBase64VLQ(*sourceMap,
-                   int32_t(loc->columnNumber - lastLoc.columnNumber));
-    lastLoc = *loc;
     lastOffset = offset;
+    if (loc) {
+      writeBase64VLQ(*sourceMap, int32_t(loc->fileIndex - lastLoc.fileIndex));
+      writeBase64VLQ(*sourceMap, int32_t(loc->lineNumber - lastLoc.lineNumber));
+      writeBase64VLQ(*sourceMap,
+                     int32_t(loc->columnNumber - lastLoc.columnNumber));
+      lastLoc = *loc;
+    } else {
+      std::cout << "waka waka write a 1-er\n";
+    }
   }
   *sourceMap << "\"}";
 }
@@ -1341,6 +1345,14 @@ void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
     auto iter = debugLocations.find(curr);
     if (iter != debugLocations.end()) {
       writeDebugLocation(iter->second);
+    } else {
+      // This expression has no debug location. Emit a size 1 segment to stop
+      // the current segment (otherwise one instruction with debug info would
+      // "smear" its debug info on everything after it).
+      // waka waka this is good i hope
+      // TODO: don't write repeated ones.
+      sourceMapLocations.emplace_back(o.size(), nullptr);
+      initializeDebugInfo(); // XXX
     }
   }
   // If this is an instruction in a function, and if the original wasm had
@@ -2816,7 +2828,28 @@ void WasmBinaryReader::readNextDebugLocation() {
       throw MapParseException("Unexpected delimiter");
     }
 
+
+
+  // TODO refactor to share
+  auto maybeReadChar = [&](char expected) {
+    if (sourceMap->peek() != expected) {
+      return false;
+    }
+    sourceMap->get();
+    return true;
+  };
+
     int32_t positionDelta = readBase64VLQ(*sourceMap);
+    // waka waka good I thinks
+    if (maybeReadChar(',') || maybeReadChar('\"')) {
+std::cout << "waka read 1-length\n";
+      // This is a 1-length entry, and after it is either another one or the
+      // end of the entire record.
+      nextDebugLocation.availablePos = 0; // XXX we need this updated below don't we?
+      break;
+    }
+
+
     uint32_t position = nextDebugLocation.availablePos + positionDelta;
     int32_t fileIndexDelta = readBase64VLQ(*sourceMap);
     uint32_t fileIndex = nextDebugLocation.next.fileIndex + fileIndexDelta;
diff --git a/test/lit/debug/source-map-stop.wast b/test/lit/debug/source-map-stop.wast
new file mode 100644
index 00000000000..ae5731c6b2a
--- /dev/null
+++ b/test/lit/debug/source-map-stop.wast
@@ -0,0 +1,37 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
+
+;; RUN: wasm-opt %s -g -o %s.wasm -osm %s.wasm.map
+;; RUN: wasm-opt %s.wasm -ism %s.wasm.map -S -o - | 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.
+
+(module
+  ;; CHECK:      (func $func (param $0 i32) (result i32)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (call $func
+  ;; CHECK-NEXT:    ;;@ waka:100:1
+  ;; CHECK-NEXT:    (i32.const 1)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (i32.const 2)
+  ;; CHECK-NEXT: )
+  (func $func (param i32) (result i32)
+    ;; This drop has on debug info, and should stay that way. Specifically the
+    ;; instruction right before it in the binary (the const 1) should not
+    ;; smear its debug info on it. And the drop is between an instruction that
+    ;; has debug info (the const 1) and another (the i32.const 2): we should not
+    ;; receive the debug info of either. (This is a regression test for a bug
+    ;; that only happens in that state: removing the debug info either before or
+    ;; after would avoid the bug.)
+    (drop
+      (call $func
+        ;;@ waka:100:1
+        (i32.const 1)
+      )
+    )
+    ;;@ waka:200:2
+    (i32.const 2)
+  )
+)

From 2aa5e0696dcbdd603921e144e3a6ff298cc123cb Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Mon, 28 Aug 2023 17:14:57 -0700
Subject: [PATCH 02/15] terrible idea

---
 src/wasm/wasm-binary.cpp | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index be1dfbbc7d5..ee04002a4ae 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -1352,7 +1352,7 @@ void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
       // waka waka this is good i hope
       // TODO: don't write repeated ones.
       sourceMapLocations.emplace_back(o.size(), nullptr);
-      initializeDebugInfo(); // XXX
+//      initializeDebugInfo(); // XXX
     }
   }
   // If this is an instruction in a function, and if the original wasm had
@@ -2840,17 +2840,20 @@ void WasmBinaryReader::readNextDebugLocation() {
   };
 
     int32_t positionDelta = readBase64VLQ(*sourceMap);
+    uint32_t position = nextDebugLocation.availablePos + positionDelta;
+
+    nextDebugLocation.previousPos = nextDebugLocation.availablePos;
+    nextDebugLocation.availablePos = position;
+
     // waka waka good I thinks
     if (maybeReadChar(',') || maybeReadChar('\"')) {
 std::cout << "waka read 1-length\n";
       // This is a 1-length entry, and after it is either another one or the
       // end of the entire record.
-      nextDebugLocation.availablePos = 0; // XXX we need this updated below don't we?
+      debugLocation.clear();
       break;
     }
 
-
-    uint32_t position = nextDebugLocation.availablePos + positionDelta;
     int32_t fileIndexDelta = readBase64VLQ(*sourceMap);
     uint32_t fileIndex = nextDebugLocation.next.fileIndex + fileIndexDelta;
     int32_t lineNumberDelta = readBase64VLQ(*sourceMap);
@@ -2859,9 +2862,7 @@ std::cout << "waka read 1-length\n";
     uint32_t columnNumber =
       nextDebugLocation.next.columnNumber + columnNumberDelta;
 
-    nextDebugLocation = {position,
-                         nextDebugLocation.availablePos,
-                         {fileIndex, lineNumber, columnNumber}};
+    nextDebugLocation.next = {fileIndex, lineNumber, columnNumber};
   }
 }
 

From ff7832823f72154905a2e14b575cf4e8dc09bb7f Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 11:32:22 -0700
Subject: [PATCH 03/15] work

---
 src/wasm-binary.h        |  7 +++++--
 src/wasm/wasm-binary.cpp | 30 ++++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index a384d2cd5f9..924413539e4 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -1490,8 +1490,11 @@ class WasmBinaryWriter {
 
   MixedArena allocator;
 
-  // storage of source map locations until the section is placed at its final
-  // location (shrinking LEBs may cause changes there)
+  // Storage of source map locations until the section is placed at its final
+  // location (shrinking LEBs may cause changes there).
+  //
+  // A null DebugLocation* indicates we have no debug information for that
+  // location.
   std::vector<std::pair<size_t, const Function::DebugLocation*>>
     sourceMapLocations;
   size_t sourceMapLocationsSizeAtSectionStart;
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index ee04002a4ae..6ba730a09d5 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -1188,6 +1188,8 @@ void WasmBinaryWriter::writeSourceMapEpilog() {
     writeBase64VLQ(*sourceMap, int32_t(offset - lastOffset));
     lastOffset = offset;
     if (loc) {
+      // There is debug information for this location, so emit the next 3
+      // fields and update lastLoc.
       writeBase64VLQ(*sourceMap, int32_t(loc->fileIndex - lastLoc.fileIndex));
       writeBase64VLQ(*sourceMap, int32_t(loc->lineNumber - lastLoc.lineNumber));
       writeBase64VLQ(*sourceMap,
@@ -1344,15 +1346,27 @@ void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
     auto& debugLocations = func->debugLocations;
     auto iter = debugLocations.find(curr);
     if (iter != debugLocations.end()) {
+      // There is debug information here, write it out.
       writeDebugLocation(iter->second);
     } else {
-      // This expression has no debug location. Emit a size 1 segment to stop
-      // the current segment (otherwise one instruction with debug info would
-      // "smear" its debug info on everything after it).
+      // 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.
       // waka waka this is good i hope
-      // TODO: don't write repeated ones.
-      sourceMapLocations.emplace_back(o.size(), nullptr);
-//      initializeDebugInfo(); // XXX
+      if (!sourceMapLocations.empty() && sourceMapLocations.back() != 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();
+      }
     }
   }
   // If this is an instruction in a function, and if the original wasm had
@@ -2848,8 +2862,8 @@ void WasmBinaryReader::readNextDebugLocation() {
     // waka waka good I thinks
     if (maybeReadChar(',') || maybeReadChar('\"')) {
 std::cout << "waka read 1-length\n";
-      // This is a 1-length entry, and after it is either another one or the
-      // end of the entire record.
+      // This is a 1-length entry, so this is a location without debug info on
+      // it.
       debugLocation.clear();
       break;
     }

From cb3c8219fe9f10d948b509e3419214590b584e27 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 11:33:30 -0700
Subject: [PATCH 04/15] fix

---
 src/wasm/wasm-binary.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 6ba730a09d5..86e846d7829 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -1358,7 +1358,8 @@ void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
       // us is valid no longer. We also don't need to write one if there is
       // nothing before us.
       // waka waka this is good i hope
-      if (!sourceMapLocations.empty() && sourceMapLocations.back() != nullptr) {
+      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,

From ae5d50682a5387c8d7a953fe53487b2f5bf6c9ab Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:13:51 -0700
Subject: [PATCH 05/15] wek

---
 src/wasm-binary.h                   | 17 +++++++++--
 src/wasm/wasm-binary.cpp            | 47 +++++++++++++++++++----------
 test/lit/debug/source-map-stop.wast |  7 ++++-
 3 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 924413539e4..0a7df654565 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -1525,13 +1525,26 @@ class WasmBinaryReader {
   Module& wasm;
   MixedArena& allocator;
   const std::vector<char>& input;
+
   std::istream* sourceMap;
+
   struct NextDebugLocation {
     uint32_t availablePos;
     uint32_t previousPos;
     Function::DebugLocation next;
-  };
-  NextDebugLocation nextDebugLocation;
+  } nextDebugLocation;
+
+  // Whether debug info is present next or not in the next debug location. A
+  // debug location can contain debug info (file:line:col) or it might not. We
+  // need to track this boolean alongside |nextDebugLocation| - that is, we
+  // can' justt do something like std::optional<DebugLocation> or such) - as we
+  // still need to track the values in |next|, as later positions are relative
+  // to them. That is, if we have line number 100, then no debug info, and then
+  // line number 500, then when we get to 500 we will see "+400" which is
+  // relative to the last existing line number (we "skip" over the place without
+  // debug info).
+  bool nextDebugLocationHasDebugInfo;
+
   bool debugInfo = true;
   bool DWARF = false;
   bool skipFunctionBodies = false;
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 86e846d7829..84e1b63d8a6 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -140,6 +140,7 @@ void WasmBinaryWriter::finishSection(int32_t start) {
   // things backwards.
   auto adjustmentForLEBShrinking = MaxLEB32Bytes - sizeFieldSize;
   if (adjustmentForLEBShrinking) {
+std::cout << "waka adjusttt down section " << adjustmentForLEBShrinking << '\n';
     // we can save some room, nice
     assert(sizeFieldSize < MaxLEB32Bytes);
     std::move(&o[start] + MaxLEB32Bytes,
@@ -421,6 +422,7 @@ void WasmBinaryWriter::writeFunctions() {
     // things backwards.
     auto adjustmentForLEBShrinking = MaxLEB32Bytes - sizeFieldSize;
     if (adjustmentForLEBShrinking) {
+std::cout << "waka adjusttt down func " << adjustmentForLEBShrinking << '\n';
       // we can save some room, nice
       assert(sizeFieldSize < MaxLEB32Bytes);
       std::move(&o[start], &o[start] + size, &o[sizePos] + sizeFieldSize);
@@ -1195,6 +1197,7 @@ void WasmBinaryWriter::writeSourceMapEpilog() {
       writeBase64VLQ(*sourceMap,
                      int32_t(loc->columnNumber - lastLoc.columnNumber));
       lastLoc = *loc;
+      std::cout << "waka write 4-er " << loc->lineNumber << ":" << loc->columnNumber << "\n";
     } else {
       std::cout << "waka waka write a 1-er\n";
     }
@@ -1348,6 +1351,9 @@ void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
     if (iter != debugLocations.end()) {
       // There is debug information here, write it out.
       writeDebugLocation(iter->second);
+
+std::cout << "waka write debug location " << iter->second.lineNumber << ":" << iter->second.columnNumber << " for " << getExpressionName(curr) << " : " << *curr << " at offset " << o.size() << '\n';
+
     } 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
@@ -1361,6 +1367,7 @@ void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
       if (!sourceMapLocations.empty() &&
           sourceMapLocations.back().second != nullptr) {
         sourceMapLocations.emplace_back(o.size(), nullptr);
+std::cout << "waka write NULL debug location at offset " << o.size() << "\n";
         // 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
@@ -1641,7 +1648,7 @@ WasmBinaryReader::WasmBinaryReader(Module& wasm,
                                    FeatureSet features,
                                    const std::vector<char>& input)
   : wasm(wasm), allocator(wasm.allocator), input(input),
-    sourceMap(nullptr), nextDebugLocation{0, 0, {0, 0, 0}}, debugLocation() {
+    sourceMap(nullptr), nextDebugLocation{0, 0, {0, 0, 0}}, nextDebugLocationHasDebugInfo(false), debugLocation() {
   wasm.features = features;
 }
 
@@ -2810,9 +2817,13 @@ void WasmBinaryReader::readSourceMapHeader() {
   uint32_t columnNumber = readBase64VLQ(*sourceMap);
   nextDebugLocation = {
     position, position, {fileIndex, lineNumber, columnNumber}};
+  nextDebugLocationHasDebugInfo = true;
+std::cout << "read initial deblug log waka " << position << " : " << lineNumber << ':' << columnNumber << '\n';
 }
 
 void WasmBinaryReader::readNextDebugLocation() {
+std::cout << "readNextDebugLoc! For pos " << pos << " compared to prev, avail: " << nextDebugLocation.previousPos << " .. " << nextDebugLocation.availablePos << '\n';
+
   if (!sourceMap) {
     return;
   }
@@ -2830,11 +2841,18 @@ void WasmBinaryReader::readNextDebugLocation() {
     debugLocation.clear();
     // use debugLocation only for function expressions
     if (currFunction) {
-      debugLocation.insert(nextDebugLocation.next);
+      if (nextDebugLocationHasDebugInfo) {
+std::cout << " insert nextDebugLocation: " << nextDebugLocation.next.lineNumber << ":" << nextDebugLocation.next.columnNumber << "\n";
+        debugLocation.insert(nextDebugLocation.next);
+      } else {
+std::cout << " insert null nextDebugLocation\n";
+            debugLocation.clear();
+      }
     }
 
     char ch;
     *sourceMap >> ch;
+    std::cout << "read a char '" << ch << "' (" << int(ch) << ")\n";
     if (ch == '\"') { // end of records
       nextDebugLocation.availablePos = 0;
       break;
@@ -2845,15 +2863,6 @@ void WasmBinaryReader::readNextDebugLocation() {
 
 
 
-  // TODO refactor to share
-  auto maybeReadChar = [&](char expected) {
-    if (sourceMap->peek() != expected) {
-      return false;
-    }
-    sourceMap->get();
-    return true;
-  };
-
     int32_t positionDelta = readBase64VLQ(*sourceMap);
     uint32_t position = nextDebugLocation.availablePos + positionDelta;
 
@@ -2861,11 +2870,11 @@ void WasmBinaryReader::readNextDebugLocation() {
     nextDebugLocation.availablePos = position;
 
     // waka waka good I thinks
-    if (maybeReadChar(',') || maybeReadChar('\"')) {
-std::cout << "waka read 1-length\n";
-      // This is a 1-length entry, so this is a location without debug info on
-      // it.
-      debugLocation.clear();
+    auto peek = sourceMap->peek();
+    if (peek == ',' || peek == '\"') {
+std::cout << "waka read 1-length for position " << position << " so .next is -1\n";
+      // This is a 1-length entry, so the next location has no debug info.
+      nextDebugLocationHasDebugInfo = false;
       break;
     }
 
@@ -2878,6 +2887,8 @@ std::cout << "waka read 1-length\n";
       nextDebugLocation.next.columnNumber + columnNumberDelta;
 
     nextDebugLocation.next = {fileIndex, lineNumber, columnNumber};
+    nextDebugLocationHasDebugInfo = true;
+    std::cout << "read a 4-length at pos " << position << " : " << lineNumber << ':' << columnNumber << '\n';
   }
 }
 
@@ -3823,6 +3834,8 @@ BinaryConsts::ASTNodes WasmBinaryReader::readExpression(Expression*& curr) {
     throwError("Reached function end without seeing End opcode");
   }
   BYN_TRACE("zz recurse into " << ++depth << " at " << pos << std::endl);
+
+std::cout << "waka readd a new instr. We are at pos " << pos << " and start with debug info NOW\n";
   readNextDebugLocation();
   std::set<Function::DebugLocation> currDebugLocation;
   if (debugLocation.size()) {
@@ -4207,9 +4220,11 @@ BinaryConsts::ASTNodes WasmBinaryReader::readExpression(Expression*& curr) {
     }
   }
   if (curr) {
+std::cout << " waka readd a " << getExpressionName(curr) << " at pos " << startPos << '\n';
     if (currDebugLocation.size()) {
       requireFunctionContext("debugLocation");
       currFunction->debugLocations[curr] = *currDebugLocation.begin();
+std::cout << "  waka readdd debug location " << currFunction->debugLocations[curr].lineNumber << ":" << currFunction->debugLocations[curr].columnNumber << " for " << getExpressionName(curr) << '\n';
     }
     if (DWARF && currFunction) {
       currFunction->expressionLocations[curr] =
diff --git a/test/lit/debug/source-map-stop.wast b/test/lit/debug/source-map-stop.wast
index ae5731c6b2a..0305d4211a7 100644
--- a/test/lit/debug/source-map-stop.wast
+++ b/test/lit/debug/source-map-stop.wast
@@ -9,12 +9,13 @@
 
 (module
   ;; CHECK:      (func $func (param $0 i32) (result i32)
+  ;; CHECK-NEXT:  ;;@ waka:100:1
   ;; CHECK-NEXT:  (drop
   ;; CHECK-NEXT:   (call $func
-  ;; CHECK-NEXT:    ;;@ waka:100:1
   ;; CHECK-NEXT:    (i32.const 1)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  ;;@ waka:200:2
   ;; CHECK-NEXT:  (i32.const 2)
   ;; CHECK-NEXT: )
   (func $func (param i32) (result i32)
@@ -35,3 +36,7 @@
     (i32.const 2)
   )
 )
+
+;; TODO test with same debug info after, so 200:2 is 100:1
+;; TOOD; test with no debug info for the first thing in the function
+;; TODO: add another debug location before the first, so 100:1 isn't first (the very first is special)

From 65d061ca4e04753022d7f9c735e7d0d65155fc56 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:17:12 -0700
Subject: [PATCH 06/15] clean

---
 src/wasm/wasm-binary.cpp | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 84e1b63d8a6..524a4242ec5 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -140,7 +140,6 @@ void WasmBinaryWriter::finishSection(int32_t start) {
   // things backwards.
   auto adjustmentForLEBShrinking = MaxLEB32Bytes - sizeFieldSize;
   if (adjustmentForLEBShrinking) {
-std::cout << "waka adjusttt down section " << adjustmentForLEBShrinking << '\n';
     // we can save some room, nice
     assert(sizeFieldSize < MaxLEB32Bytes);
     std::move(&o[start] + MaxLEB32Bytes,
@@ -422,7 +421,6 @@ void WasmBinaryWriter::writeFunctions() {
     // things backwards.
     auto adjustmentForLEBShrinking = MaxLEB32Bytes - sizeFieldSize;
     if (adjustmentForLEBShrinking) {
-std::cout << "waka adjusttt down func " << adjustmentForLEBShrinking << '\n';
       // we can save some room, nice
       assert(sizeFieldSize < MaxLEB32Bytes);
       std::move(&o[start], &o[start] + size, &o[sizePos] + sizeFieldSize);
@@ -1197,9 +1195,6 @@ void WasmBinaryWriter::writeSourceMapEpilog() {
       writeBase64VLQ(*sourceMap,
                      int32_t(loc->columnNumber - lastLoc.columnNumber));
       lastLoc = *loc;
-      std::cout << "waka write 4-er " << loc->lineNumber << ":" << loc->columnNumber << "\n";
-    } else {
-      std::cout << "waka waka write a 1-er\n";
     }
   }
   *sourceMap << "\"}";
@@ -1351,9 +1346,6 @@ void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {
     if (iter != debugLocations.end()) {
       // There is debug information here, write it out.
       writeDebugLocation(iter->second);
-
-std::cout << "waka write debug location " << iter->second.lineNumber << ":" << iter->second.columnNumber << " for " << getExpressionName(curr) << " : " << *curr << " at offset " << o.size() << '\n';
-
     } 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
@@ -1363,11 +1355,10 @@ std::cout << "waka write debug location " << iter->second.lineNumber << ":" << i
       // 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.
-      // waka waka this is good i hope
       if (!sourceMapLocations.empty() &&
           sourceMapLocations.back().second != nullptr) {
         sourceMapLocations.emplace_back(o.size(), nullptr);
-std::cout << "waka write NULL debug location at offset " << o.size() << "\n";
+
         // 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
@@ -2818,12 +2809,9 @@ void WasmBinaryReader::readSourceMapHeader() {
   nextDebugLocation = {
     position, position, {fileIndex, lineNumber, columnNumber}};
   nextDebugLocationHasDebugInfo = true;
-std::cout << "read initial deblug log waka " << position << " : " << lineNumber << ':' << columnNumber << '\n';
 }
 
 void WasmBinaryReader::readNextDebugLocation() {
-std::cout << "readNextDebugLoc! For pos " << pos << " compared to prev, avail: " << nextDebugLocation.previousPos << " .. " << nextDebugLocation.availablePos << '\n';
-
   if (!sourceMap) {
     return;
   }
@@ -2842,17 +2830,14 @@ std::cout << "readNextDebugLoc! For pos " << pos << " compared to prev, avail: "
     // use debugLocation only for function expressions
     if (currFunction) {
       if (nextDebugLocationHasDebugInfo) {
-std::cout << " insert nextDebugLocation: " << nextDebugLocation.next.lineNumber << ":" << nextDebugLocation.next.columnNumber << "\n";
         debugLocation.insert(nextDebugLocation.next);
       } else {
-std::cout << " insert null nextDebugLocation\n";
-            debugLocation.clear();
+        debugLocation.clear();
       }
     }
 
     char ch;
     *sourceMap >> ch;
-    std::cout << "read a char '" << ch << "' (" << int(ch) << ")\n";
     if (ch == '\"') { // end of records
       nextDebugLocation.availablePos = 0;
       break;
@@ -2861,18 +2846,14 @@ std::cout << " insert null nextDebugLocation\n";
       throw MapParseException("Unexpected delimiter");
     }
 
-
-
     int32_t positionDelta = readBase64VLQ(*sourceMap);
     uint32_t position = nextDebugLocation.availablePos + positionDelta;
 
     nextDebugLocation.previousPos = nextDebugLocation.availablePos;
     nextDebugLocation.availablePos = position;
 
-    // waka waka good I thinks
     auto peek = sourceMap->peek();
     if (peek == ',' || peek == '\"') {
-std::cout << "waka read 1-length for position " << position << " so .next is -1\n";
       // This is a 1-length entry, so the next location has no debug info.
       nextDebugLocationHasDebugInfo = false;
       break;
@@ -2888,7 +2869,6 @@ std::cout << "waka read 1-length for position " << position << " so .next is -1\
 
     nextDebugLocation.next = {fileIndex, lineNumber, columnNumber};
     nextDebugLocationHasDebugInfo = true;
-    std::cout << "read a 4-length at pos " << position << " : " << lineNumber << ':' << columnNumber << '\n';
   }
 }
 
@@ -3835,7 +3815,6 @@ BinaryConsts::ASTNodes WasmBinaryReader::readExpression(Expression*& curr) {
   }
   BYN_TRACE("zz recurse into " << ++depth << " at " << pos << std::endl);
 
-std::cout << "waka readd a new instr. We are at pos " << pos << " and start with debug info NOW\n";
   readNextDebugLocation();
   std::set<Function::DebugLocation> currDebugLocation;
   if (debugLocation.size()) {
@@ -4220,11 +4199,9 @@ std::cout << "waka readd a new instr. We are at pos " << pos << " and start with
     }
   }
   if (curr) {
-std::cout << " waka readd a " << getExpressionName(curr) << " at pos " << startPos << '\n';
     if (currDebugLocation.size()) {
       requireFunctionContext("debugLocation");
       currFunction->debugLocations[curr] = *currDebugLocation.begin();
-std::cout << "  waka readdd debug location " << currFunction->debugLocations[curr].lineNumber << ":" << currFunction->debugLocations[curr].columnNumber << " for " << getExpressionName(curr) << '\n';
     }
     if (DWARF && currFunction) {
       currFunction->expressionLocations[curr] =

From 0c2b390c354f79a7ede6b88850f9762ba8521c52 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:17:42 -0700
Subject: [PATCH 07/15] end

---
 test/lit/debug/source-map-stop.wast | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/lit/debug/source-map-stop.wast b/test/lit/debug/source-map-stop.wast
index 0305d4211a7..0020a4d10d4 100644
--- a/test/lit/debug/source-map-stop.wast
+++ b/test/lit/debug/source-map-stop.wast
@@ -9,9 +9,9 @@
 
 (module
   ;; CHECK:      (func $func (param $0 i32) (result i32)
-  ;; CHECK-NEXT:  ;;@ waka:100:1
   ;; CHECK-NEXT:  (drop
   ;; CHECK-NEXT:   (call $func
+  ;; CHECK-NEXT:    ;;@ waka:100:1
   ;; CHECK-NEXT:    (i32.const 1)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )

From 57c940b3914f769fe357d13bfad2111342d4ad7e Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:20:30 -0700
Subject: [PATCH 08/15] end

---
 test/lit/debug/source-map-stop.wast | 48 ++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/test/lit/debug/source-map-stop.wast b/test/lit/debug/source-map-stop.wast
index 0020a4d10d4..c69a567bcd4 100644
--- a/test/lit/debug/source-map-stop.wast
+++ b/test/lit/debug/source-map-stop.wast
@@ -18,7 +18,7 @@
   ;; CHECK-NEXT:  ;;@ waka:200:2
   ;; CHECK-NEXT:  (i32.const 2)
   ;; CHECK-NEXT: )
-  (func $func (param i32) (result i32)
+  (func $test (param i32) (result i32)
     ;; This drop has on debug info, and should stay that way. Specifically the
     ;; instruction right before it in the binary (the const 1) should not
     ;; smear its debug info on it. And the drop is between an instruction that
@@ -35,8 +35,46 @@
     ;;@ waka:200:2
     (i32.const 2)
   )
-)
 
-;; TODO test with same debug info after, so 200:2 is 100:1
-;; TOOD; test with no debug info for the first thing in the function
-;; TODO: add another debug location before the first, so 100:1 isn't first (the very first is special)
+  (func $same-later (param i32) (result i32)
+    ;; As the first, but now the later debug info is also 100:1.
+    (drop
+      (call $func
+        ;;@ waka:100:1
+        (i32.const 1)
+      )
+    )
+    ;;@ waka:100:1
+    (i32.const 2)
+  )
+
+  (func $more-before (param i32) (result i32)
+    ;; As the first, but now there is more debug info before the 100:1 (the
+    ;; very first debug info in a function has special handling, so we test it
+    ;; more carefully).
+    ;;@ waka:50:5
+    (nop)
+    (drop
+      (call $func
+        ;;@ waka:100:1
+        (i32.const 1)
+      )
+    )
+    ;;@ waka:200:2
+    (i32.const 2)
+  )
+
+  (func $nothing-before (param i32) (result i32)
+    ;; As before, but no debug info on the nop before us (so the first
+    ;; instruction in the function no longer has a debug annotation).
+    (nop)
+    (drop
+      (call $func
+        ;;@ waka:100:1
+        (i32.const 1)
+      )
+    )
+    ;;@ waka:200:2
+    (i32.const 2)
+  )
+)

From 6cb80566879c4d504cf8d3508be0c2f7e8a9e4c8 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:22:17 -0700
Subject: [PATCH 09/15] almost

---
 test/lit/debug/source-map-stop.wast | 47 +++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/test/lit/debug/source-map-stop.wast b/test/lit/debug/source-map-stop.wast
index c69a567bcd4..beff5721c01 100644
--- a/test/lit/debug/source-map-stop.wast
+++ b/test/lit/debug/source-map-stop.wast
@@ -8,9 +8,9 @@
 ;; be identical to the input.
 
 (module
-  ;; CHECK:      (func $func (param $0 i32) (result i32)
+  ;; CHECK:      (func $test (param $0 i32) (result i32)
   ;; CHECK-NEXT:  (drop
-  ;; CHECK-NEXT:   (call $func
+  ;; CHECK-NEXT:   (call $test
   ;; CHECK-NEXT:    ;;@ waka:100:1
   ;; CHECK-NEXT:    (i32.const 1)
   ;; CHECK-NEXT:   )
@@ -27,7 +27,7 @@
     ;; that only happens in that state: removing the debug info either before or
     ;; after would avoid the bug.)
     (drop
-      (call $func
+      (call $test
         ;;@ waka:100:1
         (i32.const 1)
       )
@@ -36,10 +36,20 @@
     (i32.const 2)
   )
 
+  ;; CHECK:      (func $same-later (param $0 i32) (result i32)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (call $test
+  ;; CHECK-NEXT:    ;;@ waka:100:1
+  ;; CHECK-NEXT:    (i32.const 1)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  ;;@ waka:100:1
+  ;; CHECK-NEXT:  (i32.const 2)
+  ;; CHECK-NEXT: )
   (func $same-later (param i32) (result i32)
     ;; As the first, but now the later debug info is also 100:1.
     (drop
-      (call $func
+      (call $test
         ;;@ waka:100:1
         (i32.const 1)
       )
@@ -48,14 +58,28 @@
     (i32.const 2)
   )
 
+  ;; CHECK:      (func $more-before (param $0 i32) (result i32)
+  ;; CHECK-NEXT:  ;;@ waka:50:5
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  ;;@ waka:50:5
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (call $test
+  ;; CHECK-NEXT:    ;;@ waka:100:1
+  ;; CHECK-NEXT:    (i32.const 1)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  ;;@ waka:200:2
+  ;; CHECK-NEXT:  (i32.const 2)
+  ;; CHECK-NEXT: )
   (func $more-before (param i32) (result i32)
     ;; As the first, but now there is more debug info before the 100:1 (the
     ;; very first debug info in a function has special handling, so we test it
     ;; more carefully).
     ;;@ waka:50:5
     (nop)
+    ;; XXX bad!
     (drop
-      (call $func
+      (call $test
         ;;@ waka:100:1
         (i32.const 1)
       )
@@ -64,12 +88,23 @@
     (i32.const 2)
   )
 
+  ;; CHECK:      (func $nothing-before (param $0 i32) (result i32)
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (call $test
+  ;; CHECK-NEXT:    ;;@ waka:100:1
+  ;; CHECK-NEXT:    (i32.const 1)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  ;;@ waka:200:2
+  ;; CHECK-NEXT:  (i32.const 2)
+  ;; CHECK-NEXT: )
   (func $nothing-before (param i32) (result i32)
     ;; As before, but no debug info on the nop before us (so the first
     ;; instruction in the function no longer has a debug annotation).
     (nop)
     (drop
-      (call $func
+      (call $test
         ;;@ waka:100:1
         (i32.const 1)
       )

From 77ae0d982bb4a698d344c046ea80d3fd7e45365a Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:28:31 -0700
Subject: [PATCH 10/15] test

---
 test/lit/debug/source-map-stop.wast | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/lit/debug/source-map-stop.wast b/test/lit/debug/source-map-stop.wast
index beff5721c01..40a3234cac5 100644
--- a/test/lit/debug/source-map-stop.wast
+++ b/test/lit/debug/source-map-stop.wast
@@ -75,9 +75,11 @@
     ;; As the first, but now there is more debug info before the 100:1 (the
     ;; very first debug info in a function has special handling, so we test it
     ;; more carefully).
+    ;;
+    ;; The s-parser actually smears 50:5 on the drop and call after it, so the
+    ;; output here looks incorrect. This may be a bug there, TODO
     ;;@ waka:50:5
     (nop)
-    ;; XXX bad!
     (drop
       (call $test
         ;;@ waka:100:1

From ed4c6e7bae10cdd49cc2008e9737ea618f321a48 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:29:13 -0700
Subject: [PATCH 11/15] format

---
 src/wasm/wasm-binary.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 524a4242ec5..49693ab12bc 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -1639,7 +1639,8 @@ WasmBinaryReader::WasmBinaryReader(Module& wasm,
                                    FeatureSet features,
                                    const std::vector<char>& input)
   : wasm(wasm), allocator(wasm.allocator), input(input),
-    sourceMap(nullptr), nextDebugLocation{0, 0, {0, 0, 0}}, nextDebugLocationHasDebugInfo(false), debugLocation() {
+    sourceMap(nullptr), nextDebugLocation{0, 0, {0, 0, 0}},
+    nextDebugLocationHasDebugInfo(false), debugLocation() {
   wasm.features = features;
 }
 

From 00955b2b293f7e1b8379ad15e305d486ae35c982 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:47:44 -0700
Subject: [PATCH 12/15] fix

---
 src/wasm-binary.h        | 2 +-
 src/wasm/wasm-binary.cpp | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 0a7df654565..4b445da67fe 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -1537,7 +1537,7 @@ class WasmBinaryReader {
   // Whether debug info is present next or not in the next debug location. A
   // debug location can contain debug info (file:line:col) or it might not. We
   // need to track this boolean alongside |nextDebugLocation| - that is, we
-  // can' justt do something like std::optional<DebugLocation> or such) - as we
+  // can't just do something like std::optional<DebugLocation> or such) - as we
   // still need to track the values in |next|, as later positions are relative
   // to them. That is, if we have line number 100, then no debug info, and then
   // line number 500, then when we get to 500 we will see "+400" which is
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 49693ab12bc..c9d425a5b98 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -2802,6 +2802,10 @@ void WasmBinaryReader::readSourceMapHeader() {
     return;
   }
   // read first debug location
+  // TODO: Handle the case where the very first one has only a position but ont
+  //       debug info. In practice that does not happen, which needs
+  //       investigation (if it does, it will assert in readBase64VLQ, so it
+  //       would not be a silent error at least).
   uint32_t position = readBase64VLQ(*sourceMap);
   uint32_t fileIndex = readBase64VLQ(*sourceMap);
   uint32_t lineNumber =

From 03b144bd1bcefb888862583a247b13d3d45bf826 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 13:51:35 -0700
Subject: [PATCH 13/15] clean

---
 src/wasm/wasm-binary.cpp            |  1 -
 test/lit/debug/source-map-stop.wast | 15 ++++++++++-----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index c9d425a5b98..33c279a7893 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -3819,7 +3819,6 @@ BinaryConsts::ASTNodes WasmBinaryReader::readExpression(Expression*& curr) {
     throwError("Reached function end without seeing End opcode");
   }
   BYN_TRACE("zz recurse into " << ++depth << " at " << pos << std::endl);
-
   readNextDebugLocation();
   std::set<Function::DebugLocation> currDebugLocation;
   if (debugLocation.size()) {
diff --git a/test/lit/debug/source-map-stop.wast b/test/lit/debug/source-map-stop.wast
index 40a3234cac5..90bf739c17d 100644
--- a/test/lit/debug/source-map-stop.wast
+++ b/test/lit/debug/source-map-stop.wast
@@ -19,13 +19,14 @@
   ;; CHECK-NEXT:  (i32.const 2)
   ;; CHECK-NEXT: )
   (func $test (param i32) (result i32)
-    ;; This drop has on debug info, and should stay that way. Specifically the
-    ;; instruction right before it in the binary (the const 1) should not
+    ;; The drop&call have no debug info, and should remain so. Specifically the
+    ;; instruction right before them in the binary (the const 1) should not
     ;; smear its debug info on it. And the drop is between an instruction that
     ;; has debug info (the const 1) and another (the i32.const 2): we should not
     ;; receive the debug info of either. (This is a regression test for a bug
     ;; that only happens in that state: removing the debug info either before or
-    ;; after would avoid the bug.)
+    ;; after would avoid that bug.)
+
     (drop
       (call $test
         ;;@ waka:100:1
@@ -47,7 +48,9 @@
   ;; CHECK-NEXT:  (i32.const 2)
   ;; CHECK-NEXT: )
   (func $same-later (param i32) (result i32)
-    ;; As the first, but now the later debug info is also 100:1.
+    ;; As the first, but now the later debug info is also 100:1. No debug info
+    ;; should change here.
+
     (drop
       (call $test
         ;;@ waka:100:1
@@ -78,6 +81,7 @@
     ;;
     ;; The s-parser actually smears 50:5 on the drop and call after it, so the
     ;; output here looks incorrect. This may be a bug there, TODO
+
     ;;@ waka:50:5
     (nop)
     (drop
@@ -103,7 +107,8 @@
   ;; CHECK-NEXT: )
   (func $nothing-before (param i32) (result i32)
     ;; As before, but no debug info on the nop before us (so the first
-    ;; instruction in the function no longer has a debug annotation).
+    ;; instruction in the function no longer has a debug annotation). Nothing
+    ;; should change in the debug info.
     (nop)
     (drop
       (call $test

From e86e1de71c64795929c8e444f3556e1ba2d4f643 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 14:02:30 -0700
Subject: [PATCH 14/15] clean

---
 src/wasm-binary.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 4b445da67fe..0931a818005 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -1537,7 +1537,7 @@ class WasmBinaryReader {
   // Whether debug info is present next or not in the next debug location. A
   // debug location can contain debug info (file:line:col) or it might not. We
   // need to track this boolean alongside |nextDebugLocation| - that is, we
-  // can't just do something like std::optional<DebugLocation> or such) - as we
+  // can't just do something like std::optional<DebugLocation> or such - as we
   // still need to track the values in |next|, as later positions are relative
   // to them. That is, if we have line number 100, then no debug info, and then
   // line number 500, then when we get to 500 we will see "+400" which is

From 0c62b402fa37e2777cf384677f604e24b78bbf71 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 14:04:10 -0700
Subject: [PATCH 15/15] clean

---
 src/wasm/wasm-binary.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 33c279a7893..d6eb3b23504 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -2802,7 +2802,7 @@ void WasmBinaryReader::readSourceMapHeader() {
     return;
   }
   // read first debug location
-  // TODO: Handle the case where the very first one has only a position but ont
+  // TODO: Handle the case where the very first one has only a position but not
   //       debug info. In practice that does not happen, which needs
   //       investigation (if it does, it will assert in readBase64VLQ, so it
   //       would not be a silent error at least).