about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2016-02-06 13:16:03 +0000
committerbors <bors@rust-lang.org>2016-02-06 13:16:03 +0000
commit695c907dcc7c81c5fc052f426f7764476c12cf15 (patch)
tree31579fe2c0d9619d1a2101e5021cb266a8e7e4af
parent35635aebab321ff2a4708aeb172351356ad63cf7 (diff)
parenta76cb45e340f762d83a9501452db0581ba56db52 (diff)
downloadrust-695c907dcc7c81c5fc052f426f7764476c12cf15.tar.gz
rust-695c907dcc7c81c5fc052f426f7764476c12cf15.zip
Auto merge of #31410 - rkruppe:issue31109, r=pnkfelix
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.
-rw-r--r--src/etc/test-float-parse/_common.rs2
-rw-r--r--src/etc/test-float-parse/few-ones.rs2
-rw-r--r--src/etc/test-float-parse/huge-pow10.rs2
-rw-r--r--src/etc/test-float-parse/long-fractions.rs27
-rw-r--r--src/etc/test-float-parse/many-digits.rs4
-rw-r--r--src/etc/test-float-parse/rand-f64.rs2
-rw-r--r--src/etc/test-float-parse/runtests.py11
-rw-r--r--src/etc/test-float-parse/short-decimals.rs4
-rw-r--r--src/etc/test-float-parse/subnorm.rs4
-rw-r--r--src/etc/test-float-parse/tiny-pow10.rs2
-rw-r--r--src/etc/test-float-parse/u32-small.rs2
-rw-r--r--src/etc/test-float-parse/u64-pow2.rs8
-rw-r--r--src/libcore/num/dec2flt/algorithm.rs4
-rw-r--r--src/libcore/num/dec2flt/mod.rs36
-rw-r--r--src/libcoretest/num/dec2flt/mod.rs23
-rw-r--r--src/librustc/middle/const_eval.rs12
-rw-r--r--src/test/compile-fail/issue-31109.rs15
17 files changed, 119 insertions, 41 deletions
diff --git a/src/etc/test-float-parse/_common.rs b/src/etc/test-float-parse/_common.rs
index b4a2a593e01..725a715f7cf 100644
--- a/src/etc/test-float-parse/_common.rs
+++ b/src/etc/test-float-parse/_common.rs
@@ -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) };
diff --git a/src/etc/test-float-parse/few-ones.rs b/src/etc/test-float-parse/few-ones.rs
index 390f4da6f63..2486df44466 100644
--- a/src/etc/test-float-parse/few-ones.rs
+++ b/src/etc/test-float-parse/few-ones.rs
@@ -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());
             }
         }
     }
diff --git a/src/etc/test-float-parse/huge-pow10.rs b/src/etc/test-float-parse/huge-pow10.rs
index e62afc78515..9d12a03dae2 100644
--- a/src/etc/test-float-parse/huge-pow10.rs
+++ b/src/etc/test-float-parse/huge-pow10.rs
@@ -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));
         }
     }
 }
diff --git a/src/etc/test-float-parse/long-fractions.rs b/src/etc/test-float-parse/long-fractions.rs
new file mode 100644
index 00000000000..9598bd12a0d
--- /dev/null
+++ b/src/etc/test-float-parse/long-fractions.rs
@@ -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);
+            }
+        }
+    }
+}
diff --git a/src/etc/test-float-parse/many-digits.rs b/src/etc/test-float-parse/many-digits.rs
index 0cbf57183df..674c30ad84e 100644
--- a/src/etc/test-float-parse/many-digits.rs
+++ b/src/etc/test-float-parse/many-digits.rs
@@ -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);
     }
 }
 
diff --git a/src/etc/test-float-parse/rand-f64.rs b/src/etc/test-float-parse/rand-f64.rs
index 762c3d95ec6..1d82912054e 100644
--- a/src/etc/test-float-parse/rand-f64.rs
+++ b/src/etc/test-float-parse/rand-f64.rs
@@ -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;
         }
     }
diff --git a/src/etc/test-float-parse/runtests.py b/src/etc/test-float-parse/runtests.py
index 27af63a5876..896d63b9f0a 100644
--- a/src/etc/test-float-parse/runtests.py
+++ b/src/etc/test-float-parse/runtests.py
@@ -21,8 +21,9 @@ random non-exhaustive tests for covering everything else.
 
 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.
@@ -64,9 +65,9 @@ If a test binary writes *anything at all* to stderr or exits with an
 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.
diff --git a/src/etc/test-float-parse/short-decimals.rs b/src/etc/test-float-parse/short-decimals.rs
index baefb9c9305..4909f7c58f8 100644
--- a/src/etc/test-float-parse/short-decimals.rs
+++ b/src/etc/test-float-parse/short-decimals.rs
@@ -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));
         }
     }
 }
diff --git a/src/etc/test-float-parse/subnorm.rs b/src/etc/test-float-parse/subnorm.rs
index 70682c9b218..04a7cc27466 100644
--- a/src/etc/test-float-parse/subnorm.rs
+++ b/src/etc/test-float-parse/subnorm.rs
@@ -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));
     }
 }
diff --git a/src/etc/test-float-parse/tiny-pow10.rs b/src/etc/test-float-parse/tiny-pow10.rs
index a01c6d5a078..50ca5e32609 100644
--- a/src/etc/test-float-parse/tiny-pow10.rs
+++ b/src/etc/test-float-parse/tiny-pow10.rs
@@ -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));
         }
     }
 }
diff --git a/src/etc/test-float-parse/u32-small.rs b/src/etc/test-float-parse/u32-small.rs
index a4e8488e745..571ac80e5b0 100644
--- a/src/etc/test-float-parse/u32-small.rs
+++ b/src/etc/test-float-parse/u32-small.rs
@@ -14,6 +14,6 @@ use _common::validate;
 
 fn main() {
     for i in 0..(1 << 19) {
-        validate(i.to_string());
+        validate(&i.to_string());
     }
 }
diff --git a/src/etc/test-float-parse/u64-pow2.rs b/src/etc/test-float-parse/u64-pow2.rs
index a31304d3f68..5b25c839931 100644
--- a/src/etc/test-float-parse/u64-pow2.rs
+++ b/src/etc/test-float-parse/u64-pow2.rs
@@ -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());
     }
 }
diff --git a/src/libcore/num/dec2flt/algorithm.rs b/src/libcore/num/dec2flt/algorithm.rs
index 82d3389edc4..e33c2814bf2 100644
--- a/src/libcore/num/dec2flt/algorithm.rs
+++ b/src/libcore/num/dec2flt/algorithm.rs
@@ -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().
@@ -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;
diff --git a/src/libcore/num/dec2flt/mod.rs b/src/libcore/num/dec2flt/mod.rs
index 6acc621a613..c0690c24bbb 100644
--- a/src/libcore/num/dec2flt/mod.rs
+++ b/src/libcore/num/dec2flt/mod.rs
@@ -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);
@@ -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 {
@@ -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());
diff --git a/src/libcoretest/num/dec2flt/mod.rs b/src/libcoretest/num/dec2flt/mod.rs
index 7b25333e21e..fe6f52406fb 100644
--- a/src/libcoretest/num/dec2flt/mod.rs
+++ b/src/libcoretest/num/dec2flt/mod.rs
@@ -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));
         }
     })
 }
@@ -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>());
diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs
index 07171e66077..e5fc83cc5f3 100644
--- a/src/librustc/middle/const_eval.rs
+++ b/src/librustc/middle/const_eval.rs
@@ -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;
@@ -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)),
@@ -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) => {
@@ -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)
     }
diff --git a/src/test/compile-fail/issue-31109.rs b/src/test/compile-fail/issue-31109.rs
new file mode 100644
index 00000000000..63b3d58b823
--- /dev/null
+++ b/src/test/compile-fail/issue-31109.rs
@@ -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
+}