uuid\seed_rng.hpp sha1_random_digest_state_ not random

[qiaozhiqiang@…]

boost_1_45_0\boost\uuid\seed_rng.hpp

static unsigned int * sha1_random_digest_state_()

{

intentionally left uninitialized ERROR 1: not uninitialized! static value always zero all. ERROR 2: always return the same pointer of static value should delete 'static' 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

ERROR 1: not uninitialized! static value always zero all. so state is all zero

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

ERROR 2: always return the same pointer of static value so ps is same ( ps === ps)

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 ) );

[qiaozhiqiang@…]

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 ) );

[anonymous]

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]

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.

[Andy Tompkins]

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@…]

"{{{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]

I understand now.

Thanks.

Comment removed, rev 80501