wasmtime / PR #12381 Add fallible allocation methods to `... · git-wasmtime

As we go down this route I think that ideally we'd keep all the unsafe fiddly bits in one place, e.g. the wasmtime-environ::collections module. In that sense I'd prefer if we didn't have to dip into unsafe for cases like this. To that extent WDYT of one of:

Switching this to using a Vec which means no unsafe needed

Adding fallible-slice-allocation-primitives to wasmtime_environ::collections

Build a Vec here, fallibly, and then convert it to Box<[T]>

Wasmtime GitHub notifications bot (Jan 21 2026 at 21:44):

alexcrichton created PR review comment:

"how low... can you go..."

Wasmtime GitHub notifications bot (Jan 21 2026 at 21:57):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 21 2026 at 21:57):

fitzgen created PR review comment:

Adding fallible-slice-allocation-primitives to wasmtime_environ::collections

This one won't work in this case because of crate dependencies.

Build a Vec here, fallibly, and then convert it to Box<[T]>

This one won't work (at least without some extra care) because Vec<T>-to-Box<[T]> will resize to the exact length so as not to leak the extra capacity, which requires (re)allocation.

So I guess that leaves us with switching to a Vec of these options.

But also, while I agree we don't want unsafe fiddly bits spread all over the place willy nilly, I don't think that having unsafe here is really violating that principle in spirit because it is limited to just this collection's internals and is well encapsulated. That is, although this cannot move to wasmtime_environ::collections due to crate deps, it feels to me like it sort of morally is the same thing that wasmtime_environ::collections is.

How strongly do you feel in this particular case? (with the acknowledgement that we want to coalesce and encapsulate all the unsafe OOM-handling collections stuff as much as possible)

The _other_ alternative would be to add a new crate in our workspace to hold the unsafe allocation bits for collections and move existing utilities like try_alloc into that new crate. This way we would be able to put all the little unsafe helpers in one place, but it kind of feels like a lot of work for not that much gain...

Wasmtime GitHub notifications bot (Jan 21 2026 at 22:03):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 21 2026 at 22:03):

alexcrichton created PR review comment:

What I'm also worried about is that if any of this code panics:
        for (i, bitset) in self
            .elems
            .iter()
            .copied()
            .chain(iter::repeat(ScalarBitSet::new()).take(to_grow))
            .enumerate()
        {
            // Safety: the pointer points to a block that is valid for an array
            // of length `new_len` and indexing by `i` is within that block.
            debug_assert!(i < new_len);
            unsafe {
                ptr.add(i).write(bitset);
            }
        }
then the memory is leaked (no dtor for anything in that range).

We could move wasmtime_environ::collections into its own crate to solve the crate dep problem. I'm sort of a fan of consolidating wasmtime-{internal-error,internal-math} and collections into one crate (e.g. wasmtime-internal-core) where the main purpose of the crate is "only no_std dependencies, also very limited deps"

I don't think that having unsafe here is really violating that principle in spirit because it is limited to just this collection's internals and is well encapsulated

I agree with this, but I think we should strive even further than this and encapsulating more. For example I don't think that cranelift-bitset's tests are run through Miri (although they almost certainly could be), but all of wasmtime_environ's tests are already run through Miri. I think there's basically a lot of benefit to absolutely minimizing unsafe code and what exactly the contract is doing. I understand it's simpler to just do a bunch of one-off small-ish edits, but this is sort of blazing the trail for custom collections in Wasmtime and I'd prefer idioms that favor safe/small functions with limited unsafe inside and exposing as much as we can via type signatures.

Wasmtime GitHub notifications bot (Jan 21 2026 at 23:48):

github-actions[bot] added the label cranelift on PR #12381.

Wasmtime GitHub notifications bot (Jan 21 2026 at 23:48):

github-actions[bot] added the label fuzzing on PR #12381.

Wasmtime GitHub notifications bot (Jan 21 2026 at 23:48):

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

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "fuzzing"

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

fitzgen: fuzzing

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 (Jan 22 2026 at 19:46):

fitzgen requested wasmtime-core-reviewers for a review on PR #12381.

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:46):

fitzgen updated PR #12381.

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:46):

fitzgen requested pchickey for a review on PR #12381.

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:47):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 19:47):

fitzgen created PR review comment:

Okay in the most recently pushed commits I created the wasmtime-internal-core crate and moved the low-level allocation helpers into there and made CompoundBitSet::try_ensure_capacity fully safe.

Wasmtime GitHub notifications bot (Jan 22 2026 at 20:13):

fitzgen requested alexcrichton for a review on PR #12381.

Wasmtime GitHub notifications bot (Jan 22 2026 at 21:59):

alexcrichton created PR review comment:

Personally I'd say that wasmtime-{math,error} could get folded in here too (probably in a future PR), but wanted to confirm/deny your thoughts on that?

Wasmtime GitHub notifications bot (Jan 22 2026 at 21:59):

alexcrichton created PR review comment:

I think boxed.assume_init() will work here?

Wasmtime GitHub notifications bot (Jan 22 2026 at 21:59):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:07):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:07):

fitzgen created PR review comment:

I'm half of the mind that there is no need to eagerly merge crates if we don't have any reason to do so, but I don't feel strongly at all. Anyways, I don't think there is anything blocking that, but I don't have any plans on doing it myself.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:08):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:08):

fitzgen created PR review comment:

Thanks, I was searching all over because I was sure that this existed, but couldn't find it. rustdoc is not great for surfacing methods on like Box<[T]> instead of Box<T>.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:11):

fitzgen updated PR #12381.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:11):

fitzgen has enabled auto merge for PR #12381.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:17):

alexcrichton commented on PR #12381:

Oh also, along the lines of crate consolidation, can you add this crate to the Miri-tested crates in CI? I'll do a follow-up to merge our core utilities into this crate

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:24):

fitzgen added PR #12381 Add fallible allocation methods to cranelift_bitset::CompoundBitSet to the merge queue

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:47):

fitzgen merged PR #12381.

Wasmtime GitHub notifications bot (Jan 22 2026 at 23:47):

fitzgen removed PR #12381 Add fallible allocation methods to cranelift_bitset::CompoundBitSet from the merge queue

Wasmtime GitHub notifications bot (Jan 23 2026 at 02:21):

alexcrichton commented on PR #12381:

can you add this crate to the Miri-tested crates in CI

I'm doing this in https://github.com/bytecodealliance/wasmtime/pull/12398

Last updated: Jan 29 2026 at 13:25 UTC