The C infinite for-loop gotcha is one of the less frequent issues I find with static analysis, but I feel it is worth documenting because it is obscure but easy to do.
Consider the following C example:
Since i is a 8 bit integer, it will wrap around to zero when it reaches the maximum 8 bit value of 255 and so we end up with an infinite loop if the upper limit of the loop n is 256 or more.
The fix is simple, always ensure the loop counter is at least as wide as the type of the maximum limit of the loop. This example, variable i should be a uint32_t type.
I've seen this occur in the Linux kernel a few times. Sometimes it is because the loop counter is being passed into a function call that expects a specific type such as a u8, u16. In other occasions I've seen a u16 (or short) integer being used presumably because it was expected to produce faster code, however, most commonly 32 bit integers just as fast (or sometimes faster) than 16 bit integers for this kind of operation.
It should always be "int i;".
ReplyDeleteOn my system it takes 8 seconds to count to U32_MAX. There are definitely times when we need to loop over size_t but it's not very common in the kernel. If you're doing a special long loop like this then use a size_t type and call the variable page_idx or something so that everyone knows it's not a regular loop.
When you have a huge loop the the type should never be u32. It should be long or long long. There isn't that big of a jump between 2 and 4 million so very few problems are solved by u32.
In the kernel when you enable W=3 then GCC tells you to make all your loop iterators into u32. This is terrible advice and leads to a lot of signedness bugs. GCC should be fixed.