Boost C++ Libraries: Ticket #5187: uuid\seed_rng.hpp sha1_random_digest_state_ not random

qiaozhiqiang@… — Tue, 15 Feb 2011 01:15:59 GMT

see this:

boost_1_45_0\boost\uuid\seed_rng.hpp

first time, state is all zero,sha1_random_digest_() will modify it.

but always return the same pointer of static value, so ps is not random.

 static unsigned int * sha1_random_digest_state_()
     {
         // intentionally left uninitialized
         ///////////
         // !!!!!!!!  first time, state is all zero,sha1_random_digest_() will modify it.
         // but always return the same pointer of static value
         ///////////
         static unsigned int state[ 5 ];
         return state;
     }
 unsigned int * ps = sha1_random_digest_state_();
         unsigned int state[ 5 ];
         std::memcpy( state, ps, sizeof( state ) ); // harmless data race
         sha.process_bytes( (unsigned char const*)state, sizeof( state ) );
        /////////////////
        // !!!!!!!!  ERROR: sha1_random_digest_state_() always return the same pointer of the static value
        // so should delete next line??
         ////////////////
         sha.process_bytes( (unsigned char const*)&ps, sizeof( ps ) );
 namespace boost {
 namespace uuids {
 namespace detail {
 // should this be part of Boost.Random?
 class seed_rng
 {
 static unsigned int * sha1_random_digest_state_()
     {
         // intentionally left uninitialized
         static unsigned int state[ 5 ];
         return state;
     }
     void sha1_random_digest_()
     {
         boost::uuids::detail::sha1 sha;
         unsigned int * ps = sha1_random_digest_state_();
         unsigned int state[ 5 ];
         std::memcpy( state, ps, sizeof( state ) ); // harmless data race
         sha.process_bytes( (unsigned char const*)state, sizeof( state ) );
         sha.process_bytes( (unsigned char const*)&ps, sizeof( ps ) );

component changed; owner set

Steven Watanabe — Thu, 17 Feb 2011 03:05:56 GMT

owner set to Andy Tompkins
component None → uuid

anonymous — Thu, 17 Feb 2011 03:38:51 GMT

the static value is always initialized, so we should delete
        "// intentionally left uninitialized" only ?
but the ps in this line is different in different program,
different build, different module, different DLL...
so ps is random.
     static unsigned int * sha1_random_digest_state_()
     {
         // intentionally left uninitialized
         static unsigned int state[ 5 ];
         return state;
     }
     sha.process_bytes( (unsigned char const*)&ps, sizeof( ps ) );

Andy Tompkins — Sat, 05 Mar 2011 23:52:43 GMT

The static value does produce different values for me. Some compilers will initialize it in debug builds, or with certain flags.

I believe this code is correct. Although I don't completely understand the data race.

status changed; resolution set

Andy Tompkins — Sun, 09 Sep 2012 19:12:42 GMT

status new → closed
resolution → invalid

I'm finally getting back to this after some research and thinking.

The static variable in sha1_random_digest_state_(),

static unsigned int state[5];

may be set to zero with some compilers / flags (often in debug builds). But the program _does_ change the values in it every time sha1_random_digest_() is called (at the bottom of the function). Thus the values in the static unsigned int state[5]; are not constant for the duration of the program. So at worst it is initialized to zero, at best it does contain random data initially, but in either case it is changing data that is mixed in.

The function sha1_random_digest_(); uses many different kinds of allocation for unitialized data. It uses a static array in sha1_random_digest_state_();, it uses a local array in sha1_random_digest_();, unsigned int state[5]. It uses data on the heap as well, unsigned int * p = new unsigned int.

sha1_random_digest_state_() does always return the same pointer and so in sha1_random_digest_(), unsigned int * ps = sha1_random_digest_state_(); ps always points to the same data but the line we use it in,

sha.process_bytes( (unsigned char const*)&ps, sizeof( ps ) );

does not use the value of ps, but the address of ps, &ps, which is a value on the stack since this is a local variable. Thus &ps is different each time.

I still believe the code is correct.

I am closing the ticket as "invalid" since the other choices do not seem to apply.

qiaozhiqiang@… — Mon, 10 Sep 2012 01:47:26 GMT

"{{{sha.process_bytes( (unsigned char const*)&ps, sizeof( ps ) );}}}
 does not use the value of {{{ps}}}, but the address of {{{ps}}},
{{{&ps}}}, which is a value on the stack since this is a local variable."
no, sha.process_bytes is use the ps value [4 bytes] pass by pointer.
 but ps value [address of static unsigned int state[5] ] will be
different in different program, different build, different module, different DLL...
see comment:3 Changed 19 months ago by anonymous
////////////////////
"Thus the values in the static unsigned int state[5]; are not constant for
 the duration of the program. So at worst it is initialized to zero, at best
it does contain random data initially."
static unsigned int state[5] is a static function variable will be always
initialized to zero in ISO C or ISO C++, static unsigned int state[5] ===
 static unsigned int state[5] = {0,0,0,0,0}
so, Yes, the code is correct.
but the comments "// intentionally left uninitialized" is not correct.

Andy Tompkins — Wed, 12 Sep 2012 01:15:18 GMT

I understand now.

Thanks.

Comment removed, rev 80501