Stream: git-wasmtime

Topic: wasmtime / PR #3013 Fix access to VMMemoryDefinition::cur...

Wasmtime GitHub notifications bot (Jun 22 2021 at 12:00):

uweigand opened PR #3013 from fix-currentlength to main:

The current_length member is defined as "usize" in Rust code,
but generated wasm code refers to it as if it were "u32". Not
sure why this is case, and it happens to work on little-endian
machines (as long as the length is < 4GB), but it will always
fail on big-endian machines.

Add the minimal fixes to make it work on big-endian as well.
(Note: this patch does not attempt to fix the actual underlying
type mismatch ...)

<!--

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 (Jun 22 2021 at 22:24):

uweigand updated PR #3013 from fix-currentlength to main.

Wasmtime GitHub notifications bot (Jun 23 2021 at 15:35):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 23 2021 at 15:35):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 23 2021 at 15:35):

alexcrichton created PR review comment:

Ideally we want to remove this restriction one day, so could this be listed as an extra check after this if with a FIXME pointing to https://github.com/bytecodealliance/wasmtime/issues/3022? That way when the issue is fixed we can simply delete the if without having to worry about spec correctness.

Wasmtime GitHub notifications bot (Jun 23 2021 at 15:35):

alexcrichton created PR review comment:

Like above, could the existing check be left as-is and this added as a new check with a FIXME which we can easily delete in the future?

Wasmtime GitHub notifications bot (Jun 23 2021 at 15:35):

alexcrichton created PR review comment:

Could this be tagged with a FIXME as well? I also think it's fine to change this to == since that's the only case we're specifically disallowing here, any other cases should be disallowed normally somewhere else.

Wasmtime GitHub notifications bot (Jun 23 2021 at 16:11):

uweigand updated PR #3013 from fix-currentlength to main.

Wasmtime GitHub notifications bot (Jun 23 2021 at 16:12):

uweigand submitted PR review.

Wasmtime GitHub notifications bot (Jun 23 2021 at 16:12):

uweigand created PR review comment:

Makes sense to me -- all checks now updated accordingly.

Wasmtime GitHub notifications bot (Jun 23 2021 at 16:16):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 23 2021 at 16:45):

alexcrichton merged PR #3013.

Last updated: Oct 23 2024 at 20:03 UTC