Stream: git-wasmtime

Topic: wasmtime / PR #2461 Implement imported/exported modules/i...

Wasmtime GitHub notifications bot (Dec 01 2020 at 22:57):

alexcrichton opened PR #2461 from import-export-instances-modules to main:

This commit implements the final piece of the module linking proposal
which is to flesh out the support for importing/exporting instances and
modules. This ended up having a few changes:

• Two more PrimaryMap instances are now stored in an Instance. The value
  for instances is InstanceHandle (pretty easy) and for modules it's
  Box<dyn Any> (less easy).

• The custom host state for InstanceHandle for wasmtime is now
  Arc<TypeTables to be able to fully reconstruct an instance's types
  just from its instance.

• Type matching for imports now has been updated to take
  instances/modules into account.

One of the main downsides of this implementation is that type matching
of imports is duplicated between wasmparser and wasmtime, leading to
posssible bugs especially in the subtelties of module linking. I'm not
sure how best to unify these two pieces of validation, however, and it
may be more trouble than it's worth.

cc #2094

Note that this is built on https://github.com/bytecodealliance/wasmtime/pull/2451 so only the last commit is relevant.

Wasmtime GitHub notifications bot (Dec 02 2020 at 22:58):

alexcrichton updated PR #2461 from import-export-instances-modules to main:

This commit implements the final piece of the module linking proposal
which is to flesh out the support for importing/exporting instances and
modules. This ended up having a few changes:

• Two more PrimaryMap instances are now stored in an Instance. The value
  for instances is InstanceHandle (pretty easy) and for modules it's
  Box<dyn Any> (less easy).

• The custom host state for InstanceHandle for wasmtime is now
  Arc<TypeTables to be able to fully reconstruct an instance's types
  just from its instance.

• Type matching for imports now has been updated to take
  instances/modules into account.

One of the main downsides of this implementation is that type matching
of imports is duplicated between wasmparser and wasmtime, leading to
posssible bugs especially in the subtelties of module linking. I'm not
sure how best to unify these two pieces of validation, however, and it
may be more trouble than it's worth.

cc #2094

Note that this is built on https://github.com/bytecodealliance/wasmtime/pull/2451 so only the last commit is relevant.

Wasmtime GitHub notifications bot (Dec 02 2020 at 23:24):

alexcrichton updated PR #2461 from import-export-instances-modules to main:

This commit implements the final piece of the module linking proposal
which is to flesh out the support for importing/exporting instances and
modules. This ended up having a few changes:

• Two more PrimaryMap instances are now stored in an Instance. The value
  for instances is InstanceHandle (pretty easy) and for modules it's
  Box<dyn Any> (less easy).

• The custom host state for InstanceHandle for wasmtime is now
  Arc<TypeTables to be able to fully reconstruct an instance's types
  just from its instance.

• Type matching for imports now has been updated to take
  instances/modules into account.

One of the main downsides of this implementation is that type matching
of imports is duplicated between wasmparser and wasmtime, leading to
posssible bugs especially in the subtelties of module linking. I'm not
sure how best to unify these two pieces of validation, however, and it
may be more trouble than it's worth.

cc #2094

Note that this is built on https://github.com/bytecodealliance/wasmtime/pull/2451 so only the last commit is relevant.

Wasmtime GitHub notifications bot (Dec 02 2020 at 23:24):

alexcrichton requested peterhuene for a review on PR #2461.

Wasmtime GitHub notifications bot (Dec 03 2020 at 01:54):

peterhuene submitted PR Review.

Wasmtime GitHub notifications bot (Dec 03 2020 at 01:54):

peterhuene submitted PR Review.

Wasmtime GitHub notifications bot (Dec 03 2020 at 01:54):

peterhuene created PR Review Comment:

Clippy nit:

        ty

Wasmtime GitHub notifications bot (Dec 03 2020 at 01:54):

peterhuene created PR Review Comment:

Clippy nit:

        sig

Wasmtime GitHub notifications bot (Dec 03 2020 at 01:54):

peterhuene created PR Review Comment:

:bike:shed: perhaps this method should be renamed to build now given the mutation and "builder" name?

Wasmtime GitHub notifications bot (Dec 03 2020 at 01:54):

peterhuene created PR Review Comment:

This comment seems incongruent with the method.

Wasmtime GitHub notifications bot (Dec 03 2020 at 01:54):

peterhuene created PR Review Comment:

            // we acquired the handle from an instance in the store.

Wasmtime GitHub notifications bot (Dec 03 2020 at 01:54):

peterhuene created PR Review Comment:

    /// Validates that the `actual_imports` list of module imports matches the

Wasmtime GitHub notifications bot (Dec 03 2020 at 15:17):

alexcrichton updated PR #2461 from import-export-instances-modules to main:

This commit implements the final piece of the module linking proposal
which is to flesh out the support for importing/exporting instances and
modules. This ended up having a few changes:

• Two more PrimaryMap instances are now stored in an Instance. The value
  for instances is InstanceHandle (pretty easy) and for modules it's
  Box<dyn Any> (less easy).

• The custom host state for InstanceHandle for wasmtime is now
  Arc<TypeTables to be able to fully reconstruct an instance's types
  just from its instance.

• Type matching for imports now has been updated to take
  instances/modules into account.

One of the main downsides of this implementation is that type matching
of imports is duplicated between wasmparser and wasmtime, leading to
posssible bugs especially in the subtelties of module linking. I'm not
sure how best to unify these two pieces of validation, however, and it
may be more trouble than it's worth.

cc #2094

Note that this is built on https://github.com/bytecodealliance/wasmtime/pull/2451 so only the last commit is relevant.

Wasmtime GitHub notifications bot (Dec 03 2020 at 15:33):

alexcrichton updated PR #2461 from import-export-instances-modules to main:

This commit implements the final piece of the module linking proposal
which is to flesh out the support for importing/exporting instances and
modules. This ended up having a few changes:

• Two more PrimaryMap instances are now stored in an Instance. The value
  for instances is InstanceHandle (pretty easy) and for modules it's
  Box<dyn Any> (less easy).

• The custom host state for InstanceHandle for wasmtime is now
  Arc<TypeTables to be able to fully reconstruct an instance's types
  just from its instance.

• Type matching for imports now has been updated to take
  instances/modules into account.

One of the main downsides of this implementation is that type matching
of imports is duplicated between wasmparser and wasmtime, leading to
posssible bugs especially in the subtelties of module linking. I'm not
sure how best to unify these two pieces of validation, however, and it
may be more trouble than it's worth.

cc #2094

Note that this is built on https://github.com/bytecodealliance/wasmtime/pull/2451 so only the last commit is relevant.

Wasmtime GitHub notifications bot (Dec 03 2020 at 16:15):

alexcrichton merged PR #2461.

Last updated: Dec 23 2024 at 13:07 UTC