Skip to content
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.

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

wasm-split error without an element segment #6572

Closed
kripken opened this issue May 7, 2024 · 0 comments · Fixed by #6651
Closed

wasm-split error without an element segment #6572

kripken opened this issue May 7, 2024 · 0 comments · Fixed by #6651

Comments

@kripken
Copy link
Member

kripken commented May 7, 2024

(module
 (table 0 funcref)
 (export "func_94_invoker" (func $0))
 (func $0
  (nop)
 )
)
$ bin/wasm-split a.wat -all --split -o1 a.wasm -o2 b.wasm -S --split-funcs=0
warning: not keeping any functions in the primary module
wasm-split: src/support/name.cpp:44: std::ostream& wasm::Name::print(std::ostream&) const: Assertion `*this && "Cannot print an empty name"' failed.
Aborted

It seems to have a null Name for the table when it creates a CallIndirect. At a glance at the code I thought this might work:

diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp
index e0fba8ae8..31b85b906 100644
--- a/src/ir/module-splitting.cpp
+++ b/src/ir/module-splitting.cpp
@@ -158,6 +158,10 @@ TableSlotManager::TableSlotManager(Module& module) : module(module) {
   }
 
   activeTable = it->get();
+  if (activeTable == nullptr) {
+    activeTable = makeTable();
+    activeBase = {activeTable->name, "", 0};
+  }
   ModuleUtils::iterTableSegments(
     module, activeTable->name, [&](ElementSegment* segment) {
       activeTableSegments.push_back(segment);
@@ -220,11 +227,6 @@ TableSlotManager::Slot TableSlotManager::getSlot(Name func, HeapType type) {
 
   // If there are no segments yet, allocate one.
   if (activeSegment == nullptr) {
-    if (activeTable == nullptr) {
-      activeTable = makeTable();
-      activeBase = {activeTable->name, "", 0};
-    }
-
     // None of the existing segments should refer to the active table
     assert(std::all_of(module.elementSegments.begin(),
                        module.elementSegments.end(),

That fixes this one but other testcases start to fail so I am not sure if I understand what "active base/table/segment" mean here - some seem to need to be set in the constructor but others not? Anyhow this might be something trivial I am missing, @tlively

tlively added a commit that referenced this issue Jun 11, 2024
The module splitting code incorrectly assumed that there would be at least one
active element segment and failed to initialize the table slot manager with a
function table if that was not the case. Fix the bug by setting the table even
when there are no active segments and add a test.

Fixes #6572.
tlively added a commit that referenced this issue Jun 11, 2024
The module splitting code incorrectly assumed that there would be at least one
active element segment and failed to initialize the table slot manager with a
function table if that was not the case. Fix the bug by setting the table even
when there are no active segments and add a test.

Fixes #6572 and #6637.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant