[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