Integer overflow in rayon-core 1.8 causes process abort on 32 bit CPUs

[ebarnard]

I have some software that aborts when compiled as a 32-bit executable but runs fine as a 64-bit executable. It seems that Counters::plus() is causing word to overflow leading to Rayon aborting the process.

The abort is in a deadlock detection test that tries to trigger a race condition that affected the software's original deadlock detection, so it ends up calling rayon::join quite a lot in a short space of time (but not anywhere near 2^32 times). It occurs in debug and release mode (with overflow checks enabled).

Reverting to rayon 1.3.1 and rayon-core 1.7.1 fixes the issue.

I'm working on a minimal example as I can't share the original software.

Backtrace

thread '<unnamed>' panicked at 'attempt to add with overflow', /home/pi/.cargo/registry/src/github.com-1285ae84e5963aae/rayon-core-1.8.0/src/sleep/counters.rs:226:19
thread '<unnamed>' panicked at 'attempt to add with overflow', /home/pi/.cargo/registry/src/github.com-1285ae84e5963aae/rayon-core-1.8.0/src/sleep/counters.rs:226:19
thread '<unnamed>' panicked at 'attempt to add with overflow', /home/pi/.cargo/registry/src/github.com-1285ae84e5963aae/rayon-core-1.8.0/src/sleep/counters.rs:226:19
thread '<unnamed>' panicked at 'attempt to add with overflow', /home/pi/.cargo/registry/src/github.com-1285ae84e5963aae/rayon-core-1.8.0/src/sleep/counters.rs:226:19
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1076
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1537
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:217
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:526
  11: rust_begin_unwind
             at src/libstd/panicking.rs:437
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
  13: core::panicking::panic
             at src/libcore/panicking.rs:50
  14: rayon_core::sleep::Sleep::announce_sleepy
  15: rayon_core::registry::WorkerThread::wait_until_cold
  16: rayon_core::registry::ThreadBuilder::run
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Rayon: detected unexpected panic; aborting

[cuviper]

Hmm, I thought the JEC was supposed to use wrapping add -- @nikomatsakis do you know why that's a bare addition?

[cuviper]

Prior to a0efb4a it was using fetch_add, which implicitly wraps.

[ebarnard]

A minimal reproduction of this bug on 32-bit platforms is:

fn main() {
    let mut i = 0;
    let mut j = 0;

    for _ in 0..u32::MAX {
        rayon::join(|| i += 1, || j += 1);
    }

    println!("{} {}", i, j);
}

[cuviper]

Thanks - I can reliably reproduce the overflow panic with 100,000 iterations, and it works when changing Counters::plus to use wrapping_add. With u32::MAX iterations, it hasn't completed yet, but at least it hasn't crashed yet either... 😅

[nikomatsakis]

@cuviper I believe it was meant to be a wrapping add indeed, I think I just wasn't thinking. I guess I will review the code to double check my understanding.

[nikomatsakis]

From reading the code, and the README.md in particular, I definitely think it was meant to rollover.