From dfaca1ebd14c2940d43e58d1780bb4da0a76a783 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 10 Jul 2025 08:53:24 -0700 Subject: [PATCH 1/4] fix --- src/wasm-binary.h | 5 +++++ src/wasm/wasm-binary.cpp | 30 +++++++++++++----------------- test/lit/name-overlap.wast | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 17 deletions(-) create mode 100644 test/lit/name-overlap.wast diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 21b53f34329..8cadd5c0072 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1658,6 +1658,11 @@ class WasmBinaryReader { std::unordered_map dataNames; std::unordered_map elemNames; + // The names that are already used (either from the names section, or that we + // generate as internal names for un-named things). + std::unordered_set usedFunctionNames, usedTableNames, usedMemoryNames, + usedGlobalNames, usedTagNames; + Function* currFunction = nullptr; // before we see a function (like global init expressions), there is no end of // function to check diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 39b5e7fc77f..54dad9079c7 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2529,17 +2529,15 @@ getOrMakeName(const std::unordered_map& nameMap, void WasmBinaryReader::readMemories() { auto num = getU32LEB(); auto numImports = wasm.memories.size(); - std::unordered_set usedNames; for (auto& [index, name] : memoryNames) { if (index >= num + numImports) { std::cerr << "warning: memory index out of bounds in name section: " << name << " at index " << index << '\n'; } - usedNames.insert(name); } for (size_t i = 0; i < num; i++) { auto [name, isExplicit] = - getOrMakeName(memoryNames, numImports + i, makeName("", i), usedNames); + getOrMakeName(memoryNames, numImports + i, makeName("", i), usedMemoryNames); auto memory = Builder::makeMemory(name); memory->hasExplicitName = isExplicit; getResizableLimits(memory->initial, @@ -2871,8 +2869,6 @@ void WasmBinaryReader::getResizableLimits(Address& initial, void WasmBinaryReader::readImports() { size_t num = getU32LEB(); Builder builder(wasm); - std::unordered_set usedFunctionNames, usedTableNames, usedMemoryNames, - usedGlobalNames, usedTagNames; for (size_t i = 0; i < num; i++) { auto module = getInlineString(); auto base = getInlineString(); @@ -3007,13 +3003,14 @@ void WasmBinaryReader::setLocalNames(Function& func, Index i) { void WasmBinaryReader::readFunctionSignatures() { size_t num = getU32LEB(); auto numImports = wasm.functions.size(); - std::unordered_set usedNames; +std::cout << "adding existing\n"; for (auto& [index, name] : functionNames) { if (index >= num + numImports) { std::cerr << "warning: function index out of bounds in name section: " << name << " at index " << index << '\n'; } - usedNames.insert(name); + usedFunctionNames.insert(name); +std::cout << " adding: " << name << "\n"; } // Also check that the function indices in the local names subsection are // in-bounds, even though we don't use them here. @@ -3026,7 +3023,7 @@ void WasmBinaryReader::readFunctionSignatures() { } for (size_t i = 0; i < num; i++) { auto [name, isExplicit] = - getOrMakeName(functionNames, numImports + i, makeName("", i), usedNames); + getOrMakeName(functionNames, numImports + i, makeName("", i), usedFunctionNames); auto index = getU32LEB(); HeapType type = getTypeByIndex(index); functionTypes.push_back(type); @@ -4761,17 +4758,15 @@ Name WasmBinaryReader::getIndexedString() { void WasmBinaryReader::readGlobals() { size_t num = getU32LEB(); auto numImports = wasm.globals.size(); - std::unordered_set usedNames; for (auto& [index, name] : globalNames) { if (index >= num + numImports) { std::cerr << "warning: global index out of bounds in name section: " << name << " at index " << index << '\n'; } - usedNames.insert(name); } for (size_t i = 0; i < num; i++) { auto [name, isExplicit] = getOrMakeName( - globalNames, numImports + i, makeName("global$", i), usedNames); + globalNames, numImports + i, makeName("global$", i), usedGlobalNames); auto type = getConcreteType(); auto mutable_ = getU32LEB(); if (mutable_ & ~1) { @@ -4860,17 +4855,15 @@ void WasmBinaryReader::readDataSegments() { void WasmBinaryReader::readTableDeclarations() { auto num = getU32LEB(); auto numImports = wasm.tables.size(); - std::unordered_set usedNames; for (auto& [index, name] : tableNames) { if (index >= num + numImports) { std::cerr << "warning: table index out of bounds in name section: " << name << " at index " << index << '\n'; } - usedNames.insert(name); } for (size_t i = 0; i < num; i++) { auto [name, isExplicit] = - getOrMakeName(tableNames, numImports + i, makeName("", i), usedNames); + getOrMakeName(tableNames, numImports + i, makeName("", i), usedTableNames); auto elemType = getType(); if (!elemType.isRef()) { throwError("Table type must be a reference type"); @@ -4977,18 +4970,16 @@ void WasmBinaryReader::readElementSegments() { void WasmBinaryReader::readTags() { size_t num = getU32LEB(); auto numImports = wasm.tags.size(); - std::unordered_set usedNames; for (auto& [index, name] : tagNames) { if (index >= num + numImports) { std::cerr << "warning: tag index out of bounds in name section: " << name << " at index " << index << '\n'; } - usedNames.insert(name); } for (size_t i = 0; i < num; i++) { getInt8(); // Reserved 'attribute' field auto [name, isExplicit] = - getOrMakeName(tagNames, numImports + i, makeName("tag$", i), usedNames); + getOrMakeName(tagNames, numImports + i, makeName("tag$", i), usedTagNames); auto typeIndex = getU32LEB(); auto tag = Builder::makeTag(name, getSignatureByTypeIndex(typeIndex)); tag->hasExplicitName = isExplicit; @@ -5081,6 +5072,7 @@ void WasmBinaryReader::readNames(size_t sectionPos, size_t payloadLen) { auto rawName = getInlineString(); auto name = processor.process(rawName); functionNames[index] = name; + usedFunctionNames.insert(name); } } else if (nameType == Subsection::NameLocal) { auto numFuncs = getU32LEB(); @@ -5112,6 +5104,7 @@ void WasmBinaryReader::readNames(size_t sectionPos, size_t payloadLen) { auto rawName = getInlineString(); auto name = processor.process(rawName); tableNames[index] = name; + usedTableNames.insert(name); } } else if (nameType == Subsection::NameElem) { auto num = getU32LEB(); @@ -5130,6 +5123,7 @@ void WasmBinaryReader::readNames(size_t sectionPos, size_t payloadLen) { auto rawName = getInlineString(); auto name = processor.process(rawName); memoryNames[index] = name; + usedMemoryNames.insert(name); } } else if (nameType == Subsection::NameData) { auto num = getU32LEB(); @@ -5148,6 +5142,7 @@ void WasmBinaryReader::readNames(size_t sectionPos, size_t payloadLen) { auto rawName = getInlineString(); auto name = processor.process(rawName); globalNames[index] = name; + usedGlobalNames.insert(name); } } else if (nameType == Subsection::NameField) { auto numTypes = getU32LEB(); @@ -5170,6 +5165,7 @@ void WasmBinaryReader::readNames(size_t sectionPos, size_t payloadLen) { auto rawName = getInlineString(); auto name = processor.process(rawName); tagNames[index] = name; + usedTagNames.insert(name); } } else { std::cerr << "warning: unknown name subsection with id " diff --git a/test/lit/name-overlap.wast b/test/lit/name-overlap.wast new file mode 100644 index 00000000000..ea97fa5e7ce --- /dev/null +++ b/test/lit/name-overlap.wast @@ -0,0 +1,16 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: foreach %s %t wasm-opt -all --instrument-branch-hints --roundtrip -S -o - | filecheck %s + +;; Two imports exist here, and instrument-branch-hints will add another. The +;; name "fimport$2" happens to be the name that would be chosen for that new +;; import, leading to a situation that the existing import has a forced name +;; from the names section (it is named here in the wat) while we pick an +;; internal name (not from the name section) that overlaps with it, causing an +;; error if we do not make sure to avoid duplication between import and non- +;; import names. + +(module + (import "fuzzing-support" "log-i64" (func $fimport$2 (param i64))) + (import "fuzzing-support" "log-f32" (func $fimport$3 (param f32))) +) From 9a69c342cd4b81acc29ae3c9bf7f1063644e73ba Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 10 Jul 2025 08:53:53 -0700 Subject: [PATCH 2/4] format --- src/wasm/wasm-binary.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 54dad9079c7..4af7c939d72 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2536,8 +2536,8 @@ void WasmBinaryReader::readMemories() { } } for (size_t i = 0; i < num; i++) { - auto [name, isExplicit] = - getOrMakeName(memoryNames, numImports + i, makeName("", i), usedMemoryNames); + auto [name, isExplicit] = getOrMakeName( + memoryNames, numImports + i, makeName("", i), usedMemoryNames); auto memory = Builder::makeMemory(name); memory->hasExplicitName = isExplicit; getResizableLimits(memory->initial, @@ -3003,14 +3003,12 @@ void WasmBinaryReader::setLocalNames(Function& func, Index i) { void WasmBinaryReader::readFunctionSignatures() { size_t num = getU32LEB(); auto numImports = wasm.functions.size(); -std::cout << "adding existing\n"; for (auto& [index, name] : functionNames) { if (index >= num + numImports) { std::cerr << "warning: function index out of bounds in name section: " << name << " at index " << index << '\n'; } usedFunctionNames.insert(name); -std::cout << " adding: " << name << "\n"; } // Also check that the function indices in the local names subsection are // in-bounds, even though we don't use them here. @@ -3022,8 +3020,8 @@ std::cout << " adding: " << name << "\n"; } } for (size_t i = 0; i < num; i++) { - auto [name, isExplicit] = - getOrMakeName(functionNames, numImports + i, makeName("", i), usedFunctionNames); + auto [name, isExplicit] = getOrMakeName( + functionNames, numImports + i, makeName("", i), usedFunctionNames); auto index = getU32LEB(); HeapType type = getTypeByIndex(index); functionTypes.push_back(type); @@ -4862,8 +4860,8 @@ void WasmBinaryReader::readTableDeclarations() { } } for (size_t i = 0; i < num; i++) { - auto [name, isExplicit] = - getOrMakeName(tableNames, numImports + i, makeName("", i), usedTableNames); + auto [name, isExplicit] = getOrMakeName( + tableNames, numImports + i, makeName("", i), usedTableNames); auto elemType = getType(); if (!elemType.isRef()) { throwError("Table type must be a reference type"); @@ -4978,8 +4976,8 @@ void WasmBinaryReader::readTags() { } for (size_t i = 0; i < num; i++) { getInt8(); // Reserved 'attribute' field - auto [name, isExplicit] = - getOrMakeName(tagNames, numImports + i, makeName("tag$", i), usedTagNames); + auto [name, isExplicit] = getOrMakeName( + tagNames, numImports + i, makeName("tag$", i), usedTagNames); auto typeIndex = getU32LEB(); auto tag = Builder::makeTag(name, getSignatureByTypeIndex(typeIndex)); tag->hasExplicitName = isExplicit; From 9c887578a1ac38e7306bfec342ffda41f5e573a4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 10 Jul 2025 08:56:04 -0700 Subject: [PATCH 3/4] test --- test/lit/name-overlap.wast | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/lit/name-overlap.wast b/test/lit/name-overlap.wast index ea97fa5e7ce..4caa4785e56 100644 --- a/test/lit/name-overlap.wast +++ b/test/lit/name-overlap.wast @@ -11,6 +11,15 @@ ;; import names. (module + ;; CHECK: (type $0 (func (param i64))) + + ;; CHECK: (type $1 (func (param f32))) + + ;; CHECK: (type $2 (func (param i32 i32 i32))) + + ;; CHECK: (import "fuzzing-support" "log-i64" (func $fimport$2 (type $0) (param i64))) (import "fuzzing-support" "log-i64" (func $fimport$2 (param i64))) + ;; CHECK: (import "fuzzing-support" "log-f32" (func $fimport$3 (type $1) (param f32))) (import "fuzzing-support" "log-f32" (func $fimport$3 (param f32))) ) +;; CHECK: (import "fuzzing-support" "log-branch" (func $fimport$2_2 (type $2) (param i32 i32 i32))) From 39308c50f28fcc32a14411881d8d9a06345d246d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 10 Jul 2025 09:00:00 -0700 Subject: [PATCH 4/4] redundant --- src/wasm/wasm-binary.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 4af7c939d72..1437f2a8219 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -3008,7 +3008,6 @@ void WasmBinaryReader::readFunctionSignatures() { std::cerr << "warning: function index out of bounds in name section: " << name << " at index " << index << '\n'; } - usedFunctionNames.insert(name); } // Also check that the function indices in the local names subsection are // in-bounds, even though we don't use them here.