Bug 85494 - implementation of random_device on mingw is useless
Summary: implementation of random_device on mingw is useless
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: 10.0
Assignee: Jonathan Wakely
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-22 13:40 UTC by ookawa_mi
Modified: 2021-03-26 19:12 UTC (History)
2 users (show)

See Also:
Host:
Target: *-*-mingw*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-04-23 00:00:00


Attachments
implements proper random_device for mingw-w64 (1.35 KB, patch)
2018-07-06 12:04 UTC, Dimitrij Mijoski
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ookawa_mi 2018-04-22 13:40:27 UTC
In mingw environment, std::random_device uses mt19937 with fixed seed.

As the main purpose of random_device is seed of mt19937, it is inappropriate.

Please replace to REAL random number generator by rand_s or CryptoAPI.
Comment 1 Jonathan Wakely 2018-04-23 09:06:52 UTC
(In reply to ookawa_mi from comment #0)
> As the main purpose of random_device is seed of mt19937, it is inappropriate.

It has more uses than just that.

> Please replace to REAL random number generator by rand_s or CryptoAPI.

Confirmed.
Comment 2 Dimitrij Mijoski 2018-07-06 12:01:42 UTC
I have created a patch that fixes this for mingw-w64 using rand_s() https://msdn.microsoft.com/en-us/library/sxtz2fa8.aspx

It does not work with mingw.org (reverts to mt19973 as now). mingw.org does not have rand_s() declared in its headers.
Comment 3 Dimitrij Mijoski 2018-07-06 12:04:21 UTC
Created attachment 44358 [details]
implements proper random_device for mingw-w64
Comment 4 Jonathan Wakely 2018-07-06 12:26:25 UTC
Thanks, but please see https://gcc.gnu.org/contribute.html#legal

We can't use a patch without those steps being completed.

Also patches should be sent to the mailing lists, not attached to bugzilla, see https://gcc.gnu.org/contribute.html#patches

(N.B. I've already started work on an alternative implementation that uses RtlGenRandom instead).
Comment 5 Jonathan Wakely 2018-10-16 22:24:55 UTC
Proposed patch posted to https://gcc.gnu.org/ml/libstdc++/2018-10/msg00082.html

I would be grateful for any testing you can do on mingw targets.
Comment 6 Dimitrij Mijoski 2018-10-18 11:14:35 UTC
I read the patch couple of times and seems completely OK. Can you push it to the repository (or to a fork)?
Comment 7 Jonathan Wakely 2018-10-18 12:02:55 UTC
It has some problems which I've fixed in branch pr85494 at 
https://github.com/jwakely/gcc/tree/pr85494

I'll commit it to the GCC repo at some point.
Comment 8 Jonathan Wakely 2019-05-29 14:46:10 UTC
Author: redi
Date: Wed May 29 14:45:35 2019
New Revision: 271740

URL: https://gcc.gnu.org/viewcvs?rev=271740&root=gcc&view=rev
Log:
PR libstdc++/85494 use rdseed and rand_s in std::random_device

Add support for additional sources of randomness to std::random_device,
to allow using RDSEED for Intel CPUs and rand_s for Windows. When
supported these can be selected using the tokens "rdseed" and "rand_s".
For *-w64-mingw32 targets the "default" token will now use rand_s, and
for other i?86-*-* and x86_64-*-* targets it will try to use "rdseed"
first, then "rdrand", and finally "/dev/urandom".

To simplify the declaration of std::random_device in <bits/random.h> the
constructors now unconditionally call _M_init instead of _M_init_pretr1,
and the function call operator now unconditionally calls _M_getval. The
library code now decides whether _M_init and _M_getval should use a real
source of randomness or the mt19937 engine.

Existing code compiled against old libstdc++ headers will still call
_M_init_pretr1 and _M_getval_pretr1, but those functions now forward to
_M_init and _M_getval if a real source of randomness is available. This
means existing code compiled for mingw-w64 will start to use rand_s just
by linking to a new libstdc++.dll.

	* acinclude.m4 (GLIBCXX_CHECK_X86_RDSEED): Define macro to check if
	the assembler supports rdseed.
	* config.h.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Use GLIBCXX_CHECK_X86_RDSEED.
	* config/os/mingw32-w64/os_defines.h (_GLIBCXX_USE_CRT_RAND_S): Define.
	* doc/html/*: Regenerate.
	* doc/xml/manual/status_cxx2011.xml: Document new tokens.
	* include/bits/random.h (random_device::random_device()): Always call
	_M_init rather than _M_init_pretr1.
	(random_device::random_device(const string&)): Likewise.
	(random_device::operator()()): Always call _M_getval().
	(random_device::_M_file): Replace first member of union with an
	anonymous struct, with _M_file as its first member.
	* src/c++11/random.cc [_GLIBCXX_X86_RDRAND] (USE_RDRAND): Define.
	[_GLIBCXX_X86_RDSEED] (USE_RDSEED): Define.
	(USE_MT19937): Define if none of the above are defined.
	(USE_POSIX_FILE_IO): Define.
	(_M_strtoul): Remove.
	[USE_RDSEED] (__x86_rdseed): Define new function.
	[_GLIBCXX_USE_CRT_RAND_S] (__winxp_rand_s): Define new function.
	(random_device::_M_init(const string&)): Initialize new union members.
	Add support for "rdseed" and "rand_s" tokens. Decide what the
	"default" token does according to which USE_* macros are defined.
	[USE_POSIX_FILE_IO]: Store a file descriptor.
	[USE_MT19937]: Forward to _M_init_pretr1 instead.
	(random_device::_M_init_pretr1(const string&)) [USE_MT19937]: Inline
	code from _M_strtoul.
	[!USE_MT19937]: Call _M_init, transforming the old default token or
	numeric tokens to "default".
	(random_device::_M_fini()) [USE_POSIX_FILE_IO]: Use close not fclose.
	(random_device::_M_getval()): Use new union members to obtain a
	random number from the stored function pointer or file descriptor.
	[USE_MT19937]: Obtain a value from the mt19937 engine.
	(random_device::_M_getval_pretr1()): Call _M_getval().
	(random_device::_M_getentropy()) [USE_POSIX_FILE_IO]: Use _M_fd
	instead of fileno.
	[!USE_MT19937] (mersenne_twister): Do not instantiate when not needed.
	* testsuite/26_numerics/random/random_device/85494.cc: New test.

Added:
    trunk/libstdc++-v3/testsuite/26_numerics/random/random_device/85494.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/acinclude.m4
    trunk/libstdc++-v3/config.h.in
    trunk/libstdc++-v3/config/os/mingw32-w64/os_defines.h
    trunk/libstdc++-v3/configure
    trunk/libstdc++-v3/configure.ac
    trunk/libstdc++-v3/doc/html/manual/appendix_contributing.html
    trunk/libstdc++-v3/doc/html/manual/status.html
    trunk/libstdc++-v3/doc/xml/manual/status_cxx2011.xml
    trunk/libstdc++-v3/include/bits/random.h
    trunk/libstdc++-v3/src/c++11/random.cc
Comment 9 Jonathan Wakely 2019-05-29 14:49:38 UTC
This is now fixed for mingw-w64.

It's also fixed for mingw.org if the CPU supports either RDSEED or RDRAND. For mingw.org binaries running on older CPUs it will still use the mt19937 PRNG.
Comment 10 Jonathan Wakely 2019-05-29 22:01:29 UTC
Author: redi
Date: Wed May 29 22:00:57 2019
New Revision: 271756

URL: https://gcc.gnu.org/viewcvs?rev=271756&root=gcc&view=rev
Log:
PR libstdc++/85494 fix failing test

This test now fails on mingw-w64 because it's no longer always true that
the mt19937 engine is used when _GLIBCXX_USE_DEV_RANDOM is not defined.

Add tests for all the known tokens to ensure that at least one is
accepted.

	* testsuite/26_numerics/random/random_device/cons/token.cc: Fix test
	that fails on mingw-w64.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/testsuite/26_numerics/random/random_device/cons/token.cc
Comment 11 Michel Morin 2019-06-25 08:10:04 UTC
Could this be back-ported to GCC-9.2?
Comment 12 Jonathan Wakely 2019-06-25 09:55:00 UTC
In theory it could, but it's a non-trivial change, and I don't know how much testing it's had on mingw-w64 yet.
Comment 13 Michel Morin 2019-06-26 18:08:06 UTC
Pushing this into GCC-9 might help finding bugs (if there are some) and 
I think having non-deterministic `random_device` _with possible small bugs_ would be more important than having deterministic one.
Comment 14 Jonathan Wakely 2019-06-26 19:03:23 UTC
Backporting to stable branches should not be done as a way to find bugs. Quite the opposite!

The patch affects all targets, not just the ones currently using a PRNG, so it's not as simple as just saying it must be better.

I'm afraid you aren't making a very good case for backporting it.
Comment 15 Michel Morin 2019-06-26 19:47:44 UTC
> Backporting to stable branches should not be done as a way to find bugs.

Makes sense. I stand corrected.

> The patch affects all targets, not just the ones currently using a PRNG, so it's not as simple as just saying it must be better.

Ah, I didn't notice that.

> I'm afraid you aren't making a very good case for backporting it.

I just felt that waiting for GCC-10 (i.e. about a year) is a bit too long.
(Some of my colleagues suffer from this issue; so I hoped that this fix gets released earlier... but anyway, I'll probably provide them a wrapper to workaround the issue.)
Comment 16 Jonathan Wakely 2019-06-27 11:32:01 UTC
Author: redi
Date: Thu Jun 27 11:31:02 2019
New Revision: 272748

URL: https://gcc.gnu.org/viewcvs?rev=272748&root=gcc&view=rev
Log:
PR libstdc++/85494 use rand_s in std::random_device

This is a minimal fix for the use of a deterministic RNG on mingw-w64,
simply using rand_s unconditionally. The rest of the r271740 changes on
trunk are not included. That means that RDSEED and RDRAND are not
available for mingw-w64 and the token passed to the constructor is
ignored completely.

	PR libstdc++/85494 use rand_s in std::random_device
	* config/os/mingw32-w64/os_defines.h (_GLIBCXX_USE_CRT_RAND_S): Define.
	* src/c++11/cow-string-inst.cc (random_device::_M_init_pretr1)
	[_GLIBCXX_USE_CRT_RAND_S]: Do nothing if rand_s will be used.
	* src/c++11/random.cc [_GLIBCXX_USE_CRT_RAND_S] (__winxp_rand_s):
	Define new function.
	(random_device::_M_init_pretr1) [_GLIBCXX_USE_CRT_RAND_S]: Do nothing
	if rand_s will be used.
	(random_device::_M_getval_pretr1) [_GLIBCXX_USE_CRT_RAND_S]: Use
	__winxp_rand_s().
	* testsuite/26_numerics/random/random_device/85494.cc: New test.

Added:
    branches/gcc-9-branch/libstdc++-v3/testsuite/26_numerics/random/random_device/85494.cc
Modified:
    branches/gcc-9-branch/libstdc++-v3/ChangeLog
    branches/gcc-9-branch/libstdc++-v3/config/os/mingw32-w64/os_defines.h
    branches/gcc-9-branch/libstdc++-v3/src/c++11/cow-string-inst.cc
    branches/gcc-9-branch/libstdc++-v3/src/c++11/random.cc
Comment 17 Jonathan Wakely 2019-06-27 11:38:56 UTC
(In reply to Michel Morin from comment #15)
> I just felt that waiting for GCC-10 (i.e. about a year) is a bit too long.

That's how new features work. If you want to use them, you need the new version. The time for adding new features to GCC 9 was last year.

Anyway, it should be fixed for mingw-w64 in the gcc-9-branch, so for the GCC 9.2 release.
Comment 18 Michel Morin 2019-06-28 06:59:58 UTC
Thanks, much appreciated!

(In reply to Jonathan Wakely from comment #17)
> That's how new features work. If you want to use them, you need the new
> version. The time for adding new features to GCC 9 was last year.

Agreed in general. If this was a C++2A feature, I would not ask for releasing it in a hurry. New features should be tested enough and we developers can test/use the feature with a gcc-trunk snapshot. (I made this backport request because I (mistakenly) thought that this was a small fix and not a new feature.)
Comment 19 GCC Commits 2021-03-26 19:12:53 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:5f070ba29803c99a5fe94ed7632d7b8c55593df3

commit r11-7867-g5f070ba29803c99a5fe94ed7632d7b8c55593df3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Mar 26 18:39:49 2021 +0000

    libstdc++: Add PRNG fallback to std::random_device
    
    This makes std::random_device usable on VxWorks when running on older
    x86 hardware. Since the r10-728 fix for PR libstdc++/85494 the library
    will use the new code unconditionally on x86, but the cpuid checks for
    RDSEED and RDRAND can fail at runtime, depending on the hardware where
    the code is executing. If the OS does not provide /dev/urandom then this
    means the std::random_device constructor always fails. In previous
    releases if /dev/urandom is unavailable then std::mt19937 was used
    unconditionally.
    
    This patch adds a fallback for the case where the runtime cpuid checks
    for x86 hardware instructions fail, and no /dev/urandom is available.
    When this happens a std::linear_congruential_engine object will be used,
    with a seed based on hashing the engine's address and the current time.
    Distinct std::random_device objects will use different seeds, unless an
    object is created and destroyed and a new object created at the same
    memory location within the clock tick. This is not great, but is better
    than always throwing from the constructor, and better than always using
    std::mt19937 with the same seed (as GCC 9 and earlier do).
    
    libstdc++-v3/ChangeLog:
    
            * src/c++11/random.cc (USE_LCG): Define when a pseudo-random
            fallback is needed.
            [USE_LCG] (bad_seed, construct_lcg_at, destroy_lcg_at, __lcg):
            New helper functions and callback.
            (random_device::_M_init): Add 'prng' and 'all' enumerators.
            Replace switch with fallthrough with a series of 'if' statements.
            [USE_LCG]: Construct an lcg_type engine and use __lcg when cpuid
            checks fail.
            (random_device::_M_init_pretr1) [USE_MT19937]: Accept "prng"
            token.
            (random_device::_M_getval): Check for callback unconditionally
            and always pass _M_file pointer.
            * testsuite/26_numerics/random/random_device/85494.cc: Remove
            effective-target check. Use new random_device_available helper.
            * testsuite/26_numerics/random/random_device/94087.cc: Likewise.
            * testsuite/26_numerics/random/random_device/cons/default-cow.cc:
            Remove effective-target check.
            * testsuite/26_numerics/random/random_device/cons/default.cc:
            Likewise.
            * testsuite/26_numerics/random/random_device/cons/token.cc: Use
            new random_device_available helper. Test "prng" token.
            * testsuite/util/testsuite_random.h (random_device_available):
            New helper function.