Friday, 21 May 2021

Adjacent C string concatenation gotcha

C has the useful feature of adjacent allowing literal strings to be automatically concatenated. This is described in K&R "The C programming language" 2nd edition, page 194, section A2.6 "String Literals":

"Adjacent string literals are concatenated into a single string."

Unfortunately over the years I've seen several occasions where this useful feature has led to bugs not being detected, for example, the following code:

 

A simple typo in the "Does Not Compute" error message ends up with the last two literal strings being "silently" concatenated, causing an array out of bounds read when error_msgs[7] is accessed.

This can also bite when a new literal string is added to the end of the array.  The previous string requires a comma to be added when a new string is added to the array, and sometimes this is overlooked.  An example of this is in ACPICA commit 81eb9c383e6dee0f1b6620e91e5c3dbb48234831  - fortunately static analysis detected this and it has been fixed with commit 3f8c79fc22fd64b739f51268654a6783a874520e

The concerning issue is that this useful string concatenation feature can produce hazardous outcomes with coding mistakes that are simple to make but hard to notice.

 

                                              

Friday, 23 April 2021

C Ternary operator gotcha (type conversions)

The C ternary operator expr1 ? expr2 : expr3 has a subtle side note described in K&R 2nd edition, page 52, section 2.11:

"If expr2 and expr3 are of different types, the type of the result is determined by the conversion rules discussed earlier in the chapter".

This refers to page 44, section 2.7 "Type Conversions". It's worth reading a few times along with section A6.5 "Arithmetic Conversions".

Here is an example of a type conversion gotcha:

 

At a glance one would think the program would print out -1 as the output.  Note that the expr2 is actually type converted to unsigned int so the result is 4294967295 if int types are 32 bits wide.

One solution is to type convert expr2 and/or expr3 to a long int and because that is wider than the unsigned int x this takes precedence. Alternatively just use:

References: soc: aspeed: fix a ternary sign expansion bug

Tuesday, 30 March 2021

A C for-loop Gotcha

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.

Sunday, 28 March 2021

A Common C Integer Multiplication Mistake

Multiplying integers in C is easy.  It is also easy to get it wrong.  A common issue found using static analysis on the Linux kernel is the integer overflow before widening gotcha.

Consider the following code that takes the 2 unsigned 32 bit integers, multiplies them together and returns the unsigned 64 bit result:

The multiplication is performed using unsigned 32 bit arithmetic and the unsigned 32 bit results is widened to an unsigned 64 bit when assigned to ret. A way to fix this is to explicitly cast a to a uint64_t before the multiplication to ensure an unsigned 64 bit multiplication is performed:

Fortunately static analysis finds these issues.  Unfortunately it is a bug that keeps on occurring in new code.