SIGN IN SIGN UP
oven-sh / bun UNCLAIMED

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one

0 0 150 Rust

collections: fix LinearFifo::ordered_remove_item wrapped-buffer bounds (#31565)

Fixes #31563.

## Problem

`LinearFifo::ordered_remove_item` (`src/collections/linear_fifo.rs`)
uses the
wrong slice bounds when the readable region wraps around the end of the
backing
buffer. When the data wraps, the readable region splits into two
segments:

- tail:           `buf[head..buf_len)`
- wrapped prefix: `buf[0..wrap_len)`

where the correct prefix length is `wrap_len = head + count - buf_len`.

The wrapped branch instead used:

- `count - head` for the `index < head` sub-branch (remove from wrapped
prefix)
- `head - count`  for the `index >= head` sub-branch (remove from tail)

Neither equals `wrap_len`. In wrapped layouts these underflow `usize`
and panic
with an out-of-range slice index; in the narrow cases where the wrong
bound
happens to stay in range, the shift moves the wrong number of elements
and
silently corrupts FIFO contents.

This is a faithful port of a latent bug in the Zig reference
(`orderedRemoveItem`), which uses the same `self.count - self.head` /
`self.head - self.count` bounds. It was never correct, so it is not a
regression from a prior Bun release.

### Reachability

The one in-tree caller is `weak_refs.ordered_remove_item(i)` in
`src/runtime/bake/dev_server/source_map_store.rs` (`weak_refs` is a
`LinearFifo<_, StaticBuffer<_, 16>>`). The `read_item`/`write_item`
churn in
`add_weak_ref` advances `head` and wraps the tail, so a wrapped layout
with
`head != 0` is reachable during normal dev-server operation.
(github-actions
linked #23617 — a crash in `SourceMapStore.removeOrUpgradeWeakRef`, the
only
caller of this method.)

## Fix

Compute `wrap_len = head + count - buf_len` once and use it in both
wrapped
sub-branches:

```rust
let wrap_len = head + count - buf_len;
if index < head {
    shift_down_one(&mut buf[index..wrap_len]);
} else {
    let wrap = unsafe { ptr::read(buf.as_ptr()) };
    shift_down_one(&mut buf[index..]);
    unsafe { ptr::write(buf.as_mut_ptr().add(buf_len - 1), wrap) };
    shift_down_one(&mut buf[..wrap_len]);
}
```

Since `count <= buf_len` always holds, `wrap_len <= head`, so the prefix
`buf[..wrap_len]` stays entirely inside the wrapped prefix and never
overlaps
the tail `buf[head..]`. The wrapped branch is only entered when
`buf_len - head < count`, i.e. `head + count > buf_len`, so `wrap_len >=
1` and
the subtraction cannot underflow.

## Tests

Two layers:

1. **Rust unit tests** in the crate's inline `#[cfg(test)] mod tests`
(`src/collections/linear_fifo.rs`) reconstruct the exact wrapped states
from
the issue via the public API on a `LinearFifo<i32, StaticBuffer<i32,
16>>`
   (cap 16 matches the real dev-server `weak_refs` FIFO):
   - tail sub-branch (`index >= head`, `head < count`),
   - wrapped-prefix sub-branch (`index < head`, `head > count`),
   - an exhaustive per-offset comparison against a reference `Vec`.

These run in CI on the Miri lane (`.github/workflows/miri.yml` → `cargo
miri
test -p bun_collections`, triggered on `src/collections/**`), which also
verifies the `unsafe` `ptr::read`/`ptr::write`/memmove path is UB-free.

2. **`test/internal/linear-fifo.test.ts`** — a deterministic JS
regression test.
`LinearFifo` has no JS-visible surface and its only caller uses CSPRNG
keys +
an async timer, so the wrapped branch can't be reached deterministically
from
a normal test. A small pure-Rust probe (`linearFifoOrderedRemoveProbe`,
   `src/runtime/linear_fifo_testing.rs`) is exposed through
   `bun:internal-for-testing`; it rebuilds the two wrapped states, calls
`ordered_remove_item`, and returns the resulting FIFO contents for the
test
to assert FIFO order is preserved. The probe lives in `bun_runtime`
(which
   depends on both `bun_collections` and `bun_jsc`) and is wired via the
existing `$newZigFunction` js2native path — the `.zig` filename is only
the
   codegen key; the body is Rust.

### Verification

```console
# Rust unit tests — fail without the fix (matches the issue), pass with it:
$ cargo test -p bun_collections linear_fifo
  ...attempt to subtract with overflow   # count-head / head-count
  test result: FAILED. 3 passed; 3 failed   # (buggy bounds)
  test result: ok. 6 passed; 0 failed       # (fixed)

# full crate under Miri (Tree Borrows) — no UB:
$ MIRIFLAGS=-Zmiri-tree-borrows cargo miri test -p bun_collections
  test result: ok. 36 passed; 0 failed

# JS test, debug build — passes with the fix; with the buggy bounds the
# probe's ordered_remove_item call panics (usize underflow):
$ bun bd test test/internal/linear-fifo.test.ts
  (pass) ...wrapped tail sub-branch (head < count)
  (pass) ...wrapped prefix sub-branch (head > count)
```
R
robobun committed
16f34ba649db9d514a969defdc323686737b026d
Parent: 9d953cc
Committed by GitHub <noreply@github.com> on 5/29/2026, 5:55:28 PM