diff options
| -rw-r--r-- | src/wasm/wasm-binary.cpp | 43 | ||||
| -rw-r--r-- | test/lit/binary/name-overlap.test | 22 | ||||
| -rw-r--r-- | test/lit/binary/name-overlap.test.wasm | bin | 0 -> 44 bytes |
3 files changed, 65 insertions, 0 deletions
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 967356210..28c22aed3 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -3325,18 +3325,52 @@ Name WasmBinaryBuilder::escape(Name name) { return escaped; } +namespace { + // Performs necessary processing of names from the name section before using // them. Specifically it escapes and deduplicates them. +// +// Deduplication is not trivial, since we can't only consider things in the name +// section itself. The issue is that we have already given everything a +// temporary name. Consider if we gave these two temp names: +// +// $foo$0 +// $foo$1 +// +// and imagine that the first appears in the name section, where it is given the +// the name $foo$1, and the second does not appear in the name section. In that +// case, we'd rename the second to the same name as the first. If we left things +// there that would be invalid, so we need to pick another temp name for the +// second item to resolve that. class NameProcessor { public: + // Returns a unique, escaped name. Notes that name for the items to follow to + // keep them unique as well. Name process(Name name) { return deduplicate(WasmBinaryBuilder::escape(name)); } + // After processing the names section entries, which set explicit names, we + // also handle the remaining items here, which handles the corner case + // described above. + // + // TODO: This handles vectors of Named objects; we should also do this for + // local names and type names etc. + template<typename T> void deduplicateUnexplicitlyNamed(std::vector<T>& vec) { + for (auto& x : vec) { + if (!x->hasExplicitName) { + x->name = deduplicate(x->name); + } + } + } + private: std::unordered_set<Name> usedNames; Name deduplicate(Name base) { + // TODO: Consider using Names::getValidNameGivenExisting but that does give + // longer names, and it is very noticeable in this location, so + // perhaps optimize that first. Name name = base; // De-duplicate names by appending .1, .2, etc. for (int i = 1; !usedNames.insert(name).second; ++i) { @@ -3346,6 +3380,8 @@ private: } }; +} // anonymous namespace + void WasmBinaryBuilder::readNames(size_t payloadLen) { BYN_TRACE("== readNames\n"); auto sectionPos = pos; @@ -3378,6 +3414,7 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) { << std::to_string(index) << std::endl; } } + processor.deduplicateUnexplicitlyNamed(wasm.functions); } else if (nameType == Subsection::NameLocal) { auto numFuncs = getU32LEB(); for (size_t i = 0; i < numFuncs; i++) { @@ -3454,6 +3491,7 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) { << std::to_string(index) << std::endl; } } + processor.deduplicateUnexplicitlyNamed(wasm.tables); } else if (nameType == Subsection::NameElem) { auto num = getU32LEB(); NameProcessor processor; @@ -3471,6 +3509,7 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) { << std::to_string(index) << std::endl; } } + processor.deduplicateUnexplicitlyNamed(wasm.elementSegments); } else if (nameType == Subsection::NameMemory) { auto num = getU32LEB(); NameProcessor processor; @@ -3487,6 +3526,7 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) { << std::to_string(index) << std::endl; } } + processor.deduplicateUnexplicitlyNamed(wasm.memories); } else if (nameType == Subsection::NameData) { auto num = getU32LEB(); NameProcessor processor; @@ -3503,6 +3543,7 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) { << std::to_string(index) << std::endl; } } + processor.deduplicateUnexplicitlyNamed(wasm.dataSegments); } else if (nameType == Subsection::NameGlobal) { auto num = getU32LEB(); NameProcessor processor; @@ -3519,6 +3560,7 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) { << std::to_string(index) << std::endl; } } + processor.deduplicateUnexplicitlyNamed(wasm.globals); } else if (nameType == Subsection::NameField) { auto numTypes = getU32LEB(); for (size_t i = 0; i < numTypes; i++) { @@ -3555,6 +3597,7 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) { << std::to_string(index) << std::endl; } } + processor.deduplicateUnexplicitlyNamed(wasm.tags); } else { std::cerr << "warning: unknown name subsection with id " << std::to_string(nameType) << " at " << pos << std::endl; diff --git a/test/lit/binary/name-overlap.test b/test/lit/binary/name-overlap.test new file mode 100644 index 000000000..7f5318737 --- /dev/null +++ b/test/lit/binary/name-overlap.test @@ -0,0 +1,22 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. +;; RUN: wasm-opt %s.wasm -S -o - | filecheck %s + +;; Test that we emit deduplicated names even in the corner case that one item +;; is in the names section and the other is not, and the latter's temp name +;; coincides with the name the name section gives for the first. Specifically, +;; the input wasm looks like this: +;; +;; (module +;; (global $global$1 i32 (i32.const 1)) ;; In names section. +;; (global $........ i32 (i32.const 0)) ;; *Not* in names section. +;; ) +;; +;; We give them temp names $global$0, $global$1 to begin with, then apply the +;; names section which turns the first's name into $global$1 - identical to the +;; second. We must then deduplicate the second name to avoid a collision. (Note +;; that we leave the name from the names section as it is, and only adjust the +;; temp name.) + +;; CHECK: (global $global$1 i32 (i32.const 1)) +;; CHECK-NEXT: (global $global$1.1 i32 (i32.const 0)) + diff --git a/test/lit/binary/name-overlap.test.wasm b/test/lit/binary/name-overlap.test.wasm Binary files differnew file mode 100644 index 000000000..3d01b4a95 --- /dev/null +++ b/test/lit/binary/name-overlap.test.wasm |