Skip to content

checked_div can attempt to negate with overflow #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
evnu opened this issue Mar 17, 2020 · 4 comments
Closed

checked_div can attempt to negate with overflow #73

evnu opened this issue Mar 17, 2020 · 4 comments
Labels

Comments

@evnu
Copy link

evnu commented Mar 17, 2020

use num_rational::Ratio;
use num_traits::ops::checked::CheckedDiv;

fn main() {
    let x = Ratio::from(-9223372036854775808_i64);
    x.checked_div(&x);
}
$ ./target/debug/repro
thread 'main' panicked at 'attempt to negate with overflow', /home/mo/.cargo/registry/src/github.com-1ecc6299db9ec823/num-traits-0.2.11/src/sign.rs:49:41
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
$ RUST_BACKTRACE=1 ./target/debug/repro
...snip...
  13: core::panicking::panic
             at src/libcore/panicking.rs:52
  14: <i64 as num_traits::sign::Signed>::abs
             at /home/mo/.cargo/registry/src/github.com-1ecc6299db9ec823/num-traits-0.2.11/src/sign.rs:49
  15: <i64 as num_integer::Integer>::gcd
             at /home/mo/.cargo/registry/src/github.com-1ecc6299db9ec823/num-integer-0.1.42/src/lib.rs:478
  16: <num_rational::Ratio<T> as num_traits::ops::checked::CheckedDiv>::checked_div
             at /home/mo/.cargo/registry/src/github.com-1ecc6299db9ec823/num-rational-0.2.3/src/lib.rs:875
  17: repro::main
             at src/repro.rs:6
...snip...

This might be related to #72.

I found this with afl.rs. Setup:

use std::io::prelude::*;
use std::fs::File;

fn main() {
    let files: Vec<_> = std::env::args().skip(1).collect();
    for file in &files {
        let mut f = File::open(file).unwrap();
        let mut buf = [0; 8];
        f.read_exact(&mut buf).unwrap();

        println!("{}: {}", file, i64::from_le_bytes(buf));
    }
}
@cuviper cuviper added the bug label Mar 17, 2020
@cuviper
Copy link
Member

cuviper commented Mar 17, 2020

Thanks - I do expect this also changed with #42, like issue #72, but the exact cause and fix will be distinct.

@cuviper
Copy link
Member

cuviper commented Mar 17, 2020

So the issue here is that GCD wants to return a positive value, but fails in unchecked abs(). Here's another related case: Rational::one().checked_div(&isize::min_value().into()) will panic in reduce() when we try to normalize the denominator to positive. I would generally accept such panics, but checked operations are supposed to do better.

bors bot added a commit that referenced this issue Mar 17, 2020
76: Fix CheckedDiv with minimum values r=cuviper a=cuviper

Fixes #73.

Co-authored-by: Josh Stone <cuviper@gmail.com>
@cuviper
Copy link
Member

cuviper commented Mar 17, 2020

This should be fixed in 0.2.4.

@cuviper cuviper closed this as completed Mar 17, 2020
@evnu
Copy link
Author

evnu commented Mar 18, 2020

Awesome, thank you for the fast response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants