Stream: git-wasmtime

Topic: wasmtime / PR #8363 c-api: Fix alignment of `wasmtime_val_*`

Wasmtime GitHub notifications bot (Apr 14 2024 at 01:19):

alexcrichton opened PR #8363 from alexcrichton:fix-val-alignment-in-c-api to bytecodealliance:main:

This commit fixes an issue where wasmtime_val_raw_t had an incorrect alignment. In Rust ValRaw contains a u128 which has an alignment of 16 but in C the representation had a smaller alignment meaning that the alignment of the two structures was different. This was seen to cause alignment faults when structure were passed from C++ to Rust, for example.

This then additionally changes the representation of wasmtime_val_union in Rust to use a u128 instead of [u8; 16] to ensure that the same 16-byte-aligned wasmtime_v128 type in C can represent both payloads in wasmtime_val_union and wasmtime_val_raw_t.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Apr 14 2024 at 01:19):

alexcrichton requested fitzgen for a review on PR #8363.

Wasmtime GitHub notifications bot (Apr 14 2024 at 01:19):

alexcrichton requested wasmtime-core-reviewers for a review on PR #8363.

Wasmtime GitHub notifications bot (Apr 14 2024 at 02:22):

github-actions[bot] commented on PR #8363:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:c-api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Apr 14 2024 at 19:10):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2024 at 19:46):

alexcrichton updated PR #8363.

Wasmtime GitHub notifications bot (Apr 14 2024 at 19:47):

alexcrichton has enabled auto merge for PR #8363.

Wasmtime GitHub notifications bot (Apr 14 2024 at 19:50):

alexcrichton updated PR #8363.

Wasmtime GitHub notifications bot (Apr 14 2024 at 19:57):

alexcrichton updated PR #8363.

Wasmtime GitHub notifications bot (Apr 14 2024 at 20:15):

alexcrichton updated PR #8363.

Wasmtime GitHub notifications bot (Apr 14 2024 at 20:15):

alexcrichton edited PR #8363:

This commit fixes an issue where wasmtime_val_raw_t had an incorrect
alignment. In Rust ValRaw contains a u128 which has an alignment of
16 but in C the representation had a smaller alignment meaning that the
alignment of the two structures was different. This was seen to cause
alignment faults when structure were passed from C++ to Rust, for
example.

This commit changes the Rust representation of ValRaw's v128 field
to do the same as C which is to use [u8; 16]. This avoids the need to
raise the alignment in C which appears to be nontrivial. Cranelift is
appropriately adjusted to understand that loads/stores from ValRaw are
no longer aligned. Technically this only applies to the v128 field but
it's not too bad to apply it to the other fields as well.

Wasmtime GitHub notifications bot (Apr 14 2024 at 20:15):

alexcrichton commented on PR #8363:

I ended up going the reverse direction here instead, lowering the alignment in Rust instead of raising the alignment in the C API. I don't know how to portably raise the alignment in C so I'd rather tweak the Rust side of things.

Wasmtime GitHub notifications bot (Apr 14 2024 at 20:56):

alexcrichton updated PR #8363.

Wasmtime GitHub notifications bot (Apr 14 2024 at 22:03):

alexcrichton merged PR #8363.

Last updated: Nov 22 2024 at 17:03 UTC