Dr. Martin Ettl‎ > ‎Articles‎ > ‎

Detecting Undefined Behavior by Verifying Atmel® Software Framework with Cppcheck

Detecting Undefined Behavior by

Verifying Atmel® Software Framework

with

Cppcheck

2016-06-16

 

This article provides insight about some potential programming issues, that have been detected by verifying the Atmel® Software Framework (ASF) with the static code analysis tool Cppcheck.

The ASF is a collection of embedded software for Atmel® flash Micro-Controller Units (MCUs), which can be downloaded from ASF. For this article, the ASF standalone archive for GCC and IAR Embedded Workbench, version ASF-3.34.2 has been used (date of download 2017-06-10). A backup of the code is available here.

Cppcheck is a static code analysis tool for the C and C++ programming languages. This tool is free software and released under the GNU General Public License. It has become an essential helper for many developers to improve the quality of their code-base. The latest stable version 1.80, which has been used for this analysis, has more than 200 different checks that are categorized into five different severity: errors, warnings, style, performance, and portability. 

What is Static Code Analysis?

A static code analysis program performs verification of a software without executing the program. The verification is normally performed by parsing and analyzing the source code.

What is Undefined Behavior?

Quoted from Wikipedia (2017-06-16):

In computer programming, undefined behavior (UB) is the result of executing computer code whose behavior is not prescribed by the language specification to which the code adheres, for the current state of the program (e.g. memory). This happens when the translator of the source code makes certain assumptions, but these assumptions are not satisfied during execution.

This report focuses mainly on issues resulting in undefined behavior.

Examples for UB are:

Integer Division by Zero

According the language standards C99 and C++-03, in case an integer is divided by a zero value, the behavior of the program is undefined:

C99 6.5.5p5 - The result of the / operator is the quotient from the division of the first operand by the second; the result of the % operator is the remainder. In both operations, if the value of the second operand is zero, the behavior is undefined.

C++03 5.6.4 - The binary / operator yields the quotient, and the binary % operator yields the remainder from the division of the first expression by the second. If the second operand of / or % is zero the behavior is undefined.

The C99 and C++03-standard defines division by zero as undefined behavior

Array Index Out of Bounds

An array index out of bounds error will result in undefined behavior, when data is written to a memory location, that is not allocated. A simple example for an array index out of bounds error is given as follows:

int main() {

    int a[2];

    a[0] = 0;

    a[1] = 0;

    a[2] = 0;

    return a[0];

}

Example code to demonstrate array index out of bounds

Cppcheck reports for this code:

(error) Array 'a[2]' accessed at index 2, which is out of bounds.

In the example code an array of two integers are allocated and the array at index 2 is accessed. This memory is allocated and leads to undefined program behavior.

Usage of Uninitialized Memory

The usage of uninitialized variables can lead to undefined behavior. Local variables and structures are not initialized by default. Therefore, memory is allocated, but the returned value is undefined. It’s the duty of the programmer to ensure usage of uninitialized variables are prohibited.

 


Potential Undefined Behavior found in ASF

This section provides the issues that have been detected by letting Cppcheck analyze the ASF source code.


Integer Division by Zero in ASF

Cppcheck reports a division by zero for this function:

Code from xdk-asf-3.34.2/avr32/drivers/abdac/abdac.c

unsigned long abdac_get_dac_hz(volatile avr32_abdac_t *abdac, const unsigned long bus_hz) {

  volatile avr32_pm_t *pm = &AVR32_PM;

  unsigned short div = 0;

  if (pm->gcctrl[ABDAC_GCLK] & GCLK_BIT(DIVEN)) {

    div = 2 * (GCLK_BFEXT(DIV, pm->gcctrl[ABDAC_GCLK]) + 1);

  }

  return (bus_hz / div);

}

Note: This function has been formatted.

As illustrated, Cppcheck is correct. In case the if-condition is evaluated to logically false during executing abdac_get_dac_hz, the variable bus_hz will be divided by a zero value, which results in undefined program behavior.


Conditional Integer Division by Zero

A conditional integer division by zero warning from Cppcheck normally points out a logical error in the source code. For clarification, this simple example is provided:

void f(int x) {

    int y = 42/x;

    if( x > 0 ) {}

}

Analyzing this example with Cppcheck shows following warning:

(warning) Either the condition 'x>0' is redundant or there is division by zero at line 2.

This provides insight on how this special division by zero checks operates. In case it detects an integer division (line 2), it tracks the divisor (variable x) for being checked against 0 (line 3). Therefore, this check helps to reveal inconsistences in checking variable values that can potentially lead to division by zero.


Conditional Integer Division by Zero in ASF

Cppcheck reports while scanning the file pll.h:

Either the condition 'div>0' is redundant or there is division by zero at line 142.

 Code from xdk-asf-3.34.2/common/services/clock/uc3a0_a1/pll.h

static inline void pll_config_init(struct pll_config *cfg, enum pll_source src, unsigned int div, unsigned int mul) {

     uint32_t vco_hz;

     Assert(src < PLL_NR_SOURCES);

     /* Calculate internal VCO frequency */

    vco_hz = osc_get_rate(src) * mul;

    vco_hz /= div;

   

    Assert((mul > 2) && (mul <= 16));

    Assert((div > 0) && (div <= 15));

 

   cfg->ctrl |= ((mul - 1) << AVR32_PM_PLL0_PLLMUL) | (div << AVR32_PM_PLL0_PLLDIV)

                 | (PLL_MAX_STARTUP_CYCLES << AVR32_PM_PLL0_PLLCOUNT)

                 | (src << AVR32_PM_PLL0_PLLOSC);

}

Note: This function has been formatted. In addition, code that is not relevant to demonstrate this issue has been removed. This is indicated by three dots (…).

As the colorized code from shows, Cppcheck is correct. First, a division is performed and after that it is checked if the divisor was zero. It is suggested to move the Assert before the division.

This issue has been detected multiple times. The files where similar code has been found are:

  • xdk-asf-3.34.2/common/services/clock/uc3a0_a1/pll.h
  • xdk-asf-3.34.2/common/services/clock/uc3a3_a4/pll.h
  • xdk-asf-3.34.2/common/services/clock/uc3b0_b1/pll.h
  • xdk-asf-3.34.2/common/services/clock/uc3c/pll.h
  • xdk-asf-3.34.2/common/services/clock/uc3d/pll.h
  • xdk-asf-3.34.2/common/services/clock/uc3c/pll.h

  

Usage of Uninitialized Variables in ASF

This section contains several examples where the usage of uninitialized variables has been identified.

Returning an Uninitialized Value

Cppcheck reports while scanning main.c:

     Uninitialized variable: messages

 Code from xdk-asf-3.34.2/avr32/services/dsp/dsplib/utils/programs/data_print/main.c

int WINAPI WinMain(HINSTANCE hThisInstance, HINSTANCE hPrevInstance, LPSTR lpszArgument, int nFunsterStil)

{

  MSG messages;

  WNDCLASSEX wincl;

  HBRUSH hbrush;

  char szClassName[] = "c_" TITLE;

 

    // if no argument is passed to this function, return a usage message

    if (!*lpszArgument)

    {

       SET_ERROR("Usage: DataPrinter filename");

       return messages.wParam;

    }


Note
: This function has been formatted. In addition, code that is not relevant to demonstrate this issue has been removed. This is indicated by three dots (…).

The variable MSG is a struct from the Windows API (MSG struct), which is not initialized after its creation. As shown, the struct member wParam is returned without being initialized.

 


Another issue has been found at xdk-asf-3.34.2/sam/drivers/pwm/pwm_sync_example/pwm_sync_example.c. The variable ul_numkey is only conditionally initialized within the if-clauses. In case none of the if-conditions are evaluated to logically ‘true’, the value of ul_numkey is still undefined and it will be returned:

static uint8_t get_num_value(void) {

                uint32_t ul_numkey;

                uint8_t uc_key1, uc_key2;

                puts("New value : ");

             

                while (uart_read(CONSOLE_UART, &uc_key1));

                printf("%c", uc_key1);

                while (uart_read(CONSOLE_UART, &uc_key2));

                printf("%c", uc_key2);

                puts("\r\n");

                if ('0' <= uc_key1 && '9' >= uc_key1) {

                                ul_numkey = (uc_key1 - '0');

                }

                if ('0' <= uc_key2 && '9' >= uc_key2) {

                                ul_numkey *= 10;

                                ul_numkey += (uc_key2 - '0');

                }

                return (uint8_t) ul_numkey;

}

Code from xdk-asf-3.34.2/sam/drivers/pwm/pwm_sync_example/pwm_sync_example.c

Note: This function has been formatted. In addition, code that is not relevant to demonstrate this issue has been removed. This is indicated by three dots (…).

Other cases of uninitialized variable usage have been detected in the xdk-asf-3.34.2/thirdparty-folder. They will be omitted in this article.

Array Index Out of Bounds in ASF

A potential array index out of bounds issue has been found at xdk-asf-3.34.2/avr32/services/network/lin/lin.c. In this case, the array (lin_descript_list_node0) has a size of 8 elements. The corresponding array size is defined at xdk-asf-3.34.2/avr32/services/network/lin/lin.h:

Code from xdk-asf-3.34.2/avr32/services/network/lin/lin.h

#define NUMBER_OF_LIN_FRAMES_NODE0    8


As shown in this reduce code excerpt, it is possible that the array index goes out of bounds:

  Code from xdk-asf-3.34.2/avr32/services/network/lin/lin.c 

  volatile st_lin_message   lin_descript_list_node0[NUMBER_OF_LIN_FRAMES_NODE0];

   

   // Here the communication go on only in case no error is detected!!!

   else {

        usart_lin_set_node_action(usart_lin_node0,

                                  lin_descript_list_node0[l_handle].l_cmd);

        if(l_handle != 0xFF)

        {

  

The potential array index out of bounds issue could have been avoided by simply first checking the index variable against being lower than NUMBER_OF_LIN_FRAMES_NODE0:

  volatile st_lin_message   lin_descript_list_node0[NUMBER_OF_LIN_FRAMES_NODE0];

   

   // Here the communication go on only in case no error is detected!!!

   else {

        if(l_handle < NUMBER_OF_LIN_FRAMES_NODE0)

        {

        usart_lin_set_node_action(usart_lin_node0,

                                  lin_descript_list_node0[l_handle].l_cmd);

   


Another array index out of bounds issue has been detected at xdk-asf-3.34.2/thirdparty/hd/example/ports/avr32/gui_getstring.c. The following code shows the lines where Cppcheck raised a buffer access out of bounds warning:


Code from xdk-asf-3.34.2/thirdparty/hd/example/ports/avr32/gui_getstring.c  

     

       else  {

                /* add current char */

                chr = char_array[get_string_data.row][get_string_data.col];

                getstring[gs_idx] = chr;

                gs_idx++;

                if (gs_idx > sizeof getstring) {

                        gs_idx = sizeof getstring;

                }

                getstring[gs_idx] = 0;

        }

      

In this context, the variable gs_idx is a global variable, which accesses the buffer getstring out of bounds. For better illustration, a reduced C++ example has been created (test.cpp):

#include <iostream>

#define MAX 10

char getstring[MAX];

int gs_idx = 0;

void f(char chr) {

    if (gs_idx > sizeof getstring {

        gs_idx = sizeof getstring;

    }

    getstring[gs_idx] = 0;

}

int main() {

                gs_idx = MAX;

                f('x');

                return 0;

}

As shown in this reduced example, the index gs_idx is set to 10, because of sizeof getstring. After that, a zero-value is written to the memory location one element past the end of the allocated array.

Side-note: These issues can also be detected by using the AddressSanitizer.

 

Conclusion

As we have seen, it is very simple to identify potential programming errors with the help of a static code analysis tool such as Cppcheck. 

Since Cppcheck focuses on finding bugs instead of stylistic issues, the results are accurate, as the tool is designed to avoid false warnings.

Therefore, the developers of ASF could simply use Cppcheck for non-cost, to increase the quality of their code. 


References:

Comments