[Xcb] undefined behaviour in xcb_mask()
Ulrich Eckhardt
doomster at knuut.de
Thu Aug 7 11:55:20 PDT 2008
Hi!
The function xcb_mask[1] from xcb_bitops.h has a problem when it is called
with the value 32, because shifting a value by its size in bits yields
undefined behaviour and indeed some compilers will simply generate a no-op
(sic!) from it.
I'm not sure if and where this is used and if it matters at all, so I'm not
even sure this will cause a problem, but I would at least suggest
an "assert(n<32);" in the function.
Other suggestions that allow the value 32 are:
- Use a bigger integer:
return (((uint64_t)1) << n) - 1;
- Use a special case:
if(n==32) return ~(uint32_t)0;
return (1 << n) - 1;
- Shift in the opposite direction, using two steps to prevent UB:
unsigned k = 32-n;
return ((~(uint32_t)0) >> k/2) >> (k-k/2);
- Use a lookup table.
- Use switch statement.
For now, I would use the first one, since this doesn't involve any branches
(bad for CPU pipelines) and possibly not even any additional operation, just
an additional/bigger register compared to the current implementation.
Further, I would add the assertion that n is less or equal to 32, but that of
course adds a dependency on <assert.h>.
Uli
[1] Current implementation:
uint32_t xcb_mask(uint32_t n)
{
return (1 << n) - 1;
}
More information about the Xcb
mailing list