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

Fix custom-page-sizes + pooling-allocator + CoW #9533

Conversation

alexcrichton
Copy link
Member

This commit fixes a runtime error that happened with the pooling allocator when combined with the custom-page-sizes proposal. The bug that happened was that sizes flowing into the pooling allocator were no longer host-page-size-aligned which caused syscalls to return an error unexpectedly. This meant that situations which were supposed to work were in fact not working.

The fix in this commit is to page-align incoming sizes into the CoW-pieces of memory management. This is coupled with a few more assertions that the accessible field is always page-aligned, as expected.

@alexcrichton alexcrichton requested a review from a team as a code owner October 31, 2024 20:21
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team October 31, 2024 20:21
@alexcrichton alexcrichton added the fuzz-bug Bugs found by a fuzzer label Oct 31, 2024
This commit fixes a runtime error that happened with the pooling
allocator when combined with the custom-page-sizes proposal. The bug
that happened was that sizes flowing into the pooling allocator were no
longer host-page-size-aligned which caused syscalls to return an error
unexpectedly. This meant that situations which were supposed to work
were in fact not working.

The fix in this commit is to page-align incoming sizes into the
CoW-pieces of memory management. This is coupled with a few more
assertions that the `accessible` field is always page-aligned, as
expected.
@alexcrichton alexcrichton force-pushed the fix-custom-page-sizes-and-pooling-and-cow branch from d2e540f to 173f5e9 Compare October 31, 2024 21:03
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Oct 31, 2024
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.

Thanks!

tests/all/pooling_allocator.rs Outdated Show resolved Hide resolved
@alexcrichton alexcrichton added this pull request to the merge queue Nov 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 1, 2024
@alexcrichton alexcrichton added this pull request to the merge queue Nov 1, 2024
Merged via the queue into bytecodealliance:main with commit 44da056 Nov 1, 2024
40 checks passed
@alexcrichton alexcrichton deleted the fix-custom-page-sizes-and-pooling-and-cow branch November 1, 2024 18:38
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Nov 3, 2024
This commit fixes a similar panic to one found in bytecodealliance#9533 where the
pooling allocator was combined with modules using custom page sizes. The
fix is similar where a variable needs page-aligning where previously it
wasn't necessary due to wasm sizes always being page-aligned.
github-merge-queue bot pushed a commit that referenced this pull request Nov 4, 2024
This commit fixes a similar panic to one found in #9533 where the
pooling allocator was combined with modules using custom page sizes. The
fix is similar where a variable needs page-aligning where previously it
wasn't necessary due to wasm sizes always being page-aligned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzz-bug Bugs found by a fuzzer wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants