Debug info handling for new EH try-catch

src/passes/Print.cpp

@@ -1902,6 +1902,21 @@ struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
     }
   }
 
+  // Prints debug info for a delimiter in an expression.
+  void printDebugDelimiterLocation(Expression* curr, Index i) {
+    if (currFunction && debugInfo) {
+      auto iter = currFunction->delimiterLocations.find(curr);
+      if (iter != currFunction->delimiterLocations.end()) {
+        auto& locations = iter->second;
+        Colors::grey(o);
+        o << ";; code offset: 0x" << std::hex << locations[i] << std::dec
+          << '\n';
+        restoreNormalColor(o);
+        doIndent(o, indent);
+      }
+    }
+  }
+
   void visit(Expression* curr) {
     printDebugLocation(curr);
     OverriddenVisitor<PrintSExpression>::visit(curr);
@@ -2020,6 +2035,10 @@ struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
     printFullLine(curr->condition);
     maybePrintImplicitBlock(curr->ifTrue, false);
     if (curr->ifFalse) {
+      // Note: debug info here is not used as LLVM does not emit ifs, and since
+      // LLVM is the main source of DWARF, effectively we never encounter ifs
+      // with DWARF.
+      printDebugDelimiterLocation(curr, BinaryLocations::Else);
       maybePrintImplicitBlock(curr->ifFalse, false);
     }
     decIndent();
@@ -2380,6 +2399,7 @@ struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
     o << "\n";
     for (size_t i = 0; i < curr->catchEvents.size(); i++) {
       doIndent(o, indent);
+      printDebugDelimiterLocation(curr, i);
       o << "(catch ";
       printName(curr->catchEvents[i], o);
       incIndent();
@@ -2389,6 +2409,7 @@ struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
     }
     if (curr->hasCatchAll()) {
       doIndent(o, indent);
+      printDebugDelimiterLocation(curr, curr->catchEvents.size());
       o << "(catch_all";
       incIndent();
       maybePrintImplicitBlock(curr->catchBodies.back(), true);

src/support/small_vector.h

@@ -181,6 +181,30 @@ template<typename T, size_t N> class SmallVector {
   ConstIterator end() const { return ConstIterator(this, size()); }
 };
 
+// A SmallVector for which some values may be read before they are written, and
+// in that case they have the value zero.
+template<typename T, size_t N>
+struct ZeroInitSmallVector : public SmallVector<T, N> {
+  T& operator[](size_t i) {
+    if (i >= this->size()) {
+      resize(i + 1);
+    }
+    return SmallVector<T, N>::operator[](i);
+  }
+
+  const T& operator[](size_t i) const {
+    return const_cast<ZeroInitSmallVector<T, N>&>(*this)[i];
+  }
+
+  void resize(size_t newSize) {
+    auto oldSize = this->size();
+    SmallVector<T, N>::resize(newSize);
+    for (size_t i = oldSize; i < this->size(); i++) {
+      (*this)[i] = 0;
+    }
+  }
+};
+
 } // namespace wasm
 
 #endif // wasm_support_small_vector_h

src/wasm-binary.h

@@ -1177,9 +1177,7 @@ class WasmBinaryWriter {
   void writeDebugLocation(const Function::DebugLocation& loc);
   void writeDebugLocation(Expression* curr, Function* func);
   void writeDebugLocationEnd(Expression* curr, Function* func);
-  void writeExtraDebugLocation(Expression* curr,
-                               Function* func,
-                               BinaryLocations::DelimiterId id);
+  void writeExtraDebugLocation(Expression* curr, Function* func, size_t id);
 
   // helpers
   void writeInlineString(const char* name);
@@ -1406,10 +1404,6 @@ class WasmBinaryBuilder {
   // Called when we parse the beginning of a control flow structure.
   void startControlFlow(Expression* curr);
 
-  // Called when we parse a later part of a control flow structure, like "end"
-  // or "else".
-  void continueControlFlow(BinaryLocations::DelimiterId id, BinaryLocation pos);
-
   // set when we know code is unreachable in the sense of the wasm spec: we are
   // in a block and after an unreachable element. this helps parse stacky wasm
   // code, which can be unsuitable for our IR when unreachable.

src/wasm.h

@@ -1530,20 +1530,15 @@ struct BinaryLocations {
   // Track the extra delimiter positions that some instructions, in particular
   // control flow, have, like 'end' for loop and block. We keep these in a
   // separate map because they are rare and we optimize for the storage space
-  // for the common type of instruction which just needs a Span. We implement
-  // this as a simple array with one element at the moment (more elements may
-  // be necessary in the future).
-  // TODO: If we are sure we won't need more, make this a single value?
-  struct DelimiterLocations : public std::array<BinaryLocation, 1> {
-    DelimiterLocations() {
-      // Ensure zero-initialization.
-      for (auto& item : *this) {
-        item = 0;
-      }
-    }
-  };
+  // for the common type of instruction which just needs a Span.
+  // For "else" (from an if) we use index 0, and for catch (from a try) we use
+  // indexes 0 and above.
+  // We use automatic zero-initialization here because that indicates a "null"
+  // debug value, indicating the information is not present.
+  using DelimiterLocations = ZeroInitSmallVector<BinaryLocation, 1>;
+
+  enum DelimiterId : size_t { Else = 0, Invalid = size_t(-1) };
 
-  enum DelimiterId { Else = 0, Catch = 0, Invalid = -1 };
   std::unordered_map<Expression*, DelimiterLocations> delimiters;
 
   // DWARF debug info can refer to multiple interesting positions in a function.

src/wasm/wasm-binary.cpp

@@ -915,8 +915,9 @@ void WasmBinaryWriter::writeDebugLocationEnd(Expression* curr, Function* func) {
   }
 }
 
-void WasmBinaryWriter::writeExtraDebugLocation(
-  Expression* curr, Function* func, BinaryLocations::DelimiterId id) {
+void WasmBinaryWriter::writeExtraDebugLocation(Expression* curr,
+                                               Function* func,
+                                               size_t id) {
   if (func && !func->expressionLocations.empty()) {
     binaryLocations.delimiters[curr][id] = o.size();
   }
@@ -2836,13 +2837,30 @@ BinaryConsts::ASTNodes WasmBinaryBuilder::readExpression(Expression*& curr) {
       }
       break;
     case BinaryConsts::Else:
+    case BinaryConsts::Catch: {
       curr = nullptr;
-      continueControlFlow(BinaryLocations::Else, startPos);
-      break;
-    case BinaryConsts::Catch:
-      curr = nullptr;
-      continueControlFlow(BinaryLocations::Catch, startPos);
+      if (DWARF && currFunction) {
+        assert(!controlFlowStack.empty());
+        auto currControlFlow = controlFlowStack.back();
+        BinaryLocation delimiterId;
+        // Else and CatchAll have the same binary ID, so differentiate them
+        // using the control flow stack.
+        static_assert(BinaryConsts::CatchAll == BinaryConsts::Else,
+                      "Else and CatchAll should have identical codes");
+        if (currControlFlow->is<If>()) {
+          delimiterId = BinaryLocations::Else;
+        } else {
+          // Both Catch and CatchAll can simply append to the list as we go, as
+          // we visit them in the right order in the binary, and like the binary
+          // we store the CatchAll at the end.
+          delimiterId =
+            currFunction->delimiterLocations[currControlFlow].size();
+        }
+        currFunction->delimiterLocations[currControlFlow][delimiterId] =
+          startPos - codeSectionLocation;
+      }
       break;
+    }
     case BinaryConsts::RefNull:
       visitRefNull((curr = allocator.alloc<RefNull>())->cast<RefNull>());
       break;
@@ -3102,18 +3120,6 @@ void WasmBinaryBuilder::startControlFlow(Expression* curr) {
   }
 }
 
-void WasmBinaryBuilder::continueControlFlow(BinaryLocations::DelimiterId id,
-                                            BinaryLocation pos) {
-  if (DWARF && currFunction) {
-    assert(!controlFlowStack.empty());
-    auto currControlFlow = controlFlowStack.back();
-    // We are called after parsing the byte, so we need to subtract one to
-    // get its position.
-    currFunction->delimiterLocations[currControlFlow][id] =
-      pos - codeSectionLocation;
-  }
-}
-
 void WasmBinaryBuilder::pushBlockElements(Block* curr,
                                           Type type,
                                           size_t start) {

src/wasm/wasm-debug.cpp

@@ -361,7 +361,7 @@ struct AddrExprMap {
   // bloat the common case which is represented in the earlier maps.
   struct DelimiterInfo {
     Expression* expr;
-    BinaryLocations::DelimiterId id;
+    size_t id;
   };
   std::unordered_map<BinaryLocation, DelimiterInfo> delimiterMap;
 
@@ -414,8 +414,7 @@ struct AddrExprMap {
     for (Index i = 0; i < delimiter.size(); i++) {
       if (delimiter[i] != 0) {
         assert(delimiterMap.count(delimiter[i]) == 0);
-        delimiterMap[delimiter[i]] =
-          DelimiterInfo{expr, BinaryLocations::DelimiterId(i)};
+        delimiterMap[delimiter[i]] = DelimiterInfo{expr, i};
       }
     }
   }

src/wasm/wasm-stack.cpp

@@ -1887,9 +1887,8 @@ void BinaryInstWriter::emitCatch(Try* curr, Index i) {
   assert(!breakStack.empty());
   breakStack.pop_back();
   breakStack.emplace_back(IMPOSSIBLE_CONTINUE);
-  // TODO Fix handling of BinaryLocations for the new EH spec
   if (func && !sourceMap) {
-    parent.writeExtraDebugLocation(curr, func, BinaryLocations::Catch);
+    parent.writeExtraDebugLocation(curr, func, i);
   }
   o << int8_t(BinaryConsts::Catch)
     << U32LEB(parent.getEventIndex(curr->catchEvents[i]));
@@ -1899,7 +1898,9 @@ void BinaryInstWriter::emitCatchAll(Try* curr) {
   assert(!breakStack.empty());
   breakStack.pop_back();
   breakStack.emplace_back(IMPOSSIBLE_CONTINUE);
-  // TODO Fix handling of BinaryLocations for the new EH spec
+  if (func && !sourceMap) {
+    parent.writeExtraDebugLocation(curr, func, curr->catchBodies.size());
+  }
   o << int8_t(BinaryConsts::CatchAll);
 }