Very strange Bug

[adamh]

No problem! One more question, can you repro this in 64-bit debug compiles as well? If release only, does removing optimization remove the prob?

[Gerd Isenberg]

hmm, maybe it is not important what we can see in the second assembly,
maybe it s more important what we (at least i) can not see.

Where is the variable rookAttacks gone ?

The variable rookAttacks is an implicit register variable, here the byte register r8b:

Code: Select all

   or   r8b, al
   test   r8b, 32               ; 00000020H

[Gerd Isenberg]

No problem! One more question, can you repro this in 64-bit debug compiles as well? If release only, does removing optimization remove the prob?

This seems a compiler bug to me, probaly caused by wrong optimization, but anyway. Unaries and static casts should have higher precedence than shift left. Even if only the low byte is needed finally, it is an error to use non 64-bit but 32-bit shift due to the not specified shift behaviour with shift amounts >= register width, for x86 modulo 32 or modulo 64.

Code: Select all

ui64_t lo = -(ui64_t) 1 << bsr64((lmsk->lineLo & occ)|1); 

Assuming only the low byte is needed, the VC2008 express makes the following out of that:

Code: Select all

unsigned char lo = (unsigned char)255 << bsr64((lmsk->lineLo & occ)|1); 

Now, if bsr returns exactly 32, the shift makes nothing and low becomes 255 instead the expected zero ...

May be the compiler is "confused" by the two unaries -(cast), and I guess there is an easy work around.

[Volker Annuss]

Code: Select all

 -(ui64_t) 1

That's it. This is an overflow and the compiler is free to optimize it to any nonsense it wants.

[Chan Rasjid]

[quote="Desperado"]

Code: Select all

if((attackR(KING(black),OCCUPIED) & bF1)) {codeblock}

- attackR is returning (unsigned __int64)
- bF1 is static const ui64_t

The problem is the following: in my 64 bit compile (MS Express VS2008).
the codeblock is entered although the Attackgetter returns 0, when 0 is the correct value.

Code: Select all

...
thx, Michael[/quote]

As Bob pointed out, mixing 32 bits integers with 64 bits integers are bug prone. 

Since {codeblock} is entered and bF1 should be ok, it means 'attackR' did not return 0 when 0 should be the correct value! The likely bug may be the sign bit was set.

I remember posting (and confounded with) what is (unsigned _int64) (-1) ? The type of -1 is likely 32 bit signed integer and there is no sane way to cast and convert (and extend 32 0 bits) to an unsigned 64 bit integer because of type incompatibility. Even if C standards does know what to extend, it could be bug prone.

Your attackR() could have bugs from variables of type signed int32 and integer constants (default type signed int32) mixed with unsigned int64 in expressions.

Rasjid.

[Gerd Isenberg]

Code: Select all

 -(ui64_t) 1

That's it. This is an overflow and the compiler is free to optimize it to any nonsense it wants.

Really? Unary minus aka Two's complement on any unsigned overlows and has to borrow from hidden 2^n bit, not?

So what would be the correct solution?

Code: Select all

ui64_t lo = (ui64_t) -1 // seems wrong as well
ui64_t lo = -1ULL
ui64_t lo = ~(ui64_t) 0
ui64_t lo = 0xffffffffffffffffULL

[adamh]

So if this is really true the result is unspecified according to the language standard. And if it is unspecified it cannot be called a bug ....

[michiguel]

[quote="Chan Rasjid"][quote="Desperado"]

Code: Select all

if((attackR(KING(black),OCCUPIED) & bF1)) {codeblock}

- attackR is returning (unsigned __int64)
- bF1 is static const ui64_t

The problem is the following: in my 64 bit compile (MS Express VS2008).
the codeblock is entered although the Attackgetter returns 0, when 0 is the correct value.

Code: Select all

...
thx, Michael[/quote]

As Bob pointed out, mixing 32 bits integers with 64 bits integers are bug prone. 

Since {codeblock} is entered and bF1 should be ok, it means 'attackR' did not return 0 when 0 should be the correct value! The likely bug may be the sign bit was set.

I remember posting (and confounded with) what is (unsigned _int64) (-1) ? The type of -1 is likely 32 bit signed integer and there is no sane way to cast and convert (and extend 32 0 bits) to an unsigned 64 bit integer because of type incompatibility. Even if C standards does know what to extend, it could be bug prone.

[/quote]
This is well defined, but it is tricky. If you want all bits set to one, most likely you mean to have 
uint64_t all_bits_are_one = -(uint64_t)(1);
rather than
uint64_t lower32bits_are_one = (uint64_t)(-1);

Miguel
[quote]
Your attackR() could have bugs from variables of type signed int32 and integer constants (default type signed int32) mixed with unsigned int64 in expressions.

Rasjid.[/quote]

[Gerd Isenberg]

So if this is really true the result is unspecified according to the language standard. And if it is unspecified it cannot be called a bug ....

The bug (if any, see Volker's remark) is using a 32-bit shift rather than a 64-bit shift with a shift amounts from 0 to 63. It is implementation dependent and may vary between different platforms. As mentioned on x86 32-bit shift is implicit only using the 5 lower bits of the shift amount (modulo 32).

[Chan Rasjid]

...
I remember posting (and confounded with) what is (unsigned _int64) (-1) ? The type of -1 is likely 32 bit signed integer and there is no sane way to cast and convert (and extend 32 0 bits) to an unsigned 64 bit integer because of type incompatibility. Even if C standards does know what to extend, it could be bug prone.

This is well defined, but it is tricky. If you want all bits set to one, most likely you mean to have
uint64_t all_bits_are_one = -(uint64_t)(1);
rather than
uint64_t lower32bits_are_one = (uint64_t)(-1);

Miguel

My compiler gives (uint64_t)(-1) to be 0xffffffffffffffff;

My system is linux gcc 64 bits and my debuggger crashed when I wanted to see what is -1 in hex. I am not sure of the type of -1 (32 or 64 bits).

Rasjid.