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

Add fn for splitting borrow of memory & data in Ctx, use in WASI #1069

Merged
merged 4 commits into from
Dec 17, 2019

Conversation

MarkMcCaskey
Copy link
Contributor

Fixes a soundness issue / some undefined behavior

Review

  • Add a short description of the the change to the CHANGELOG.md file

@MarkMcCaskey MarkMcCaskey added 📦 lib-deprecated About the deprecated crates 📦 lib-wasi About wasmer-wasi labels Dec 16, 2019
Copy link
Contributor

@nlewycky nlewycky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? It appears to be a convenience method?

@@ -401,6 +401,21 @@ impl Ctx {
}
}

/// Splits the borrow, getting both Memory and a mutable reference to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Splits the borrow" sounds like a function that takes a borrow and splits it. Instead, this is a variation on memory that returns a Memory and a mutable reference to the Ctx.data.

Also, I'd replace "a hostcall" with simply "the host".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's required by the Rust borrow checker; it's a split borrow in that one borrow comes in and 2 come out that otherwise is not possible with safe Rust. There's a function for splitting borrows over slices I believe &mut [u8] -> (&mut [u8], &mut [u8]) which is where I got the terminology from

lib/runtime-core/src/vm.rs Outdated Show resolved Hide resolved
Co-Authored-By: nlewycky <nick@wasmer.io>
@MarkMcCaskey
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Dec 17, 2019
1069: Add fn for splitting borrow of memory & data in Ctx, use in WASI r=MarkMcCaskey a=MarkMcCaskey

Fixes a soundness issue / some undefined behavior

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <mark@wasmer.io>
Co-authored-by: Mark McCaskey <5770194+markmccaskey@users.noreply.github.com>
@MarkMcCaskey
Copy link
Contributor Author

bors r-

@bors
Copy link
Contributor

bors bot commented Dec 17, 2019

Canceled

@MarkMcCaskey
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Dec 17, 2019
1069: Add fn for splitting borrow of memory & data in Ctx, use in WASI r=MarkMcCaskey a=MarkMcCaskey

Fixes a soundness issue / some undefined behavior

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <mark@wasmer.io>
Co-authored-by: Mark McCaskey <5770194+markmccaskey@users.noreply.github.com>
@bors
Copy link
Contributor

bors bot commented Dec 17, 2019

Build succeeded

@bors bors bot merged commit 3a2881e into master Dec 17, 2019
@bors bors bot deleted the feature/split-borrow-on-ctx branch December 17, 2019 01:35
@Hywan Hywan added the 🎉 enhancement New feature! label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-deprecated About the deprecated crates 📦 lib-wasi About wasmer-wasi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants