Skip to content

Commit 84e22f2

Browse files
committed
auto merge of #6108 : gifnksm/rust/bigint-shift-bug, r=brson
`std::bigint` contains the following code. ```rust borrow = *elem << (uint::bits - n_bits); ``` The code above contains a bug that the value of the right operand of the shift operator exceeds the size of the left operand, because sizeof(*elem) == 32, and 0 <= n_bits < 32 in 64bit architecture. If `--opt-level` option is not given to rustc, the code above runs as if the right operand is `(uint::bits - n_bits) % 32`, but if --opt-level is given, `borrow` is always zero. I wonder why this bug is not catched in the libstd's testsuite (I try the `rustc --test --opt-level=2 bigint.rs` before fixing the bug, but the unittest passes normally.) This pull request also removes the implicit vector copies in `bigint.rs`.
2 parents 48f50ac + ffa31d2 commit 84e22f2

File tree

1 file changed

+39
-21
lines changed

1 file changed

+39
-21
lines changed

src/libstd/num/bigint.rs

+39-21
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ A BigUint is represented as an array of BigDigits.
1616
A BigInt is a combination of BigUint and Sign.
1717
*/
1818

19+
#[deny(vecs_implicitly_copyable)];
20+
#[deny(deprecated_mutable_fields)];
21+
1922
use core::cmp::{Eq, Ord, TotalEq, TotalOrd, Ordering, Less, Equal, Greater};
2023
use core::num::{IntConvertible, Zero, One, ToStrRadix, FromStrRadix};
2124
use core::*;
@@ -355,16 +358,24 @@ impl Integer for BigUint {
355358
let mut (d0, d_unit, b_unit) = div_estimate(&m, &b, n);
356359
let mut prod = b * d0;
357360
while prod > m {
358-
d0 -= d_unit;
359-
prod -= b_unit;
361+
// FIXME(#6050): overloaded operators force moves with generic types
362+
// d0 -= d_unit
363+
d0 = d0 - d_unit;
364+
// FIXME(#6050): overloaded operators force moves with generic types
365+
// prod = prod - b_unit;
366+
prod = prod - b_unit
360367
}
361368
if d0.is_zero() {
362369
n = 2;
363370
loop;
364371
}
365372
n = 1;
366-
d += d0;
367-
m -= prod;
373+
// FIXME(#6102): Assignment operator for BigInt causes ICE
374+
// d += d0;
375+
d = d + d0;
376+
// FIXME(#6102): Assignment operator for BigInt causes ICE
377+
// m -= prod;
378+
m = m - prod;
368379
}
369380
return (d, m);
370381
}
@@ -411,7 +422,7 @@ impl Integer for BigUint {
411422
#[inline(always)]
412423
fn gcd(&self, other: &BigUint) -> BigUint {
413424
// Use Euclid's algorithm
414-
let mut m = *self, n = *other;
425+
let mut m = copy *self, n = copy *other;
415426
while !m.is_zero() {
416427
let temp = m;
417428
m = n % temp;
@@ -547,14 +558,18 @@ impl BigUint {
547558
loop {
548559
let start = uint::max(end, unit_len) - unit_len;
549560
match uint::parse_bytes(vec::slice(buf, start, end), radix) {
550-
Some(d) => n += BigUint::from_uint(d) * power,
561+
// FIXME(#6102): Assignment operator for BigInt causes ICE
562+
// Some(d) => n += BigUint::from_uint(d) * power,
563+
Some(d) => n = n + BigUint::from_uint(d) * power,
551564
None => return None
552565
}
553566
if end <= unit_len {
554567
return Some(n);
555568
}
556569
end -= unit_len;
557-
power *= base_num;
570+
// FIXME(#6050): overloaded operators force moves with generic types
571+
// power *= base_num;
572+
power = power * base_num;
558573
}
559574
}
560575

@@ -569,15 +584,15 @@ impl BigUint {
569584
}
570585

571586
#[inline(always)]
572-
priv fn shl_unit(self, n_unit: uint) -> BigUint {
573-
if n_unit == 0 || self.is_zero() { return self; }
587+
priv fn shl_unit(&self, n_unit: uint) -> BigUint {
588+
if n_unit == 0 || self.is_zero() { return copy *self; }
574589

575590
return BigUint::new(vec::from_elem(n_unit, 0) + self.data);
576591
}
577592

578593
#[inline(always)]
579-
priv fn shl_bits(self, n_bits: uint) -> BigUint {
580-
if n_bits == 0 || self.is_zero() { return self; }
594+
priv fn shl_bits(&self, n_bits: uint) -> BigUint {
595+
if n_bits == 0 || self.is_zero() { return copy *self; }
581596

582597
let mut carry = 0;
583598
let shifted = do vec::map(self.data) |elem| {
@@ -592,23 +607,23 @@ impl BigUint {
592607
}
593608

594609
#[inline(always)]
595-
priv fn shr_unit(self, n_unit: uint) -> BigUint {
596-
if n_unit == 0 { return self; }
610+
priv fn shr_unit(&self, n_unit: uint) -> BigUint {
611+
if n_unit == 0 { return copy *self; }
597612
if self.data.len() < n_unit { return Zero::zero(); }
598613
return BigUint::from_slice(
599614
vec::slice(self.data, n_unit, self.data.len())
600615
);
601616
}
602617

603618
#[inline(always)]
604-
priv fn shr_bits(self, n_bits: uint) -> BigUint {
605-
if n_bits == 0 || self.data.is_empty() { return self; }
619+
priv fn shr_bits(&self, n_bits: uint) -> BigUint {
620+
if n_bits == 0 || self.data.is_empty() { return copy *self; }
606621

607622
let mut borrow = 0;
608623
let mut shifted = ~[];
609624
for self.data.each_reverse |elem| {
610625
shifted = ~[(*elem >> n_bits) | borrow] + shifted;
611-
borrow = *elem << (uint::bits - n_bits);
626+
borrow = *elem << (BigDigit::bits - n_bits);
612627
}
613628
return BigUint::new(shifted);
614629
}
@@ -1070,7 +1085,7 @@ pub impl BigInt {
10701085
start = 1;
10711086
}
10721087
return BigUint::parse_bytes(vec::slice(buf, start, buf.len()), radix)
1073-
.map(|bu| BigInt::from_biguint(sign, *bu));
1088+
.map_consume(|bu| BigInt::from_biguint(sign, bu));
10741089
}
10751090
10761091
#[inline(always)]
@@ -1198,6 +1213,7 @@ mod biguint_tests {
11981213
check(~[1 << 2], 2, ~[1]);
11991214
check(~[1, 2], 3, ~[1 << (BigDigit::bits - 2)]);
12001215
check(~[1, 1, 2], 3 + BigDigit::bits, ~[1 << (BigDigit::bits - 2)]);
1216+
check(~[0, 1], 1, ~[0x80000000]);
12011217
test_shr_bits();
12021218

12031219
#[cfg(target_arch = "x86_64")]
@@ -1376,10 +1392,10 @@ mod biguint_tests {
13761392
let c = BigUint::from_slice(cVec);
13771393

13781394
if !a.is_zero() {
1379-
assert!(c.quot_rem(&a) == (b, Zero::zero()));
1395+
assert!(c.quot_rem(&a) == (copy b, Zero::zero()));
13801396
}
13811397
if !b.is_zero() {
1382-
assert!(c.quot_rem(&b) == (a, Zero::zero()));
1398+
assert!(c.quot_rem(&b) == (copy a, Zero::zero()));
13831399
}
13841400
}
13851401

@@ -1503,7 +1519,7 @@ mod biguint_tests {
15031519
let &(n, rs) = num_pair;
15041520
for rs.each |str_pair| {
15051521
let &(radix, str) = str_pair;
1506-
assert_eq!(Some(n), FromStrRadix::from_str_radix(str, radix));
1522+
assert_eq!(&n, &FromStrRadix::from_str_radix(str, radix).get());
15071523
}
15081524
}
15091525

@@ -1517,7 +1533,9 @@ mod biguint_tests {
15171533
fn factor(n: uint) -> BigUint {
15181534
let mut f= One::one::<BigUint>();
15191535
for uint::range(2, n + 1) |i| {
1520-
f *= BigUint::from_uint(i);
1536+
// FIXME(#6102): Assignment operator for BigInt causes ICE
1537+
// f *= BigUint::from_uint(i);
1538+
f = f * BigUint::from_uint(i);
15211539
}
15221540
return f;
15231541
}

0 commit comments

Comments
 (0)