1 << 31 and related issues

[Bruce Evans]

On Mon, 25 Nov 2013, Eitan Adler wrote:

> There are a few cases in FreeBSD where the expression (1 << 31) is used.

More than a few.

Compilers are broken for not warning about this.  clang and gcc both
warn for (1 << 30) + (1 << 30).  Apparently, they intentionally turn
off their overflow checking for bitwise expressions.

> However this produces undefined behavior as '1' is of type int.  (see
> 6.4.4p5: The type of an unmarked integer constant is the first of the
> following list in which its value can be represented: int, long int,
> long long int).  The shift of 31 is illegal (see 6.5.7p4) if the size
> of an int is 32.  The same issue arises with 2 << 30 and 3 << 30.
>
> I have been working on fixing this issue in a few different projects.
> The correct fix here is to use 1U as the literal instead.  adrian@ has

Not really correct.  This assumes >= 32-bit unsigned ints.  1UL doesn't
assume anything, but might break the type of the result, e.g., for
printf()ing.  Even using 1U may break the type -- perhaps a signed type
with the usual undefined behaviour of a value of INT32_MIN is what is
wanted.

> advocated for the macros BIT32(bit) and BIT64(bit) which I would also
> support.

I don't like these much.  1ULL would work for fixing (wrong_case)1 << 64,
but uses the long long abomination.  There must already be casts or type
suffixes for 1 in 1 << 64 because this just won't work without them.
   (The long long abomination is now used 1017 times in /sys in just the
   form 1ULL.  This is up from zero outside of alpha in FreeBSD-3 and 61
   outside of alpha in FreeBSD-5.2. :-().  I enforced this for parts of
   the kernel that I used up to about 1999 by building with K&R and C90
   compilers.  This mainly involved changing long long literals to
   int64_t ones.  quad_t was used excessively, and it wasn't long long
   so compilers accepted it.  Even "K&R" and "C90" ones, by declaring
   int64_t as an extension (this was done up to about FreeBSD-4).  Now,
   int64_t is used even more excessively, and it is declared as long
   long for 32-bit arches.  C90 compilers are just not supported.)
The macro would hide the details of the cast, and this seems negatively
useful since you still need to know the result type for some uses.

> I have patches which fix the issue by using 1U in these cases.  I did
> not change non-broken cases.  I also did not change contributed code.

It is also reasonable not to touch code in arch-dependent headers.

> An incomplete listing of the issues available here:
> http://people.freebsd.org/~eadler/files/1..31.txt

The first few in the list (in binutils) are probably correct, since
the 1 is cast (not when the cast is to bfd_signed_vma though).  These
are in contrib'ed code and casting is not very common.

Bruce