Skip to content

fix(cranelift): use saturating_sub in PassTimes::total to prevent panic#12694

Closed
A386official wants to merge 2 commits intobytecodealliance:mainfrom
A386official:fix/timing-duration-overflow
Closed

fix(cranelift): use saturating_sub in PassTimes::total to prevent panic#12694
A386official wants to merge 2 commits intobytecodealliance:mainfrom
A386official:fix/timing-duration-overflow

Conversation

@A386official
Copy link

Bug

PassTimes::total() panics with "overflow when subtracting durations" when compiling certain WASM modules. This was reported by Zed via telemetry on Windows x86 with Cranelift 33.0.2.

Stack trace:

core::time::impl$3::sub (time.rs:1164)
cranelift_codegen::timing::enabled::impl$0::total::closure$0 (timing.rs:174)
...
cranelift_codegen::timing::enabled::PassTimes::total (timing.rs:174)
wasmtime_cranelift::compiler::impl$3::compile_function (compiler.rs:284)

Root Cause

PassTimes::total() computes self-time per pass as p.total - p.child using the - operator on Duration, which panics on underflow. When timing tokens are dropped out of order (which is guarded by a debug_assert that is elided in release builds), the accumulated child duration can exceed total, triggering the panic.

The Display implementation for PassTimes already handles this edge case correctly by using checked_sub:

if let Some(s) = time.total.checked_sub(time.child) {
    fmtdur(s, f)?;
}

But total() was not given the same treatment.

Fix

Replace p.total - p.child with p.total.saturating_sub(p.child) in PassTimes::total(). This is consistent with how the Display impl handles the same subtraction and prevents the panic in production builds where the ordering assertion is not checked.

Closes #12692

PassTimes::total() subtracts child duration from total duration using
the `-` operator, which panics on underflow. When timing tokens are
dropped out of order (possible in release builds where the debug_assert
in Drop is elided), child can exceed total, causing:

  "overflow when subtracting durations"

The Display impl already handles this correctly using checked_sub.
Use saturating_sub in total() for consistency and to prevent the panic
in production.

Fixes #12692
@A386official A386official requested a review from a team as a code owner February 28, 2026 09:36
@A386official A386official requested review from alexcrichton and removed request for a team February 28, 2026 09:36
Break method chain across multiple lines to satisfy rustfmt's
line length requirements.

Signed-off-by: A386official <a386official@users.noreply.github.com>
@A386official
Copy link
Author

Closing in favor of #12693 which addresses the same issue.

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.

Cranelift: panic calculating timing total

1 participant