GNOME Bugzilla – Bug 371631
Bug in g_bit_nth_lsf?
Last modified: 2007-01-03 20:10:33 UTC
Ok, this is most probably not a glib bug, but I'm confused and I'll paste it here for others to test. With this code: #include <glib.h> int main (void) { gulong i; for (i = 0; i < 20; i++) g_message ("%lu %d", i, g_bit_nth_lsf (i, 31)); return 0; } I'm getting: ** Message: 0 -1 ** Message: 1 32 ** Message: 2 -1 ** Message: 3 32 ** Message: 4 -1 ** Message: 5 32 ** Message: 6 -1 ** Message: 7 32 ** Message: 8 -1 ** Message: 9 32 ** Message: 10 -1 ** Message: 11 32 ** Message: 12 -1 ** Message: 13 32 ** Message: 14 -1 ** Message: 15 32 ** Message: 16 -1 ** Message: 17 32 ** Message: 18 -1 ** Message: 19 32 The 32s are all wrong. If I change the loop to loop up to 10, I get the correct result: ** Message: 0 -1 ** Message: 1 -1 ** Message: 2 -1 ** Message: 3 -1 ** Message: 4 -1 ** Message: 5 -1 ** Message: 6 -1 ** Message: 7 -1 ** Message: 8 -1 ** Message: 9 -1 I get one or the other depending on the gcc -O flags too. gcc doesn't warn about anything stupid, neither does valgrind. I suspected the inlining of g_bit_nth_lsf, but a local copy exposes the same behavior. I tracked it down that while "(mask & (1UL << nth_bit))" for mask=1 and nth_bit=32 prints out 0, the conditional on this succeeds and returns the 32 value. IIRC, <<32 is undefined, so that may just be the bug, but I appreciate any more insights. gcc (GCC) 4.1.1 20060525 (Red Hat 4.1.1-1)
So yeah, I'm now clearly seeing 1<<32 failing. This code: g_message ("%d, %lu %lu", nth_bit, (1UL<<(nth_bit+1)), (1UL<<(nth_bit))<<1); is writing this: ** Message: 31, 1 0
Rewriting both g_bit_nth_lsf and g_bit_nth_msf as while() {} instead of do {} while () fixes the problem.
(In reply to comment #2) > Rewriting both g_bit_nth_lsf and g_bit_nth_msf as while() {} instead of do {} > while () fixes the problem. In fact, to be completely correct, one should check nth_bit to be in range from both ways, or to document these functions as undefined if nth_bit is out of [0..sizeof(long)*8).
My first attachment in bug 371670 has correct code for nth_lsf and nth_msf that checks all ranges and doesn't rely on undefined C code.
2007-01-03 Behdad Esfahbod <behdad@gnome.org> * glib/gutils.h: Fix bug in g_bit_nth_lsf (#371631) and use __builtin_clzl for g_bit_storage if available (#371670). * tests/Makefile.am: * tests/bit-test.c: New test, to test g_bit_* operations against naive and builtin implementations.