Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Catch panics on the FFI boundary between the runtime and the host for wasmtime #11189

Conversation

koute
Copy link
Contributor

@koute koute commented Apr 8, 2022

Currently if the runtime calls into the host and the host panics we're left with no backtrace from within the runtime since the panic is only caught here after the whole execution context is already dropped. (And the host functions can panic.)

This PR makes it so that we catch any panics on the FFI boundary between the runtime and the host, and turn that into a trap. This allows us to extract and print out the backtrace from within the runtime as part of the error message and see what exactly within the runtime called the host function which panicked.

This is done only for wasmtime since at this time wasmi doesn't support backtraces anyway.

@koute koute added A0-please_review Pull request needs code review. J0-enhancement An additional feature request. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Apr 8, 2022
@koute koute requested review from pepyakin and a team April 8, 2022 08:10
@koute koute added the B0-silent Changes should not be mentioned in any release notes label Apr 8, 2022
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Nice. I had exactly the same idea, so that we get a proper backtrace :)

client/executor/runtime-test/src/lib.rs Outdated Show resolved Hide resolved
@bkchr
Copy link
Member

bkchr commented Apr 8, 2022

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Bot will approve on the behalf of @bkchr, since they are a team lead, in an attempt to reach the minimum approval count

@paritytech-processbot paritytech-processbot bot merged commit 994bb57 into paritytech:master Apr 8, 2022
DaviRain-Su pushed a commit to octopus-network/substrate that referenced this pull request Aug 23, 2022
… `wasmtime` (paritytech#11189)

* Catch panics on the FFI boundary between the runtime and the host for `wasmtime`

* Use an already existing test runtime function

* Merge the tests together
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
… `wasmtime` (paritytech#11189)

* Catch panics on the FFI boundary between the runtime and the host for `wasmtime`

* Use an already existing test runtime function

* Merge the tests together
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit J0-enhancement An additional feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants