[libcxx-commits] [PATCH] D116124: [libc++][ABI BREAK] Start using `arc4random()` to implement `std::random_device` on Apple

[Louis Dionne via Phabricator via libcxx-commits]

ldionne created this revision.
Herald added a subscriber: krytarowski.
ldionne requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

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 passing any custom token to the constructor will result in
an exception. 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116124

Files:
  libcxx/docs/ReleaseNotes.rst
  libcxx/include/__config
  libcxx/include/__random/random_device.h


Index: libcxx/include/__random/random_device.h
===================================================================
--- libcxx/include/__random/random_device.h
+++ libcxx/include/__random/random_device.h
@@ -55,7 +55,7 @@
 //      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(__Fuchsia__) || defined(__wasi__)
 #  define _LIBCPP_USING_GETENTROPY
@@ -72,6 +72,18 @@
 #ifdef _LIBCPP_USING_DEV_RANDOM
     int __f_;
 #endif
+
+#ifndef _LIBCPP_ABI_NO_RANDOM_DEVICE_COMPATIBILITY_LAYOUT
+    // 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 ...
+#endif
+
 public:
     // types
     typedef unsigned result_type;
Index: libcxx/include/__config
===================================================================
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -107,6 +107,12 @@
 #  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. 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
Index: libcxx/docs/ReleaseNotes.rst
===================================================================
--- libcxx/docs/ReleaseNotes.rst
+++ libcxx/docs/ReleaseNotes.rst
@@ -114,6 +114,13 @@
   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``. Using any implementation-defined token different
+  from ``"/dev/urandom"`` when constructing a ``std::random_device`` will now be an error.
+  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, but would now be ignored.
+
 Build System Changes
 --------------------
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116124.395739.patch
Type: text/x-patch
Size: 3057 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20211221/6071d5b0/attachment-0001.bin>