• AVR Freaks

Helpful ReplyHot!Overwriting of local function variables (XC8 bug)

Author
rjc101
Super Member
  • Total Posts : 108
  • Reward points : 0
  • Joined: 2016/07/08 14:56:34
  • Location: 0
  • Status: online
2018/08/11 05:47:34 (permalink)
5 (1)

Overwriting of local function variables (XC8 bug)

I have been trying to debug a problem that I think is caused by the way I create various menus.  They are declared along the lines of... 
 
typedef struct {
    uint8_t text[17];
    void (*ptrFunction)(uint8_t);
} MENU_t;

 
I then create the menu with the following 
 
const MENU_t system_meny[] = {
    {" menu opt 1 ", menu_opt_1},
    {" menu opt 2 ", menu_opt_2},
    {" menu opt 3 ", menu_opt_3},
}

 
The text is displayed on an OLED display, so I can easily step up and down the menu displaying the choices when then jump to the menu routine with...
 
if (button(&button_ok, BUTTON_ON_RELEASE) == BUTTON_ACTIVE) {
            if (system_menu[menu_item].ptrFunction != NULL) {

                Display_Cmd(DISPLAY_CLEAR);
                system_menu[menu_item].ptrFunction(menu_item);
                Display_Cmd(DISPLAY_CLEAR);
                // 1234567890123456
                Display_Const_Out(1, 1, " Main Menu ");
                Display_Const_Out(2, 1, system_menu[menu_item].text);
            }
        }

 
This all works just fine.  However I have various functions that again offer choices to the user, and to save duplicate of code to update the display I use a subroutine to do that.  For example...
 
void menu_frame_rate_upd(uint8_t frame_rate_disp) {
    uint8_t display_buffer[17];
    sprintf(display_buffer, " %02d F", frame_rate_disp);
    Display_Cmd(DISPLAY_CURSOR_OFF);
    Display_Out(2, 1, display_buffer);
    Display_Cmd(DISPLAY_CURSOR_ON);
}

void menu_frame_rate(uint8_t menu_item) {
    BUTTON_ENUM_t button_state;
    uint8_t frame_rate;
    /* update the display to show what menu item we are in */
    Display_Const_Out(1, 1, system_menu[menu_item].text);

    menu_frame_rate_upd(frame_rate);

    do {

        button_state = button_advanced(&button_up, 1000);

        if (button_state != BUTTON_INACTIVE) {
            if (button_state == BUTTON_ACTIVE) {
                if (frame_rate < MAX_FRAME_RATE) {
                    frame_rate++;
                    menu_frame_rate_upd(frame_rate);
                }
            }
            if (button_state == BUTTON_ACTIVE_LONG) {
                while (button_advanced(&button_up, 500) == BUTTON_ACTIVE_LONG) {
                    if ((frame_rate + 10) < MAX_FRAME_RATE) {
                        frame_rate += 10;
                        menu_frame_rate_upd(frame_rate);
                    }
                }
            }
        }

        button_state = button_advanced(&button_down, 1000);
        if (button_state != BUTTON_INACTIVE) {
            if (button_state == BUTTON_ACTIVE) {
                if (frame_rate > 0) {
                    frame_rate--;
                    menu_frame_rate_upd(frame_rate);
                }
            }
            if (button_state == BUTTON_ACTIVE_LONG) {
                while (button_advanced(&button_down, 500) == BUTTON_ACTIVE_LONG) {
                    if ((frame_rate - 10) > 0) {
                        frame_rate -= 10;
                        menu_frame_rate_upd(frame_rate);
                    }
                }
            }
        }

        if (button(&button_menu, BUTTON_ON_RELEASE) == BUTTON_ACTIVE) {
            frame_rate = 0;
            menu_frame_rate_upd(frame_rate);
        }
    } while ((button(&button_ok, BUTTON_ON_RELEASE) != BUTTON_ACTIVE));
    Display_Cmd(DISPLAY_CURSOR_OFF);
    
}

 
What I have been seeing is the 'frame_rate' variable in the menu_frame_rate function ends up being corrupted.  Looking in the debugger at what is going on I see that the 'display_buffer' variable in 'menu_frame_rate_upd' function has been given an overlapping memory address to 'frame_rate' - so when I update the display the 'frame_rate' various becomes corrupted.
 
If stop using the menu structure to traverse the menu choices (i.e. stop calling functions by pointer) and just use a long switch/case statement things seem to be as expected.
 
My question is why cannot I use the function pointer method to create menu structures like this?
 
I'm using a PIC18F47K42, XC8 1.45 or 2.00 (both do the same), MPLAB 5.0.  
I can see the call graph for my main menu routine doesn't show the menu items, but the menu_frame_rate routing does show the _upd subroutine so I don't understand why memory space is be re-used causing the corruption.
 
What have I missed?
post edited by rjc101 - 2018/08/12 03:19:29
#1
mbrowning
USNA79
  • Total Posts : 1533
  • Reward points : 0
  • Joined: 2005/03/16 14:32:56
  • Location: Melbourne, FL
  • Status: online
Re: Overwriting of local function variables 2018/08/11 07:10:29 (permalink) ☄ Helpfulby rjc101 2018/08/11 07:11:24
+3 (3)
XC8 by default uses a “compiled” stack for local variables. Basically locals have fixed addresses but share addresses to save memory. The compiler figures out when they are in scope and arranges them so that locals that might be in use at the same time don’t share addresses.

Unfortunately it’s not good at this when function pointers are used. I think you could call it a bug. There was a thread about this a few months ago.

Maybe define the problem local variable as static as a work around. Or use the emulated stack mode, but that’s fairly inefficient.

Go Navy! Beat Army!
#2
rjc101
Super Member
  • Total Posts : 108
  • Reward points : 0
  • Joined: 2016/07/08 14:56:34
  • Location: 0
  • Status: online
Re: Overwriting of local function variables 2018/08/11 07:14:41 (permalink)
+1 (1)
mbrowning
XC8 by default uses a “compiled” stack for local variables. Basically locals have fixed addresses but share addresses to save memory. The compiler figures out when they are in scope and arranges them so that locals that might be in use at the same time don’t share addresses.

Unfortunately it’s not good at this when function pointers are used. I think you could call it a bug. There was a thread about this a few months ago.

Maybe define the problem local variable as static as a work around. Or use the emulated stack mode, but that’s fairly inefficient.



Hi did look at an alternative stack but the overhead was horrible.  I thought compiled stack basically worked on the assumption that all local vars were static.  The call graphs in the .map file correctly show the compiler "knows" what is calling what, but still re-uses memory.  
 
I might open a ticket, as it took *ages* to figure out what was going wrong.
#3
rjc101
Super Member
  • Total Posts : 108
  • Reward points : 0
  • Joined: 2016/07/08 14:56:34
  • Location: 0
  • Status: online
Re: Overwriting of local function variables 2018/08/11 08:15:41 (permalink)
0
I've raised a support ticket.  
 
Not looking forward to re-working things, there are dozens of options as I use the same structure for a displayed menu and for processing commands arriving via a serial port.
#4
qɥb
Monolothic Member
  • Total Posts : 3332
  • Reward points : 0
  • Joined: 2017/09/09 05:07:30
  • Location: Jupiter
  • Status: offline
Re: Overwriting of local function variables 2018/08/11 13:31:24 (permalink)
+2 (2)
rjc101
...
  I thought compiled stack basically worked on the assumption that all local vars were static. 

That would be a bad assumption.
As mentioned, it overlays them wherever it thinks they are not required at the same time.
 
 
The call graphs in the .map file correctly show the compiler "knows" what is calling what, but still re-uses memory.

Point it out in a support ticket then, and possibly something will get fixed.

This forum is mis-configured so it only works correctly if you access it via https protocol.
The Microchip website links to it using http protocol. Will they ever catch on?
PicForum "it just works"
#5
rjc101
Super Member
  • Total Posts : 108
  • Reward points : 0
  • Joined: 2016/07/08 14:56:34
  • Location: 0
  • Status: online
Re: Overwriting of local function variables 2018/08/12 03:18:28 (permalink)
+1 (1)
This is a snip from the .map file, to me it shows the compiler has worked out that 'menu_system' calls 'menu_opt_1' that in turn can call 'menu_opt_1_upd'.  Which is pretty neat as this is all done with pointers.  However you can see that in  'menu_opt_1_upd' is allocated memory at 123[BANK0 ] for char array that neatly stomps over the variables in the calling function.
 
I've updated the ticket already with the information but no idea when this will appear on someone's radar,  ticket number 00320364 is anyone on the inside wants to give it a poke.  
 
Going through the forums it seems this has kind of thing may have bitten people a few times in the past, fixed by making variables in functions static.  Which kind of steps round the root of the problem.
 
*************** function _menu_opt_1 *****************
 Defined at:
  line 1733 in file "main.c"
 Parameters: Size Location Type
  _menu_item 1 wreg unsigned char
 Auto vars: Size Location Type
  var1 1 125[BANK0 ] unsigned char
  var2 1 127[BANK0 ] unsigned char
  var3 1 126[BANK0 ] unsigned char
  var4 1 124[BANK0 ] unsigned char
  var5 1 123[BANK0 ] unsigned char
 Return value: Size Location Type
                  1 wreg void
 
 This function calls:
  _menu_opt_1_upd
 This function is called by:
  _menu_system
 This function uses a non-reentrant model


*************** function _menu_opt_1_upd *****************
 Defined at:
  line 1655 in file "main.c"
 Parameters: Size Location Type
  par1 1 wreg unsigned char
  par2 1 121[BANK0 ] unsigned char
  par3 1 122[BANK0 ] unsigned char
 Auto vars: Size Location Type
  var1 1 143[BANK0 ] unsigned char
  var2 10 133[BANK0 ] unsigned char [10]
  var3 10 123[BANK0 ] unsigned char [10]
  var4 4 144[BANK0 ] float
 Return value: Size Location Type
                  1 wreg void

 This function calls: This function is called by:
  _menu_opt_1
 This function uses a non-reentrant model

#6
rjc101
Super Member
  • Total Posts : 108
  • Reward points : 0
  • Joined: 2016/07/08 14:56:34
  • Location: 0
  • Status: online
Re: Overwriting of local function variables 2018/08/22 00:54:44 (permalink)
+2 (2)
Microchip can duplicate this issue, looks like there is a bug in the way the compiler is generating the Auto Parameter Block causing variable overlap.  
 
Now been escalated to the compiler development team.
#7
rjc101
Super Member
  • Total Posts : 108
  • Reward points : 0
  • Joined: 2016/07/08 14:56:34
  • Location: 0
  • Status: online
Re: Overwriting of local function variables 2018/09/29 14:50:49 (permalink)
+3 (3)
Confirmed as a bug, reference XC8-1775, advised to check the readme in future releases to see when this gets fixed.
It seems this has bitten a few people before so hoping for a fix sooner rather than later.
#8
DavidBLit
Super Member
  • Total Posts : 1579
  • Reward points : 0
  • Joined: 2012/02/18 13:08:48
  • Location: The Land of Confusion
  • Status: offline
Re: Overwriting of local function variables 2018/09/29 16:59:18 (permalink)
+2 (2)
Thanks for reporting back, too few ever do.

Yeah, "//Code and stuff".
#9
rjc101
Super Member
  • Total Posts : 108
  • Reward points : 0
  • Joined: 2016/07/08 14:56:34
  • Location: 0
  • Status: online
Re: Overwriting of local function variables 2019/01/11 14:34:06 (permalink)
0
No fix in the v2.05 release read-me :-( 
#10
rjc101
Super Member
  • Total Posts : 108
  • Reward points : 0
  • Joined: 2016/07/08 14:56:34
  • Location: 0
  • Status: online
Re: Overwriting of local function variables 2019/09/02 02:55:22 (permalink)
+2 (2)
No fix in the v2.10 release read-me :-(
#11
Jump to:
© 2019 APG vNext Commercial Version 4.5