Stream: git-wasmtime

Topic: wasmtime / PR #2550 wiggle: introduce Trap enum

Wasmtime GitHub notifications bot (Jan 06 2021 at 02:23):

pchickey opened PR #2550 from pch/wiggle_trapping to main:

<!--

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 (Jan 06 2021 at 02:31):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

<!--

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 (Jan 06 2021 at 15:00):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jan 06 2021 at 18:57):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

<!--

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 (Jan 06 2021 at 20:01):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

<!--

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 (Jan 06 2021 at 21:25):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

<!--

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 (Jan 06 2021 at 21:49):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

<!--

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 (Jan 06 2021 at 22:09):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

<!--

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 (Jan 07 2021 at 19:45):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

<!--

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 (Jan 07 2021 at 21:49):

pchickey edited PR #2550 from pch/wiggle_trapping to main:

This improves the design of Wiggle's ability to terminate WebAssembly execution by introducing a new
enum Trap { I32Exit(i32), String(String) }.

Previously, Wiggle users could terminate WebAssembly by giving an Err(String) in the UserErrorConversion impl. This has evolved into Err(wiggle::Trap), so that the termination can either give a human-readable trap, or an i32 exit code.

The big win here is we can finally implement wasi's proc_exit function in "pure" Wiggle, without the function override facility. So, this change has been threaded through the wasi-common crate and then into wasi-wasmtime.

I even uncovered some cli tests that technically didn't conform to the Wasi command spec because they didn't export a memory - wiggle now catches this, the old proc_exit override did not. These were trivially fixed.

The function override facility has been left in Wiggle for the time being, but I expect to remove it soon.

<!--

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 (Jan 07 2021 at 22:05):

pchickey edited PR #2550 from pch/wiggle_trapping to main:

This improves the design of Wiggle's ability to terminate WebAssembly execution by introducing a new
enum Trap { I32Exit(i32), String(String) }.

Previously, Wiggle users could terminate WebAssembly by giving an Err(String) in the UserErrorConversion impl. This has evolved into Err(wiggle::Trap), so that the termination can either give a human-readable trap, or an i32 exit code.

The big win here is we can finally implement wasi's proc_exit function in "pure" Wiggle, without the function override facility. So, this change has been threaded through the wasi-common crate and then into wasi-wasmtime.

I even uncovered some cli tests that technically didn't conform to the Wasi command spec because they didn't export a memory - wiggle now catches this, the old proc_exit override did not. These were trivially fixed.

Finally, instead of leaving the unimplemented! macro in the several spots in the wasi trait, I replaced them with an Error::Unsupported variant that produces a Trap.

The function override facility has been left in Wiggle for the time being, but I expect to remove it soon.

<!--

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 (Jan 07 2021 at 22:05):

pchickey has marked PR #2550 as ready for review.

Wasmtime GitHub notifications bot (Jan 07 2021 at 22:06):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

This improves the design of Wiggle's ability to terminate WebAssembly execution by introducing a new
enum Trap { I32Exit(i32), String(String) }.

Previously, Wiggle users could terminate WebAssembly by giving an Err(String) in the UserErrorConversion impl. This has evolved into Err(wiggle::Trap), so that the termination can either give a human-readable trap, or an i32 exit code.

The big win here is we can finally implement wasi's proc_exit function in "pure" Wiggle, without the function override facility. So, this change has been threaded through the wasi-common crate and then into wasi-wasmtime.

I even uncovered some cli tests that technically didn't conform to the Wasi command spec because they didn't export a memory - wiggle now catches this, the old proc_exit override did not. These were trivially fixed.

Finally, instead of leaving the unimplemented! macro in the several spots in the wasi trait, I replaced them with an Error::Unsupported variant that produces a Trap.

The function override facility has been left in Wiggle for the time being, but I expect to remove it soon.

<!--

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 (Jan 07 2021 at 22:06):

pchickey requested alexcrichton for a review on PR #2550.

Wasmtime GitHub notifications bot (Jan 07 2021 at 22:07):

pchickey edited PR #2550 from pch/wiggle_trapping to main:

This improves the design of Wiggle's ability to terminate WebAssembly execution by introducing a new
enum Trap { I32Exit(i32), String(String) }. This is just a simple version of Wasmtime's own Trap, which will be compatible with Lucet as well.

Previously, Wiggle users could terminate WebAssembly by giving an Err(String) in the UserErrorConversion impl. This has evolved into Err(wiggle::Trap), so that the termination can either give a human-readable trap, or an i32 exit code.

The big win here is we can finally implement wasi's proc_exit function in "pure" Wiggle, without the function override facility. So, this change has been threaded through the wasi-common crate and then into wasi-wasmtime.

I even uncovered some cli tests that technically didn't conform to the Wasi command spec because they didn't export a memory - wiggle now catches this, the old proc_exit override did not. These were trivially fixed.

Finally, instead of leaving the unimplemented! macro in the several spots in the wasi trait, I replaced them with an Error::Unsupported variant that produces a Trap.

The function override facility has been left in Wiggle for the time being, but I expect to remove it soon.

<!--

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 (Jan 07 2021 at 22:42):

pchickey edited PR #2550 from pch/wiggle_trapping to main:

This improves the design of Wiggle's ability to terminate WebAssembly execution by introducing a new
enum Trap { I32Exit(i32), String(String) }. This is just a simple version of Wasmtime's own Trap, which will be compatible with Lucet as well.

Previously, Wiggle users could terminate WebAssembly by giving an Err(String) in the UserErrorConversion impl. This has evolved into Err(wiggle::Trap), so that the termination can either give a human-readable trap, or an i32 exit code.

Additionally, functions which are (@witx noreturn) now have a return type of wiggle::Trap, using the type system to enforce that they terminate WebAssembly execution.

The big win here is we can finally implement wasi's proc_exit function in "pure" Wiggle, without the function override facility. So, this change has been threaded through the wasi-common crate and then into wasi-wasmtime.

I even uncovered some cli tests that technically didn't conform to the Wasi command spec because they didn't export a memory - wiggle now catches this, the old proc_exit override did not. These were trivially fixed.

Finally, instead of leaving the unimplemented! macro in the several spots in the wasi trait, I replaced them with an Error::Unsupported variant that produces a Trap.

The function override facility has been left in Wiggle for the time being, but I expect to remove it soon.

<!--

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 (Jan 07 2021 at 22:46):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jan 07 2021 at 22:47):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

This improves the design of Wiggle's ability to terminate WebAssembly execution by introducing a new
enum Trap { I32Exit(i32), String(String) }. This is just a simple version of Wasmtime's own Trap, which will be compatible with Lucet as well.

Previously, Wiggle users could terminate WebAssembly by giving an Err(String) in the UserErrorConversion impl. This has evolved into Err(wiggle::Trap), so that the termination can either give a human-readable trap, or an i32 exit code.

Additionally, functions which are (@witx noreturn) now have a return type of wiggle::Trap, using the type system to enforce that they terminate WebAssembly execution.

The big win here is we can finally implement wasi's proc_exit function in "pure" Wiggle, without the function override facility. So, this change has been threaded through the wasi-common crate and then into wasi-wasmtime.

I even uncovered some cli tests that technically didn't conform to the Wasi command spec because they didn't export a memory - wiggle now catches this, the old proc_exit override did not. These were trivially fixed.

Finally, instead of leaving the unimplemented! macro in the several spots in the wasi trait, I replaced them with an Error::Unsupported variant that produces a Trap.

The function override facility has been left in Wiggle for the time being, but I expect to remove it soon.

<!--

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 (Jan 07 2021 at 22:57):

pchickey updated PR #2550 from pch/wiggle_trapping to main:

This improves the design of Wiggle's ability to terminate WebAssembly execution by introducing a new
enum Trap { I32Exit(i32), String(String) }. This is just a simple version of Wasmtime's own Trap, which will be compatible with Lucet as well.

Previously, Wiggle users could terminate WebAssembly by giving an Err(String) in the UserErrorConversion impl. This has evolved into Err(wiggle::Trap), so that the termination can either give a human-readable trap, or an i32 exit code.

Additionally, functions which are (@witx noreturn) now have a return type of wiggle::Trap, using the type system to enforce that they terminate WebAssembly execution.

The big win here is we can finally implement wasi's proc_exit function in "pure" Wiggle, without the function override facility. So, this change has been threaded through the wasi-common crate and then into wasi-wasmtime.

I even uncovered some cli tests that technically didn't conform to the Wasi command spec because they didn't export a memory - wiggle now catches this, the old proc_exit override did not. These were trivially fixed.

Finally, instead of leaving the unimplemented! macro in the several spots in the wasi trait, I replaced them with an Error::Unsupported variant that produces a Trap.

The function override facility has been left in Wiggle for the time being, but I expect to remove it soon.

<!--

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 (Jan 08 2021 at 00:23):

fitzgen merged PR #2550.

Last updated: Nov 22 2024 at 16:03 UTC