Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for integrating validation in Wasmtime #62

Merged

Conversation

alexcrichton
Copy link
Member

This commit is a batch of updates I've made and found when integrating
wasmparser's validation more intrusively into Wasmtime. The changes made
here are:

  • WasmFeatures is now exposed as a standalone type so it can be shared
    with other crates. This way only one crate, wasmparser, declares the
    "bag of features" and everyone else can just reference that one bag.

  • Parsing locals is deferred in code_section_entry to actual
    validation. Helper methods have been added to advance a BinaryReader
    by reading the locals, as well as methods added to define locals
    yourself. The intention here is to avoid parsing the locals section
    twice, once in wasmparser and once by the application. This also
    removed a few read_local* methods from BinaryReader to defer
    validation to Validator.

  • WasmTypeDef and type_at have been removed from
    WasmModuleResources. These weren't ever really that useful and
    callers only ever cared about function types anyway. The previous
    func_type_at method has been repurposed for looking up a type index
    and a new type_of_function looks up the type of a function by index.

  • Parsing for element segments was slightly updated to ensure that
    externref segments don't have ref.func in them.

@alexcrichton
Copy link
Member Author

Hm actually I forgot there's one more large-ish feature, so I'll be adding that on here.

@alexcrichton
Copy link
Member Author

Ok I've added the API intended, which was FuncValidator::new to create an arbitrary function validator with a WasmModuleReferences instance.

This commit is a batch of updates I've made and found when integrating
wasmparser's validation more intrusively into Wasmtime. The changes made
here are:

* `WasmFeatures` is now exposed as a standalone type so it can be shared
  with other crates. This way only one crate, wasmparser, declares the
  "bag of features" and everyone else can just reference that one bag.

* Parsing locals is deferred in `code_section_entry` to actual
  validation. Helper methods have been added to advance a `BinaryReader`
  by reading the locals, as well as methods added to define locals
  yourself. The intention here is to avoid parsing the locals section
  twice, once in `wasmparser` and once by the application. This also
  removed a few `read_local*` methods from `BinaryReader` to defer
  validation to `Validator`.

* `WasmTypeDef` and `type_at` have been removed from
  `WasmModuleResources`. These weren't ever really that useful and
  callers only ever cared about function types anyway. The previous
  `func_type_at` method has been repurposed for looking up a type index
  and a new `type_of_function` looks up the type of a function by index.

* Parsing for element segments was slightly updated to ensure that
  `externref` segments don't have `ref.func` in them.

* Add a `FuncValidator::new` API which allows creating one independent
  of a `Validator`.
@alexcrichton
Copy link
Member Author

Ok, and another commit for WasmFuncType::{inputs,outputs} to get at the inputs/outputs a bit more ergonomically.

@@ -73,7 +73,7 @@ impl Range {
}

/// A binary reader of the WebAssembly structures and types.
#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder how/where Hash will be used -- buffer can be long.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW this is used here where we has the contents of functions for the internal cache.

Migrate the old iterator constructors to these trait methods and tidy up
the implementations a bit to rely on `Range` for all the heavy lifting.
@alexcrichton alexcrichton force-pushed the wasmtime-parser-updates branch from a00963d to ca1475a Compare July 22, 2020 15:15
Copy link
Contributor

@yurydelendik yurydelendik left a comment

Choose a reason for hiding this comment

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

The changes make sense. Thank you.

@alexcrichton alexcrichton merged commit 6ac060c into bytecodealliance:main Jul 22, 2020
@alexcrichton alexcrichton deleted the wasmtime-parser-updates branch July 22, 2020 15:39
fitzgen pushed a commit to fitzgen/wasm-tools that referenced this pull request Oct 21, 2020
dhil added a commit to dhil/wasm-tools that referenced this pull request Feb 23, 2024
This patch merges with upstream and attempts to fix the CI publish job.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants