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

Implement libstd HashMap seeding on OS X #686

Closed
Aaron1011 opened this issue Apr 11, 2019 · 13 comments · Fixed by rust-lang/rust#60156 or #1134
Closed

Implement libstd HashMap seeding on OS X #686

Aaron1011 opened this issue Apr 11, 2019 · 13 comments · Fixed by rust-lang/rust#60156 or #1134
Labels
A-mac Area: Affects only macOS targets A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement

Comments

@Aaron1011
Copy link
Member

PR #683 implemented support for random number generation on Windows and (relatively recent) Linux. This was accomplished by implementing RtlGenRandom and getrandom for Windows and Linux respectively.

For OS X (and other Unix systems without getrandom), implementing random number generation is significantly more complicated. We need to implement the following:

  • Calling open/openat with /dev/random and /dev/urandom
  • Calling read and close on the 'file descriptor' created above.

On Linux, file descriptors appear to be assigned sequentially. However, I don't believe that anything actually guarantees this - in other words, this could potentially be considered a source of non-determinism.

@RalfJung RalfJung added A-mac Area: Affects only macOS targets A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement labels Apr 12, 2019
@RalfJung
Copy link
Member

Uh, yes, this will be awful.

Alternatively we could "just" grant the interpreter FS access. But of course that would break cross-execution.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 12, 2019

In order to keep file descriptors deterministic, we can use "symbolic file descriptors" (generate a new AllocId on every open call)

@RalfJung
Copy link
Member

What is the point, if we allow programs to open and read files they are not deterministic any more anyway?

For a given seed, we'll always assign the same file descriptors. Small changes elsewhere in the program might affect that, but the same is true for the non-determinism that we return.

@Aaron1011
Copy link
Member Author

This is actually going to be really simple once rust-lang/rust#59879 is merged, since SecRandomCopyBytes is essentially the same as all of the other random-generating functions.

@etaoins
Copy link

etaoins commented Apr 17, 2019

@Aaron1011

Assuming there are no other open file descriptors the sequential allocation is guaranteed by POSIX:

The open() function shall return a file descriptor for the named file that is the lowest file descriptor not currently open for that process.

@uberjay
Copy link

uberjay commented Apr 17, 2019

This is actually going to be really simple once rust-lang/rust#59879 is merged, since SecRandomCopyBytes is essentially the same as all of the other random-generating functions.

Unfortunately it turns out that SecRandomCopyBytes was not such a great solution after all: rust-lang/rust#59879 (comment). :(

@RalfJung
Copy link
Member

Dang. :/

There is no precedent for this, but in principle we could also patch libstd to use the miri config flag as an indication to follow the iOS code path. Then we could still rely on SecRandomCopyBytes.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 19, 2019

I think for a bunch of these APIs we should not reproduce the platform APIs, but add some attributes to a high level function that all platforms go through and hook miri into that.

@RalfJung
Copy link
Member

Actually, for the rand crate, SecRandomCopyBytes is being used (through https://github.com/rust-random/getrandom/blob/master/src/macos.rs). So implementing that would still help.

@RalfJung
Copy link
Member

See #709 and rust-lang/rust#60156

bors added a commit to rust-lang/rust that referenced this issue May 2, 2019
use SecRandomCopyBytes on macOS in Miri

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](https://crates.io/crates/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
@RalfJung RalfJung changed the title Implement random number generation on OS X Implement libstd HashMap seeding on OS X May 13, 2019
@RalfJung
Copy link
Member

Status update: libstd is moving to using the getrandom crate for seeding HashMap. That would unify that code path with what the rand crate uses. Currently getrandom works on macOS in Miri because they use SecRandomCopyBytes, but that has been causing problems and they are still discussing what to do on macOS.

@RalfJung
Copy link
Member

RalfJung commented Jun 29, 2019

The current plan for getrandom seems to be to use the getentropy syscall, loaded via dlsym. So if we could revive and build on @oli-obk's hack for getting dlsym function pointers to work in Miri, that should actually work. :)

@RalfJung
Copy link
Member

RalfJung commented Jul 16, 2019

Current status: should be fixed by rust-lang/rust#62082, but it may be a while until that lands.

jrmuizel added a commit to jrmuizel/bacon-rajan-cc that referenced this issue Jul 27, 2019
HashMap doesn't work with miri on macOS yet because of
rust-lang/miri#686
jrmuizel added a commit to jrmuizel/bacon-rajan-cc that referenced this issue Jul 27, 2019
HashMap doesn't work with miri on macOS yet because of
rust-lang/miri#686
jrmuizel added a commit to fitzgen/bacon-rajan-cc that referenced this issue Jul 28, 2019
HashMap doesn't work with miri on macOS yet because of
rust-lang/miri#686
@bors bors closed this as completed in 86d7db4 Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mac Area: Affects only macOS targets A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement
Projects
None yet
5 participants