-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Use getrandom crate #62082
Changes from 8 commits
28ac843
621ef04
98be6f7
79ffb24
26a1108
66e799d
b6c7673
0056e1d
67ea529
e6d3ed3
568e492
560dd3e
0a63f37
7ac37e2
facdc28
5d0ae59
1fdf620
f94d928
dd9af37
4f3e2a0
93c89fe
f584f92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2445,13 +2445,20 @@ 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]; | ||
// in case of `Error::UNAVAILABLE` we use a constant value | ||
match getrandom::getrandom(&mut buf) { | ||
Ok(()) | Err(Error::UNAVAILABLE) => (), | ||
Err(err) => panic!("getrandom failure: {:?}", err), | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On if false {
getrandom::getrandom(&mut buf).unwrap();
} ... so it does require the If you There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(..)]
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 } | ||
}) | ||
} | ||
|
This file was deleted.
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caveat: this won't work on bare WASM since
getrandom
currently requires a feature flag to enable eitherstdweb
orwasm_bindgen
. There was mention of makingwasm_bindgen
the default; we might want to do this first?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would prefer if a separate target will be introduced and
wasm32-unknown-wunknown
will stay assumptions-free regarding executor environment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC without enabled
stdweb
orwasm_bindgen
features outside of WASI and Emscripten it will use a dummy impl, so it will work in the same way as it does today by using a constant seed value.