Skip to content

Commit

Permalink
Auto merge of #31410 - rkruppe:issue31109, r=pnkfelix
Browse files Browse the repository at this point in the history
Issue #31109 uncovered two semi-related problems:

* A panic in `str::parse::<f64>`
* A panic in `rustc::middle::const_eval::lit_to_const` where the result of float parsing was unwrapped.

This series of commits fixes both issues and also drive-by-fixes some things I noticed while tracking down the parsing panic.
  • Loading branch information
bors committed Feb 6, 2016
2 parents 35635ae + a76cb45 commit 695c907
Show file tree
Hide file tree
Showing 17 changed files with 119 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/etc/test-float-parse/_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::mem::transmute;
#[allow(dead_code)]
pub const SEED: [u32; 3] = [0x243f_6a88, 0x85a3_08d3, 0x1319_8a2e];

pub fn validate(text: String) {
pub fn validate(text: &str) {
let mut out = io::stdout();
let x: f64 = text.parse().unwrap();
let f64_bytes: u64 = unsafe { transmute(x) };
Expand Down
2 changes: 1 addition & 1 deletion src/etc/test-float-parse/few-ones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn main() {
for a in &pow {
for b in &pow {
for c in &pow {
validate((a | b | c).to_string());
validate(&(a | b | c).to_string());
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/etc/test-float-parse/huge-pow10.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use _common::validate;
fn main() {
for e in 300..310 {
for i in 0..100000 {
validate(format!("{}e{}", i, e));
validate(&format!("{}e{}", i, e));
}
}
}
27 changes: 27 additions & 0 deletions src/etc/test-float-parse/long-fractions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

mod _common;

use std::char;
use _common::validate;

fn main() {
for n in 0..10 {
let digit = char::from_digit(n, 10).unwrap();
let mut s = "0.".to_string();
for _ in 0..400 {
s.push(digit);
if s.parse::<f64>().is_ok() {
validate(&s);
}
}
}
}
4 changes: 2 additions & 2 deletions src/etc/test-float-parse/many-digits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ fn main() {
let mut rnd = IsaacRng::from_seed(&SEED);
let mut range = Range::new(0, 10);
for _ in 0..5_000_000u64 {
let num_digits = rnd.gen_range(100, 300);
let num_digits = rnd.gen_range(100, 400);
let digits = gen_digits(num_digits, &mut range, &mut rnd);
validate(digits);
validate(&digits);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/etc/test-float-parse/rand-f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn main() {
let bits = rnd.next_u64();
let x: f64 = unsafe { transmute(bits) };
if x.is_finite() {
validate(format!("{:e}", x));
validate(&format!("{:e}", x));
i += 1;
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/etc/test-float-parse/runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@
The actual tests (generating decimal strings and feeding them to dec2flt) is
performed by a set of stand-along rust programs. This script compiles, runs,
and supervises them. In particular, the programs report the strings they
generate and the floating point numbers they converted those strings to.
and supervises them. The programs report the strings they generate and the
floating point numbers they converted those strings to, and this script
checks that the results are correct.
You can run specific tests rather than all of them by giving their names
(without .rs extension) as command line parameters.
Expand Down Expand Up @@ -64,9 +65,9 @@
exit code that's not 0, the test fails.
The output on stdout is treated as (f64, f32, decimal) record, encoded thusly:
- The first eight bytes are a binary64 (native endianness).
- The following four bytes are a binary32 (native endianness).
- Then the corresponding string input follows, in ASCII (no newline).
- First, the bits of the f64 encoded as an ASCII hex string.
- Second, the bits of the f32 encoded as an ASCII hex string.
- Then the corresponding string input, in ASCII
- The record is terminated with a newline.
Incomplete records are an error. Not-a-Number bit patterns are invalid too.
Expand Down
4 changes: 2 additions & 2 deletions src/etc/test-float-parse/short-decimals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ fn main() {
if i % 10 == 0 {
continue;
}
validate(format!("{}e{}", i, e));
validate(format!("{}e-{}", i, e));
validate(&format!("{}e{}", i, e));
validate(&format!("{}e-{}", i, e));
}
}
}
4 changes: 2 additions & 2 deletions src/etc/test-float-parse/subnorm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use _common::validate;
fn main() {
for bits in 0u32..(1 << 21) {
let single: f32 = unsafe { transmute(bits) };
validate(format!("{:e}", single));
validate(&format!("{:e}", single));
let double: f64 = unsafe { transmute(bits as u64) };
validate(format!("{:e}", double));
validate(&format!("{:e}", double));
}
}
2 changes: 1 addition & 1 deletion src/etc/test-float-parse/tiny-pow10.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use _common::validate;
fn main() {
for e in 301..327 {
for i in 0..100000 {
validate(format!("{}e-{}", i, e));
validate(&format!("{}e-{}", i, e));
}
}
}
2 changes: 1 addition & 1 deletion src/etc/test-float-parse/u32-small.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ use _common::validate;

fn main() {
for i in 0..(1 << 19) {
validate(i.to_string());
validate(&i.to_string());
}
}
8 changes: 4 additions & 4 deletions src/etc/test-float-parse/u64-pow2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ use std::u64;
fn main() {
for exp in 19..64 {
let power: u64 = 1 << exp;
validate(power.to_string());
validate(&power.to_string());
for offset in 1..123 {
validate((power + offset).to_string());
validate((power - offset).to_string());
validate(&(power + offset).to_string());
validate(&(power - offset).to_string());
}
}
for offset in 0..123 {
validate((u64::MAX - offset).to_string());
validate(&(u64::MAX - offset).to_string());
}
}
4 changes: 2 additions & 2 deletions src/libcore/num/dec2flt/algorithm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ fn algorithm_r<T: RawFloat>(f: &Big, e: i16, z0: T) -> T {
// This is written a bit awkwardly because our bignums don't support
// negative numbers, so we use the absolute value + sign information.
// The multiplication with m_digits can't overflow. If `x` or `y` are large enough that
// we need to worry about overflow, then they are also large enough that`make_ratio` has
// we need to worry about overflow, then they are also large enough that `make_ratio` has
// reduced the fraction by a factor of 2^64 or more.
let (d2, d_negative) = if x >= y {
// Don't need x any more, save a clone().
Expand Down Expand Up @@ -278,7 +278,7 @@ fn quick_start<T: RawFloat>(u: &mut Big, v: &mut Big, k: &mut i16) {
// The target ratio is one where u/v is in an in-range significand. Thus our termination
// condition is log2(u / v) being the significand bits, plus/minus one.
// FIXME Looking at the second bit could improve the estimate and avoid some more divisions.
let target_ratio = f64::sig_bits() as i16;
let target_ratio = T::sig_bits() as i16;
let log2_u = u.bit_length() as i16;
let log2_v = v.bit_length() as i16;
let mut u_shift: i16 = 0;
Expand Down
36 changes: 28 additions & 8 deletions src/libcore/num/dec2flt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,18 +230,15 @@ fn convert<T: RawFloat>(mut decimal: Decimal) -> Result<T, ParseFloatError> {
if let Some(x) = trivial_cases(&decimal) {
return Ok(x);
}
// AlgorithmM and AlgorithmR both compute approximately `f * 10^e`.
let max_digits = decimal.integral.len() + decimal.fractional.len() +
decimal.exp.abs() as usize;
// Remove/shift out the decimal point.
let e = decimal.exp - decimal.fractional.len() as i64;
if let Some(x) = algorithm::fast_path(decimal.integral, decimal.fractional, e) {
return Ok(x);
}
// Big32x40 is limited to 1280 bits, which translates to about 385 decimal digits.
// If we exceed this, perhaps while calculating `f * 10^e` in Algorithm R or Algorithm M,
// we'll crash. So we error out before getting too close, with a generous safety margin.
if max_digits > 375 {
// If we exceed this, we'll crash, so we error out before getting too close (within 10^10).
let upper_bound = bound_intermediate_digits(&decimal, e);
if upper_bound > 375 {
return Err(pfe_invalid());
}
let f = digits_to_big(decimal.integral, decimal.fractional);
Expand All @@ -251,7 +248,7 @@ fn convert<T: RawFloat>(mut decimal: Decimal) -> Result<T, ParseFloatError> {
// FIXME These bounds are rather conservative. A more careful analysis of the failure modes
// of Bellerophon could allow using it in more cases for a massive speed up.
let exponent_in_range = table::MIN_E <= e && e <= table::MAX_E;
let value_in_range = max_digits <= T::max_normal_digits();
let value_in_range = upper_bound <= T::max_normal_digits() as u64;
if exponent_in_range && value_in_range {
Ok(algorithm::bellerophon(&f, e))
} else {
Expand Down Expand Up @@ -288,13 +285,36 @@ fn simplify(decimal: &mut Decimal) {
}
}

/// Quick and dirty upper bound on the size (log10) of the largest value that Algorithm R and
/// Algorithm M will compute while working on the given decimal.
fn bound_intermediate_digits(decimal: &Decimal, e: i64) -> u64 {
// We don't need to worry too much about overflow here thanks to trivial_cases() and the
// parser, which filter out the most extreme inputs for us.
let f_len: u64 = decimal.integral.len() as u64 + decimal.fractional.len() as u64;
if e >= 0 {
// In the case e >= 0, both algorithms compute about `f * 10^e`. Algorithm R proceeds to
// do some complicated calculations with this but we can ignore that for the upper bound
// because it also reduces the fraction beforehand, so we have plenty of buffer there.
f_len + (e as u64)
} else {
// If e < 0, Algorithm R does roughly the same thing, but Algorithm M differs:
// It tries to find a positive number k such that `f << k / 10^e` is an in-range
// significand. This will result in about `2^53 * f * 10^e` < `10^17 * f * 10^e`.
// One input that triggers this is 0.33...33 (375 x 3).
f_len + (e.abs() as u64) + 17
}
}

/// Detect obvious overflows and underflows without even looking at the decimal digits.
fn trivial_cases<T: RawFloat>(decimal: &Decimal) -> Option<T> {
// There were zeros but they were stripped by simplify()
if decimal.integral.is_empty() && decimal.fractional.is_empty() {
return Some(T::zero());
}
// This is a crude approximation of ceil(log10(the real value)).
// This is a crude approximation of ceil(log10(the real value)). We don't need to worry too
// much about overflow here because the input length is tiny (at least compared to 2^64) and
// the parser already handles exponents whose absolute value is greater than 10^18
// (which is still 10^19 short of 2^64).
let max_place = decimal.exp + decimal.integral.len() as i64;
if max_place > T::inf_cutoff() {
return Some(T::infinity());
Expand Down
23 changes: 16 additions & 7 deletions src/libcoretest/num/dec2flt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ macro_rules! test_literal {
let x64: f64 = $x;
let inputs = &[stringify!($x).into(), format!("{:?}", x64), format!("{:e}", x64)];
for input in inputs {
if input != "inf" {
assert_eq!(input.parse(), Ok(x64));
assert_eq!(input.parse(), Ok(x32));
let neg_input = &format!("-{}", input);
assert_eq!(neg_input.parse(), Ok(-x64));
assert_eq!(neg_input.parse(), Ok(-x32));
}
assert_eq!(input.parse(), Ok(x64));
assert_eq!(input.parse(), Ok(x32));
let neg_input = &format!("-{}", input);
assert_eq!(neg_input.parse(), Ok(-x64));
assert_eq!(neg_input.parse(), Ok(-x32));
}
})
}
Expand Down Expand Up @@ -136,6 +134,17 @@ fn massive_exponent() {
assert_eq!(format!("1e{}000", max).parse(), Ok(f64::INFINITY));
}

#[test]
fn borderline_overflow() {
let mut s = "0.".to_string();
for _ in 0..375 {
s.push('3');
}
// At the time of this writing, this returns Err(..), but this is a bug that should be fixed.
// It makes no sense to enshrine that in a test, the important part is that it doesn't panic.
let _ = s.parse::<f64>();
}

#[bench]
fn bench_0(b: &mut test::Bencher) {
b.iter(|| "0.0".parse::<f64>());
Expand Down
12 changes: 9 additions & 3 deletions src/librustc/middle/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use middle::ty::{self, Ty};
use middle::astconv_util::ast_ty_to_prim_ty;
use util::num::ToPrimitive;
use util::nodemap::NodeMap;
use session::Session;

use graphviz::IntoCow;
use syntax::ast;
Expand Down Expand Up @@ -1117,7 +1118,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
debug!("const call({:?})", call_args);
try!(eval_const_expr_partial(tcx, &**result, ty_hint, Some(&call_args)))
},
hir::ExprLit(ref lit) => lit_to_const(&**lit, ety),
hir::ExprLit(ref lit) => lit_to_const(tcx.sess, e.span, &**lit, ety),
hir::ExprBlock(ref block) => {
match block.expr {
Some(ref expr) => try!(eval_const_expr_partial(tcx, &**expr, ty_hint, fn_args)),
Expand Down Expand Up @@ -1319,7 +1320,7 @@ fn cast_const<'tcx>(tcx: &ty::ctxt<'tcx>, val: ConstVal, ty: Ty) -> CastResult {
}
}

fn lit_to_const(lit: &ast::Lit, ty_hint: Option<Ty>) -> ConstVal {
fn lit_to_const(sess: &Session, span: Span, lit: &ast::Lit, ty_hint: Option<Ty>) -> ConstVal {
match lit.node {
ast::LitStr(ref s, _) => Str((*s).clone()),
ast::LitByteStr(ref data) => {
Expand All @@ -1339,7 +1340,12 @@ fn lit_to_const(lit: &ast::Lit, ty_hint: Option<Ty>) -> ConstVal {
ast::LitInt(n, ast::UnsignedIntLit(_)) => Uint(n),
ast::LitFloat(ref n, _) |
ast::LitFloatUnsuffixed(ref n) => {
Float(n.parse::<f64>().unwrap() as f64)
if let Ok(x) = n.parse::<f64>() {
Float(x)
} else {
// FIXME(#31407) this is only necessary because float parsing is buggy
sess.span_bug(span, "could not evaluate float literal (see issue #31407)");
}
}
ast::LitBool(b) => Bool(b)
}
Expand Down
15 changes: 15 additions & 0 deletions src/test/compile-fail/issue-31109.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn main() {
// FIXME(#31407) this error should go away, but in the meantime we test that it
// is accompanied by a somewhat useful error message.
let _: f64 = 1234567890123456789012345678901234567890e-340; //~ ERROR could not evaluate float
}

0 comments on commit 695c907

Please sign in to comment.