From 1edfa6c88d99b8cbd72f219da6eb94a05930060e Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 27 Jun 2018 14:43:32 +0100 Subject: [PATCH 1/3] Use a weak lang item for hashmap_random_keys --- src/librustc/middle/lang_items.rs | 2 ++ src/librustc/middle/weak_lang_items.rs | 1 + src/libstd/collections/hash/map.rs | 35 ++++++++++---------------- src/libstd/collections/mod.rs | 29 +++++++++++++++++++++ 4 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/librustc/middle/lang_items.rs b/src/librustc/middle/lang_items.rs index fe676919a7d14..c6046c809fb59 100644 --- a/src/librustc/middle/lang_items.rs +++ b/src/librustc/middle/lang_items.rs @@ -355,6 +355,8 @@ language_item_table! { AlignOffsetLangItem, "align_offset", align_offset_fn; TerminationTraitLangItem, "termination", termination; + + HashMapRandomKeysLangItem, "hashmap_random_keys", hashmap_random_keys; } impl<'a, 'tcx, 'gcx> TyCtxt<'a, 'tcx, 'gcx> { diff --git a/src/librustc/middle/weak_lang_items.rs b/src/librustc/middle/weak_lang_items.rs index 180e75df1a66e..fce42bf47990e 100644 --- a/src/librustc/middle/weak_lang_items.rs +++ b/src/librustc/middle/weak_lang_items.rs @@ -166,4 +166,5 @@ weak_lang_items! { eh_personality, EhPersonalityLangItem, rust_eh_personality; eh_unwind_resume, EhUnwindResumeLangItem, rust_eh_unwind_resume; oom, OomLangItem, rust_oom; + hashmap_random_keys,HashMapRandomKeysLangItem, rust_hashmap_random_keys; } diff --git a/src/libstd/collections/hash/map.rs b/src/libstd/collections/hash/map.rs index 91912e5f2412e..52473b063f239 100644 --- a/src/libstd/collections/hash/map.rs +++ b/src/libstd/collections/hash/map.rs @@ -12,7 +12,6 @@ use self::Entry::*; use self::VacantEntryState::*; use collections::CollectionAllocErr; -use cell::Cell; use borrow::Borrow; use cmp::max; use fmt::{self, Debug}; @@ -21,7 +20,6 @@ use hash::{Hash, Hasher, BuildHasher, SipHasher13}; use iter::{FromIterator, FusedIterator}; use mem::{self, replace}; use ops::{Deref, Index}; -use sys; use super::table::{self, Bucket, EmptyBucket, Fallibility, FullBucket, FullBucketMut, RawTable, SafeHash}; @@ -2592,29 +2590,22 @@ impl RandomState { /// ``` #[inline] #[allow(deprecated)] - // rand #[stable(feature = "hashmap_build_hasher", since = "1.7.0")] pub fn new() -> RandomState { - // Historically this function did not cache keys from the OS and instead - // simply always called `rand::thread_rng().gen()` twice. In #31356 it - // was discovered, however, that because we re-seed the thread-local RNG - // from the OS periodically that this can cause excessive slowdown when - // many hash maps are created on a thread. To solve this performance - // trap we cache the first set of randomly generated keys per-thread. - // - // Later in #36481 it was discovered that exposing a deterministic - // iteration order allows a form of DOS attack. To counter that we - // increment one of the seeds on every RandomState creation, giving - // every corresponding HashMap a different iteration order. - thread_local!(static KEYS: Cell<(u64, u64)> = { - Cell::new(sys::hashmap_random_keys()) - }); + // Use a weak lang item to get random keys without a hard dependency + // on libstd. + #[cfg(not(stage0))] + #[allow(improper_ctypes)] + extern { + #[lang = "hashmap_random_keys"] + #[unwind(allowed)] + fn hashmap_random_keys() -> (u64, u64); + } + #[cfg(stage0)] + unsafe fn hashmap_random_keys() -> (u64, u64) { (0, 0) } - KEYS.with(|keys| { - let (k0, k1) = keys.get(); - keys.set((k0.wrapping_add(1), k1)); - RandomState { k0: k0, k1: k1 } - }) + let (k0, k1) = unsafe { hashmap_random_keys() }; + RandomState { k0: k0, k1: k1 } } } diff --git a/src/libstd/collections/mod.rs b/src/libstd/collections/mod.rs index 8d2c82bc0aa84..1939a1f3445a7 100644 --- a/src/libstd/collections/mod.rs +++ b/src/libstd/collections/mod.rs @@ -455,3 +455,32 @@ pub mod hash_set { #[stable(feature = "rust1", since = "1.0.0")] pub use super::hash::set::*; } + +#[cfg(not(stage0))] +#[cfg_attr(not(test), lang = "hashmap_random_keys")] +#[cfg_attr(test, allow(dead_code))] +fn hashmap_random_keys() -> (u64, u64) { + use sys; + use cell::Cell; + + // Historically this function did not cache keys from the OS and instead + // simply always called `rand::thread_rng().gen()` twice. In #31356 it + // was discovered, however, that because we re-seed the thread-local RNG + // from the OS periodically that this can cause excessive slowdown when + // many hash maps are created on a thread. To solve this performance + // trap we cache the first set of randomly generated keys per-thread. + // + // Later in #36481 it was discovered that exposing a deterministic + // iteration order allows a form of DOS attack. To counter that we + // increment one of the seeds on every RandomState creation, giving + // every corresponding HashMap a different iteration order. + thread_local!(static KEYS: Cell<(u64, u64)> = { + Cell::new(sys::hashmap_random_keys()) + }); + + KEYS.with(|keys| { + let (k0, k1) = keys.get(); + keys.set((k0.wrapping_add(1), k1)); + (k0, k1) + }) +} From 4331ada79d93af9f5bbc487ef9c7cae7d9ce4736 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 27 Jun 2018 14:54:02 +0100 Subject: [PATCH 2/3] Move HashMap to liballoc --- .../collections/hash/bench.rs | 0 .../collections/hash/map.rs | 27 ++++++++++--------- .../collections/hash/mod.rs | 0 .../collections/hash/set.rs | 13 ++++----- .../collections/hash/table.rs | 14 +++++----- src/liballoc/collections/mod.rs | 23 ++++++++++++++++ src/liballoc/lib.rs | 4 ++- .../{collections/mod.rs => collections.rs} | 21 ++------------- src/libstd/lib.rs | 1 - src/test/ui/missing-allocator.rs | 3 +++ 10 files changed, 59 insertions(+), 47 deletions(-) rename src/{libstd => liballoc}/collections/hash/bench.rs (100%) rename src/{libstd => liballoc}/collections/hash/map.rs (99%) rename src/{libstd => liballoc}/collections/hash/mod.rs (100%) rename src/{libstd => liballoc}/collections/hash/set.rs (99%) rename src/{libstd => liballoc}/collections/hash/table.rs (99%) rename src/libstd/{collections/mod.rs => collections.rs} (97%) diff --git a/src/libstd/collections/hash/bench.rs b/src/liballoc/collections/hash/bench.rs similarity index 100% rename from src/libstd/collections/hash/bench.rs rename to src/liballoc/collections/hash/bench.rs diff --git a/src/libstd/collections/hash/map.rs b/src/liballoc/collections/hash/map.rs similarity index 99% rename from src/libstd/collections/hash/map.rs rename to src/liballoc/collections/hash/map.rs index 52473b063f239..0f0379990f808 100644 --- a/src/libstd/collections/hash/map.rs +++ b/src/liballoc/collections/hash/map.rs @@ -12,14 +12,14 @@ use self::Entry::*; use self::VacantEntryState::*; use collections::CollectionAllocErr; -use borrow::Borrow; -use cmp::max; -use fmt::{self, Debug}; +use core::borrow::Borrow; +use core::cmp::max; +use core::fmt::{self, Debug}; #[allow(deprecated)] -use hash::{Hash, Hasher, BuildHasher, SipHasher13}; -use iter::{FromIterator, FusedIterator}; -use mem::{self, replace}; -use ops::{Deref, Index}; +use core::hash::{Hash, Hasher, BuildHasher, SipHasher13}; +use core::iter::{FromIterator, FusedIterator}; +use core::mem::{self, replace}; +use core::ops::{Deref, Index}; use super::table::{self, Bucket, EmptyBucket, Fallibility, FullBucket, FullBucketMut, RawTable, SafeHash}; @@ -2559,7 +2559,7 @@ impl<'a, K, V, S> Extend<(&'a K, &'a V)> for HashMap /// instances are unlikely to produce the same result for the same values. /// /// [`HashMap`]: struct.HashMap.html -/// [`Hasher`]: ../../hash/trait.Hasher.html +/// [`Hasher`]: ../../../std/hash/trait.Hasher.html /// /// # Examples /// @@ -2625,7 +2625,7 @@ impl BuildHasher for RandomState { /// not be relied upon over releases. /// /// [`RandomState`]: struct.RandomState.html -/// [`Hasher`]: ../../hash/trait.Hasher.html +/// [`Hasher`]: ../../../std/hash/trait.Hasher.html #[stable(feature = "hashmap_default_hasher", since = "1.13.0")] #[allow(deprecated)] #[derive(Clone, Debug)] @@ -2759,11 +2759,12 @@ mod test_map { use super::HashMap; use super::Entry::{Occupied, Vacant}; use super::RandomState; - use cell::RefCell; + use core::cell::RefCell; use rand::{thread_rng, Rng}; - use realstd::collections::CollectionAllocErr::*; - use realstd::mem::size_of; - use realstd::usize; + use collections::CollectionAllocErr::*; + use std::mem::size_of; + use std::usize; + use vec::Vec; #[test] fn test_zero_capacities() { diff --git a/src/libstd/collections/hash/mod.rs b/src/liballoc/collections/hash/mod.rs similarity index 100% rename from src/libstd/collections/hash/mod.rs rename to src/liballoc/collections/hash/mod.rs diff --git a/src/libstd/collections/hash/set.rs b/src/liballoc/collections/hash/set.rs similarity index 99% rename from src/libstd/collections/hash/set.rs rename to src/liballoc/collections/hash/set.rs index 5ac3e8f9cf7d3..f9590d49cf068 100644 --- a/src/libstd/collections/hash/set.rs +++ b/src/liballoc/collections/hash/set.rs @@ -8,11 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use borrow::Borrow; -use fmt; -use hash::{Hash, BuildHasher}; -use iter::{Chain, FromIterator, FusedIterator}; -use ops::{BitOr, BitAnd, BitXor, Sub}; +use core::borrow::Borrow; +use core::fmt; +use core::hash::{Hash, BuildHasher}; +use core::iter::{Chain, FromIterator, FusedIterator}; +use core::ops::{BitOr, BitAnd, BitXor, Sub}; use super::Recover; use super::map::{self, HashMap, Keys, RandomState}; @@ -1403,6 +1403,7 @@ fn assert_covariance() { mod test_set { use super::HashSet; use super::super::map::RandomState; + use vec::Vec; #[test] fn test_zero_capacities() { @@ -1712,7 +1713,7 @@ mod test_set { #[test] fn test_replace() { - use hash; + use core::hash; #[derive(Debug)] struct Foo(&'static str, i32); diff --git a/src/libstd/collections/hash/table.rs b/src/liballoc/collections/hash/table.rs similarity index 99% rename from src/libstd/collections/hash/table.rs rename to src/liballoc/collections/hash/table.rs index 2b319186a8db2..16047dcdb7a92 100644 --- a/src/libstd/collections/hash/table.rs +++ b/src/liballoc/collections/hash/table.rs @@ -10,13 +10,13 @@ use alloc::{Global, Alloc, Layout, LayoutErr, handle_alloc_error}; use collections::CollectionAllocErr; -use hash::{BuildHasher, Hash, Hasher}; -use marker; -use mem::{size_of, needs_drop}; -use mem; -use ops::{Deref, DerefMut}; -use ptr::{self, Unique, NonNull}; -use hint; +use core::hash::{BuildHasher, Hash, Hasher}; +use core::marker; +use core::mem::{size_of, needs_drop}; +use core::mem; +use core::ops::{Deref, DerefMut}; +use core::ptr::{self, Unique, NonNull}; +use core::hint; use self::BucketState::*; diff --git a/src/liballoc/collections/mod.rs b/src/liballoc/collections/mod.rs index 96e0eb633b2f5..d98bfcd78fce1 100644 --- a/src/liballoc/collections/mod.rs +++ b/src/liballoc/collections/mod.rs @@ -14,6 +14,7 @@ pub mod binary_heap; mod btree; +mod hash; pub mod linked_list; pub mod vec_deque; @@ -31,6 +32,20 @@ pub mod btree_set { pub use super::btree::set::*; } +#[stable(feature = "rust1", since = "1.0.0")] +pub mod hash_map { + //! A hash map implemented with linear probing and Robin Hood bucket stealing. + #[stable(feature = "rust1", since = "1.0.0")] + pub use super::hash::map::*; +} + +#[stable(feature = "rust1", since = "1.0.0")] +pub mod hash_set { + //! A hash set implemented as a `HashMap` where the value is `()`. + #[stable(feature = "rust1", since = "1.0.0")] + pub use super::hash::set::*; +} + #[stable(feature = "rust1", since = "1.0.0")] #[doc(no_inline)] pub use self::binary_heap::BinaryHeap; @@ -43,6 +58,14 @@ pub use self::btree_map::BTreeMap; #[doc(no_inline)] pub use self::btree_set::BTreeSet; +#[stable(feature = "rust1", since = "1.0.0")] +#[doc(no_inline)] +pub use self::hash_map::HashMap; + +#[stable(feature = "rust1", since = "1.0.0")] +#[doc(no_inline)] +pub use self::hash_set::HashSet; + #[stable(feature = "rust1", since = "1.0.0")] #[doc(no_inline)] pub use self::linked_list::LinkedList; diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 493448eaf88fa..e650cbd9b8b0b 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -69,7 +69,7 @@ html_favicon_url = "https://doc.rust-lang.org/favicon.ico", html_root_url = "https://doc.rust-lang.org/nightly/", issue_tracker_base_url = "https://github.com/rust-lang/rust/issues/", - test(no_crate_inject, attr(allow(unused_variables), deny(warnings))))] + test(no_crate_inject, attr(allow(unused_variables, unused_mut), deny(warnings))))] #![no_std] #![needs_allocator] #![deny(missing_debug_implementations)] @@ -122,6 +122,8 @@ #![feature(inclusive_range_methods)] #![feature(rustc_const_unstable)] #![feature(const_vec_new)] +#![feature(unwind_attributes)] +#![feature(hashmap_internals)] #![cfg_attr(not(test), feature(fn_traits, i128))] #![cfg_attr(test, feature(test))] diff --git a/src/libstd/collections/mod.rs b/src/libstd/collections.rs similarity index 97% rename from src/libstd/collections/mod.rs rename to src/libstd/collections.rs index 1939a1f3445a7..e8ba75b0b0ee4 100644 --- a/src/libstd/collections/mod.rs +++ b/src/libstd/collections.rs @@ -431,31 +431,14 @@ pub use alloc_crate::collections::{LinkedList, VecDeque}; pub use alloc_crate::collections::{binary_heap, btree_map, btree_set}; #[stable(feature = "rust1", since = "1.0.0")] pub use alloc_crate::collections::{linked_list, vec_deque}; - #[stable(feature = "rust1", since = "1.0.0")] -pub use self::hash_map::HashMap; +pub use alloc_crate::collections::{HashMap, HashSet}; #[stable(feature = "rust1", since = "1.0.0")] -pub use self::hash_set::HashSet; +pub use alloc_crate::collections::{hash_map, hash_set}; #[unstable(feature = "try_reserve", reason = "new API", issue="48043")] pub use alloc_crate::collections::CollectionAllocErr; -mod hash; - -#[stable(feature = "rust1", since = "1.0.0")] -pub mod hash_map { - //! A hash map implemented with linear probing and Robin Hood bucket stealing. - #[stable(feature = "rust1", since = "1.0.0")] - pub use super::hash::map::*; -} - -#[stable(feature = "rust1", since = "1.0.0")] -pub mod hash_set { - //! A hash set implemented as a `HashMap` where the value is `()`. - #[stable(feature = "rust1", since = "1.0.0")] - pub use super::hash::set::*; -} - #[cfg(not(stage0))] #[cfg_attr(not(test), lang = "hashmap_random_keys")] #[cfg_attr(test, allow(dead_code))] diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index d73cb1f8349a6..d82abedc2604e 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -264,7 +264,6 @@ #![feature(fnbox)] #![feature(futures_api)] #![feature(generator_trait)] -#![feature(hashmap_internals)] #![feature(int_error_internals)] #![feature(integer_atomics)] #![feature(into_cow)] diff --git a/src/test/ui/missing-allocator.rs b/src/test/ui/missing-allocator.rs index 24282631b7eea..e49ecdef2a04f 100644 --- a/src/test/ui/missing-allocator.rs +++ b/src/test/ui/missing-allocator.rs @@ -23,4 +23,7 @@ fn panic(_: &core::panic::PanicInfo) -> ! { #[lang = "oom"] fn oom() {} +#[lang = "hashmap_random_keys"] +fn hashmap_random_keys() {} + extern crate alloc; From 4d567312141978ead3a8688b2fbc0e20bf76ac5f Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 2 Jul 2018 23:53:15 +0100 Subject: [PATCH 3/3] Make HashMap unstable in liballoc but stable in libstd --- src/liballoc/collections/mod.rs | 8 ++++---- src/libstd/collections.rs | 14 +++++++++++++- src/libstd/lib.rs | 1 + 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/liballoc/collections/mod.rs b/src/liballoc/collections/mod.rs index d98bfcd78fce1..9a19feef23438 100644 --- a/src/liballoc/collections/mod.rs +++ b/src/liballoc/collections/mod.rs @@ -32,14 +32,14 @@ pub mod btree_set { pub use super::btree::set::*; } -#[stable(feature = "rust1", since = "1.0.0")] +#[unstable(feature = "alloc_hashmap", reason = "HashMap in liballoc is unstable", issue="0")] pub mod hash_map { //! A hash map implemented with linear probing and Robin Hood bucket stealing. #[stable(feature = "rust1", since = "1.0.0")] pub use super::hash::map::*; } -#[stable(feature = "rust1", since = "1.0.0")] +#[unstable(feature = "alloc_hashmap", reason = "HashMap in liballoc is unstable", issue="0")] pub mod hash_set { //! A hash set implemented as a `HashMap` where the value is `()`. #[stable(feature = "rust1", since = "1.0.0")] @@ -58,11 +58,11 @@ pub use self::btree_map::BTreeMap; #[doc(no_inline)] pub use self::btree_set::BTreeSet; -#[stable(feature = "rust1", since = "1.0.0")] +#[unstable(feature = "alloc_hashmap", reason = "HashMap in liballoc is unstable", issue="0")] #[doc(no_inline)] pub use self::hash_map::HashMap; -#[stable(feature = "rust1", since = "1.0.0")] +#[unstable(feature = "alloc_hashmap", reason = "HashMap in liballoc is unstable", issue="0")] #[doc(no_inline)] pub use self::hash_set::HashSet; diff --git a/src/libstd/collections.rs b/src/libstd/collections.rs index e8ba75b0b0ee4..2aec29bc7d8c8 100644 --- a/src/libstd/collections.rs +++ b/src/libstd/collections.rs @@ -433,8 +433,20 @@ pub use alloc_crate::collections::{binary_heap, btree_map, btree_set}; pub use alloc_crate::collections::{linked_list, vec_deque}; #[stable(feature = "rust1", since = "1.0.0")] pub use alloc_crate::collections::{HashMap, HashSet}; + +// We can't just re-export the modules directly since they are unstable in liballoc +#[stable(feature = "rust1", since = "1.0.0")] +pub mod hash_map { + //! A hash map implemented with linear probing and Robin Hood bucket stealing. + #[stable(feature = "rust1", since = "1.0.0")] + pub use alloc_crate::collections::hash_map::*; +} #[stable(feature = "rust1", since = "1.0.0")] -pub use alloc_crate::collections::{hash_map, hash_set}; +pub mod hash_set { + //! A hash set implemented as a `HashMap` where the value is `()`. + #[stable(feature = "rust1", since = "1.0.0")] + pub use alloc_crate::collections::hash_set::*; +} #[unstable(feature = "try_reserve", reason = "new API", issue="48043")] pub use alloc_crate::collections::CollectionAllocErr; diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index d82abedc2604e..b2ce6e2eaef40 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -322,6 +322,7 @@ #![feature(float_internals)] #![feature(panic_info_message)] #![feature(panic_implementation)] +#![feature(alloc_hashmap)] #![default_lib_allocator]