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

Fix tableidx overflow on call_indirect #303

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MaartenS11
Copy link
Member

@MaartenS11 MaartenS11 commented Jan 25, 2025

On some programs, the vm can run into an error like this Unsigned LEB at byte 0x12800b011 overflow. This overflow occurs when reading table indexes on the call_indirect function. It occurs because the table index in WARDuino can at most consist of 7 or 1 bits (the maximum amount of bit differs in two parts of the vm). In the wasm specification however, the table index is specified to be a u32. You would expect this error to only occur on modules using multiple tables but as it turns out this also happens on rust binaries that use just a single table. The reason for this is that the rust compiler can sometimes use a 5 byte table index 0x80 0x80 0x80 0x80 0x00 to encode table index 0 as described in this blog post. Any binary using such an index will fail to execute, this PR fixes that.

This PR also adds an error message for stack overflows during setup_call when in debug/trace/warn/info mode. This way users attempting to run bigger programs will more quickly realize that they just need to increase the stack size (or run their program in release mode) and it's not an issue with the vm implementation.

…EBUG/TRACE/WARN/INFO mode

This way you get a friendly error when trying to figure out why something is not working instead of a vague segfault.
@tolauwae tolauwae self-requested a review February 10, 2025 07:07
Copy link
Member

@tolauwae tolauwae left a comment

Choose a reason for hiding this comment

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

Thanks for the change. Looks good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants