Stream: git-wasmtime

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


view this post on Zulip 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.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 22:24):

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

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2021 at 15:35):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2021 at 15:35):

alexcrichton submitted PR review.

view this post on Zulip 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.

view this post on Zulip 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?

view this post on Zulip 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.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2021 at 16:11):

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

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2021 at 16:12):

uweigand submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2021 at 16:12):

uweigand created PR review comment:

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

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2021 at 16:16):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2021 at 16:45):

alexcrichton merged PR #3013.


Last updated: Dec 23 2024 at 12:05 UTC