[libcxx-commits] [PATCH] D116045: [libc++] Use arc4random instead of /dev/urandom on Apple platforms

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Turns out this is an ABI break:

1. `random_device` contains this, so we'd be changing the layout of the type:

  class random_device {
  #ifdef _LIBCPP_USING_DEV_RANDOM
      int __f_;
  #endif
      ....
  };



2. When using `arc4random`, `std::random_device`'s constructor will throw whenever the token isn't exactly `"/dev/urandom"`. Previously, it would work with any token that represents a path that we can `std::open()`.

If we're willing to break (2), then I think there's definitely a way we can still roll this out by retaining the same field in the struct.

Are we willing to break (2)? Well, http://eel.is/c++draft/rand.device#lib:random_device,constructor says:

> Throws: A value of an implementation-defined type derived from exception if the `random_device` cannot be initialized.
> Remarks: The semantics of the `token` parameter and the token value used by the default constructor are implementation-defined.

So technically, users should not be expecting a whole lot of portability if they are using these custom tokens. In practice, I suspect users don't really provide a custom token cause we've never documented our "implementation-defined" behavior, but I could be wrong.

For the above reasons, and considering the benefits of using `arc4random`, I would be willing to give a shot at this. The strategy would basically be to keep the `int __f_` around to preserve the layout of `std::random_device`, but not do anything with it. Since all the class methods are defined in the dylib, any old code compiled against the previous definition of `std::random_device` will still work when run against the new dylib, but it won't use the `int __f_` in the struct. Any code compiled against the new library (headers + dylib) but running against an older dylib will also work, since it will still have the `int __f_` member for the old dylib to use `std::open()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116045/new/

https://reviews.llvm.org/D116045