in my little chess engine I get some crash reports from testers running it on their systems. As most of them use a 64 bit system and I don't I wonder whether this can be the cause of trouble.
As the crashes seem to happen when the hash is reset I looked at my code here and ask myself if this is the proper way of coding this
// enforce memory alignment to 64 byte boundary
// allocate some headroom in buffer and let the actual hash point to a 64 byte aligned address
buffer = (unsigned char *) malloc(sizeof(*hash) + 63);
hash = (THash *) (((uintptr_t) buffer + 63) & ~63);
memset(hash,0,sizeof(*hash));
The cast to uintptr_t seems problematic. From the definition uintptr_t
"It is an unsigned int that is guaranteed to be the same size as a pointer."
but this guarantee is only valid at compile time, right ? So when compiling for a 32bit target it will be a 4byte pointer, can this cause a crash when the binary is run on a 64 bit system then ?
If so, is there a better way of coding this (a long long hack or so) or do I have to create a dedicated 32 bit (only) and a 64 bit (only) binary ?
// enforce memory alignment to 64 byte boundary
// allocate some headroom in buffer and let the actual hash point to a 64 byte aligned address
buffer = (unsigned char *) malloc(sizeof(*hash) + 63);
hash = (THash *) (((uintptr_t) buffer + 63) & ~63);
memset(hash,0,sizeof(*hash));
The cast to uintptr_t seems problematic. From the definition uintptr_t
"It is an unsigned int that is guaranteed to be the same size as a pointer."
but this guarantee is only valid at compile time, right ? So when compiling for a 32bit target it will be a 4byte pointer, can this cause a crash when the binary is run on a 64 bit system then ?
Depends.
If they're running the compiled 32 bit binary, then it'll be fine since it runs in 32 bit mode. If they're recompiling it to 64 bit native code, then there may be a problem because ~63 is an int, which may still be 32 bits (always is in Windows and flavours of Linux, as far as I'm aware), so it's masking out the upper 32 bits of the pointer, which may be incorrect (but probably won't show up if there's < 4 GB of RAM).
If so, is there a better way of coding this (a long long hack or so) or do I have to create a dedicated 32 bit (only) and a 64 bit (only) binary ?
If you want to benefit from running on a 64 bit OS, then yes, you do need a 64 bit binary.
I have to say that I'm not a fan of this type of alignment tricks (I think the code looks ugly), but they should generally be ok. Just make sure that when you're freeing/reallocing the memory, you use the original "buffer" pointer rather than the mangled "hash" pointer.
Thanks, I agree it looks ugly and it is the only place in all my 15.000 lines of source code where I use an explicit malloc and do such pointer tricks.
Even if it is OK, I will remove it and go back to a simple
private:
THash hash;
statement.
My engine is still far to weak to need such tricks to improve. Maybe in a distant future when everything else is done, I might have another look.
You may try to change uintptr_t to intptr_t to see if it works (even if ~63 is a 32-bit int it's negative so converting to signed 64-bit int should keep the sign bit over 32 msbits). Even better would probably be to use something like & ~63LLU or ~((uint64_t)63)).
thanks. The difficulty here is that whatever I tried I don't get the engine to fail on my systems and I tried 5 different by now. So for the moment I will remove the alignment to be safe, I don't probe the hash in QS so only in a small % of total nodes the hash is probed so I hope it does not hurt that much.
The aligned hash did not safe me yesterday anyway against cheng
in my little chess engine I get some crash reports from testers running it on their systems. As most of them use a 64 bit system and I don't I wonder whether this can be the cause of trouble.
As the crashes seem to happen when the hash is reset I looked at my code here and ask myself if this is the proper way of coding this
// enforce memory alignment to 64 byte boundary
// allocate some headroom in buffer and let the actual hash point to a 64 byte aligned address
buffer = (unsigned char *) malloc(sizeof(*hash) + 63);
hash = (THash *) (((uintptr_t) buffer + 63) & ~63);
memset(hash,0,sizeof(*hash));
The cast to uintptr_t seems problematic. From the definition uintptr_t
"It is an unsigned int that is guaranteed to be the same size as a pointer."
but this guarantee is only valid at compile time, right ? So when compiling for a 32bit target it will be a 4byte pointer, can this cause a crash when the binary is run on a 64 bit system then ?
If so, is there a better way of coding this (a long long hack or so) or do I have to create a dedicated 32 bit (only) and a 64 bit (only) binary ?
Thanks
Thomas...
If it runs in 32 bit mode, you can't have a 64 bit pointer. There are no 64 bit registers either. I'm afraid to guess what (I assume the O/S is windows) windows might do internally. I'd certainly do a 32 bit and 64 bit version myself, just because on the 64 bit machine, you get to access the extra 8 registers that will make the program faster. A 32 bit binary won't even know they are there because 32 bit processors don't have 'em and the compiler won't emit code to access them.
In any case, watch out for constants. ~63 could zap you since on windows an int is 32 bits, even on a 64 bit architecture (not true on linux, but that's a different O/S.)
thanks. The difficulty here is that whatever I tried I don't get the engine to fail on my systems and I tried 5 different by now. So for the moment I will remove the alignment to be safe, I don't probe the hash in QS so only in a small % of total nodes the hash is probed so I hope it does not hurt that much.
The aligned hash did not safe me yesterday anyway against cheng
Thomas...
Hi Thomas,
It seems strange to me anyway. Are you 100% sure problem is in the alignment? Those random crashes are difficult to trace. Besides I think I do something similar in cheng and had no crash reports (yet).
Well too bad for iCE we met in round 1 (I haven't seen the match - I mean iCE didn't crash, did it?). I hope that iCE recovers and that it makes it safely to group G. Good luck.
I think there is an operator precedence bug here. The cast to uintptr_t has a higher precedence than the binary addition "+ 63". Your pointer ends up outside the allocated memory because you advance it by sizeof(*uintptr_t) * 63, whereas your malloc call is in bytes.
Last edited by Gian-Carlo Pascutto on Mon Sep 05, 2011 5:48 pm, edited 1 time in total.
No, it did not crash in Olivers tournament. Cheng got a regular win by better play which was not totally unexpected. Your engine is very strong.
I got some crash reports from Graham that ice crashes in the 2nd game it plays. I don't think it is the alignment that let's it crash because then it would probably not survive the first game if its really broken here.
By looking through the code I found some minor bugs in the hash handling (that had nothing to do with the alignment) and those bugs seemed to cancel each other out so the code worked. They should not be serious enough to crash the engine but who knows. I now fixed them, and I'm running a few hundreds test games to look whether I did not break anything else while fixing them.
While I was debugging it I looked at the addresses from buffer and hash and the malloc call always returned already an 64byte aligned address. So at the end the address of buffer and hash were the same.
I tried several times to cause malloc to return a not aligned address (by allocating some dummy memory before that call). I got different addresses for buffer each time but they were always already aligned.
However your explanation makes sense. It looks like a bug now.
