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

Revert "Update to wasmi v0.32 and add lazy validation (#1488)" #1527

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Dec 28, 2023

Reverts #1488

cc @Robbepop

While working on #1483, I noticed that some Wasm runtime calls fail with:

panicked at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/alloc/src/collections/btree/navigate.rs:535:47:\ncalled Option::unwrap() on a None value

Reverting #1488 fixes this issue, so I'm going to revert for now.

The code that is panicking is very low-level, so I'm suspecting some kind of memory corruption or miscompilation or something like that.

@Robbepop
Copy link

Okay thanks for letting me know. Going to investigate as soon as possible.

@tomaka
Copy link
Contributor Author

tomaka commented Dec 28, 2023

To clarify, the panic in btree/navigate.rs comes from the Rust code that was compiled to Wasm, and executing this Wasm with wasmi 0.32 triggers it while executing it with wasmi 0.31 doesn't.

@tomaka
Copy link
Contributor Author

tomaka commented Dec 28, 2023

I could make a reproducible case, but it's a bit complicated because there are a lot of imported functions and a lot of calls to these imported functions.

@tomaka tomaka added this pull request to the merge queue Dec 28, 2023
Merged via the queue into smol-dot:main with commit 961c920 Dec 28, 2023
23 checks passed
@tomaka tomaka deleted the revert-wasmi branch December 28, 2023 11:20
@Robbepop
Copy link

I could make a reproducible case, but it's a bit complicated because there are a lot of imported functions and a lot of calls to these imported functions.

I would really appreciate that if you would do that but if not I can also try doing it. Very certainly going to be needed.

@Robbepop
Copy link

@tomaka I just release Wasmi v0.32.0-beta.3 with many bug fixes. I will continue the bug hunt but maybe this new version fixes the problems you encountered with one of the Polkadot runtimes?

https://github.com/paritytech/wasmi/releases/tag/v0.32.0-beta.3

@tomaka
Copy link
Contributor Author

tomaka commented Jan 12, 2024

It seems to work now!

tomaka added a commit to tomaka/smoldot that referenced this pull request Jan 12, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jan 16, 2024
…"" (#1577)

* Revert "Revert "Update to wasmi v0.32 and add lazy validation (#1488)" (#1527)"

This reverts commit 961c920.

* Update wasmi

* Fix compilation

* Update CHANGELOG

* Update wasmi

* Cargo.lock changes didn't commit

* Another update
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 this pull request may close these issues.

2 participants