Capstone: audit and repair an unsound abstraction

You are handed a working slab allocator. It compiles, it is clippy-clean apart from one lint, and a reasonable test suite passes. It contains five planted defects, four of which are soundness bugs. Find them, fix them without changing the public API’s behaviour, and leave a correct // SAFETY: comment on every remaining unsafe block.

pub fn audit_script(ops: Vec<String>) -> Vec<String>

Almost nobody writes a new unsafe abstraction from scratch. Everybody reviews one. Auditing is the job, and it is also the only exercise where you have to choose which concept applies to unfamiliar code rather than being told which chapter you are in.

The abstraction

mod arena is a slab: values live in reusable slots addressed by a Handle. Each slot carries a generation counter, so a handle to a freed value can be detected and rejected rather than silently resolving to whatever moved in afterwards. There is a fixed capacity, an unsafe fn fast path, and a Drain guard.

Handle‘s fields are pub, so any script can forge one. That must be harmless — a slab whose safety depends on callers only using handles it issued is not a safe abstraction, it is a convention. Validation happens on entry, every time.

audit_script is given and correct. Read it to learn the vocabulary: alloc <text>, get <i> <g>, raw <i> <g>, free <i> <g>, live, drain, forget. Capacity is 3.

What “unsound” means here

A safe function is sound if there is no input at all — including hostile, absurd or forged input — for which it can cause undefined behaviour. Not “no input a sensible caller would pass”. No input.

So the audit question for every safe pub fn in that module is: what is the worst thing a caller can do with this? And for every unsafe block: what invariant makes this operation defined, and is that invariant actually guaranteed at this point in the code?

The species you are looking for

Not a list of the bugs — a list of the kinds, which is what you would have in a real review:

1. An off-by-one in a bounds check that feeds an unchecked access. <= where < was meant. The // SAFETY: comment above it says “bounds checked above”, and it is a lie by exactly one element.
2. A stale handle that resolves. The generation mechanism only works if every path that vacates a slot bumps the counter. Miss one and the handle-validity guarantee is gone, silently.
3. State updated before a fallible operation. A counter incremented at the top of a function that can still return early leaves the object describing a world that does not exist.
4. A guard whose soundness depends on its destructor running. Destructors are not guaranteed to run — mem::forget is a safe function, and so is leaking through an Rc cycle. An abstraction that is only sound if you are polite is not sound.
5. A pub unsafe fn with no # Safety section. This one clippy will tell you about; it is here so you can see the difference between the bugs a tool finds and the bugs it cannot.

::: question Why is defect 4 — the drain guard — the same bug as the one that once made Vec::drain unsound? Because both relied on a destructor to restore an invariant, and destructors are optional.

The historical bug is called leak amplification. Vec::drain used to set the vector’s length correctly only in Drop. During the drain, the vector’s len still covered elements that had been moved out. Call mem::forget on the guard — a safe function — and you were left with a Vec whose len claimed ownership of elements that had already been given away. Reading them was a use-after-free reachable from entirely safe code.

The fix, which is what std does today and what you should do here, is leak amplification: put the container into its final valid state up front, before the guard is handed out, and make the guard’s Drop a convenience rather than a requirement. Forgetting the guard then leaks the values — which is safe, if wasteful — instead of corrupting the container.

This is the general rule and it is worth memorising: never let a safe API’s soundness depend on a destructor. Rust makes leaking safe on purpose (see Box::leak, mem::forget, ManuallyDrop, Rc cycles), so any design that assumes cleanup will happen is unsound by construction. :::

How you are graded

The cases come in two banks.

The behaviour bank (b1–b3) is the naive test suite. It exercises the ordinary happy paths and it passes on the broken code exactly as it passes on the fixed code. That is the point: it is what a well-meaning colleague would have written, and it is why the bugs are still there.

The soundness bank (s1–s5) is the adversarial one. Each script is precisely the sequence that makes one planted bug observable: free-then-alloc-then-get; fill past capacity and count; drain-then-forget; index exactly at the length; and touch an empty arena.

Be honest about what this proves. There is no Miri here. Defect 1 is a genuine out-of-bounds read, and undefined behaviour is not obliged to misbehave: on another machine, or with a different optimiser, the broken build might have printed the right answer and passed. The test catching it is luck reinforced by design, not a verification. When you have finished, write down which of the five defects the harness could not reliably have caught — that list is the actual output of this exercise.

Rules

• Do not change audit_script, and do not change what the public API does on any input the behaviour bank exercises.
• Do not add #[allow(...)] anywhere. Silencing missing_safety_doc rather than writing the section is exactly the move this item exists to train you out of. Nothing in this harness can detect it; a reviewer can, and will.
• Every remaining unsafe block needs a // SAFETY: comment that is true after your fix. Two of the ones in the file are currently false.

  Loading visualization…