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

Validate the effective type size of modules #215

Merged

Conversation

alexcrichton
Copy link
Member

This commit implements a fix for #214 where the effective size of the
type of any item in a module (or a module itself) is now required to be
bounded. The goal here is to accept reasonable modules but at the same
time protecting implementations like wasmtime from having to worry about
deeply recursive and nested module types. The general fix here is to
account for the size of all types as we parse them, and at all times the
size is bounded.

Additionally wasm-smith is updated with similar accounting for the size
of types and such to prevent generating modules with massively recursive
types.

Closes #214

This commit implements a fix for bytecodealliance#214 where the effective size of the
type of any item in a module (or a module itself) is now required to be
bounded. The goal here is to accept reasonable modules but at the same
time protecting implementations like wasmtime from having to worry about
deeply recursive and nested module types. The general fix here is to
account for the size of all types as we parse them, and at all times the
size is bounded.

Additionally wasm-smith is updated with similar accounting for the size
of types and such to prevent generating modules with massively recursive
types.

Closes bytecodealliance#214
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

r=me with doc clarification below

crates/wasm-smith/src/config.rs Show resolved Hide resolved
crates/wasm-smith/src/lib.rs Outdated Show resolved Hide resolved
@alexcrichton alexcrichton merged commit 6bb6c24 into bytecodealliance:main Jan 29, 2021
@alexcrichton alexcrichton deleted the bound-recursive-types branch January 29, 2021 15:01
alexcrichton added a commit to alexcrichton/wasm-tools that referenced this pull request Nov 16, 2022
…lliance#215)

* Split out the `Option` and `Expected` types from `Variant`

This commit is like prior PRs to split out specializations of types into
their own AST type to avoid conflicting with the main type (in this case
`variant`). I originally thought these two types would be relatively
simple but this is probably one of the more complicated transitions, as
evidenced by the lines changed here. The main churn was that variants
already have a significant amount of code to support them and this is in
some places "duplicating" code for option/expected and in other cases
splitting what was already an if/else.

Overall I think that the generated code gets a little better since it's
clear when something is and `option` vs `expected` now rather than
trying to have everything shoehorned into one. Notably the C code
generator now generates descriptive fields like `bool is_some` or `bool
is_err` instead of a bland `uint8_t tag` with some comments about how to
use it.

* Remove `Variant::as_{option,expected}`

... as these are separate variants now.

* Review comments
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.

Somehow protect against hugely-recursive module types
2 participants