Date: Tue, 26 Nov 2013 07:54:37 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Eitan Adler <lists@eitanadler.com> Cc: "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: 1 << 31 and related issues Message-ID: <20131126070729.N3780@besplex.bde.org> In-Reply-To: <CAF6rxgm9Q9ckhKR75sKRjAmebGGNM_jpDjiUqeUd%2B=WbCf6TRw@mail.gmail.com> References: <CAF6rxgm9Q9ckhKR75sKRjAmebGGNM_jpDjiUqeUd%2B=WbCf6TRw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131126070729.N3780>