Stream: git-wasmtime

Topic: wasmtime / PR #4825 components: Limit the recursive size ...

Wasmtime GitHub notifications bot (Aug 30 2022 at 19:12):

alexcrichton opened PR #4825 from limit-component-recursion to main:

This commit is aimed at fixing #4814 by placing a hard limit on the
maximal recursive depth a type may have in the component model. The
component model theoretically allows for infinite recursion but many
various types of operations within the component model are naturally
written as recursion over the structure of a type which can lead to
stack overflow with deeply recursive types. Some examples of recursive
operations are:

• Lifting and lowering a type - currently the recursion here is modeled
  in Rust directly with #[derive] implementations as well as the
  implementations for the Val type.

• Compilation of adapter trampolines which iterates over the type
  structure recursively.

• Historically many various calculations like the size of a type, the
  flattened representation of a type, etc, were all done recursively.
  Many of these are more efficiently done via other means but it was
  still natural to implement these recursively initially.

By placing a hard limit on type recursion Wasmtime won't be able to load
some otherwise-valid modules. The hope, though, is that no human-written
program is likely to ever reach this limit. This limit can be revised
and/or the locations with recursion revised if it's ever reached.

The implementation of this feature is done by generalizing the current
flattened-representation calculation which now keeps track of a type's
depth and size. The size calculation isn't used just yet but I plan to
use it in fixing #4816 and it was natural enough to write here as well.
The depth is checked after a type is translated and if it exceeds the
maximum then an error is returned.

Additionally the Arbitrary for Type implementation was updated to
prevent generation of a type that's too-recursive.

Closes #4814

<!--

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 (Aug 30 2022 at 21:53):

alexcrichton requested fitzgen for a review on PR #4825.

Wasmtime GitHub notifications bot (Aug 31 2022 at 15:18):

alexcrichton updated PR #4825 from limit-component-recursion to main.

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:46):

fitzgen created PR review comment:

Why not 99?

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:46):

fitzgen created PR review comment:

todo

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:46):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:46):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:51):

alexcrichton updated PR #4825 from limit-component-recursion to main.

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:51):

alexcrichton created PR review comment:

Oops this was actually intended for a separate PR so I'm going to remove this

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:51):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:53):

alexcrichton updated PR #4825 from limit-component-recursion to main.

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:53):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:53):

alexcrichton created PR review comment:

Originally I didn't really want to deal with off-by-one bugs with these sorts of arbitrary limits, but I suppose there's no harm in fixing them if they come up.

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:53):

alexcrichton has enabled auto merge for PR #4825.

Wasmtime GitHub notifications bot (Aug 31 2022 at 17:11):

alexcrichton updated PR #4825 from limit-component-recursion to main.

Wasmtime GitHub notifications bot (Aug 31 2022 at 18:29):

alexcrichton merged PR #4825.

Last updated: Dec 23 2024 at 12:05 UTC