Skip to content

[wasm-split] Fix table naming conflicts #8708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aheejin wants to merge 3 commits into
base: main
Choose a base branch
from
+69 −6
Open

[wasm-split] Fix table naming conflicts #8708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9abfe78
[wasm-split] Fix table naming conflicts
aheejin May 15, 2026
d38d9fa
Disable Split fuzzer
aheejin May 15, 2026
57d63fc
Make a set of secondary names first
aheejin May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/fuzz_opt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2525,7 +2525,7 @@ def handle(self, wasm):
TrapsNeverHappen(),
CtorEval(),
Merge(),
Split(),
# Split(), # Will reenable after stabilized
RoundtripText(),
ClusterFuzz(),
Two(),
Expand Down
28 changes: 23 additions & 5 deletions src/ir/module-splitting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,15 @@ struct TableSlotManager {
Expression* makeExpr(Module& module);
};
Module& module;
const std::vector<std::unique_ptr<Module>>& secondaries;
Table* activeTable = nullptr;
ElementSegment* activeSegment = nullptr;
Slot activeBase;
std::map<Name, Slot> funcIndices;
std::vector<ElementSegment*> activeTableSegments;

TableSlotManager(Module& module);
TableSlotManager(Module& module,
const std::vector<std::unique_ptr<Module>>& secondaries);

Table* makeTable();
ElementSegment* makeElementSegment();
Expand Down Expand Up @@ -149,7 +151,9 @@ void TableSlotManager::addSlot(Name func, Slot slot) {
funcIndices.insert({func, slot});
}

TableSlotManager::TableSlotManager(Module& module) : module(module) {
TableSlotManager::TableSlotManager(
Module& module, const std::vector<std::unique_ptr<Module>>& secondaries)
: module(module), secondaries(secondaries) {
// If possible, just create a new table to manage all primary-to-secondary
// calls lazily. Do not re-use slots for functions that will already be in
// existing tables, since that is not correct in the face of table mutations.
Expand Down Expand Up @@ -233,8 +237,22 @@ TableSlotManager::TableSlotManager(Module& module) : module(module) {
}

Table* TableSlotManager::makeTable() {
return module.addTable(
Builder::makeTable(Names::getValidTableName(module, Name::fromInt(0))));
std::unordered_set<Name> secondaryTableNames;
for (auto& secondary : secondaries) {
for (auto& table : secondary->tables) {
secondaryTableNames.insert(table->name);
}
}
Name name = Names::getValidName("0", [&](Name test) {
if (module.getTableOrNull(test)) {
return false;
}
if (secondaryTableNames.count(test)) {
return false;
}
return true;
});
return module.addTable(Builder::makeTable(name));
}

ElementSegment* TableSlotManager::makeElementSegment() {
Expand Down Expand Up @@ -347,7 +365,7 @@ struct ModuleSplitter {
void setupTablePatching();

ModuleSplitter(Module& primary, const Config& config)
: config(config), primary(primary), tableManager(primary),
: config(config), primary(primary), tableManager(primary, secondaries),
exportedPrimaryFuncs(initExportedPrimaryFuncs(primary)),
exportedPrimaryItems(initExportedPrimaryItems(primary)) {
classifyFunctions();
Expand Down
45 changes: 45 additions & 0 deletions test/lit/wasm-split/table-name-conflict.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
;; RUN: wasm-split %s -all -g -o1 %t.1.wasm -o2 %t.2.wasm --split-funcs=split
;; RUN: wasm-dis %t.1.wasm | filecheck %s --check-prefix PRIMARY
;; RUN: wasm-dis %t.2.wasm | filecheck %s --check-prefix SECONDARY

;; Regression test for a bug when an existing table, which is to be split to the
;; secondary module, has the name '0'. The newly created active table should
;; have a different name.

(module
(table $0 0 externref)
(export "split" (func $split))
(func $split
(table.set $0
(i32.const 0)
(ref.null extern)
)
)
)

;; PRIMARY: (module
;; PRIMARY-NEXT: (type $0 (func))
;; PRIMARY-NEXT: (import "placeholder.deferred" "0" (func $placeholder_0))
;; PRIMARY-NEXT: (table $0 1 funcref)
Copy link
Copy Markdown
Member Author

@aheejin aheejin May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Table names are not actually shown in the output because its hasExternalName is false. But without this patch this test crashes because it tries to reuse the existing externref table in the secondary module as the active table.

Copy link
Copy Markdown
Member

@tlively tlively May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying that $0 is not the internal name of the table? Why are we emitting a name at all, then?

Copy link
Copy Markdown
Member Author

@aheejin aheejin May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I understand. So the input file's table name is0:

 (table $0 0 externref)

And when it tries to make an active table, we start from 0, but because it collides with the existing secondary table, getValidName adds _0 to the name, so the active table's name will be 0_0.

But because the table's hasExternalName is not set, when we write the binary, that name is not separately written to the name section. And this test expectation is printed by wasm-dis. wasm-dis doesn't have any name information on tables, so it just blindly assign numbers from 0. That's why both tables, one in the primary (newly created active table) and secondary (existing externref table split out) are named 0.

As I said in #8708 (comment), we can make it preserved if debug option is enabled.

Copy link
Copy Markdown
Member

@tlively tlively May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the new table is still called $0?

Copy link
Copy Markdown
Member Author

@aheejin aheejin May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#8708 (comment)
We can set hasExternalName true depending on the debug information. Given that there will be only one active table, I'm not sure if it's worth it, but we can try, like even naming it "active_table" and make it shown in debug mode. (But this would be better as a follow-up, if we do it)

;; PRIMARY-NEXT: (elem $0 (i32.const 0) $placeholder_0)
;; PRIMARY-NEXT: (export "split" (func $trampoline_split))
;; PRIMARY-NEXT: (export "table" (table $0))
;; PRIMARY-NEXT: (func $trampoline_split
;; PRIMARY-NEXT: (call_indirect (type $0)
;; PRIMARY-NEXT: (i32.const 0)
;; PRIMARY-NEXT: )
;; PRIMARY-NEXT: )
;; PRIMARY-NEXT: )

;; SECONDARY: (module
;; SECONDARY-NEXT: (type $0 (func))
;; SECONDARY-NEXT: (import "primary" "table" (table $timport$0 1 funcref))
;; SECONDARY-NEXT: (table $0 0 externref)
;; SECONDARY-NEXT: (elem $0 (table $timport$0) (i32.const 0) func $split)
;; SECONDARY-NEXT: (func $split
;; SECONDARY-NEXT: (table.set $0
;; SECONDARY-NEXT: (i32.const 0)
;; SECONDARY-NEXT: (ref.null noextern)
;; SECONDARY-NEXT: )
;; SECONDARY-NEXT: )
;; SECONDARY-NEXT: )
Loading