Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use getrandom crate #62082

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1050,11 +1050,14 @@ dependencies = [

[[package]]
name = "getrandom"
version = "0.1.8"
version = "0.1.9"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update Cargo.lock here, as 0.1.9 has been yanked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can wait until landing of VxWorks support.

source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"cfg-if 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
"compiler_builtins 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.60 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-std-workspace-core 1.0.0",
"wasi 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
Expand Down Expand Up @@ -2246,7 +2249,7 @@ name = "rand"
version = "0.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"getrandom 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
"getrandom 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.60 (registry+https://github.com/rust-lang/crates.io-index)",
"rand_chacha 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
"rand_core 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -2286,7 +2289,7 @@ name = "rand_core"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"getrandom 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
"getrandom 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
Expand Down Expand Up @@ -3534,6 +3537,7 @@ dependencies = [
"core 0.0.0",
"dlmalloc 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"fortanix-sgx-abi 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
"getrandom 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)",
"hashbrown 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.60 (registry+https://github.com/rust-lang/crates.io-index)",
"panic_abort 0.0.0",
Expand Down Expand Up @@ -4269,6 +4273,11 @@ dependencies = [
"try-lock 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "wasi"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"

[[package]]
name = "winapi"
version = "0.2.8"
Expand Down Expand Up @@ -4443,7 +4452,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum fwdansi 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "34dd4c507af68d37ffef962063dfa1944ce0dd4d5b82043dbab1dabe088610c3"
"checksum generic-array 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ef25c5683767570c2bbd7deba372926a55eaae9982d7726ee2a1050239d45b9d"
"checksum getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)" = "72327b15c228bfe31f1390f93dd5e9279587f0463836393c9df719ce62a3e450"
"checksum getrandom 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "34f33de6f0ae7c9cb5e574502a562e2b512799e32abb801cd1e79ad952b62b49"
"checksum getrandom 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)" = "2512b3191f22e2763a5db387f1c9409379772e2050841722eb4a8c4f497bf096"
"checksum git2 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)" = "8cb400360e8a4d61b10e648285bbfa919bbf9519d0d5d5720354456f44349226"
"checksum git2-curl 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2293de73491c3dc4174c5949ef53d2cc037b27613f88d72032e3f5237247a7dd"
"checksum glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
Expand Down Expand Up @@ -4703,6 +4712,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum vte 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "4f42f536e22f7fcbb407639765c8fd78707a33109301f834a594758bedd6e8cf"
"checksum walkdir 2.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "9d9d7ed3431229a144296213105a390676cc49c9b6a72bd19f3176c98e129fa1"
"checksum want 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b6395efa4784b027708f7451087e647ec73cc74f5d9bc2e418404248d679a230"
"checksum wasi 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fd5442abcac6525a045cc8c795aedb60da7a2e5e89c7bf18a0d5357849bb23c7"
"checksum winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a"
"checksum winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "92c1eb33641e276cfa214a0522acad57be5c56b10cb348b3c5117db75f3ac4b0"
"checksum winapi-build 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2d315eee3b34aca4797b2da6b13ed88266e6d612562a0c46390af8299fc699bc"
Expand Down
3 changes: 3 additions & 0 deletions src/libstd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ profiler_builtins = { path = "../libprofiler_builtins", optional = true }
unwind = { path = "../libunwind" }
hashbrown = { version = "0.4.0", features = ['rustc-dep-of-std'] }

[target.'cfg(not(all(target_arch = "wasm32", target_vendor = "unknown", target_os = "unknown")))'.dependencies]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the usage of if cfg! the getrandom crate needs to be avialable on wasm32-unknown-unknown, and I think that's fine in that this header shouldn't be necessary anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without an enabled dummy,stdweb or wasm-bindgen feature the current version of getrandom will emit a compilation error when compiled for wasm32-unknown-unknown. If I am not mistaken without this config getrandom will be compiled unconditionally even if it was not used in the crate.

getrandom = { version = "0.1.9", features = ['rustc-dep-of-std'] }

[dependencies.backtrace]
version = "0.3.34"
default-features = false # don't use coresymbolication on OSX
Expand Down
2 changes: 0 additions & 2 deletions src/libstd/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ fn main() {
println!("cargo:rustc-link-lib=resolv");
} else if target.contains("uwp") {
println!("cargo:rustc-link-lib=ws2_32");
// For BCryptGenRandom
println!("cargo:rustc-link-lib=bcrypt");
} else if target.contains("windows") {
println!("cargo:rustc-link-lib=advapi32");
println!("cargo:rustc-link-lib=ws2_32");
Expand Down
21 changes: 16 additions & 5 deletions src/libstd/collections/hash/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use crate::fmt::{self, Debug};
use crate::hash::{BuildHasher, Hash, Hasher, SipHasher13};
use crate::iter::{FromIterator, FusedIterator};
use crate::ops::Index;
use crate::sys;

/// A hash map implemented with quadratic probing and SIMD lookup.
///
Expand Down Expand Up @@ -2445,13 +2444,25 @@ impl RandomState {
// 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())
thread_local!(static KEYS: Cell<[u64; 2]> = {
let mut buf = [0u8; 16];
// Use a constant seed on wasm32-unknown-unknown.
// `cfg(target = "..")` does not work right now, see:
// https://github.com/rust-lang/rust/issues/63217
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second part of this comment can be removed, it's fine to just say that on wasm32-unknown-unknown we skip this.

if cfg!(not(all(
target_arch = "wasm32",
target_vendor = "unknown",
target_os = "unknown",
))) {
getrandom::getrandom(&mut buf).unwrap();
}
Copy link
Member

@cuviper cuviper Aug 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On wasm32-unknown-unknown, this will effectively be:

if false {
    getrandom::getrandom(&mut buf).unwrap();
}

... so it does require the getrandom dependency, even though this is dead code.

If you #[cfg(...)] gate this expression instead, it won't always need getrandom.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But IIUC Cargo determines crates to be compiled only by looking at Cargo.toml, without taking crate code into account. So even if a dependency is not used, it will be still compiled, which in our case means a compilation error.

I forgot that we can use cfg like this:

#[cfg(..)]
getrandom::getrandom(&mut buf).unwrap();

I will replace those lines in the next commit.

let n = u128::from_ne_bytes(buf);
Cell::new([n as u64, (n >> 64) as u64])
});

KEYS.with(|keys| {
let (k0, k1) = keys.get();
keys.set((k0.wrapping_add(1), k1));
let [k0, k1] = keys.get();
keys.set([k0.wrapping_add(1), k1]);
RandomState { k0: k0, k1: k1 }
})
}
Expand Down
11 changes: 0 additions & 11 deletions src/libstd/sys/cloudabi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,3 @@ pub unsafe fn abort_internal() -> ! {
}

pub use libc::strlen;

pub fn hashmap_random_keys() -> (u64, u64) {
unsafe {
let mut v: mem::MaybeUninit<(u64, u64)> = mem::MaybeUninit::uninit();
libc::arc4random_buf(
v.as_mut_ptr() as *mut libc::c_void,
mem::size_of_val(&v)
);
v.assume_init()
}
}
15 changes: 0 additions & 15 deletions src/libstd/sys/sgx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,21 +137,6 @@ pub unsafe extern "C" fn __rust_abort() {
abort_internal();
}

pub fn hashmap_random_keys() -> (u64, u64) {
fn rdrand64() -> u64 {
unsafe {
let mut ret: u64 = 0;
for _ in 0..10 {
if crate::arch::x86_64::_rdrand64_step(&mut ret) == 1 {
return ret;
}
}
rtabort!("Failed to obtain random data");
}
}
(rdrand64(), rdrand64())
}

pub use crate::sys_common::{AsInner, FromInner, IntoInner};

pub trait TryIntoInner<Inner>: Sized {
Expand Down
2 changes: 0 additions & 2 deletions src/libstd/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use crate::io::ErrorKind;
#[cfg(all(not(rustdoc), target_os = "hermit"))] pub use crate::os::hermit as platform;
#[cfg(all(not(rustdoc), target_os = "redox"))] pub use crate::os::redox as platform;

pub use self::rand::hashmap_random_keys;
pub use libc::strlen;

#[macro_use]
Expand Down Expand Up @@ -48,7 +47,6 @@ pub mod os;
pub mod path;
pub mod pipe;
pub mod process;
pub mod rand;
pub mod rwlock;
pub mod stack_overflow;
pub mod thread;
Expand Down
189 changes: 0 additions & 189 deletions src/libstd/sys/unix/rand.rs

This file was deleted.

2 changes: 0 additions & 2 deletions src/libstd/sys/vxworks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use crate::io::ErrorKind;

pub use crate::os::vxworks as platform;
pub use self::rand::hashmap_random_keys;
pub use libc::strlen;

pub mod alloc;
Expand All @@ -24,7 +23,6 @@ pub mod os;
pub mod path;
pub mod pipe;
pub mod process;
pub mod rand;
pub mod rwlock;
pub mod stack_overflow;
pub mod thread;
Expand Down
Loading