Skip to content

Conversation

@alexcrichton
Copy link
Member

This commit removes preexisting unsoundness in Wasmtime where a
&mut StoreOpaque borrow was "widened" into encompassing the limiter on
the T in StoreInner<T>, for example, by using the self-pointer
located in an instance or the store. This fix is done by threading
&mut StoreOpaque as a parameter separately from a
StoreResourceLimiter. This means that various callers now take a new
Option<&mut StoreResourceLimiter<'_>> parameter in various locations.

Closes #11409

@alexcrichton alexcrichton requested a review from a team as a code owner August 20, 2025 21:47
@alexcrichton alexcrichton requested review from dicej and removed request for a team August 20, 2025 21:47
@alexcrichton
Copy link
Member Author

Currently built on #11480, which is in turn built on #11470, so the first three commits aren't part of this PR

);
store.on_fiber(|store| self.instantiate_impl(store)).await?
pub async fn instantiate_async(&self, store: impl AsContextMut<Data = T>) -> Result<Instance> {
self._instantiate(store).await
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we not need an assert!(store.0.async_support()) here anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

A good question! That's part of the previous commits and Nick was also curious here, with some rationale in my response over there

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.

Nice!

This commit removes preexisting unsoundness in Wasmtime where a
`&mut StoreOpaque` borrow was "widened" into encompassing the limiter on
the `T` in `StoreInner<T>`, for example, by using the self-pointer
located in an instance or the store. This fix is done by threading
`&mut StoreOpaque` as a parameter separately from a
`StoreResourceLimiter`. This means that various callers now take a new
`Option<&mut StoreResourceLimiter<'_>>` parameter in various locations.

Closes bytecodealliance#11409
@alexcrichton alexcrichton force-pushed the remove-unsound-widening-borrows branch from 20ee87b to 1288134 Compare August 21, 2025 01:11
@alexcrichton alexcrichton enabled auto-merge August 21, 2025 01:11
@alexcrichton alexcrichton added this pull request to the merge queue Aug 21, 2025
Merged via the queue into bytecodealliance:main with commit 155ea7f Aug 21, 2025
44 checks passed
@alexcrichton alexcrichton deleted the remove-unsound-widening-borrows branch August 21, 2025 01:45
bongjunj pushed a commit to prosyslab/wasmtime that referenced this pull request Oct 20, 2025
* Remove unsoundness of widening store borrows

This commit removes preexisting unsoundness in Wasmtime where a
`&mut StoreOpaque` borrow was "widened" into encompassing the limiter on
the `T` in `StoreInner<T>`, for example, by using the self-pointer
located in an instance or the store. This fix is done by threading
`&mut StoreOpaque` as a parameter separately from a
`StoreResourceLimiter`. This means that various callers now take a new
`Option<&mut StoreResourceLimiter<'_>>` parameter in various locations.

Closes bytecodealliance#11409

* Fix gc-less build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implementation of GC is not sound with resource limiters

3 participants