Stream: git-wasmtime

Topic: wasmtime / PR #1910 Factor Wiggle's wasmtime integration ...

Wasmtime GitHub notifications bot (Jun 23 2020 at 02:36):

pchickey opened PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which I'll rename shortly.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Jun 23 2020 at 02:36):

pchickey requested alexcrichton for a review on PR #1910.

Wasmtime GitHub notifications bot (Jun 23 2020 at 02:39):

pchickey edited PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on (lucet-wiggle)[https://github.com/bytecodealliance/lucet/tree/master/lucet-wiggle], which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 23 2020 at 02:42):

pchickey edited PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on lucet-wiggle, which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 23 2020 at 19:30):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jun 23 2020 at 19:30):

alexcrichton created PR Review Comment:

Technically this isn't quite true since wasi-common supports virtual handles, right?

Wasmtime GitHub notifications bot (Jun 23 2020 at 19:30):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jun 23 2020 at 20:36):

pchickey created PR Review Comment:

The Handle trait doesn't expose GuestPtr or any types which contain one, so I think technically we are ok. All of the interesting uses of GuestPtr get translated into std::io::IoSlice{Mut} before being passed to the Handle methods.

Wasmtime GitHub notifications bot (Jun 23 2020 at 20:36):

pchickey submitted PR Review.

Wasmtime GitHub notifications bot (Jun 23 2020 at 21:21):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jun 23 2020 at 21:21):

alexcrichton created PR Review Comment:

I'm not necessarily worried about GuestPtr, moreso that if you run arbitrary code it could reenter wasm which could invalidate GuestPtr values on the stack previously. The Handle type I think can run arbitrary code, right? (and internally it can call something which reenters the original wasm module)

Wasmtime GitHub notifications bot (Jun 23 2020 at 21:46):

pchickey submitted PR Review.

Wasmtime GitHub notifications bot (Jun 23 2020 at 21:46):

pchickey created PR Review Comment:

Ah, right, I forgot about that aspect. Yes, Handle impls can execute whatever code they like, so it is definitely possible for them to re-enter the instance. That means we should be finding some way to provide a BorrowChecker that has the same lifetime as the instance, because the lifetime of the call into these functions is no longer sufficient.

I'm not sure how to do that, but I can fix it in this PR if you can outline how I should, or we can leave it for a separate PR.

Wasmtime GitHub notifications bot (Jun 23 2020 at 22:27):

pchickey updated PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on lucet-wiggle, which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 23 2020 at 22:28):

pchickey updated PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on lucet-wiggle, which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 23 2020 at 23:29):

pchickey updated PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on lucet-wiggle, which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 24 2020 at 00:42):

pchickey updated PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on lucet-wiggle, which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 24 2020 at 01:11):

pchickey updated PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on lucet-wiggle, which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 24 2020 at 15:04):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jun 24 2020 at 15:04):

alexcrichton created PR Review Comment:

There's a number of ways that we could do this, but I don't think any of them are relatively easy. What we probably want to do is to integrate this into wasmtime-the-crate itself, and that way entering into wasm could always check to make sure there are no active borrows because we have tight control over the wasm entry points.

Perhaps we could have a discussion in an issue about how to best integrate this tightly with the wasmtime crate itself? I feel like borrowing from guest memory is so common it's justified to put this in the core functionality.

Wasmtime GitHub notifications bot (Jun 24 2020 at 19:47):

pchickey updated PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on lucet-wiggle, which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 24 2020 at 21:34):

pchickey updated PR #1910 from pch/separate_wasmtime_wiggle_crate to master:

Presently, integrating wiggle with wasmtime is taken care of as a one-off proc macro in wasi-common's wig crate.

It would be ideal to provide a single proc macro that generates the adapter between wiggle::from_witx! -generated code and Wasmtime. This can be used as a substitute for the wig code in wasmtime-wasi, and in other crates.

The design of this crate is based on lucet-wiggle, which provides the same adapter between wiggle and lucet-runtime.

This PR moves and refactors the wig implementation of this adapter code to be general purpose.

• Creates a new crate wasmtime-wiggle at crates/wiggle/wasmtime. This re-exports wiggle (so users of the adapter only need to track one dep), defines the reusable runtime component WasmtimeGuestMemory, which has an impl GuestMemory, and exports the adapter macro
• New crate wasmtime-wiggle-macro is a proc-macro lib at crates/wiggle/wasmtime/macro. This exports define_struct_for_wiggle, which isn't an ideal name anymore, and I'll have to fix.

The configuration options for define_struct_for_wiggle are dramatically expanded, and re-use some code and the style of parsing configuration options from wiggle_generate.

Wasmtime GitHub notifications bot (Jun 24 2020 at 21:52):

pchickey submitted PR Review.

Wasmtime GitHub notifications bot (Jun 24 2020 at 21:52):

pchickey created PR Review Comment:

https://github.com/bytecodealliance/wasmtime/issues/1917

Wasmtime GitHub notifications bot (Jun 24 2020 at 21:52):

pchickey has marked PR #1910 as ready for review.

Wasmtime GitHub notifications bot (Jun 24 2020 at 21:52):

pchickey requested alexcrichton for a review on PR #1910.

Wasmtime GitHub notifications bot (Jun 25 2020 at 14:20):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jun 25 2020 at 16:34):

pchickey merged PR #1910.

Last updated: Nov 22 2024 at 17:03 UTC