Warn about and ignore empty local/param names in name section (#4426)

Fixes the crash in #4418

Also replace the .at() there with better logic to handle imported functions.

See WebAssembly/wabt#1799 for details on why wabt sometimes emits this.

3 files changed, 31 insertions, 3 deletions

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 4804626da..5ebc4d864 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -749,8 +749,18 @@ void WasmBinaryWriter::writeNames() {
         o << U32LEB(localsWithNames.size());
         for (auto& [indexInFunc, name] : localsWithNames) {
           // TODO: handle multivalue
-          auto indexInBinary =
-            funcMappedLocals.at(func->name)[{indexInFunc, 0}];
+          Index indexInBinary;
+          auto iter = funcMappedLocals.find(func->name);
+          if (iter != funcMappedLocals.end()) {
+            indexInBinary = iter->second[{indexInFunc, 0}];
+          } else {
+            // No data on funcMappedLocals. That is only possible if we are an
+            // imported function, where there are no locals to map, and in that
+            // case the index is unchanged anyhow: parameters always have the
+            // same index, they are not mapped in any way.
+            assert(func->imported());
+            indexInBinary = indexInFunc;
+          }
           o << U32LEB(indexInBinary);
           writeEscapedName(name.str);
         }
@@ -3092,7 +3102,11 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) {
             continue; // read and discard in case of prior error
           }
           auto localName = processor.process(rawLocalName);
-          if (localIndex < func->getNumLocals()) {
+          if (localName.size() == 0) {
+            std::cerr << "warning: empty local name at index "
+                      << std::to_string(localIndex) << " in function "
+                      << std::string(func->name.str) << std::endl;
+          } else if (localIndex < func->getNumLocals()) {
             func->localNames[localIndex] = localName;
           } else {
             std::cerr << "warning: local index out of bounds in name "
diff --git a/test/lit/binary/empty-param-name.test b/test/lit/binary/empty-param-name.test
new file mode 100644
index 000000000..2216d0bbb
--- /dev/null
+++ b/test/lit/binary/empty-param-name.test
@@ -0,0 +1,14 @@
+;; Test that we show a warning on an empty local name, and do not crash.
+;;
+;; The binary contains this, processed by wabt with debug names:
+;;
+;;   (module
+;;     (func $foo (import "imports" "foo") (param i32)))
+;;
+;; Wabt emits a name for that parameter, but it is the empty string. See
+;; https://github.com/WebAssembly/wabt/issues/1799
+
+;; RUN: wasm-opt %s.wasm 2>&1 | filecheck %s
+
+;; CHECK: warning: empty local name at index 0 in function foo
+
diff --git a/test/lit/binary/empty-param-name.test.wasm b/test/lit/binary/empty-param-name.test.wasm
new file mode 100644
index 000000000..6d08ab93a
--- /dev/null
+++ b/test/lit/binary/empty-param-name.test.wasm