This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Use ioctl when available to get random_device entropy.
ClosedPublic

Authored by curdeius on Jan 19 2021, 1:01 AM.

Details

Summary

Implemented the idea from D94571 to improve entropy on Linux.

Diff Detail

Event Timeline

curdeius requested review of this revision.Jan 19 2021, 1:01 AM
curdeius created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2021, 1:01 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
curdeius edited the summary of this revision. (Show Details)Jan 19 2021, 1:03 AM
curdeius added a subscriber: brad.
curdeius added inline comments.Jan 19 2021, 5:24 AM
libcxx/src/random.cpp
180

This could be guarded even more if we wanted. E.g. defining _LIBCPP_USING_IOCTL/LINUX_RANDOM_H, but personally I think that just checking for RNDGETENTCNT is enough.

ldionne requested changes to this revision.Jan 19 2021, 7:22 AM
ldionne added a subscriber: tcanens.

@tcanens You may want to consider updating cppreference in light of this patch when it gets in.

This LGTM except for the test nitpick!

libcxx/test/std/numerics/rand/rand.device/entropy.pass.cpp
26–27

I think we want to make those be LIBCPP_ASSERT(...) instead, since the entropy is technically not mandated by the standard. It's just that our implementation behaves that way.

This revision now requires changes to proceed.Jan 19 2021, 7:22 AM
ldionne added inline comments.Jan 19 2021, 11:48 AM
libcxx/src/random.cpp
33

Should this be __has_include(<sys/ioctl.h>) instead?

curdeius added inline comments.Jan 19 2021, 12:49 PM
libcxx/src/random.cpp
33

Oops, good catch.

libcxx/test/std/numerics/rand/rand.device/entropy.pass.cpp
26–27

The standard says this http://eel.is/c++draft/rand.device#5. So entropy should be between min and log2(max+1), where max is max of result_type.
At the same time, http://eel.is/c++draft/rand.device#2 says result_type=unsigned int.
So the only thing that can change is the size of unsigned int (which can be non-32-bit somewhere).
To be precise I should rather assert(e <= sizeof(result_type) * CHAR_BIT).
And it should be for whatever implementation of stdlib.
WDYT?

curdeius updated this revision to Diff 317807.Jan 20 2021, 1:40 AM

Fix has_include. Use result_type.

curdeius marked an inline comment as done.Jan 20 2021, 1:41 AM
ldionne accepted this revision.Jan 21 2021, 7:42 AM

LGTM, thanks!

This revision is now accepted and ready to land.Jan 21 2021, 7:42 AM