What could cause C to skip over a return statement?

Question

I am programming in C for a microcontroller so I am not sure if this question belongs here or on the electronics stackexchange. Just let me know if it doesn't fit here and I will move the question over there.

So I have Finite State Machine in C. This FSM is responsible for retrieving data over UART. Each state retrieves some data and performs some checks on it. Each state has three possible directions. If the received data is valid it will go to the next state. If not it will repeat its state. If the state has been repeated a certain amount of times it will move to a fail state. To indicate this direction the states return a code: ok, err, or repeat. This works in all states except for the last one. Here when calling return repeat I can see the debugger going to that statement, then going over it the the last } of the function and then it goes to a random location in memory which I can only see in the dissassembly view of Code Composer studio. Sometimes this is 0x0 sometimes its a random memory in RAM I cant see.

What could cause such strange behavior? I have optimization options turned off in Code Composer Studio.

All states have bodies that look like this:

enum ret_codes fc_state(void) {
    uint8_t buf[250];
    static uint8_t fail_counter = 0;
    const uint8_t max_tries = 5;

    int ret = 0;
    while (ret == 0) {
        ret = UART_read(_uart, buf, sizeof(buf));
    }
    if (ret == UART_STATUS_ERROR) {
        return err;
    }

    /*
        perform tests on buf ...
    */
    if (testFailed) {
        fail_counter++;
        if (fail_counter == max_tries) {
            // Write DEN over UART ...
            return err;
        }
        // write NACK over UART ...
        return repeat; // I can see the debugger reaching this statement in the fc_state
    }
    // write ACK over UART ...
    return ok;
} // When debugging the debugger goes here after 'return repeat' and to a random location in memory after this
// In the other states it goes back to the main loop described further below after 'return repeat'

The code for the FSM looks like this:

enum state_codes { ipv, fup, fc, fail, succ, invalid };

enum ret_codes { ok, err, repeat };
struct transition {
    enum state_codes src_state; // source state
    enum ret_codes ret_code; // the code it returns
    enum state_codes dst_state; // the state it should transfer to if the previous mentioned code was returned
};

enum ret_codes ipv_state(void);
enum ret_codes fup_state(void);
enum ret_codes fc_state(void);
enum ret_codes fail_state(void);
enum ret_codes succ_state(void);

enum ret_codes (* states[])(void) = {ipv_state, fup_state, fc_state, fail_state, succ_state};

struct transition transition_table[] = {
    {ipv, ok, fup}, // when in the ipv state and ok is returned then go to fup state
    {ipv, repeat, ipv}, // This repeat works
    {ipv, err, fail},
    {fup, ok, fc},
    {fup, repeat, fup}, // This repeats works as well
    {fup, err, fail},
    {fc, ok, succ},
    {fc, repeat, fc}, // This repeat fails for some reason
    {fc, err, fail}
};
enum state_codes lookup_transitions(enum state_codes, enum ret_codes);

The main loop for these states looks like this:

while (1) {
    state_fun = states[current_state];
    return_code = state_fun();
    if (current_state == fail) {
        return 1;
    }
    if (current_state == succ) {
        return 0;
    }
    current_state = lookup_transitions(current_state, return_code);
    if (current_state == invalid) {
        return 1;
    }
}

EDIT: I added the lookup_transitions table below. This implementation for the FSM was taken from https://mcmap.net/q/129430/-state-machines-tutorials-closed by the way. The code is written for Texas Instruments' CC2652R with an Arm Cortex-M4F CPU.

enum state_codes lookup_transitions(enum state_codes cur_state, enum ret_codes rc) {
    uint8_t i;
    for (i = 0; i < sizeof(transition_table); i++) {
        if (cur_state == transition_table[i].src_state && rc == transition_table[i].ret_code)
            return transition_table[i].dst_state;
    }
    return invalid; // the transition table is not configured correctly
}

This is what the Code Composer Studio disassembly viewer shows for fc_state:

505       enum ret_codes fc_state(void) {
          fc_state():
000529a0:   B500                push       {r14}
000529a2:   F1AD0D3C            sub.w      r13, r13, #0x3c
512           const uint8_t max_tries = 5;
000529a6:   2005                movs       r0, #5
000529a8:   F88D003B            strb.w     r0, [r13, #0x3b]
514           int ret = 0;
000529ac:   2000                movs       r0, #0
000529ae:   9000                str        r0, [r13]
515           while (ret == 0) {
000529b0:   9800                ldr        r0, [r13]
000529b2:   B948                cbnz       r0, #0x529c8
516               ret = UART_read(_uart, &rheader, sizeof(rheader));
          $C$L62:
000529b4:   4829                ldr        r0, [pc, #0xa4]
000529b6:   6800                ldr        r0, [r0]
000529b8:   A901                add        r1, r13, #4
000529ba:   222B                movs       r2, #0x2b
000529bc:   F003FFC6            bl         UART_read
000529c0:   9000                str        r0, [r13]
515           while (ret == 0) {
000529c2:   9800                ldr        r0, [r13]
000529c4:   2800                cmp        r0, #0
000529c6:   D0F5                beq        $C$L62
518           if (ret == UART_STATUS_ERROR) {
          $C$L63:
000529c8:   9800                ldr        r0, [r13]
000529ca:   F1B03FFF            cmp.w      r0, #-1
000529ce:   D101                bne        $C$L64
519               return err;
000529d0:   2001                movs       r0, #1
000529d2:   E040                b          $C$L70
522           ret = 0;
          $C$L64:
000529d4:   2000                movs       r0, #0
000529d6:   9000                str        r0, [r13]
523           while (ret == 0) {
000529d8:   9800                ldr        r0, [r13]
000529da:   B958                cbnz       r0, #0x529f4
524               ret = UART_read(_uart, &last_fup_info, rheader.length);
          $C$L65:
000529dc:   481F                ldr        r0, [pc, #0x7c]
000529de:   F8BD202D            ldrh.w     r2, [r13, #0x2d]
000529e2:   6800                ldr        r0, [r0]
000529e4:   F10D012F            add.w      r1, r13, #0x2f
000529e8:   F003FFB0            bl         UART_read
000529ec:   9000                str        r0, [r13]
523           while (ret == 0) {
000529ee:   9800                ldr        r0, [r13]
000529f0:   2800                cmp        r0, #0
000529f2:   D0F3                beq        $C$L65
526           if (ret == UART_STATUS_ERROR) {
          $C$L66:
000529f4:   9800                ldr        r0, [r13]
000529f6:   F1B03FFF            cmp.w      r0, #-1
000529fa:   D101                bne        $C$L67
527               return err;
000529fc:   2001                movs       r0, #1
000529fe:   E02A                b          $C$L70
530           if (
          $C$L67:
00052a00:   4817                ldr        r0, [pc, #0x5c]
00052a02:   F8DD1037            ldr.w      r1, [r13, #0x37]
00052a06:   6800                ldr        r0, [r0]
00052a08:   4288                cmp        r0, r1
00052a0a:   D115                bne        $C$L68
00052a0c:   4815                ldr        r0, [pc, #0x54]
00052a0e:   F8DD1037            ldr.w      r1, [r13, #0x37]
00052a12:   6800                ldr        r0, [r0]
00052a14:   4288                cmp        r0, r1
00052a16:   D10F                bne        $C$L68
00052a18:   4813                ldr        r0, [pc, #0x4c]
00052a1a:   F8DD1033            ldr.w      r1, [r13, #0x33]
00052a1e:   6800                ldr        r0, [r0]
00052a20:   4288                cmp        r0, r1
00052a22:   D109                bne        $C$L68
00052a24:   4811                ldr        r0, [pc, #0x44]
00052a26:   F8DD1033            ldr.w      r1, [r13, #0x33]
00052a2a:   6800                ldr        r0, [r0]
00052a2c:   4288                cmp        r0, r1
00052a2e:   D103                bne        $C$L68
535               UPDATEMNGR_writeAck();
00052a30:   F001F834            bl         UPDATEMNGR_writeAck
536               return ok;
00052a34:   2000                movs       r0, #0
00052a36:   E00E                b          $C$L70
539           fail_counter++;
          $C$L68:
00052a38:   490D                ldr        r1, [pc, #0x34]
00052a3a:   7808                ldrb       r0, [r1]
00052a3c:   1C40                adds       r0, r0, #1
00052a3e:   7008                strb       r0, [r1]
540           if (fail_counter != max_tries) {
00052a40:   480B                ldr        r0, [pc, #0x2c]
00052a42:   7800                ldrb       r0, [r0]
00052a44:   2805                cmp        r0, #5
00052a46:   D003                beq        $C$L69
541               UPDATEMNGR_writeNAck();
00052a48:   F001F8BC            bl         UPDATEMNGR_writeNAck
542               return repeat;
00052a4c:   2002                movs       r0, #2
00052a4e:   E002                b          $C$L70
544           UPDATEMNGR_writeDen();
          $C$L69:
00052a50:   F001F86E            bl         UPDATEMNGR_writeDen
545           return err;
00052a54:   2001                movs       r0, #1
546       }
          $C$L70:
00052a56:   B00F                add        r13, #0x3c
00052a58:   BD00                pop        {pc}
00052a5a:   46C0                mov        r8, r8

Teleview

Check for a stack overflow. Find where the stack is located (from the map file). View the end of the stack in a memory window while you're debugging. See if the stack has overflowed when the problem occurs. Or just double the stack size and see if the problem no longer occurs. – Escribe

If it reached return repeat but then returns to a random location perhaps you are clobbering the stack in /* perform tests on buf ...*/. After all uint8_t buf[250]; is a local variable. – Klong

I don't know what architecture it is and what calling conventions are used but my guess would be: This happens in failure case --> the buffer may be invalid (data/length/etc.). Could be that buf got filled beyond its limit and had overwritten the return address (in case it is kept on the stack). Please verify you never exceed the 250 bytes of the buf. – Complimentary

The UB resulting in stack clobber could be anywhere in the heirachy of functions called from this function. – Klong

@WeatherVane It looks like this could be the case but not in the fc_state but in the fup_state which gets called many times before transitioning to this state. Looking at the Stack Usage viewer from Code Composer Studio it shows a large size of the fup_state. Any tips for reducing it and doing clean up? – Teleview

Unfortunately, the disassembly is incomplete. I'm missing the label $C$L70 as well as the return instruction (afaik, that should be a ret, but could be some other kind of branch theoretically). Without that, we cannot reason about what the assembly code actually does after the branch at 00052a4e. – Alexisaley

@cmaster-reinstatemonica I added the extra lines from the disassembly. – Teleview

@VincentKenbeek It performs ‘pop’ from the stack into the instruction pointer. It indeed looks like stuck corruption – Complimentary

@AlexLop. The 'UART_read()' function is provided by the SimpleLink SDK from Texas Instruments. But I do think overflowing is the problem. First a 'header' arrives over UART containing the size of the body. I then pass this size to the second UART_read but I now see I never validated the size contained in the header. So sometimes a a massive number was passed to UART_read while the max size of the body buffer is about 15 bytes. – Teleview

@VincentKenbeek If so I guess this is the reason for the behaviour you observed. – Complimentary

@AlexLop. I just did some more tests and it does look like that was the problem. Thank you very much Alex. – Teleview

@VincentKenbeek You are welcome! Good luck! – Complimentary

Answer 1

The question is a little bit outdated but there an error in lookup_transitions() function. Loop variable can iterate beyond the array boundary and as of it function can return value from unexpected memory location. This can be a reason of calling function by an invalid pointer and undefined behavior. I think fixup should look something like this:

 for (i = 0; i < sizeof(transition_table) / sizeof(struct transition); i++) 

Benge

Answer 2

The fc and repeat enums both become the same integer since they are in the same position in the enum lists. You can fix this by setting the enums to different starting values that do not overlap. This is why the second repeat fails.

Suicide

This isn't the problem. See this comment to the question, and the comments that follow. – Anonym

This does not provide an answer to the question. Once you have sufficient reputation you will be able to comment on any post; instead, provide answers that don't require clarification from the asker. - From Review – Thermic