Skip to content

Commit

Permalink
Add core::num::wrapping and fix overflow errors.
Browse files Browse the repository at this point in the history
Many of the core rust libraries have places that rely on integer
wrapping behaviour. These places have been altered to use the wrapping_*
methods:

 * core::hash::sip - A number of macros
 * core::str - The `maximal_suffix` method in `TwoWaySearcher`
 * rustc::util::nodemap - Implementation of FnvHash
 * rustc_back::sha2 - A number of macros and other places
 * rand::isaac - Isaac64Rng, changed to use the Wrapping helper type

Some places had "benign" underflow. This is when underflow or overflow
occurs, but the unspecified value is not used due to other conditions.

 * collections::bit::Bitv - underflow when `self.nbits` is zero.
 * collections::hash::{map,table} - Underflow when searching an empty
   table. Did cause undefined behaviour in this case due to an
   out-of-bounds ptr::offset based on the underflowed index. However the
   resulting pointers would never be read from.
 * syntax::ext::deriving::encodable - Underflow when calculating the
   index of the last field in a variant with no fields.

These cases were altered to avoid the underflow, often by moving the
underflowing operation to a place where underflow could not happen.

There was one case that relied on the fact that unsigned arithmetic and
two's complement arithmetic are identical with wrapping semantics. This
was changed to use the wrapping_* methods.

Finally, the calculation of variant discriminants could overflow if the
preceeding discriminant was `U64_MAX`. The logic in `rustc::middle::ty`
for this was altered to avoid the overflow completely, while the
remaining places were changed to use wrapping methods. This is because
`rustc::middle::ty::enum_variants` now throws an error when the
calculated discriminant value overflows a `u64`.

This behaviour can be triggered by the following code:

```
enum Foo {
  A = U64_MAX,
  B
}
```

This commit also implements the remaining integer operators for
Wrapped<T>.
  • Loading branch information
Aatch authored and pnkfelix committed Mar 3, 2015
1 parent cdfff9d commit 1246d40
Show file tree
Hide file tree
Showing 19 changed files with 286 additions and 95 deletions.
6 changes: 3 additions & 3 deletions src/libcollections/bit.rs
Expand Up @@ -818,19 +818,19 @@ impl BitVec {
let full_value = if value { !0 } else { 0 };

// Correct the old tail word, setting or clearing formerly unused bits
let old_last_word = blocks_for_bits(self.nbits) - 1;
let num_cur_blocks = blocks_for_bits(self.nbits);
if self.nbits % u32::BITS as usize > 0 {
let mask = mask_for_bits(self.nbits);
if value {
self.storage[old_last_word] |= !mask;
self.storage[num_cur_blocks - 1] |= !mask;
} else {
// Extra bits are already zero by invariant.
}
}

// Fill in words after the old tail word
let stop_idx = cmp::min(self.storage.len(), new_nblocks);
for idx in old_last_word + 1..stop_idx {
for idx in num_cur_blocks..stop_idx {
self.storage[idx] = full_value;
}

Expand Down
12 changes: 6 additions & 6 deletions src/libcore/hash/sip.rs
Expand Up @@ -14,7 +14,7 @@

use prelude::*;
use default::Default;

use num::wrapping::WrappingOps;
use super::Hasher;

/// An implementation of SipHash 2-4.
Expand Down Expand Up @@ -71,17 +71,17 @@ macro_rules! u8to64_le {

macro_rules! rotl {
($x:expr, $b:expr) =>
(($x << $b) | ($x >> (64 - $b)))
(($x << $b) | ($x >> (64.wrapping_sub($b))))
}

macro_rules! compress {
($v0:expr, $v1:expr, $v2:expr, $v3:expr) =>
({
$v0 += $v1; $v1 = rotl!($v1, 13); $v1 ^= $v0;
$v0 = $v0.wrapping_add($v1); $v1 = rotl!($v1, 13); $v1 ^= $v0;
$v0 = rotl!($v0, 32);
$v2 += $v3; $v3 = rotl!($v3, 16); $v3 ^= $v2;
$v0 += $v3; $v3 = rotl!($v3, 21); $v3 ^= $v0;
$v2 += $v1; $v1 = rotl!($v1, 17); $v1 ^= $v2;
$v2 = $v2.wrapping_add($v3); $v3 = rotl!($v3, 16); $v3 ^= $v2;
$v0 = $v0.wrapping_add($v3); $v3 = rotl!($v3, 21); $v3 ^= $v0;
$v2 = $v2.wrapping_add($v1); $v1 = rotl!($v1, 17); $v1 ^= $v2;
$v2 = rotl!($v2, 32);
})
}
Expand Down
3 changes: 3 additions & 0 deletions src/libcore/num/mod.rs
Expand Up @@ -30,6 +30,9 @@ use option::Option::{self, Some, None};
use result::Result::{self, Ok, Err};
use str::{FromStr, StrExt};

#[unstable(feature = "core", reason = "may be removed or relocated")]
pub mod wrapping;

/// A built-in signed or unsigned integer.
#[stable(feature = "rust1", since = "1.0.0")]
pub trait Int
Expand Down
153 changes: 153 additions & 0 deletions src/libcore/num/wrapping.rs
@@ -0,0 +1,153 @@
// Copyright 2014 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.
#![allow(missing_docs)]

use ops::*;

#[cfg(not(stage0))]
use intrinsics::{overflowing_add, overflowing_sub, overflowing_mul};

pub trait WrappingOps {
fn wrapping_add(self, rhs: Self) -> Self;
fn wrapping_sub(self, rhs: Self) -> Self;
fn wrapping_mul(self, rhs: Self) -> Self;
}

#[cfg(not(stage0))]
macro_rules! wrapping_impl {
($($t:ty)*) => ($(
impl WrappingOps for $t {
#[inline(always)]
fn wrapping_add(self, rhs: $t) -> $t {
unsafe {
overflowing_add(self, rhs)
}
}
#[inline(always)]
fn wrapping_sub(self, rhs: $t) -> $t {
unsafe {
overflowing_sub(self, rhs)
}
}
#[inline(always)]
fn wrapping_mul(self, rhs: $t) -> $t {
unsafe {
overflowing_mul(self, rhs)
}
}
}
)*)
}

#[cfg(stage0)]
macro_rules! wrapping_impl {
($($t:ty)*) => ($(
impl WrappingOps for $t {
#[inline(always)]
fn wrapping_add(self, rhs: $t) -> $t {
self + rhs
}
#[inline(always)]
fn wrapping_sub(self, rhs: $t) -> $t {
self - rhs
}
#[inline(always)]
fn wrapping_mul(self, rhs: $t) -> $t {
self * rhs
}
}
)*)
}

wrapping_impl! { uint u8 u16 u32 u64 int i8 i16 i32 i64 }

#[unstable(feature = "core", reason = "may be removed, renamed, or relocated")]
#[derive(PartialEq,Eq,PartialOrd,Ord,Clone,Copy)]
pub struct Wrapping<T>(pub T);

impl<T:WrappingOps> Add for Wrapping<T> {
type Output = Wrapping<T>;

#[inline(always)]
fn add(self, other: Wrapping<T>) -> Wrapping<T> {
Wrapping(self.0.wrapping_add(other.0))
}
}

impl<T:WrappingOps> Sub for Wrapping<T> {
type Output = Wrapping<T>;

#[inline(always)]
fn sub(self, other: Wrapping<T>) -> Wrapping<T> {
Wrapping(self.0.wrapping_sub(other.0))
}
}

impl<T:WrappingOps> Mul for Wrapping<T> {
type Output = Wrapping<T>;

#[inline(always)]
fn mul(self, other: Wrapping<T>) -> Wrapping<T> {
Wrapping(self.0.wrapping_mul(other.0))
}
}

impl<T:WrappingOps+Not<Output=T>> Not for Wrapping<T> {
type Output = Wrapping<T>;

fn not(self) -> Wrapping<T> {
Wrapping(!self.0)
}
}

impl<T:WrappingOps+BitXor<Output=T>> BitXor for Wrapping<T> {
type Output = Wrapping<T>;

#[inline(always)]
fn bitxor(self, other: Wrapping<T>) -> Wrapping<T> {
Wrapping(self.0 ^ other.0)
}
}

impl<T:WrappingOps+BitOr<Output=T>> BitOr for Wrapping<T> {
type Output = Wrapping<T>;

#[inline(always)]
fn bitor(self, other: Wrapping<T>) -> Wrapping<T> {
Wrapping(self.0 | other.0)
}
}

impl<T:WrappingOps+BitAnd<Output=T>> BitAnd for Wrapping<T> {
type Output = Wrapping<T>;

#[inline(always)]
fn bitand(self, other: Wrapping<T>) -> Wrapping<T> {
Wrapping(self.0 & other.0)
}
}

impl<T:WrappingOps+Shl<uint,Output=T>> Shl<uint> for Wrapping<T> {
type Output = Wrapping<T>;

#[inline(always)]
fn shl(self, other: uint) -> Wrapping<T> {
Wrapping(self.0 << other)
}
}

impl<T:WrappingOps+Shr<uint,Output=T>> Shr<uint> for Wrapping<T> {
type Output = Wrapping<T>;

#[inline(always)]
fn shr(self, other: uint) -> Wrapping<T> {
Wrapping(self.0 >> other)
}
}
9 changes: 5 additions & 4 deletions src/libcore/str/mod.rs
Expand Up @@ -830,6 +830,7 @@ impl TwoWaySearcher {
#[inline]
#[allow(dead_code)]
fn maximal_suffix(arr: &[u8], reversed: bool) -> (usize, usize) {
use num::wrapping::WrappingOps;
let mut left = -1; // Corresponds to i in the paper
let mut right = 0; // Corresponds to j in the paper
let mut offset = 1; // Corresponds to k in the paper
Expand All @@ -839,17 +840,17 @@ impl TwoWaySearcher {
let a;
let b;
if reversed {
a = arr[left + offset];
a = arr[left.wrapping_add(offset)];
b = arr[right + offset];
} else {
a = arr[right + offset];
b = arr[left + offset];
b = arr[left.wrapping_add(offset)];
}
if a < b {
// Suffix is smaller, period is entire prefix so far.
right += offset;
offset = 1;
period = right - left;
period = right.wrapping_sub(left);
} else if a == b {
// Advance through repetition of the current period.
if offset == period {
Expand All @@ -866,7 +867,7 @@ impl TwoWaySearcher {
period = 1;
}
}
(left + 1, period)
(left.wrapping_add(1), period)
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librand/distributions/range.rs
Expand Up @@ -14,6 +14,7 @@

use core::prelude::{PartialOrd};
use core::num::Int;
use core::num::wrapping::WrappingOps;

use Rng;
use distributions::{Sample, IndependentSample};
Expand Down Expand Up @@ -97,7 +98,7 @@ macro_rules! integer_impl {
// bijection.

fn construct_range(low: $ty, high: $ty) -> Range<$ty> {
let range = high as $unsigned - low as $unsigned;
let range = (high as $unsigned).wrapping_sub(low as $unsigned);
let unsigned_max: $unsigned = Int::max_value();

// this is the largest number that fits into $unsigned
Expand Down

0 comments on commit 1246d40

Please sign in to comment.