#3536 Calling Functions in ISR Corrupts the Stack, as SDCC Makes no Attempt to Preserve it

[Tom Li]

For the PIC14 target, the current calling convention in SDCC is the following: the common RAM section 0x70 to 0x7F, accessible from all memory banks, is used as a software stack as STK00 to STK15. When calling a function, the first word is stored into the working register W, and the rest of the stored into STK00, STK01, STK02, etc.

However, the Interrupt Service Routine (ISR) seems to make no attempt to preserve the software stack. If a function is called inside an ISR, the original software stack is overwritten completely. It's only safe to call functions with no argument or only a single-word argument in an ISR.

Consider the following program fail.c:

#include <pic16f1579.h>
#include <stdint.h>

void f(uint8_t x1, uint8_t x2)
{
    PORTA = x1;
    PORTB = x2;
}

void g(uint8_t x1, uint8_t x2)
{
    PORTA = x1;
    PORTB = x2;
}

void main(void)
{
    for (;;) {
        f(0, 1);
    }
}

void isr(void) __interrupt (0)
{
    g(1, 0);
}

This generates the following assembly code (with sdcc --no-pcode-opt --use-non-free -mpic14 -p16f1579 fail.c:

_isr:
;   .line   23; "fail.c"    void isr(void) __interrupt (0)
    CLRF    PCLATH
;   .line   25; "fail.c"    g(1, 0);
    MOVLW   0x00
    MOVWF   STK00
    MOVLW   0x01
    PAGESEL _g
    CALL    _g
    PAGESEL $
END_OF_INTERRUPT:
    RETFIE  

code_fail   code
S_fail__main    code
_main:
_00114_DS_:
;   .line   19; "fail.c"    f(0, 1);
    MOVLW   0x01
    MOVWF   STK00
    MOVLW   0x00
    PAGESEL _f
    CALL    _f
    PAGESEL $
    GOTO    _00114_DS_
;   .line   21; "fail.c"    }
    RETURN  

S_fail__g   code
_g:
;   .line   10; "fail.c"    void g(uint8_t x1, uint8_t x2)
    BANKSEL _PORTA
    MOVWF   _PORTA
    MOVF    STK00,W
    MOVWF   _PORTB
;   .line   14; "fail.c"    }
    RETURN  

S_fail__f   code
_f:
;   .line   4; "fail.c" void f(uint8_t x1, uint8_t x2)
    BANKSEL _PORTA
    MOVWF   _PORTA
    MOVF    STK00,W
    MOVWF   _PORTB
;   .line   8; "fail.c" }
    RETURN  

    end
    ```

    Imagine that we have just entered function `f(x1, x2)` from `main()`, so the following code is executed:

; set x2 to 1
MOVLW   0x01
MOVWF   STK00

; set x1 to 0
MOVLW   0x00

; start calling f(0, 1)
PAGESEL _f

; an interrupt fires before f is actually called

; select default page
CLRF    PCLATH

; set x2 to 0
MOVLW   0x00
MOVWF   STK00

; set x1 to 1
MOVLW   0x01

; call g(1, 0)
PAGESEL _g
CALL    _g
PAGESEL $

; ...function body g omitted...

; return from ISR
 RETFIE

; call f(0, 1)
CALL    _f

; W is restored by the CPU to 2 after leaving the ISR,
; thus PORTA is set to 0 as expected.
BANKSEL _PORTA
MOVWF   _PORTA

; STK00 has been overwritten by the ISR to 0
MOVF    STK00,W

; Incorrect value 0 was written into PORTB,
; it should've been 0x01.
MOVWF   _PORTB

```

This behavior should be documented and must be eventually fixed. One idea is saving all arguments to a non-overlapping section of the stack when entering the ISR, e.g. STK00 to STK08, STK01 to STK09, etc.

[Benedikt Freisen]

The PIC14 and PIC16 targets, as well as their documentation, are unmaintained and have been for a few years.
Patches are welcome, though.

[Tom Li]

I just submitted a PIC patch a few days ago on adding support for new devices. Unfortunately it has no reviewer so far. The PIC targets look completely abandoned...

[Benedikt Freisen]

I'm afraid the number of people around here who can give qualified feedback on PIC-related issues and apply patch sets is rather limited.