From 9febb895a8bbefb8ed673b3553f79e1499fc0ca4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 15 May 2015 09:18:14 -0700 Subject: [PATCH 1/6] std: Make abs() panic on overflow in debug mode Debug overflow checks for arithmetic negation landed in #24500, at which time the `abs` method on signed integers was changed to using `wrapping_neg` to ensure that the function never panicked. This implied that `abs` of `INT_MIN` would return `INT_MIN`, another negative value. When this change was back-ported to beta, however, in #24708, the `wrapping_neg` function had not yet been backported, so the implementation was changed in #24785 to `!self + 1`. This change had the unintended side effect of enabling debug overflow checks for the `abs` function. Consequently, the current state of affairs is that the beta branch checks for overflow in debug mode for `abs` and the nightly branch does not. This commit alters the behavior of nightly to have `abs` always check for overflow in debug mode. This change is more consistent with the way the standard library treats overflow as well, and it is also not a breaking change as it's what the beta branch currently does (albeit if by accident). cc #25378 --- src/libcore/num/mod.rs | 15 ++++++++++++--- src/test/run-pass/int-abs-overflow.rs | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 src/test/run-pass/int-abs-overflow.rs diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 011830ddb7882..bd7286dfa3fa5 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -563,13 +563,22 @@ macro_rules! int_impl { acc } - /// Computes the absolute value of `self`. `Int::min_value()` will be - /// returned if the number is `Int::min_value()`. + /// Computes the absolute value of `self`. + /// + /// # Overflow behavior + /// + /// The absolute value of `i32::min_value()` cannot be represented as an + /// `i32`, and attempting to calculate it will cause an overflow. This + /// means that code in debug mode will trigger a panic on this case and + /// optimized code will return `i32::min_value()` without a panic. #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn abs(self) -> $T { if self.is_negative() { - self.wrapping_neg() + // Note that the #[inline] above means that the overflow + // semantics of this negation depend on the crate we're being + // inlined into. + -self } else { self } diff --git a/src/test/run-pass/int-abs-overflow.rs b/src/test/run-pass/int-abs-overflow.rs new file mode 100644 index 0000000000000..3f50a7d6c0298 --- /dev/null +++ b/src/test/run-pass/int-abs-overflow.rs @@ -0,0 +1,21 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z force-overflow-checks=on + +use std::thread; + +fn main() { + assert!(thread::spawn(|| i8::min_value().abs()).join().is_err()); + assert!(thread::spawn(|| i16::min_value().abs()).join().is_err()); + assert!(thread::spawn(|| i32::min_value().abs()).join().is_err()); + assert!(thread::spawn(|| i64::min_value().abs()).join().is_err()); + assert!(thread::spawn(|| isize::min_value().abs()).join().is_err()); +} From 0e38e7676ef5ea687d9d47f76442a43bbc51c05a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 16 May 2015 11:27:08 -0700 Subject: [PATCH 2/6] std: Fix missing stability on iter::Cloned The method was stabilized but the structure was forgotten to be stabilized. Closes #25480 --- src/libcore/iter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index cab79d938c374..091342226e0ec 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -1370,7 +1370,7 @@ impl MinMaxResult { } /// An iterator that clones the elements of an underlying iterator -#[unstable(feature = "core", reason = "recent addition")] +#[stable(feature = "iter_cloned", since = "1.1.0")] #[must_use = "iterator adaptors are lazy and do nothing unless consumed"] #[derive(Clone)] pub struct Cloned { From 322ae054605d354ace2f1a5b5221143581c082b5 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 16 May 2015 22:24:13 -0700 Subject: [PATCH 3/6] std: Reexport std::net::tcp::Incoming This iterator was mistakenly not reexported at the top level, preventing actually naming the type! Closes #25519 --- src/libstd/net/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/net/mod.rs b/src/libstd/net/mod.rs index bff9774bcd04a..a79a451305daf 100644 --- a/src/libstd/net/mod.rs +++ b/src/libstd/net/mod.rs @@ -19,7 +19,7 @@ use sys_common::net as net_imp; pub use self::ip::{IpAddr, Ipv4Addr, Ipv6Addr, Ipv6MulticastScope}; pub use self::addr::{SocketAddr, SocketAddrV4, SocketAddrV6, ToSocketAddrs}; -pub use self::tcp::{TcpStream, TcpListener}; +pub use self::tcp::{TcpStream, TcpListener, Incoming}; pub use self::udp::UdpSocket; pub use self::parser::AddrParseError; From 2cc6e8fb538d5a629501e2a55e439cf4c8f98b2c Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Thu, 28 May 2015 10:44:31 +0200 Subject: [PATCH 4/6] collections: Make BinaryHeap panic safe in sift_up / sift_down Use a struct called Hole that keeps track of an invalid location in the vector and fills the hole on drop. I include a run-pass test that the current BinaryHeap fails, and the new one passes. Fixes #25842 --- src/libcollections/binary_heap.rs | 113 +++++++++++++++----- src/test/run-pass/binary-heap-panic-safe.rs | 108 +++++++++++++++++++ 2 files changed, 196 insertions(+), 25 deletions(-) create mode 100644 src/test/run-pass/binary-heap-panic-safe.rs diff --git a/src/libcollections/binary_heap.rs b/src/libcollections/binary_heap.rs index af7112a5cb4e3..00e4002f82f4f 100644 --- a/src/libcollections/binary_heap.rs +++ b/src/libcollections/binary_heap.rs @@ -153,7 +153,7 @@ use core::prelude::*; use core::iter::{FromIterator}; -use core::mem::{zeroed, replace, swap}; +use core::mem::swap; use core::ptr; use slice; @@ -484,46 +484,42 @@ impl BinaryHeap { // The implementations of sift_up and sift_down use unsafe blocks in // order to move an element out of the vector (leaving behind a - // zeroed element), shift along the others and move it back into the - // vector over the junk element. This reduces the constant factor - // compared to using swaps, which involves twice as many moves. - fn sift_up(&mut self, start: usize, mut pos: usize) { + // hole), shift along the others and move the removed element back into the + // vector at the final location of the hole. + // The `Hole` type is used to represent this, and make sure + // the hole is filled back at the end of its scope, even on panic. + // Using a hole reduces the constant factor compared to using swaps, + // which involves twice as many moves. + fn sift_up(&mut self, start: usize, pos: usize) { unsafe { - let new = replace(&mut self.data[pos], zeroed()); + // Take out the value at `pos` and create a hole. + let mut hole = Hole::new(&mut self.data, pos); - while pos > start { - let parent = (pos - 1) >> 1; - - if new <= self.data[parent] { break; } - - let x = replace(&mut self.data[parent], zeroed()); - ptr::write(&mut self.data[pos], x); - pos = parent; + while hole.pos() > start { + let parent = (hole.pos() - 1) / 2; + if hole.removed() <= hole.get(parent) { break } + hole.move_to(parent); } - ptr::write(&mut self.data[pos], new); } } fn sift_down_range(&mut self, mut pos: usize, end: usize) { + let start = pos; unsafe { - let start = pos; - let new = replace(&mut self.data[pos], zeroed()); - + let mut hole = Hole::new(&mut self.data, pos); let mut child = 2 * pos + 1; while child < end { let right = child + 1; - if right < end && !(self.data[child] > self.data[right]) { + if right < end && !(hole.get(child) > hole.get(right)) { child = right; } - let x = replace(&mut self.data[child], zeroed()); - ptr::write(&mut self.data[pos], x); - pos = child; - child = 2 * pos + 1; + hole.move_to(child); + child = 2 * hole.pos() + 1; } - ptr::write(&mut self.data[pos], new); - self.sift_up(start, pos); + pos = hole.pos; } + self.sift_up(start, pos); } fn sift_down(&mut self, pos: usize) { @@ -554,6 +550,73 @@ impl BinaryHeap { pub fn clear(&mut self) { self.drain(); } } +/// Hole represents a hole in a slice i.e. an index without valid value +/// (because it was moved from or duplicated). +/// In drop, `Hole` will restore the slice by filling the hole +/// position with the value that was originally removed. +struct Hole<'a, T: 'a> { + data: &'a mut [T], + /// `elt` is always `Some` from new until drop. + elt: Option, + pos: usize, +} + +impl<'a, T> Hole<'a, T> { + /// Create a new Hole at index `pos`. + fn new(data: &'a mut [T], pos: usize) -> Self { + unsafe { + let elt = ptr::read(&data[pos]); + Hole { + data: data, + elt: Some(elt), + pos: pos, + } + } + } + + #[inline(always)] + fn pos(&self) -> usize { self.pos } + + /// Return a reference to the element removed + #[inline(always)] + fn removed(&self) -> &T { + self.elt.as_ref().unwrap() + } + + /// Return a reference to the element at `index`. + /// + /// Panics if the index is out of bounds. + /// + /// Unsafe because index must not equal pos. + #[inline(always)] + unsafe fn get(&self, index: usize) -> &T { + debug_assert!(index != self.pos); + &self.data[index] + } + + /// Move hole to new location + /// + /// Unsafe because index must not equal pos. + #[inline(always)] + unsafe fn move_to(&mut self, index: usize) { + debug_assert!(index != self.pos); + let index_ptr: *const _ = &self.data[index]; + let hole_ptr = &mut self.data[self.pos]; + ptr::copy_nonoverlapping(index_ptr, hole_ptr, 1); + self.pos = index; + } +} + +impl<'a, T> Drop for Hole<'a, T> { + fn drop(&mut self) { + // fill the hole again + unsafe { + let pos = self.pos; + ptr::write(&mut self.data[pos], self.elt.take().unwrap()); + } + } +} + /// `BinaryHeap` iterator. #[stable(feature = "rust1", since = "1.0.0")] pub struct Iter <'a, T: 'a> { diff --git a/src/test/run-pass/binary-heap-panic-safe.rs b/src/test/run-pass/binary-heap-panic-safe.rs new file mode 100644 index 0000000000000..4888a8b84fc42 --- /dev/null +++ b/src/test/run-pass/binary-heap-panic-safe.rs @@ -0,0 +1,108 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(std_misc, collections, catch_panic, rand)] + +use std::__rand::{thread_rng, Rng}; +use std::thread; + +use std::collections::BinaryHeap; +use std::cmp; +use std::sync::Arc; +use std::sync::Mutex; +use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; + +static DROP_COUNTER: AtomicUsize = ATOMIC_USIZE_INIT; + +// old binaryheap failed this test +// +// Integrity means that all elements are present after a comparison panics, +// even if the order may not be correct. +// +// Destructors must be called exactly once per element. +fn test_integrity() { + #[derive(Eq, PartialEq, Ord, Clone, Debug)] + struct PanicOrd(T, bool); + + impl Drop for PanicOrd { + fn drop(&mut self) { + // update global drop count + DROP_COUNTER.fetch_add(1, Ordering::SeqCst); + } + } + + impl PartialOrd for PanicOrd { + fn partial_cmp(&self, other: &Self) -> Option { + if self.1 || other.1 { + panic!("Panicking comparison"); + } + self.0.partial_cmp(&other.0) + } + } + let mut rng = thread_rng(); + const DATASZ: usize = 32; + const NTEST: usize = 10; + + // don't use 0 in the data -- we want to catch the zeroed-out case. + let data = (1..DATASZ + 1).collect::>(); + + // since it's a fuzzy test, run several tries. + for _ in 0..NTEST { + for i in 1..DATASZ + 1 { + DROP_COUNTER.store(0, Ordering::SeqCst); + + let mut panic_ords: Vec<_> = data.iter() + .filter(|&&x| x != i) + .map(|&x| PanicOrd(x, false)) + .collect(); + let panic_item = PanicOrd(i, true); + + // heapify the sane items + rng.shuffle(&mut panic_ords); + let heap = Arc::new(Mutex::new(BinaryHeap::from_vec(panic_ords))); + let inner_data; + + { + let heap_ref = heap.clone(); + + + // push the panicking item to the heap and catch the panic + let thread_result = thread::catch_panic(move || { + heap.lock().unwrap().push(panic_item); + }); + assert!(thread_result.is_err()); + + // Assert no elements were dropped + let drops = DROP_COUNTER.load(Ordering::SeqCst); + //assert!(drops == 0, "Must not drop items. drops={}", drops); + + { + // now fetch the binary heap's data vector + let mutex_guard = match heap_ref.lock() { + Ok(x) => x, + Err(poison) => poison.into_inner(), + }; + inner_data = mutex_guard.clone().into_vec(); + } + } + let drops = DROP_COUNTER.load(Ordering::SeqCst); + assert_eq!(drops, DATASZ); + + let mut data_sorted = inner_data.into_iter().map(|p| p.0).collect::>(); + data_sorted.sort(); + assert_eq!(data_sorted, data); + } + } +} + +fn main() { + test_integrity(); +} + From 1c0b2f42c86a12dd49b164ee0df97208a9cee20e Mon Sep 17 00:00:00 2001 From: Alexis Beingessner Date: Thu, 4 Jun 2015 17:18:27 -0700 Subject: [PATCH 5/6] properly null out ptr in LinkedList::split_off - fixes #26020 --- src/libcollections/linked_list.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/libcollections/linked_list.rs b/src/libcollections/linked_list.rs index f6dc5cf7d90a0..9aae6192317f1 100644 --- a/src/libcollections/linked_list.rs +++ b/src/libcollections/linked_list.rs @@ -616,7 +616,13 @@ impl LinkedList { length: len - at }; + // Swap split_node.next with list_head (which is None), nulling out split_node.next, + // as it is the new tail. mem::swap(&mut split_node.resolve().unwrap().next, &mut splitted_list.list_head); + // Null out list_head.prev. Note this `unwrap` won't fail because if at == len + // we already branched out at the top of the fn to return the empty list. + splitted_list.list_head.as_mut().unwrap().prev = Rawlink::none(); + // Fix the tail ptr self.list_tail = split_node; self.length = at; @@ -1082,6 +1088,26 @@ mod tests { } } + #[test] + fn test_26021() { + use std::iter::ExactSizeIterator; + // There was a bug in split_off that failed to null out the RHS's head's prev ptr. + // This caused the RHS's dtor to walk up into the LHS at drop and delete all of + // its nodes. + // + // https://github.com/rust-lang/rust/issues/26021 + let mut v1 = LinkedList::new(); + v1.push_front(1u8); + v1.push_front(1u8); + v1.push_front(1u8); + v1.push_front(1u8); + let _ = v1.split_off(3); // Dropping this now should not cause laundry consumption + assert_eq!(v1.len(), 3); + + assert_eq!(v1.iter().len(), 3); + assert_eq!(v1.iter().collect::>().len(), 3); + } + #[cfg(test)] fn fuzz_test(sz: i32) { let mut m: LinkedList<_> = LinkedList::new(); From 48ff33c4ea4178e12076a2a1315e6a24cc7ffe93 Mon Sep 17 00:00:00 2001 From: Johannes Oertel Date: Wed, 27 May 2015 11:28:41 +0200 Subject: [PATCH 6/6] Remove build date from the output of --version Closes #25812. --- mk/main.mk | 4 ---- src/librustc_driver/lib.rs | 5 ----- 2 files changed, 9 deletions(-) diff --git a/mk/main.mk b/mk/main.mk index 63849abc73161..878c2db84d641 100644 --- a/mk/main.mk +++ b/mk/main.mk @@ -74,9 +74,6 @@ ifneq ($(wildcard $(subst $(SPACE),\$(SPACE),$(CFG_GIT_DIR))),) endif endif -CFG_BUILD_DATE = $(shell date +%F) -CFG_VERSION += (built $(CFG_BUILD_DATE)) - # Windows exe's need numeric versions - don't use anything but # numbers and dots here CFG_VERSION_WIN = $(CFG_RELEASE_NUM) @@ -333,7 +330,6 @@ endif ifdef CFG_VER_HASH export CFG_VER_HASH endif -export CFG_BUILD_DATE export CFG_VERSION export CFG_VERSION_WIN export CFG_RELEASE diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index a618f4b6ef675..1bdfa532beb3d 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -483,10 +483,6 @@ pub fn commit_date_str() -> Option<&'static str> { option_env!("CFG_VER_DATE") } -pub fn build_date_str() -> Option<&'static str> { - option_env!("CFG_BUILD_DATE") -} - /// Prints version information and returns None on success or an error /// message on panic. pub fn version(binary: &str, matches: &getopts::Matches) { @@ -498,7 +494,6 @@ pub fn version(binary: &str, matches: &getopts::Matches) { println!("binary: {}", binary); println!("commit-hash: {}", unw(commit_hash_str())); println!("commit-date: {}", unw(commit_date_str())); - println!("build-date: {}", unw(build_date_str())); println!("host: {}", config::host_triple()); println!("release: {}", unw(release_str())); }