129. MISRA C and defensive programming

Sections 122–128 showed where C lets you down: type promotion, undefined behavior, macros, memory layout. MISRA C is a set of rules that bans the most dangerous language constructs outright, so you do not have to keep them in mind. Defensive programming is the habit of writing code so that bad data and your own mistakes do not turn into a device failure. This section tests in practice what each tool finds: the compiler, cppcheck with the MISRA rules, gcc's execution path analysis and armclang's stack protection.

Terms used in the documentation

Term	Meaning
MISRA C	a set of C language rules for dependable systems
mandatory / required / advisory	a rule with no exceptions / one you may deviate from in writing / one left to the team
deviation	a documented exception to a rule
static analysis	checking code without running it
essential type	the type of an expression as the MISRA rules see it
defensive programming	writing code that survives bad data and its own bugs
fail-safe state	the safe state the device enters on an error
stack protector, canary	stack protection, a guard word in the stack frame
false positive	a warning about code that is actually correct
coding standard	rules for how code is written and laid out

129.1 Why rules

MISRA C was born in the automotive industry, and now it is required in medical devices, on the railways, in industrial automation. The rules come in three kinds: mandatory — no exceptions; required — you may deviate, but in writing, with a justification; advisory — at the team's discretion. A deviation is nothing to be ashamed of: what is shameful is breaking a rule without knowing it.

The rule texts are sold separately, so analyzers print only numbers, for example misra-c2012-15.6. Below, the meaning of the rules is given in our own words — that is enough to fix the code.

Even if the device needs no certification, the rules are useful as a list of what breaks most often in C. A machine checks most of them.

129.2 Experiment: parsing a command "quick and dirty"

A function parses a packet: the sum of the bytes, the command in the first byte, a copy for the log. Ordinary code, the kind you see everywhere:

uint8_t parse(const uint8_t *p, int n)
{
    int i, sum = 0;
    int level = 0;
    for (i = 0; i < n; i++)
        sum += p[i];
    if (sum > 1000)
        goto fail;
    switch (p[0]) {
    case 1:
        level++;
    case 2:
        level--;
        break;
    }
    uint8_t *copy = malloc(n);
    memcpy(copy, p, n);
    free(copy);
    return sum + level;
fail:
    return 0xFF;
}

Besides parse, the file has a global int level and a function count_below (neither is in the listing). cppcheck with the MISRA add-on found in this file 15 violations of 10 rules:

Some of these places are already bugs, and a run on the PC showed them. With n == 0 the function reads p[0] past the end of the packet, and when memory runs out memcpy writes to address zero — on the PC both cases crash. return sum + level truncates the sum to 8 bits: a packet with a sum of 1000 gives 232. Command 1 falls through into case 2 and does not change level at all — that is the forgotten break. The other places are not bugs by themselves, but each of them has led to bugs somewhere: a body without braces means a line added "inside" an if that always executes.

129.3 The same function by the rules

status_t parse(const uint8_t *p, uint32_t n, int32_t *level)
{
    status_t st = ST_OK;

    if ((p == NULL) || (level == NULL) || (n == 0u) || (n > FRAME_MAX)) {
        st = ST_BAD_ARG;
    } else {
        uint32_t sum = 0u;
        for (uint32_t i = 0u; i < n; i++) {
            sum += (uint32_t)p[i];
        }
        if (sum > SUM_LIMIT) {
            st = ST_BAD_SUM;
        } else {
            switch ((uint32_t)p[0]) {
            case CMD_UP:
                *level += 1;
                break;
            case CMD_DOWN:
                *level -= 1;
                break;
            default:
                st = ST_BAD_CMD;
                break;
            }
        }
    }
    return st;
}

What changed:

• the arguments are checked before the first access, the length is limited by FRAME_MAX;
• the function returns a result code and passes the new value back through a pointer, and the status_t enumeration names each error;
• all bodies are in braces, switch has a default, each case has a break;
• a single exit point, no goto;
• no dynamic memory;
• one declaration, one variable;
• all variables are unsigned where negative values cannot occur.

cppcheck with the MISRA rules — not a single remark; gcc with -Wconversion -Wshadow -pedantic — not a single warning. The code got longer, but every line can be understood without guessing.

129.4 The compiler is the first analyzer

The free analyzer you already have installed is the compiler's warnings. On the careless file gcc reported:

-Wall -Wextra caught only the case fallthrough and the signed/unsigned comparison. The extra options also found the local level that shadows the global one, and three implicit conversions that lose the sign or the value. The set for firmware: -Wall -Wextra -Wconversion -Wsign-conversion -Wshadow, plus -Werror in the server build, so that a warning cannot be ignored (section 110). armclang understands the same options and gave 5 warnings on the careless file, but it flags the case fallthrough only with the separate -Wimplicit-fallthrough option — it is worth adding.

129.5 Execution path analysis

Some bugs are visible only to someone who has walked every branch of the program. A file with four memory errors, for example a read after free:

int last_byte(uint8_t *f)
{
    free(f);
    return f[0];
}

What each tool found:

gcc -fanalyzer found all four and for each one printed the path by which it got there. The compiler and cppcheck found two each — and different ones. The practical conclusion: the tools complement each other, so run them together. -fanalyzer is slow — it belongs in the nightly build, not in every one.

129.6 Do not trust input data

Everything that comes from outside — over UART, radio, from a sensor, from Flash memory — can be wrong. The defensive parser was tested on corrupted packets, with the same results on three machines:

The rules behind this:

• check the length before copying, the index before accessing the array;
• return an error code and check it in the caller;
• on an error, do not change the state: a partly applied command is worse than a rejected one;
• compute in unsigned types of suitable width: a sum of 16 bytes of 255 each will not overflow uint32_t.

129.7 The safe state

default in a switch is the place where the program admits that the impossible has happened. In a state machine (section 125) the state variable can get corrupted by a write past the end of an array or by a power glitch; if default is missing, the machine silently freezes. The right thing is to go to a safe state: switch off the power outputs, record the cause, and wait for the watchdog reset (section 12.1).

Enumerations that come from external data are handled the same way: a value that matches none of the known ones is an error, not "do nothing".

129.8 Stack protection on the board

A function that trusts the length from a packet:

uint8_t buf[32];
memcpy(buf, src, n);

With n greater than 32 the copy overwrites the stack frame, including the return address. armclang with -fstack-protector-strong puts a guard word into the frame and checks it before returning. In the F103 object file the function copy_packet grew from 28 bytes to 60, and the stack frame from 0x20 to 0x28: the word __stack_chk_guard is read on entry, compared on exit, and on a mismatch __stack_chk_fail is called.

Both are written by the firmware: __stack_chk_guard is a variable that is filled with a random number at startup (on the F407 — from the hardware generator, section 43), __stack_chk_fail is a handler that puts the device into the safe state. The word is written before any protected function is called — in the startup code or on the first line of main — and the function that changes it is built with __attribute__((no_stack_protector)): otherwise, on exit from it, the check will not match and __stack_chk_fail will fire. Stack protection is insurance, not a cure: the cure is a length check before copying:

if ((src != NULL) && (n <= sizeof buf)) {

129.9 Run-time checks

• assert in a debug build stops the program where the condition is violated; in a release build NDEBUG turns it off. On a board assert is routed to your own handler: record the file and line (section 127), go to the safe state.
• The watchdog timer resets a hung program (section 12.1); refresh it only from the main loop, not from an interrupt.
• A firmware checksum at startup — a CRC of the whole Flash memory (sections 12.5 and 43.1): corrupted firmware must not start.
• Guard words at the beginning and end of important structures in RAM catch a write past the end of a neighboring array.
• The level of protection depends on the cost of an error: a corridor light is not protected like this, a door drive certainly is.

129.10 Pitfalls

The analyzer produced hundreds of remarks, and all of them were turned off. Start with new code and the mandatory rules; old files gradually, deviations in writing.

A false positive. The analyzer does not know the pointer has already been checked in another file. Rewrite the code more clearly or document a deviation instead of turning the check off entirely.

-Wconversion on old code is a wall of warnings. Enable it module by module and fix it with explicit casts where loss of value really is impossible.

__stack_chk_fail not found at link time. You enabled -fstack-protector-strong but did not write the handler and the guard word.

Defensive code "never executes" and was thrown out. Checks for bad data are not dead code; they are tested with corrupted packets.

default is there, but empty. That is the same as not having it. An impossible value is an error, and it must be handled.

129.11 Summary

• MISRA C is a list of dangerous constructs; rules are mandatory, required and advisory, deviations are made in writing.
• cppcheck with the MISRA add-on found 15 violations of 10 rules in the careless parser: braces (15.6), goto (15.1), break (16.3), default (16.4), malloc (21.3) and others; the fixed code — none.
• gcc warnings: -Wall -Wextra — 2, with -Wconversion -Wsign-conversion -Wshadow — 6.
• Memory errors: -fanalyzer found 4 out of 4, the compiler and cppcheck 2 each, and different ones; run the tools together.
• Check input data; on an error, return a result code and leave the state untouched; default leads to the safe state.
• -fstack-protector-strong: 28 → 60 bytes, frame 0x20 → 0x28, on corruption __stack_chk_fail; the main cure is a length check.

Part XV "C in depth" continues with two hands-on sections: 129а, unit tests for firmware on the PC, and 129б, static analysis.

Full source code

bad129.c — parsing a command "quick and dirty" — with violations on purpose.

/* Раздел 129: разбор команды так, как его пишут «по-быстрому».
 * Файл нарочно нарушает правила: cppcheck с дополнением MISRA и
 * предупреждения gcc обязаны это найти. Логика та же, что в good129.c. */
#include <stdint.h>
#include <stdlib.h>
#include <string.h>

int level;

uint8_t parse(const uint8_t *p, int n)
{
    int i, sum = 0;
    int level = 0;
    for (i = 0; i < n; i++)
        sum += p[i];
    if (sum > 1000)
        goto fail;
    switch (p[0]) {
    case 1:
        level++;
    case 2:
        level--;
        break;
    }
    uint8_t *copy = malloc(n);
    memcpy(copy, p, n);
    free(copy);
    return sum + level;
fail:
    return 0xFF;
}

unsigned count_below(const int *v, unsigned n, int limit)
{
    unsigned count = 0;
    for (int i = 0; i < n; i++)
        if (v[i] < limit)
            count++;
    return count;
}

good129.c — the same logic following the rules of defensive programming.

/* Раздел 129: та же логика, что в bad129.c, по правилам защитного
 * программирования: проверка аргументов, фигурные скобки, default в switch,
 * одна точка выхода, без динамической памяти и без смешения знаковых типов. */
#include <stddef.h>
#include "good129.h"

#define SUM_LIMIT 1000u
#define CMD_UP 1u
#define CMD_DOWN 2u

status_t parse(const uint8_t *p, uint32_t n, int32_t *level)
{
    status_t st = ST_OK;

    if ((p == NULL) || (level == NULL) || (n == 0u) || (n > FRAME_MAX)) {
        st = ST_BAD_ARG;
    } else {
        uint32_t sum = 0u;
        for (uint32_t i = 0u; i < n; i++) {
            sum += (uint32_t)p[i];
        }
        if (sum > SUM_LIMIT) {
            st = ST_BAD_SUM;
        } else {
            switch ((uint32_t)p[0]) {
            case CMD_UP:
                *level += 1;
                break;
            case CMD_DOWN:
                *level -= 1;
                break;
            default:
                st = ST_BAD_CMD;
                break;
            }
        }
    }
    return st;
}

uint32_t count_below(const int32_t *v, uint32_t n, int32_t limit)
{
    uint32_t count = 0u;

    if (v != NULL) {
        for (uint32_t i = 0u; i < n; i++) {
            if (v[i] < limit) {
                count++;
            }
        }
    }
    return count;
}

guard129_arm.c — stack protection on the F103: the compiler's guard word and a length check.

/* Раздел 129: защита стека на плате — armclang для F103.
 * copy_packet доверяет длине из пакета; с -fstack-protector-strong
 * компилятор ставит в кадр контрольное слово и проверяет его перед
 * возвратом. copy_packet_checked защищается проверкой длины. */
#include <stdbool.h>
#include <stdint.h>
#include <string.h>

void process(uint8_t *buf, uint32_t n);

void copy_packet(const uint8_t *src, uint32_t n)
{
    uint8_t buf[32];
    memcpy(buf, src, n);
    process(buf, sizeof buf);
}

bool copy_packet_checked(const uint8_t *src, uint32_t n)
{
    bool ok = false;
    uint8_t buf[32];

    if ((src != NULL) && (n <= sizeof buf)) {
        memcpy(buf, src, n);
        process(buf, n);
        ok = true;
    }
    return ok;
}

Next: section 129а "Unit tests for firmware on the PC: faking the hardware, a small runner of your own and coverage" — how to run a driver that writes to registers and calls HAL on a PC.