Skip to content

Check every public routine for all inputs with no_panic - #15

Merged
jadamcrain merged 3 commits into
mainfrom
no-panic-all-inputs
Oct 15, 2026
Merged

jadamcrain merged 3 commits into
mainfrom
no-panic-all-inputs

Conversation

@jadamcrain

Copy link
Copy Markdown
Member

Problem

The no_panic check from #14 could miss panics that depend on the input. The driver called each routine with particular values, and after inlining, the compiler specialized each routine to those values. To confirm it, I added if value < 0.0 { panic!() } to write_f32_le, and the check still passed, because the driver only passed non-negative floats.

The README on main also says the check "covers every input", which wasn't true.

Fix

  • tests/no_panic.rs now takes the address of every public routine (black_box::<fn(_) -> _>(ReadCursor::read_u8)) instead of calling it. That forces a standalone copy that has to handle any arguments, and that copy is what the linker checks. The negative-float panic is now detected. transaction and at_pos are checked with a callback that can't panic, since they run the caller's callback.
  • tests/no_panic_coverage.rs parses src/ with syn instead of matching text. Every public method must have the attribute and appear in the driver, matched by type. It's deliberately simple: it guards against honest mistakes in our own code, not adversarial input.
  • README and CHANGELOG now describe what's checked and what isn't: caller callbacks, derived trait impls, read_array sizes, and build configuration.

Verification

  • Ran three mutation sweeps over all 66 public methods. The list of methods came from rustdoc's JSON, so it's independent of the syn checker:
    • Injecting a panic into each method fails the link and names that method: 66/66.
    • Removing each method from the driver fails the coverage test: 66/66.
    • Removing each attribute fails the coverage test: 66/66.
  • Ran all CI steps locally: fmt, clippy, tests, and the no_panic check.

The no_panic driver called each routine with particular values. After
inlining, the compiler specialized each routine to those values, so a
panic that depends on the input could be optimized away and missed: a
panic in write_f32_le on negative values passed the check, because the
driver only passed non-negative floats.

Take the address of each routine instead. That forces a standalone copy
that has to handle any arguments, and that copy is what the linker
checks. The panic above is now detected.

Replace the text-matching coverage test with one that parses the source
with syn, so it is not affected by formatting or attribute order and
matches routines by type.
no_panic relies on a linker trick. If a toolchain change stopped it
from detecting panics, every check would pass silently. The canary is
a function that can panic; CI requires its link to fail with the
expected no_panic error.
The scan only read the top level of src/, so a public method in a
nested module directory (e.g. src/foo/mod.rs) escaped the check.
@jadamcrain
jadamcrain merged commit 7ace5b6 into main Oct 15, 2026
3 checks passed
@jadamcrain
jadamcrain deleted the no-panic-all-inputs branch October 15, 2026 19:43
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.

1 participant