[libc++] Start using `arc4random()` to implement `std::random_device` on Apple

[ldionne]

Summary

[libc++] Start using arc4random() to implement std::random_device on Apple

On Apple platforms, arc4random is faster than /dev/urandom, and it is
the recommended user-space RNG according to Apple's own OS folks.

This commit adds an ABI switch to guard ABI-break-protections in
std::random_device, and starts using arc4random instead of /dev/urandom
to implement std::random_device on Apple platforms.

Note that previously, std::random_device would allow passing a custom
token to its constructor, and that token would be interpreted as the name
of a file to read entropy from. This was implementation-defined and
undocumented. After this change, Apple platforms will be using arc4random()
instead, and any custom token passed to the constructor will be ignored.
This behavioral change will also impact other platforms that use the
arc4random() implementation, such as OpenBSD. This should be fine since
that is effectively a relaxation of the constructor's requirements.

rdar://86638350

[ldionne]

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().

[Mordante]

I assume that's something along the lines of

• #ifdef _LIBCPP_USING_DEV_RANDOM

+ #if defined(_LIBCPP_USING_DEV_RANDOM) || defined(APPLE)

With the appropriate comment and closing #endif.

I grepped for urandom and it didn't find documentation, so it seems it's only documented in the code.
It's however documented at https://en.cppreference.com/w/cpp/numeric/random/random_device/random_device
Which also mentions MSVC STL ignores the token completely.

I wonder whether it wouldn't be better to ignore the token for _LIBCPP_USING_ARC4_RANDOM instead of throwing an exception. (Probably in a separate patch.) That way even when Apple users now select a device their code will keep working after this change instead of suddenly throwing an exception.

libcxx/include/__config

381–382

This comment is somewhat misleading, since it doesn't work when the token is /dev/random.
But as commented below I think it would be good to document here that the token is ignored when using arc4random.

Documenting the behavior of token for the other devices would be nice.

[ldionne]

In D116045#3205247, @Mordante wrote:

I assume that's something along the lines of

• #ifdef _LIBCPP_USING_DEV_RANDOM

+ #if defined(_LIBCPP_USING_DEV_RANDOM) || defined(APPLE)

With the appropriate comment and closing #endif.

Yes, roughly.

I wonder whether it wouldn't be better to ignore the token for _LIBCPP_USING_ARC4_RANDOM instead of throwing an exception. (Probably in a separate patch.) That way even when Apple users now select a device their code will keep working after this change instead of suddenly throwing an exception.

I am really ambivalent on this. My understanding is that these __throws are meant to basically say "you should not ever use a non-default-constructed random_device". Failing at least ensures that users don't think they are using some other source of randomness -- they will definitely notice the change. Like I said, I would be surprised if anyone ever noticed the change, however if they did, I'm unsure whether it would be better to error out explicitly (so they can notice and fix their stuff), or to swallow the error and just disregard the token. I am currently leaning slightly towards the former, but I can easily be convinced otherwise. For now I'll update with my proposed patch if we do go for the former.

libcxx/include/__config

381–382

Under _LIBCPP_USING_DEV_RANDOM, using /dev/random as the parameter will work. I fixed the documentation issue in a separate NFC commit to keep those separate: https://github.com/llvm/llvm-project/commit/346ef5e5879e9143fe008c35c3dd6ea3c402634a

[ldionne]

In D116045#3205454, @ldionne wrote:

I am really ambivalent on this. My understanding is that these __throws are meant to basically say "you should not ever use a non-default-constructed random_device". Failing at least ensures that users don't think they are using some other source of randomness -- they will definitely notice the change. Like I said, I would be surprised if anyone ever noticed the change, however if they did, I'm unsure whether it would be better to error out explicitly (so they can notice and fix their stuff), or to swallow the error and just disregard the token. I am currently leaning slightly towards the former, but I can easily be convinced otherwise. For now I'll update with my proposed patch if we do go for the former.

I guess what I should say here to justify my leaning is that I view the act of ignoring a user's custom token as being as much of an ABI break as suddenly starting to throw an exception. One of them is just waaaay more subtle, but I can imagine users being broken by either equally (e.g. if they were specifying a custom file to read entropy from and somehow the entropy provided in that file had important properties). We're kind of stretching it, but that's what's on my mind in preferring to break users explicitly instead of more subtly.

[ldionne]

Update with ABI break mechanism and documentation

[Mordante]

I assume software on Mac dynamically links libc++.so and that normally there's only one system version of libc++.so installed. I'm somewhat concerned about this use case:

• A user has bought software that uses random_device("/dev/not_urandom")
• The user update to a new MacOS which has the proposed changes
• Now the application terminated due to an uncaught exception

When this scenario can happen it's possible to break working software by updating the random device. So I agree that not throwing is a kind of ABI-break, but I consider possibly breaking working code after a system upgrade by the user also an ABI-break and worse than the other.

Should we discuss this further on Discord?

[ldionne]

In D116045#3206713, @Mordante wrote:

I assume software on Mac dynamically links libc++.so and that normally there's only one system version of libc++.so installed. I'm somewhat concerned about this use case:

• A user has bought software that uses random_device("/dev/not_urandom")
• The user update to a new MacOS which has the proposed changes
• Now the application terminated due to an uncaught exception

When this scenario can happen it's possible to break working software by updating the random device. So I agree that not throwing is a kind of ABI-break, but I consider possibly breaking working code after a system upgrade by the user also an ABI-break and worse than the other.

Should we discuss this further on Discord?

Right -- so I tried explaining this in my commit message here:

This behavior is preferred to ignoring the custom token in order to avoid surprising changes in behavior when a custom token was expected to result in the corresponding file being used for entropy.

Basically, what I'm suggesting is that it's better to fail fast and loud than to be silently incorrect. One could argue that the behavior if we silently ignore the custom token isn't really incorrect, though. I'll ignore the token instead.

[Mordante]

In D116045#3220577, @ldionne wrote:

In D116045#3206713, @Mordante wrote:

I assume software on Mac dynamically links libc++.so and that normally there's only one system version of libc++.so installed. I'm somewhat concerned about this use case:

• A user has bought software that uses random_device("/dev/not_urandom")
• The user update to a new MacOS which has the proposed changes
• Now the application terminated due to an uncaught exception

When this scenario can happen it's possible to break working software by updating the random device. So I agree that not throwing is a kind of ABI-break, but I consider possibly breaking working code after a system upgrade by the user also an ABI-break and worse than the other.

Should we discuss this further on Discord?

Right -- so I tried explaining this in my commit message here:

This behavior is preferred to ignoring the custom token in order to avoid surprising changes in behavior when a custom token was expected to result in the corresponding file being used for entropy.

Basically, what I'm suggesting is that it's better to fail fast and loud than to be silently incorrect. One could argue that the behavior if we silently ignore the custom token isn't really incorrect, though. I'll ignore the token instead.

I read the original message. I was just wondering whether the change can break a working executables after MacOS update. Just to make sure we considered that case and made a conscious decision to accept that issue. I think it would be good the mention whether or not this can break existing executables in the commit message.

[ldionne]

Ignore the custom token instead of erroring out.

[ldionne]

This version of the patch relaxes the std::random_device constructor so it doesn't error out when a custom token is passed, which eases transition to arc4random() for vendors who have already shipped std::random_device with a different implementation.

@emaste do you know who's responsible for OpenBSD? This is technically a small behavior change on their platform since an invalid custom token is going to be ignored instead of throwing an exception. That's most likely OK but we might as well double check with them if we can get a hold of someone.

[Mordante]

LGTM after the CI is green.

[ldionne]

Try to fix CI. The test doesn't fail anymore on older macOS because the exception-throwing test isn't enabled anymore (since we're not using _LIBCPP_USING_DEV_RANDOM).

libcxx/docs/ReleaseNotes.rst

- The C++03 emulation type for ``std::nullptr_t`` has been removed in favor of
   ABI. However, previously, these users' ABI would be incompatible with any other
   binary or static archive compiled with C++11 or later. If you start seeing linker
   errors involving ``std::nullptr_t`` against previously compiled binaries, this may
   be the cause. You can define the ``_LIBCPP_ABI_USE_CXX03_NULLPTR_EMULATION`` macro
   to return to the previous behavior. That macro will be removed in LLVM 15. Please
   comment `here <https://reviews.llvm.org/D109459>`_ if you are broken by this change
   and need to define the macro.
 
+- On Apple platforms, ``std::random_device`` is now implemented on top of ``arc4random()``
+  instead of reading from ``/dev/urandom``. Any implementation-defined token used when
+  constructing a ``std::random_device`` will now be ignored instead of interpreted as a
+  file to read entropy from.
+
 Build System Changes
 --------------------
 
 - Building the libc++ shared or static library requires a C++ 20 capable compiler.
   Consider using a Bootstrapping build to build libc++ with a fresh Clang if you
   can't use the system compiler to build libc++ anymore.
 
 - Historically, there has been numerous ways of building libc++ and libc++abi. This has

libcxx/include/__config

 #  define _LIBCPP_ABI_SPAN_POINTER_ITERATORS
 // Re-worked external template instantiations for std::string with a focus on
 // performance and fast-path inlining.
 #  define _LIBCPP_ABI_STRING_OPTIMIZED_EXTERNAL_INSTANTIATION
 // Enable clang::trivial_abi on std::unique_ptr.
 #  define _LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI
 // Enable clang::trivial_abi on std::shared_ptr and std::weak_ptr
 #  define _LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI
+// std::random_device holds some state when it uses an implementation that gets
+// entropy from a file (see _LIBCPP_USING_DEV_RANDOM). When switching from this
+// implementation to another one on a platform that has already shipped
+// std::random_device, one needs to retain the same object layout to remain ABI
+// compatible. This switch removes these workarounds for platforms that don't care
+// about ABI compatibility.
+#  define _LIBCPP_ABI_NO_RANDOM_DEVICE_COMPATIBILITY_LAYOUT
 #elif _LIBCPP_ABI_VERSION == 1
 #  if !defined(_LIBCPP_OBJECT_FORMAT_COFF)
 // Enable compiling copies of now inline methods into the dylib to support
 // applications compiled against older libraries. This is unnecessary with
 // COFF dllexport semantics, since dllexport forces a non-inline definition
 // of inline functions to be emitted anyway. Our own non-inline copy would
 // conflict with the dllexport-emitted copy, so we disable it.
 #    define _LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS

 //      uses the NaCL syscall `nacl_secure_random_init()` to get entropy.
 //      When this option is used, the token passed to `std::random_device`'s
 //      constructor *must* be "/dev/urandom" -- anything else is an error.
 //
 // _LIBCPP_USING_WIN32_RANDOM
 //      Use rand_s(), for use on Windows.
 //      When this option is used, the token passed to `std::random_device`'s
 //      constructor *must* be "/dev/urandom" -- anything else is an error.
-#if defined(__OpenBSD__)
+#if defined(__OpenBSD__) || defined(__APPLE__)
 #  define _LIBCPP_USING_ARC4_RANDOM
 #elif defined(__wasi__)
 #  define _LIBCPP_USING_GETENTROPY
 #elif defined(__Fuchsia__)
 #  define _LIBCPP_USING_FUCHSIA_CPRNG
 #elif defined(__native_client__)
 #  define _LIBCPP_USING_NACL_RANDOM
 #elif defined(_LIBCPP_WIN32API)
 #  define _LIBCPP_USING_WIN32_RANDOM

libcxx/include/__random/random_device.h

 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if !defined(_LIBCPP_HAS_NO_RANDOM_DEVICE)
 
 class _LIBCPP_TYPE_VIS random_device
 {
 #ifdef _LIBCPP_USING_DEV_RANDOM
     int __f_;
-#endif
+#elif !defined(_LIBCPP_ABI_NO_RANDOM_DEVICE_COMPATIBILITY_LAYOUT)
+#   if defined(__clang__)
+#       pragma clang diagnostic push
+#       pragma clang diagnostic ignored "-Wunused-private-field"
+#   endif
+
+    // Apple platforms used to use the `_LIBCPP_USING_DEV_RANDOM` code path, and now
+    // use `arc4random()` as of this comment. In order to avoid breaking the ABI, we
+    // retain the same layout as before.
+#   if defined(__APPLE__)
+    int __padding_; // padding to fake the `__f_` field above
+#   endif
+
+    // ... vendors can add workarounds here if they switch to a different representation ...
+
+#   if defined(__clang__)
+#       pragma clang diagnostic pop
+#   endif
+#endif
+
 public:
     // types
     typedef unsigned result_type;
 
     // generator characteristics
     static _LIBCPP_CONSTEXPR const result_type _Min = 0;
     static _LIBCPP_CONSTEXPR const result_type _Max = 0xFFFFFFFFu;
 

libcxx/src/random.cpp

random_device::operator()()
     int err = getentropy(&r, n);
     if (err)
         __throw_system_error(errno, "random_device getentropy failed");
     return r;
 }
 
 #elif defined(_LIBCPP_USING_ARC4_RANDOM)
 
-random_device::random_device(const string& __token)
+random_device::random_device(const string&)
 {
-    if (__token != "/dev/urandom")
-        __throw_system_error(ENOENT, ("random device not supported " + __token).c_str());
 }
 
 random_device::~random_device()
 {
 }
 
 unsigned
 random_device::operator()()

libcxx/test/std/numerics/rand/rand.device/ctor.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // See https://llvm.org/PR20183
 // XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}
 
+// The behavior of std::random_device changed on Apple platforms with
+// https://llvm.org/D116045.
+// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13|14|15}}
+// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx{{11|12}}
+
 // UNSUPPORTED: libcpp-has-no-random-device
 
 // <random>
 
 // class random_device;
 
 // explicit random_device(const string& token = implementation-defined); // before C++20
 // random_device() : random_device(implementation-defined) {}            // C++20

 }
 
 int main(int, char**) {
   {
     std::random_device r;
   }
   // Check the validity of various tokens
   {
-    check_random_device_invalid("wrong file");
-    check_random_device_invalid("/dev/whatever");
+#if defined(_LIBCPP_USING_ARC4_RANDOM)
+    check_random_device_valid("/dev/urandom");
+    check_random_device_valid("/dev/random");
+    check_random_device_valid("/dev/null");
+    check_random_device_valid("/dev/nonexistent");
+    check_random_device_valid("wrong file");
+#elif defined(_LIBCPP_USING_DEV_RANDOM)
     check_random_device_valid("/dev/urandom");
-#if defined(_LIBCPP_USING_DEV_RANDOM)
     check_random_device_valid("/dev/random");
+    check_random_device_valid("/dev/null");
+    check_random_device_invalid("/dev/nonexistent");
+    check_random_device_invalid("wrong file");
 #else
+    check_random_device_valid("/dev/urandom");
     check_random_device_invalid("/dev/random");
+    check_random_device_invalid("/dev/null");
+    check_random_device_invalid("/dev/nonexistent");
+    check_random_device_invalid("wrong file");
 #endif
   }
 
 #if !defined(_WIN32)
 // Test that random_device(const string&) properly handles getting
 // a file descriptor with the value '0'. Do this by closing the standard
 // streams so that the descriptor '0' is available.
   {

libcxx/test/std/numerics/rand/rand.device/eval.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-// See https://llvm.org/PR20183
-// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}
-
 // UNSUPPORTED: libcpp-has-no-random-device
 
 // <random>
 
 // class random_device;
 
 // result_type operator()();