1. Home
2. Academy
3. Embedded C from Zero
4. Volume 16

16.1 Undefined behaviour, catalogued

Undefined behaviour is not "something unpredictable happens". It is the standard giving no meaning to a construct, which lets the compiler assume you never write it.

That second half is the part that surprises people. If a program can only reach a line by doing something undefined, the compiler may decide the line is unreachable and delete it.

The overflow check that is not one

bool grows_signed(int x)
{
    return x + 1 > x;        /* did adding one make it bigger? */
}

bool grows_unsigned(unsigned x)
{
    return x + 1u > x;       /* the same question, unsigned */
}

The first can only be false if x + 1 overflows. Signed overflow is undefined, so it cannot be false, so it is true. Here is what gcc -O2 produced:

endbr64
movl    $1, %eax
ret

endbr64
cmpl    $-1, %edi
setne   %al
ret

The signed version never looks at x. It returns 1. The unsigned version does real work, because unsigned arithmetic wraps by definition and the answer genuinely can be false.

It gets more pointed with the check people actually write:

bool safe_to_add_signed(int a, int b)
{
    return a + b >= a;
}

endbr64
movl    %esi, %eax
notl    %eax
shrl    $31, %eax
ret

Read that assembly: it takes b, inverts it, and shifts the sign bit down. It computes b >= 0. The compiler removed the addition entirely, because the only way the sum could be smaller is by overflowing, which cannot happen. The check no longer checks anything.

bool safe_to_add_checked(int a, int b)
{
    if (b >= 0) {
        return a <= INT_MAX - b;
    }
    return a >= INT_MIN - b;
}

The catalogue

These are the ones that reach real firmware.

Make it report itself

A compiler can insert checks. Build the test firmware with a sanitiser and undefined behaviour stops being silent.

vol16_ub.c:24:13: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
vol16_ub.c:32:45: runtime error: shift exponent 33 is too large for 32-bit type 'unsigned int'
vol16_ub.c:39:48: runtime error: index 4 out of bounds for type 'int [4]'
vol16_ub.c:39:9: runtime error: load of address 0xADDRESS with insufficient space for an object of type 'int'
vol16_ub.c:46:44: runtime error: division by zero

Each one names the file, the line and the column. Without the sanitiser the program printed numbers and carried on, which is the whole danger: undefined behaviour usually looks like it worked.

16.2 MISRA C ideas in plain words

MISRA C is a list of rules for C in safety-related software. Most of them are one idea: stay away from the parts of C whose behaviour is surprising or undefined.

You may never work on a project that requires it. The reasoning is still worth having, because the rules were written by people who had already had the bugs.

The ideas worth taking

1. Use the fixed-width types. uint16_t rather than int, so the size is the same everywhere
2. One exit from a loop, and no fall-through in a switch. Both make the control flow readable at a glance
3. Always brace a body. if (x) do_thing(); is one edit away from a bug
4. Do not mix signed and unsigned in a comparison. The signed value is converted, and a negative number becomes enormous
5. Give every switch on an enum a default. Volume 12 used it to reach a safe state
6. No dynamic memory after start-up. Volume 14 gave the reasons in detail
7. No recursion. The stack depth stops being something you can calculate
8. Check every return value, or cast it to (void) deliberately. Then ignoring one is a decision rather than an oversight

The signed and unsigned trap, since it catches everyone

for (unsigned i = 10u; i >= 0u; i--) {      /* i is never negative */
    use(i);
}

An unsigned is never less than zero, so the condition is always true. When i is 0 and the loop decrements it, it wraps to 4294967295 and carries on.

int  a = -1;
unsigned b = 1u;

if (a < b) { /* not taken */ }              /* -1 converts to 4294967295 */

16.3 Defensive coding and assertions

Defensive coding checks what other people might get wrong. An assertion checks what you believe cannot be wrong. They are different tools and they belong in different places.

status_t set_speed(int rpm)
{
    if (rpm < 0 || rpm > MAX_RPM) {
        return ERR_BAD_ARG;
    }
    /* ... */
}

static void write_pin(uint8_t pin)
{
    ASSERT(pin < 16u);
    /* a private function, and
       every caller is in this
       file */
}

The rule of thumb: if a value comes from outside your code - a caller you do not control, a sensor, a message, a user - handle it. If it comes from your own code and being wrong means you have a bug, assert it.

An assertion that suits a chip

The standard assert prints to stderr and calls abort. A microcontroller has neither. So embedded projects write their own, which records where it happened and then goes somewhere safe.

static void assert_failed(const char *file, unsigned line)
{
    last_failure.file  = file;      /* in RAM the start-up code does not clear */
    last_failure.line  = line;
    last_failure.valid = true;
    /* on a real chip: stop the motor, open the relay, then reset */
}

#define ASSERT(c) \
    do { if (!(c)) { assert_failed(__FILE__, __LINE__); } } while (0)

1. an assertion that holds, and one that does not
   speed 40 <= 100: nothing happened, safe state entered 0 times
   speed 140 <= 100: failed at vol16_assert.c line 59
   safe state entered 1 time

Storing it somewhere that survives a reset is the whole point. A board that resets and can then tell you which line failed is diagnosable. One that just resets is not.

The trap: work inside the assertion

In a release build the macro usually expands to nothing. Everything inside the brackets goes with it.

2. the trap: work inside the assertion
   debug build:   init_hardware called 1 time
   release build: init_hardware called 0 times
   the hardware is never initialised in the build you ship

bool ok = init_hardware();      /* happens in every build */
ASSERT(ok);                     /* only the check disappears */
(void)ok;                       /* so it is not unused when ASSERT is empty */

16.4 Watchdogs

A watchdog resets the chip unless the program keeps telling it not to. It only helps if the telling proves the work is happening.

The hardware is simple: a counter that counts down, and resets the chip if it reaches zero. Your code writes a value to it periodically - kicking it - to start it again.

The question is where the kick goes, and there is one answer that looks convenient and is worthless.

The version that protects nothing

Put the kick in a timer interrupt and it is neat, regular, and impossible to forget. It also keeps running perfectly while the rest of the firmware is dead.

1. kicked from a timer interrupt
   control task ran 200 times and stopped at 2000 ms
   watchdog resets in 5000 ms: 0
   the interrupt is still running, so the watchdog is happy
   the board is doing nothing useful and will never recover

Three seconds with the control loop dead, and not one reset. The watchdog was doing exactly what it was told, and what it was told had nothing to do with whether the firmware worked.

The version that works

Each task reports that it has run. The kick happens only when all of them have reported since the last one.

if (sensor_task())  { sensor_ok  = true; }
if (control_task()) { control_ok = true; }
if (display_task()) { display_ok = true; }

if (sensor_ok && control_ok && display_ok) {
    watchdog_kick();
    sensor_ok = control_ok = display_ok = false;
}

2. kicked only when every task has reported
   control task ran 200 times and stopped at 2000 ms
   watchdog fired at 2990 ms, 990 ms after the hang
   the board resets and comes back working

Within one watchdog period of the hang, the board resets and recovers.

Choosing the timeout

Long enough that the slowest legitimate pass of the loop never trips it. Short enough that a hang is noticed before it matters.

1. Measure the worst-case loop time, including the longest interrupt storm
2. Multiply by a comfortable factor, two or three
3. Check the result against how long the device may safely misbehave
4. If those two numbers do not overlap, the design needs changing, not the timeout

16.5 Error-handling patterns

C has no exceptions, so a function that can fail must say so in its return value, and every caller must look. The shape you pick decides whether that stays true as the project grows.

Shape one: a special value

int read_temperature(void)
{
    if (failed) {
        return -1000;       /* "error" */
    }
    return 235;             /* 23.5 degrees */
}

1. a sentinel value in the return
   working:  read_temperature_sentinel() = 235
   failing:  read_temperature_sentinel() = -1000
   -1000 means an error, and also means -100.0 degrees
   a caller that forgets to check gets a number, not a warning

Two problems. The sentinel is a legal reading, so a caller cannot always tell. And there is only one of it, so "timed out" and "checksum wrong" look identical.

Shape two: a status code, and the value through a pointer

status_t read_temperature(int16_t *out)
{
    if (out == NULL) {
        return ERR_BAD_ARG;
    }
    if (failed) {
        return ERR_TIMEOUT;     /* *out is left alone */
    }
    *out = 235;
    return OK;
}

2. a status code, with the value through a pointer
   working:  status OK, value 235
   failing:  status ERR_TIMEOUT, value untouched
   every reading is a valid temperature, and the status is separate
   a null pointer is caught too: ERR_BAD_ARG

The whole range of the value is available, the reason for failing is specific, and adding a new failure reason does not change any existing code.

Shape three: several things that can fail, and things to undo

This is where C's lack of exceptions is felt, and where the one legitimate use of goto lives.

static status_t send_message(const uint8_t *msg, unsigned len)
{
    status_t st;

    if (msg == NULL || len == 0u) {
        return ERR_BAD_ARG;         /* nothing acquired yet */
    }

    st = step_power_up();
    if (st != OK) {
        return st;
    }
    power_on = true;                /* record only after it succeeded */

    st = step_open_port();
    if (st != OK) {
        goto release_power;
    }
    port_open = true;

    st = step_send();
    if (st != OK) {
        goto release_port;
    }

    st = step_read_reply();

release_port:
    if (port_open) { close_port(); port_open = false; }
release_power:
    if (power_on)  { power_down(); power_on  = false; }
    return st;
}

The labels are in reverse order of acquisition, so each one undoes exactly what was acquired above it. Jumping to the right label releases everything that was taken and nothing that was not.

the port will not open
   power up        ok
   open port       failed
   power down      done
   result: ERR_NOT_READY, power off, port closed

the reply never comes
   power up        ok
   open port       ok
   send bytes      ok
   read reply      failed
   close port      done
   power down      done
   result: ERR_CHECKSUM, power off, port closed

Notice the failed open. It does not close a port that never opened, because the flag is set only after the step succeeds. That ordering is the part people get wrong.

__attribute__((warn_unused_result))
status_t send_message(const uint8_t *msg, unsigned len);

What you learned

• Undefined behaviour lets the compiler assume the construct never occurs, and optimise on that
• a + b >= a compiled to a test of b >= 0: the addition was removed entirely
• Never test for overflow by overflowing; subtract from the limit instead
• A sanitiser turns silent undefined behaviour into a message naming the file, line and column
• Keep hardware-free logic separate, so it can be run under a sanitiser on a desktop
• Most MISRA rules are one idea: avoid the parts of C that are undefined or surprising
• An unsigned is never less than zero, so i >= 0 never ends a loop
• Defensive checks are for what others may get wrong; assertions are for what cannot be wrong
• Store an assertion failure where a reset cannot erase it, or the board can tell you nothing
• Never put work inside an assertion: the release build removes the whole expression
• A watchdog kicked from a timer interrupt protected a dead board for three seconds
• Kick the watchdog only when every task has reported since the last kick
• Prefer a status code with the value through a pointer over a sentinel value
• Use one error enum for the whole project
• Acquire, then record, then release in reverse order at cleanup labels

Practice

status_t st = spi_write(cmd);
ASSERT(st == OK);
(void)st;

void configure(void)
{
    open_bus();
    write_config(0x40);
    set_mode(FAST);
    close_bus();
}

status_t configure(void)
{
    status_t st = open_bus();
    if (st != OK) {
        return st;                  /* nothing to release */
    }

    st = write_config(0x40u);
    if (st != OK) {
        goto release_bus;
    }

    st = set_mode(FAST);

release_bus:
    {
        status_t close_st = close_bus();
        if (st == OK) {
            st = close_st;          /* do not hide an earlier failure */
        }
    }
    return st;
}

Interview corner

Next, Volume 17 is about the part nobody teaches: what to do when it does not work. Reading a hard fault, using a debugger properly, printf that does not lie, and finding bugs that only happen on the real board.

Key words from this volume

Every word below has a plain-English entry in the glossary.

• Undefined behaviour
• Sanitiser
• MISRA C
• Defensive programming
• Assertion
• Safe state
• Watchdog
• Brown-out
• Error code