Skip to content

Commit 2690107

Browse files
AatchAatch
committed
Fix overflow errors
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>.
1 parent c698a1b commit 2690107

File tree

20 files changed

+186
-97
lines changed

20 files changed

+186
-97
lines changed

‎mk/main.mk

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,8 @@ endif
157157
# have to worry about the distribution of one file (with its native dynamic
158158
# dependencies)
159159
RUSTFLAGS_STAGE0 += -C prefer-dynamic
160-
# FIXME: Remove the -Z force-no-overflow-checks flags
161-
RUSTFLAGS_STAGE1 += -C prefer-dynamic -Z force-no-overflow-checks
162-
RUST_LIB_FLAGS_ST2 += -C prefer-dynamic -Z force-no-overflow-checks
160+
RUSTFLAGS_STAGE1 += -C prefer-dynamic
161+
RUST_LIB_FLAGS_ST2 += -C prefer-dynamic
163162
RUST_LIB_FLAGS_ST3 += -C prefer-dynamic
164163

165164
# Landing pads require a lot of codegen. We can get through bootstrapping faster

‎src/libcollections/bit.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -816,19 +816,19 @@ impl Bitv {
816816
let full_value = if value { !0 } else { 0 };
817817

818818
// Correct the old tail word, setting or clearing formerly unused bits
819-
let old_last_word = blocks_for_bits(self.nbits) - 1;
819+
let num_cur_blocks = blocks_for_bits(self.nbits);
820820
if self.nbits % u32::BITS > 0 {
821821
let mask = mask_for_bits(self.nbits);
822822
if value {
823-
self.storage[old_last_word] |= !mask;
823+
self.storage[num_cur_blocks - 1] |= !mask;
824824
} else {
825825
// Extra bits are already zero by invariant.
826826
}
827827
}
828828

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

‎src/libcore/hash/sip.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,17 @@ macro_rules! u8to64_le {
7171

7272
macro_rules! rotl {
7373
($x:expr, $b:expr) =>
74-
(($x << $b) | ($x >> (64 - $b)))
74+
(($x << $b) | ($x >> (64.wrapping_sub($b))))
7575
}
7676

7777
macro_rules! compress {
7878
($v0:expr, $v1:expr, $v2:expr, $v3:expr) =>
7979
({
80-
$v0 += $v1; $v1 = rotl!($v1, 13); $v1 ^= $v0;
80+
$v0 = $v0.wrapping_add($v1); $v1 = rotl!($v1, 13); $v1 ^= $v0;
8181
$v0 = rotl!($v0, 32);
82-
$v2 += $v3; $v3 = rotl!($v3, 16); $v3 ^= $v2;
83-
$v0 += $v3; $v3 = rotl!($v3, 21); $v3 ^= $v0;
84-
$v2 += $v1; $v1 = rotl!($v1, 17); $v1 ^= $v2;
82+
$v2 = $v2.wrapping_add($v3); $v3 = rotl!($v3, 16); $v3 ^= $v2;
83+
$v0 = $v0.wrapping_add($v3); $v3 = rotl!($v3, 21); $v3 ^= $v0;
84+
$v2 = $v2.wrapping_add($v1); $v1 = rotl!($v1, 17); $v1 ^= $v2;
8585
$v2 = rotl!($v2, 32);
8686
})
8787
}

‎src/libcore/intrinsics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ extern "rust-intrinsic" {
547547
pub fn u64_mul_with_overflow(x: u64, y: u64) -> (u64, bool);
548548
}
549549

550-
#[cfg(not(stage1))]
550+
#[cfg(not(stage0))]
551551
extern "rust-intrinsic" {
552552
/// Returns (a + b) mod 2^N, where N is the width of N in bits.
553553
pub fn overflowing_add<T>(a: T, b: T) -> T;

‎src/libcore/num/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use option::Option;
2929
use option::Option::{Some, None};
3030
use str::{FromStr, StrExt};
3131

32+
#[experimental = "may be removed or relocated"]
3233
pub mod wrapping;
3334

3435
/// A built-in signed or unsigned integer.

‎src/libcore/num/wrapping.rs

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ macro_rules! wrapping_impl {
5959

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

62-
#[derive(PartialEq,Eq,PartialOrd,Ord,Clone)]
62+
#[derive(PartialEq,Eq,PartialOrd,Ord,Clone,Copy)]
6363
pub struct Wrapping<T>(pub T);
6464

6565
impl<T:WrappingOps> Add for Wrapping<T> {
@@ -88,3 +88,56 @@ impl<T:WrappingOps> Mul for Wrapping<T> {
8888
Wrapping(self.0.wrapping_mul(other.0))
8989
}
9090
}
91+
92+
impl<T:WrappingOps+Not<Output=T>> Not for Wrapping<T> {
93+
type Output = Wrapping<T>;
94+
95+
fn not(self) -> Wrapping<T> {
96+
Wrapping(!self.0)
97+
}
98+
}
99+
100+
impl<T:WrappingOps+BitXor<Output=T>> BitXor for Wrapping<T> {
101+
type Output = Wrapping<T>;
102+
103+
#[inline(always)]
104+
fn bitxor(self, other: Wrapping<T>) -> Wrapping<T> {
105+
Wrapping(self.0 ^ other.0)
106+
}
107+
}
108+
109+
impl<T:WrappingOps+BitOr<Output=T>> BitOr for Wrapping<T> {
110+
type Output = Wrapping<T>;
111+
112+
#[inline(always)]
113+
fn bitor(self, other: Wrapping<T>) -> Wrapping<T> {
114+
Wrapping(self.0 | other.0)
115+
}
116+
}
117+
118+
impl<T:WrappingOps+BitAnd<Output=T>> BitAnd for Wrapping<T> {
119+
type Output = Wrapping<T>;
120+
121+
#[inline(always)]
122+
fn bitand(self, other: Wrapping<T>) -> Wrapping<T> {
123+
Wrapping(self.0 & other.0)
124+
}
125+
}
126+
127+
impl<T:WrappingOps+Shl<uint,Output=T>> Shl<uint> for Wrapping<T> {
128+
type Output = Wrapping<T>;
129+
130+
#[inline(always)]
131+
fn shl(self, other: uint) -> Wrapping<T> {
132+
Wrapping(self.0 << other)
133+
}
134+
}
135+
136+
impl<T:WrappingOps+Shr<uint,Output=T>> Shr<uint> for Wrapping<T> {
137+
type Output = Wrapping<T>;
138+
139+
#[inline(always)]
140+
fn shr(self, other: uint) -> Wrapping<T> {
141+
Wrapping(self.0 >> other)
142+
}
143+
}

‎src/libcore/str/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,7 @@ impl TwoWaySearcher {
789789
// critical factorization (u, v) and p = period(v)
790790
#[inline]
791791
fn maximal_suffix(arr: &[u8], reversed: bool) -> (uint, uint) {
792+
use num::wrapping::WrappingOps;
792793
let mut left = -1; // Corresponds to i in the paper
793794
let mut right = 0; // Corresponds to j in the paper
794795
let mut offset = 1; // Corresponds to k in the paper
@@ -798,17 +799,17 @@ impl TwoWaySearcher {
798799
let a;
799800
let b;
800801
if reversed {
801-
a = arr[left + offset];
802+
a = arr[left.wrapping_add(offset)];
802803
b = arr[right + offset];
803804
} else {
804805
a = arr[right + offset];
805-
b = arr[left + offset];
806+
b = arr[left.wrapping_add(offset)];
806807
}
807808
if a < b {
808809
// Suffix is smaller, period is entire prefix so far.
809810
right += offset;
810811
offset = 1;
811-
period = right - left;
812+
period = right.wrapping_sub(left);
812813
} else if a == b {
813814
// Advance through repetition of the current period.
814815
if offset == period {
@@ -825,7 +826,7 @@ impl TwoWaySearcher {
825826
period = 1;
826827
}
827828
}
828-
(left + 1, period)
829+
(left.wrapping_add(1), period)
829830
}
830831
}
831832

‎src/librand/isaac.rs

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ impl Isaac64Rng {
304304
fn init(&mut self, use_rsl: bool) {
305305
macro_rules! init {
306306
($var:ident) => (
307-
let mut $var = 0x9e3779b97f4a7c13;
307+
let mut $var = Wrapping(0x9e3779b97f4a7c13);
308308
)
309309
}
310310

@@ -313,14 +313,14 @@ impl Isaac64Rng {
313313

314314
macro_rules! mix {
315315
() => {{
316-
a-=e; f^=h>>9; h+=a;
317-
b-=f; g^=a<<9; a+=b;
318-
c-=g; h^=b>>23; b+=c;
319-
d-=h; a^=c<<15; c+=d;
320-
e-=a; b^=d>>14; d+=e;
321-
f-=b; c^=e<<20; e+=f;
322-
g-=c; d^=f>>17; f+=g;
323-
h-=d; e^=g<<14; g+=h;
316+
a=a-e; f=f^h>>9; h=h+a;
317+
b=b-f; g=g^a<<9; a=a+b;
318+
c=c-g; h=h^b>>23; b=b+c;
319+
d=d-h; a=a^c<<15; c=c+d;
320+
e=e-a; b=b^d>>14; d=d+e;
321+
f=f-b; c=c^e<<20; e=e+f;
322+
g=g-c; d=d^f>>17; f=f+g;
323+
h=h-d; e=e^g<<14; g=g+h;
324324
}}
325325
}
326326

@@ -332,15 +332,15 @@ impl Isaac64Rng {
332332
macro_rules! memloop {
333333
($arr:expr) => {{
334334
for i in range(0, RAND_SIZE_64 / 8).map(|i| i * 8) {
335-
a+=$arr[i ]; b+=$arr[i+1];
336-
c+=$arr[i+2]; d+=$arr[i+3];
337-
e+=$arr[i+4]; f+=$arr[i+5];
338-
g+=$arr[i+6]; h+=$arr[i+7];
335+
a=a+Wrapping($arr[i ]); b=b+Wrapping($arr[i+1]);
336+
c=c+Wrapping($arr[i+2]); d=d+Wrapping($arr[i+3]);
337+
e=e+Wrapping($arr[i+4]); f=f+Wrapping($arr[i+5]);
338+
g=g+Wrapping($arr[i+6]); h=h+Wrapping($arr[i+7]);
339339
mix!();
340-
self.mem[i ]=a; self.mem[i+1]=b;
341-
self.mem[i+2]=c; self.mem[i+3]=d;
342-
self.mem[i+4]=e; self.mem[i+5]=f;
343-
self.mem[i+6]=g; self.mem[i+7]=h;
340+
self.mem[i ]=a.0; self.mem[i+1]=b.0;
341+
self.mem[i+2]=c.0; self.mem[i+3]=d.0;
342+
self.mem[i+4]=e.0; self.mem[i+5]=f.0;
343+
self.mem[i+6]=g.0; self.mem[i+7]=h.0;
344344
}
345345
}}
346346
}
@@ -350,10 +350,10 @@ impl Isaac64Rng {
350350
} else {
351351
for i in range(0, RAND_SIZE_64 / 8).map(|i| i * 8) {
352352
mix!();
353-
self.mem[i ]=a; self.mem[i+1]=b;
354-
self.mem[i+2]=c; self.mem[i+3]=d;
355-
self.mem[i+4]=e; self.mem[i+5]=f;
356-
self.mem[i+6]=g; self.mem[i+7]=h;
353+
self.mem[i ]=a.0; self.mem[i+1]=b.0;
354+
self.mem[i+2]=c.0; self.mem[i+3]=d.0;
355+
self.mem[i+4]=e.0; self.mem[i+5]=f.0;
356+
self.mem[i+6]=g.0; self.mem[i+7]=h.0;
357357
}
358358
}
359359

@@ -364,8 +364,8 @@ impl Isaac64Rng {
364364
fn isaac64(&mut self) {
365365
self.c += 1;
366366
// abbreviations
367-
let mut a = self.a;
368-
let mut b = self.b + self.c;
367+
let mut a = Wrapping(self.a);
368+
let mut b = Wrapping(self.b) + Wrapping(self.c);
369369
const MIDPOINT: uint = RAND_SIZE_64 / 2;
370370
const MP_VEC: [(uint, uint); 2] = [(0,MIDPOINT), (MIDPOINT, 0)];
371371
macro_rules! ind {
@@ -384,13 +384,13 @@ impl Isaac64Rng {
384384
let mix = if $j == 0 {!mix} else {mix};
385385

386386
unsafe {
387-
let x = *self.mem.get_unchecked(base + mr_offset);
388-
a = mix + *self.mem.get_unchecked(base + m2_offset);
389-
let y = ind!(x) + a + b;
390-
*self.mem.get_unchecked_mut(base + mr_offset) = y;
387+
let x = Wrapping(*self.mem.get_unchecked(base + mr_offset));
388+
a = mix + Wrapping(*self.mem.get_unchecked(base + m2_offset));
389+
let y = Wrapping(ind!(x.0)) + a + b;
390+
*self.mem.get_unchecked_mut(base + mr_offset) = y.0;
391391

392-
b = ind!(y >> RAND_SIZE_64_LEN) + x;
393-
*self.rsl.get_unchecked_mut(base + mr_offset) = b;
392+
b = Wrapping(ind!(y.0 >> RAND_SIZE_64_LEN)) + x;
393+
*self.rsl.get_unchecked_mut(base + mr_offset) = b.0;
394394
}
395395
}}
396396
}
@@ -402,13 +402,13 @@ impl Isaac64Rng {
402402
let mix = if $j == 0 {!mix} else {mix};
403403

404404
unsafe {
405-
let x = *self.mem.get_unchecked(base + mr_offset);
406-
a = mix + *self.mem.get_unchecked(base + m2_offset);
407-
let y = ind!(x) + a + b;
408-
*self.mem.get_unchecked_mut(base + mr_offset) = y;
405+
let x = Wrapping(*self.mem.get_unchecked(base + mr_offset));
406+
a = mix + Wrapping(*self.mem.get_unchecked(base + m2_offset));
407+
let y = Wrapping(ind!(x.0)) + a + b;
408+
*self.mem.get_unchecked_mut(base + mr_offset) = y.0;
409409

410-
b = ind!(y >> RAND_SIZE_64_LEN) + x;
411-
*self.rsl.get_unchecked_mut(base + mr_offset) = b;
410+
b = Wrapping(ind!(y.0 >> RAND_SIZE_64_LEN)) + x;
411+
*self.rsl.get_unchecked_mut(base + mr_offset) = b.0;
412412
}
413413
}}
414414
}
@@ -420,8 +420,8 @@ impl Isaac64Rng {
420420
}
421421
}
422422

423-
self.a = a;
424-
self.b = b;
423+
self.a = a.0;
424+
self.b = b.0;
425425
self.cnt = RAND_SIZE_64;
426426
}
427427
}

‎src/librustc/metadata/decoder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,7 @@ pub fn get_enum_variants<'tcx>(intr: Rc<IdentInterner>, cdata: Cmd, id: ast::Nod
743743
_ => { /* empty */ }
744744
}
745745
let old_disr_val = disr_val;
746-
disr_val += 1;
746+
disr_val = disr_val.wrapping_add(1);
747747
Rc::new(ty::VariantInfo {
748748
args: arg_tys,
749749
arg_names: arg_names,

‎src/librustc/metadata/encoder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ fn encode_enum_variant_info(ecx: &EncodeContext,
358358

359359
ecx.tcx.map.with_path(variant.node.id, |path| encode_path(rbml_w, path));
360360
rbml_w.end_tag();
361-
disr_val += 1;
361+
disr_val = disr_val.wrapping_add(1);
362362
i += 1;
363363
}
364364
}

‎src/librustc/middle/astencode.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,9 @@ impl<'a, 'b, 'tcx> DecodeContext<'a, 'b, 'tcx> {
202202
pub fn tr_id(&self, id: ast::NodeId) -> ast::NodeId {
203203
// from_id_range should be non-empty
204204
assert!(!self.from_id_range.empty());
205-
(id - self.from_id_range.min + self.to_id_range.min)
205+
// Use wrapping arithmetic because otherwise it introduces control flow.
206+
// Maybe we should just have the control flow? -- aatch
207+
(id.wrapping_sub(self.from_id_range.min).wrapping_add(self.to_id_range.min))
206208
}
207209

208210
/// Translates an EXTERNAL def-id, converting the crate number from the one used in the encoded

‎src/librustc/middle/ty.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5317,6 +5317,7 @@ pub fn type_is_empty(cx: &ctxt, ty: Ty) -> bool {
53175317

53185318
pub fn enum_variants<'tcx>(cx: &ctxt<'tcx>, id: ast::DefId)
53195319
-> Rc<Vec<Rc<VariantInfo<'tcx>>>> {
5320+
use std::num::Int; // For checked_add
53205321
memoized(&cx.enum_var_cache, id, |id: ast::DefId| {
53215322
if ast::LOCAL_CRATE != id.krate {
53225323
Rc::new(csearch::get_enum_variants(cx, id))
@@ -5333,10 +5334,7 @@ pub fn enum_variants<'tcx>(cx: &ctxt<'tcx>, id: ast::DefId)
53335334
let mut last_discriminant: Option<Disr> = None;
53345335
Rc::new(enum_definition.variants.iter().map(|variant| {
53355336

5336-
let mut discriminant = match last_discriminant {
5337-
Some(val) => val + 1,
5338-
None => INITIAL_DISCRIMINANT_VALUE
5339-
};
5337+
let mut discriminant = INITIAL_DISCRIMINANT_VALUE;
53405338

53415339
match variant.node.disr_expr {
53425340
Some(ref e) =>
@@ -5359,7 +5357,19 @@ pub fn enum_variants<'tcx>(cx: &ctxt<'tcx>, id: ast::DefId)
53595357
*err)[]);
53605358
}
53615359
},
5362-
None => {}
5360+
None => {
5361+
if let Some(val) = last_discriminant {
5362+
if let Some(v) = val.checked_add(1) {
5363+
discriminant = v
5364+
} else {
5365+
cx.sess.span_err(
5366+
variant.span,
5367+
format!("Discriminant overflowed!")[]);
5368+
}
5369+
} else {
5370+
discriminant = INITIAL_DISCRIMINANT_VALUE;
5371+
}
5372+
}
53635373
};
53645374

53655375
last_discriminant = Some(discriminant);

‎src/librustc/util/nodemap.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl Writer for FnvHasher {
8888
let FnvHasher(mut hash) = *self;
8989
for byte in bytes.iter() {
9090
hash = hash ^ (*byte as u64);
91-
hash = hash * 0x100000001b3;
91+
hash = hash.wrapping_mul(0x100000001b3);
9292
}
9393
*self = FnvHasher(hash);
9494
}

0 commit comments

Comments
 (0)