use SecRandomCopyBytes on macOS in Miri

[RalfJung]

This is a hack to fix rust-lang/miri#686: on macOS, rustc will open /dev/urandom to initialize a HashMap. That's quite hard to emulate properly in Miri without a full-blown implementation of file descriptors. However, Miri needs an implementation of SecRandomCopyBytes anyway to support getrandom, so using it here should work just as well.

This will only have an effect when libstd is compiled specifically for Miri, but that will generally be the case when people use cargo miri.

This is clearly a hack, so I am opening this to start a discussion about whether we are okay with such a hack or not.

Cc @oli-obk

[rust-highfive]

r? @Kimundi

(rust_highfive has picked a reviewer for you, use r? to override)

[oli-obk]

We can revisit once we are able to open /dev/random with miri

[RalfJung]

Another case where we might want to use cfg(miri) is HashBrown: since that one got merged, Miri does not support HashMap any more.

[01:38:01] error[E0080]: constant evaluation error: tried to call a function with ABI RustIntrinsic using caller ABI PlatformIntrinsic
[01:38:01]    --> /checkout/src/libcore/../stdsimd/crates/core_arch/src/x86/sse2.rs:820:27
[01:38:01]     |
[01:38:01] 820 |     transmute::<i8x16, _>(simd_eq(a.as_i8x16(), b.as_i8x16()))
[01:38:01]     |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ tried to call a function with ABI RustIntrinsic using caller ABI PlatformIntrinsic
[01:38:01]     |
[01:38:01]     = note: inside call to `std::arch::x86_64::_mm_cmpeq_epi8` at /cargo/registry/src/github.com-1ecc6299db9ec823/hashbrown-0.3.0/src/raw/sse2.rs:83:23
[01:38:01]     = note: inside call to `hashbrown::raw::imp::Group::match_byte` at /cargo/registry/src/github.com-1ecc6299db9ec823/hashbrown-0.3.0/src/raw/mod.rs:853:28
[01:38:01]     = note: inside call to `hashbrown::raw::RawTable::<(i32, i32)>::find::<[closure@DefId(8/1:34 ~ hashbrown[269a]::map[0]::{{impl}}[2]::insert[0]::{{closure}}[0]) 0:&i32]>` at /cargo/registry/src/github.com-1ecc6299db9ec823/hashbrown-0.3.0/src/map.rs:844:33
[01:38:01]     = note: inside call to `hashbrown::map::HashMap::<i32, i32, std::collections::hash_map::RandomState>::insert` at /checkout/src/libstd/collections/hash/map.rs:821:9
[01:38:01] note: inside call to `std::collections::HashMap::<i32, i32>::insert` at $DIR/hashmap.rs:7:5
[01:38:01]    --> $DIR/hashmap.rs:7:5
[01:38:01]     |
[01:38:01] 7   |     map.insert(0, 0);
[01:38:01]     |     ^^^^^^^^^^^^^^^^

Cc @Amanieu

[Amanieu]

There is already a cfg(miri) here. Maybe the cfg isn't being passed when building sysroot crates?

[RalfJung]

Ah... yes we'd probably have to add that here.

[RalfJung]

@rust-lang/libs what do you think, is it reasonable to use cfg(miri) in libstd itself (like it turns out hashbrown already does)?

[sfackler]

If we're going to go the cfg(miri) route, it seems like we could do that at a higher level and replace the top-level randomness interface entirely? That way miri wouldn't have to emulate SecRandomCopyBytes and getrandom and whatever Windows uses, etc.

[RalfJung]

Probably. However, then we'd have to figure out what it is that Miri would have to hook instead.

Also, Miri would still need to emulate all of these for the getrandom crate (or that one would have to be patched as well). Right now, SecRandomCopyBytes and whatever Windows uses are already implemented in Miri.

[Centril]

r? @oli-obk

[alexcrichton]

I'd agree with @sfackler that this seems like something where over time we'll want dedicated support for this rather than a few #[cfg] here and there. For now though I don't think we've crossed that threshold so if @RalfJung prefers to take this route it seems reasonable. We should remain watchful though in the sense that if we find we're taking a lot of these PRs we may want to add a first-class miri target sorta

[RalfJung]

We should remain watchful though in the sense that if we find we're taking a lot of these PRs we may want to add a first-class miri target sorta

Alternatively, maybe Miri can reuse some of the work for WASI here. The constraints of both seem to be similar.

For now though, I think this is good enough.

@bors r=oli-obk,alexcrichton

[bors]

📌 Commit 16ad977 has been approved by oli-obk,alexcrichton

[bors]

⌛ Testing commit 16ad977 with merge 758dc9a...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk,alexcrichton
Pushing 758dc9a to master...