XC8 destroys indexes by ISR

This forum handles questions and discussions concerning Microchip’s 8-bit compilers, assemblers, linkers and related tools.

XC8 destroys indexes by ISR

Postby zlej » Mon Oct 18, 2021 3:20 pm

In my project I defined these variables:

Code: Select all
   struct T_XPOMObject {
        unsigned char XPOMInstruction[XPOM_MaxLength];
        unsigned char XPOMValue[XPOM_DataLength];     
                 int  XPOMEEAddr[XPOM_DataLength];   
        unsigned char XPOMLength;                     
        unsigned char XPOMCounter;                   
    };

    struct T_XPOMObject XPOMBuffer[XPOMBufferLength];

    unsigned char sequenceNumber_SS;


and used them in this function:

Code: Select all
void putXPOMToBuffer(unsigned char packet_idx){
   unsigned char x;
   ix=0;

   ...

   while (packet_idx<packetLength) {

      //(when following was written as one command, XC8 issued a warning that this command is candidate for compiler verify
      x=packet[packet_idx++];       
      XPOMBuffer[sequenceNumber_SS].XPOMInstruction[ix]=x;
      ix++;
   };
};


and tryied to debug my code using simulator in MPLAB X-IDE as usual.

But XPOMBuffer[sequenceNumber_SS].XPOMInstruction[0] get corrupted always when ix=2. But this was also always the moment, when Simulator issued an TMR0 IRQ during the _WMUL function. Because the code was OK for other ix, the ISR code was clear suspect. Finally I went to assembler and quickly realized that XC8 2.30 doesn't properly recover context when returning from ISR!

As you can see in the following XC8 generated asm code, although the ISR routine saves and restores all registers used as stated in manual, it's buggy.
When you check the asm code thoroughly, you'll see, that it saves 126 to ??isr+8, but restores it back from ??_isr+9 , and when you check all the saved registers, it's even worser - ALL are "recovered" same way :!:.

Code: Select all
 20332 ;; *************** function _isr *****************
 ...
 20369                           ; Regs used in _isr: [wreg-fsr0h+fsr1l+fsr1h+status,2-btemp+1+btemp+4-btemp+11+pclath+cstack]
 20370  0004  3180                  pagesel $
 20371  0005  0874                  movf    116,w
 20372  0006  0020                  movlb   0   ; select bank0
 20373  0007  00A0                  movwf   ??_isr
 20374  0008  0875                  movf    117,w
 20375  0009  00A1                  movwf   ??_isr+1
 20376  000A  0878                  movf    120,w
 20377  000B  00A2                  movwf   ??_isr+2
 20378  000C  0879                  movf    121,w
 20379  000D  00A3                  movwf   ??_isr+3
 20380  000E  087A                  movf    122,w
 20381  000F  00A4                  movwf   ??_isr+4
 20382  0010  087B                  movf    123,w
 20383  0011  00A5                  movwf   ??_isr+5
 20384  0012  087C                  movf    124,w
 20385  0013  00A6                  movwf   ??_isr+6
 20386  0014  087D                  movf    125,w
 20387  0015  00A7                  movwf   ??_isr+7
 20388  0016  087E                  movf    126,w
 20389  0017  00A8                  movwf   ??_isr+8
 20390  0018  087F                  movf    127,w
 20391  0019  00A9                  movwf   ??_isr+9
 20392  001A  3011                  movlw   low ___int_sp
 20393  001B  0086                  movwf   6
 20394  001C  3024                  movlw   high ___int_sp
 20395  001D  0087                  movwf   7
 20396  001E  0875                  movf    117,w           ;;unnecessary 2nd save of 117
 20397  001F  00AA                  movwf   ??_isr+10
 20398                           
 20399                           ;interrupts.c: 39:     if ((INTCONbits.TMR0IE) && (INTCONbits.TMR0IF)){
 
 --- here was the ISR code ---
 
 20493  0059  0020                  movlb   0   ; select bank0
 20494  005A  082A                  movf    ??_isr+10,w     ;;wrong offset 10 instead of 9
 20495  005B  00FF                  movwf   127
 20496  005C  0829                  movf    ??_isr+9,w
 20497  005D  00FE                  movwf   126
 20498  005E  0828                  movf    ??_isr+8,w
 20499  005F  00FD                  movwf   125
 20500  0060  0827                  movf    ??_isr+7,w
 20501  0061  00FC                  movwf   124
 20502  0062  0826                  movf    ??_isr+6,w
 20503  0063  00FB                  movwf   123
 20504  0064  0825                  movf    ??_isr+5,w
 20505  0065  00FA                  movwf   122
 20506  0066  0824                  movf    ??_isr+4,w
 20507  0067  00F9                  movwf   121
 20508  0068  0823                  movf    ??_isr+3,w
 20509  0069  00F8                  movwf   120
 20510  006A  0822                  movf    ??_isr+2,w
 20511  006B  00F5                  movwf   117
 20512  006C  0821                  movf    ??_isr+1,w
 20513  006D  00F4                  movwf   116
 20514  006E  0009                  retfie
 20515  006F                     __end_of_isr:


And this is the piece of code with field index computation which gets corrupted:

Code: Select all
 11755                           ;instructions.c: 123:             XPOMBuffer[sequenceNumber_SS].XPOMInstruction[ix]=x;
 11756  1A76  0021                  movlb   1   ; select bank1
 11757  1A77  084C                  movf    _ix^(0+128),w   ;volatile
 11758  1A78  00FE                  movwf   126             ;;save index to field to 126&127
 
 ;;IRQ after this point corrupts the computed address
 
 11759  1A79  01FF                  clrf    127
 11760  1A7A  3016                  movlw   22              ;;size of struct XPOM_Object
 11761  1A7B  001E                  movwi fsr1++
 11762  1A7C  3000                  movlw   0
 11763  1A7D  001E                  movwi fsr1++
 11764  1A7E  0020                  movlb   0   ; select bank0
 11765  1A7F  085F                  movf    _sequenceNumber_SS,w
 11766  1A80  001E                  movwi fsr1++
 11767  1A81  3000                  movlw   0
 11768  1A82  001E                  movwi fsr1++
 11769  1A83  31A0  200F  319A      fcall   ___wmul ; compute offset to XPOMBuffer[Sequence#SS] for XPOM_OBject
 11770  1A86  3098                  movlw   152     ;base address of XPOMInstruction[] in XPOMBUffer[0] LSB
 11771  1A87  00FC                  movwf   124
 11772  1A88  3027                  movlw   39      ;base address of XPOMInstruction[] in XPOMBUffer[0] MSB
 11773  1A89  00FD                  movwf   125
 11774  1A8A  0874                  movf    116,w   ;offset returned from WMUL to XPOMBuffer[sequenceNumber_SS].XPOMInstruction[0] LSB
 11775  1A8B  077C                  addwf   124,w
 11776  1A8C  00FA                  movwf   122
 11777  1A8D  0875                  movf    117,w   ;;offset returned from WMUL to XPOMBuffer[sequenceNumber_SS].XPOMInstruction[0] MSB
 11778  1A8E  3D7D                  addwfc  125,w   ;
 11779  1A8F  00FB                  movwf   123     ;computed base address of XPOMBuffer[sequenceNumber_SS].XPOMInstruction[0]
 11780  1A90  087E                  movf    126,w   ;and now add the ix offset LSB XPOMBuffer[sequenceNumber_SS].XPOMInstruction[ix]
 11781  1A91  077A                  addwf   122,w
 11782  1A92  00FC                  movwf   124     
 11783  1A93  087F                  movf    127,w   ;;and now add the ix offset MSB
 11784  1A94  3D7B                  addwfc  123,w
 11785  1A95  00FD                  movwf   125
 11786  1A96  087C                  movf    124,w
 11787  1A97  0084                  movwf   4       ;;copy the computed address to FSR0L+FSR0H
 11788  1A98  087D                  movf    125,w
 
 ;till this point can the faulty ISR corrupt the computed address
 
 11789  1A99  0085                  movwf   5
 11790  1A9A  0020                  movlb   0   ; select bank0
 11791  1A9B  082B                  movf    putXPOMToBuffer@x,w
 11792  1A9C  0080                  movwf   0       ;;and finally save the value X using FSR0 and IND0
 11793                           
 11794                           ;instructions.c: 124:             ix++;
 


When the IRQ appears in the middle of computing complex address this will lead to computing wrong index. If this is part of a write to a variable, you end with mysteriosly corrupted data.
Computing the address starts with saving the offset to fixed adress 126 and eventually 127. After that the base address is computed by _WMUL and finally added the offset from 126+127.

When an IRQ appears between saving the offset to 126/127 and reading it back, the adress gets corrupted. In my case the IRQ occurs during _WMUL which is a reentrant 16 bit unsigned multiply routine from library.
Because 126 get the value from 127 (which in this case is 0), the value was writen to first variable of the array [0] where it could be easilly observed.
When the IRQ should appeared later, it could more significantly change the address and the value could be written somewhere in the memory casuing really mysterious errors.

I'm really surprised, that rest of my code worked some way. May be it's so because most of the code is someway "synchronized" by TMR0 and IRQ appears almost at same places of code.
But during debugging other parts of code I realized that some code using arrays sometimes doesn't run OK, but when I added some code, the problem mysteriously disappeared.
Because this part dealed with flash memory driven by ISR, which is not in sync with rest of code, I simply couldn't found the reason.
I don't know if this is only special case, but such ISR restore makes any afflicted code unstable and useless.

My project used PIC16F1789, PIC12-16Fxxx.DFP 1.2.63, Hybrid stack, C90, XC8 2.30 with free license and optimization level S2. Tryied to use XC8 2.32 with same result :-(
BTW, during debugging I changed all local variables from ISR called functions to global and set as volatile as recommended, but without any change of behaviour.

So, how to deal with this bug?
Because my code should run in a real time, I found using _WMUL to slow. So without worrying about the ISR problem, I tried to rewrite the code to avoid using _WMUL.
The declaratinon was almost same, but instead of using indexes to array of structures as above, I used a pointer to every single structure. It seems weird, but XC8 used
the pointer as base and when addressing a variable in the structure adds a constant. This is much quicker then _WMUL and also has the effect that the XC8 doesn't
save in ISR so many registers and restores them in a proper way :D !

Code: Select all
struct T_XPOMObject XPOMBuffer[XPOMBufferLength];              //Buffer for XPOM instructions
struct T_XPOMObject *XPOMPtr[XPOMBufferLength], *XP_Ptr;    //Pointers to XPOM instructions

...

void putXPOMToBuffer(unsigned char sequenceNumber, unsigned char packet_idx){
    struct T_XPOMObject *XP;
    unsigned char i=0;

    XP=XPOMPtr[sequenceNumber];                                 

    ...

    while (packet_idx<packetLength) {                       
       XP->XPOMInstruction[i++]=packet[packet_idx++];      //ISR doesn't corrupt this and also candidate for compiler verify warning isn't issued anymore
    };
    ...
};


and right ISR routine:

Code: Select all
 20579  0004                     _isr:
 20580                           
 20581                           ;incstack = 0
 20582                           ; Regs used in _isr: [wreg-fsr0h+fsr1l+fsr1h+status,2-btemp+0+btemp+6-btemp+11+pclath+cstack]
 20583  0004  3180                  pagesel $
 20584  0005  0874                  movf    116,w
 20585  0006  0020                  movlb   0   ; select bank0
 20586  0007  00A0                  movwf   ??_isr
 20587  0008  087A                  movf    122,w
 20588  0009  00A1                  movwf   ??_isr+1
 20589  000A  087B                  movf    123,w
 20590  000B  00A2                  movwf   ??_isr+2
 20591  000C  087C                  movf    124,w
 20592  000D  00A3                  movwf   ??_isr+3
 20593  000E  087D                  movf    125,w
 20594  000F  00A4                  movwf   ??_isr+4
 20595  0010  087E                  movf    126,w
 20596  0011  00A5                  movwf   ??_isr+5
 20597  0012  087F                  movf    127,w
 20598  0013  00A6                  movwf   ??_isr+6
 20599  0014  3011                  movlw   low ___int_sp
 20600  0015  0086                  movwf   6
 20601  0016  3024                  movlw   high ___int_sp
 20602  0017  0087                  movwf   7
 20603                           
 20604                           ;interrupts.c: 39:     if ((INTCONbits.TMR0IE) && (INTCONbits.TMR0IF)){

----------- here was the ISR code ------------
                         
 20698  0051  0020                  movlb   0   ; select bank0
 20699  0052  0826                  movf    ??_isr+6,w
 20700  0053  00FF                  movwf   127
 20701  0054  0825                  movf    ??_isr+5,w
 20702  0055  00FE                  movwf   126
 20703  0056  0824                  movf    ??_isr+4,w
 20704  0057  00FD                  movwf   125
 20705  0058  0823                  movf    ??_isr+3,w
 20706  0059  00FC                  movwf   124
 20707  005A  0822                  movf    ??_isr+2,w
 20708  005B  00FB                  movwf   123
 20709  005C  0821                  movf    ??_isr+1,w
 20710  005D  00FA                  movwf   122
 20711  005E  0820                  movf    ??_isr,w
 20712  005F  00F4                  movwf   116
 20713  0060  0009                  retfie
 20714  0061                     __end_of_isr:

 


I don't know the real reason why the XC8 changed the behaviour. But the mix of 'an array in a structure in an array' was for XC8 probably some way "toxic".

What is wrong with XC8? In my opinion it's the second unnecessary save of register 117. This increases the index to 10. And this index is then used for restore code and all goes wrong :? . It cost me some time, but nearly same time I spent trying to publish this article in Microchips' Forum. Nevermore.
zlej
 
Posts: 1
Joined: Mon Oct 18, 2021 2:05 pm
PIC experience: Experienced Hobbyist

Return to MPLAB XC8

Who is online

Users browsing this forum: No registered users and 6 guests

cron