Skip to content
This repository has been archived by the owner on Mar 24, 2022. It is now read-only.

💢 Package unknown hostcall panics into termination details #572

Merged
merged 1 commit into from
Jul 30, 2020

Conversation

acfoltzer
Copy link
Contributor

This prevents panics that aren't intentionally initiated by the Lucet runtime from attempting to unwind across Wasm stack frames. When #254 lands, this will no longer be necessary, but until then this allows us to safely transport the unknown panic payload across the FFI boundary so that panicking can resume on the other side.

This prevents panics that aren't intentionally initiated by the Lucet runtime from attempting to
unwind across Wasm stack frames. When #254 lands, this will no longer be necessary, but until then
this allows us to safely transport the unknown panic payload across the FFI boundary so that
panicking can resume on the other side.
Copy link
Contributor

@iximeow iximeow left a comment

Choose a reason for hiding this comment

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

🎉 panics, but less crashy! Approving on the assumption I've properly understood the boxed-box piece.

TerminationDetails::OtherPanic(p) => lucet_terminated {
reason: lucet_terminated_reason::OtherPanic,
// double box the panic payload so that the pointer passed to FFI
// land is thin
Copy link
Contributor

Choose a reason for hiding this comment

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

took a moment to remember that non-thin-pointer types don't have FFI-stable layouts. That's what's going on here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly; Box::into_raw(x) where x is Box<dyn T> returns a *mut dyn T, which is two machine pointers wide for the vtable, so casting it to *mut c_void to store in the struct would be Bad. The extra box makes the returned raw pointer have the type *mut Box<dyn T>, which is fine because the vtable pointer is then stored in the box on the heap.

@acfoltzer acfoltzer merged commit d80e86c into main Jul 30, 2020
@acfoltzer acfoltzer deleted the acf/panic-ferry branch July 30, 2020 00:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants