DWARF: Do not reorder locals in binary writing (#2959)

The binary writer reorders locals unconditionally. I forgot about this, and so
when I made DWARF disable optimization passes that reorder, this was
left active.

Optimally the writer would not do this, and the ReorderLocals pass would.
But it looks like we need special logic for tuple locals anyhow, as they
expand into multiple locals, so some amount of local order changes seems
unavoidable atm.

Test changes are mostly just lots of offsets, and can be ignored, but
the new test test/passes/dwarf-local-order.* shows the issue. It
prints $foo once, then after a roundtrip (showing no reordering), then
it strips the DWARF section and prints after another roundtrip (which
does show reordering).

This also makes us avoid the Stack IR writer if DWARF is present, which
matches what we do with source maps. This doesn't prevent any known
bugs, but it's simpler this way and debugging + Stack IR opts is not an
important combination.

3 files changed, 33 insertions, 10 deletions

diff --git a/src/wasm-stack.h b/src/wasm-stack.h
index 09c8589b0..d80618225 100644
--- a/src/wasm-stack.h
+++ b/src/wasm-stack.h
@@ -84,8 +84,9 @@ public:
   BinaryInstWriter(WasmBinaryWriter& parent,
                    BufferWithRandomAccess& o,
                    Function* func,
-                   bool sourceMap)
-    : parent(parent), o(o), func(func), sourceMap(sourceMap) {}
+                   bool sourceMap,
+                   bool DWARF)
+    : parent(parent), o(o), func(func), sourceMap(sourceMap), DWARF(DWARF) {}
 
   void visit(Expression* curr) {
     if (func && !sourceMap) {
@@ -163,6 +164,7 @@ private:
   BufferWithRandomAccess& o;
   Function* func = nullptr;
   bool sourceMap;
+  bool DWARF;
 
   std::vector<Name> breakStack;
 
@@ -824,9 +826,10 @@ public:
   BinaryenIRToBinaryWriter(WasmBinaryWriter& parent,
                            BufferWithRandomAccess& o,
                            Function* func = nullptr,
-                           bool sourceMap = false)
+                           bool sourceMap = false,
+                           bool DWARF = false)
     : BinaryenIRWriter<BinaryenIRToBinaryWriter>(func), parent(parent),
-      writer(parent, o, func, sourceMap), sourceMap(sourceMap) {}
+      writer(parent, o, func, sourceMap, DWARF), sourceMap(sourceMap) {}
 
   void visit(Expression* curr) {
     BinaryenIRWriter<BinaryenIRToBinaryWriter>::visit(curr);
@@ -858,7 +861,7 @@ public:
 private:
   WasmBinaryWriter& parent;
   BinaryInstWriter writer;
-  bool sourceMap = false;
+  bool sourceMap;
 };
 
 // Binaryen IR to stack IR converter
@@ -901,7 +904,8 @@ public:
   StackIRToBinaryWriter(WasmBinaryWriter& parent,
                         BufferWithRandomAccess& o,
                         Function* func)
-    : writer(parent, o, func, false /* sourceMap */), func(func) {}
+    : writer(parent, o, func, false /* sourceMap */, false /* DWARF */),
+      func(func) {}
 
   void write();
 
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 8a7f0d744..bb5d420a9 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -307,6 +307,7 @@ void WasmBinaryWriter::writeFunctions() {
   BYN_TRACE("== writeFunctions\n");
   auto sectionStart = startSection(BinaryConsts::Section::Code);
   o << U32LEB(importInfo->getNumDefinedFunctions());
+  bool DWARF = Debug::hasDWARFSections(*getModule());
   ModuleUtils::iterDefinedFunctions(*wasm, [&](Function* func) {
     assert(binaryLocationTrackedExpressionsForFunc.empty());
     size_t sourceMapLocationsSizeAtFunctionStart = sourceMapLocations.size();
@@ -315,12 +316,12 @@ void WasmBinaryWriter::writeFunctions() {
     size_t start = o.size();
     BYN_TRACE("writing" << func->name << std::endl);
     // Emit Stack IR if present, and if we can
-    if (func->stackIR && !sourceMap) {
+    if (func->stackIR && !sourceMap && !DWARF) {
       BYN_TRACE("write Stack IR\n");
       StackIRToBinaryWriter(*this, o, func).write();
     } else {
       BYN_TRACE("write Binaryen IR\n");
-      BinaryenIRToBinaryWriter(*this, o, func, sourceMap).write();
+      BinaryenIRToBinaryWriter(*this, o, func, sourceMap, DWARF).write();
     }
     size_t size = o.size() - start;
     assert(size <= std::numeric_limits<uint32_t>::max());
diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp
index c6c450cfb..9e3ce2afa 100644
--- a/src/wasm/wasm-stack.cpp
+++ b/src/wasm/wasm-stack.cpp
@@ -16,6 +16,7 @@
 
 #include "wasm-stack.h"
 #include "ir/find_all.h"
+#include "wasm-debug.h"
 
 namespace wasm {
 
@@ -1780,8 +1781,25 @@ void BinaryInstWriter::mapLocalsAndEmitHeader() {
   assert(func && "BinaryInstWriter: function is not set");
   // Map params
   for (Index i = 0; i < func->getNumParams(); i++) {
-    size_t curr = mappedLocals.size();
-    mappedLocals[std::make_pair(i, 0)] = curr;
+    mappedLocals[std::make_pair(i, 0)] = i;
+  }
+  // Normally we map all locals of the same type into a range of adjacent
+  // addresses, which is more compact. However, if we need to keep DWARF valid,
+  // do not do any reordering at all - instead, do a trivial mapping that
+  // keeps everything unmoved.
+  if (DWARF) {
+    FindAll<TupleExtract> extracts(func->body);
+    if (!extracts.list.empty()) {
+      Fatal() << "DWARF + multivalue is not yet complete";
+    }
+    Index varStart = func->getVarIndexBase();
+    Index varEnd = varStart + func->getNumVars();
+    o << U32LEB(func->getNumVars());
+    for (Index i = varStart; i < varEnd; i++) {
+      mappedLocals[std::make_pair(i, 0)] = i;
+      o << U32LEB(1) << binaryType(func->getLocalType(i));
+    }
+    return;
   }
   for (auto type : func->vars) {
     for (auto t : type.expand()) {