![]() |
Forum Index : Microcontroller and PC projects : PicoMite/MMBasic-core: bug with subroutine nesting level (LocalIndex)
Page 1 of 2 ![]() ![]() |
|||||
Author | Message | ||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
Hi Peter & Geoff, This is an FYI, you might like to look into it, or you might just wait and perhaps I will get there myself. Try this for size: > option list PicoMite MMBasic Version 5.07.05RC9 OPTION SYSTEM SPI GP18,GP19,GP16 OPTION AUTORUN 1 OPTION CPUSPEED (KHz) 252000 OPTION LCDPANEL ILI9341, RLANDSCAPE,GP20,GP21,GP17 OPTION SDCARD GP22 > list "local_index_test.bas" Option Base 0 Option Default None Option Explicit On foo() End Sub foo() Local i% = 0, s$ On Error Skip 2 s$ = bar%() i% = 1 End Sub Function bar%() Exit Function > run "local_index_test.bas" Error: Invalid address - resetting And I know I'm not running the latest version, and I apologise, if you wait long enough then I might get there, or possibly someone else will test it on the latest version and confirm my findings. I think it is possibly a problem in the MMBasic-core rather than being PicoMite specific as it also doesn't work with MMB4L (derived from MMBasic for DOS) where instead of crashing it reports that 'I' is not declared at line 12. I've done some investigation and at least with MMB4L the problem is that the nesting level (LocalIndex) is getting screwed up. I think the problem is with SaveLocalIndex in MMBasic.c#ExecuteProgram() and the use of setjmp(ErrNext). Maybe something to do with https://stackoverflow.com/questions/7271313/what-is-an-automatic-variable-in-this-setjmp-longjmp-context, but my C knowledge in this area is nascent; I have a C++ background where I would never use setjmp/longjmp directly (though under the hood exception handing may well use them). I have found that using a global stack (instead of a local/automatic variable) for storing the LocalIndexes seems to fix the issue but I'm still investigating/learning before comitting to that fix. Best wishes, Tom MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
robert.rozee Guru ![]() Joined: 31/12/2012 Location: New ZealandPosts: 2442 |
hi Tom, this feels vaguely familiar as something that has been discussed some years back; essentially "ON ERROR SKIP n" does not in any way play nicely with called user functions or subroutines as i recall. if my memory is right, the consensus at the time was that making it suitably 'bullet proof' would take far more code (and an ON ERROR stack) than was justified. as it stands, there are a number of tricky/obscure error situations that the MMbasic interpreter fails to catch - fixing them all would just require far too much flash and CPU overhead. cheers, rob :-) |
||||
JohnS Guru ![]() Joined: 18/11/2011 Location: United KingdomPosts: 4044 |
I wonder if making it (SaveLocalIndex) volatile would be enough? John |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
I did try that (monkey see, monkey do) and it did not magically fix it. On further consideration I think setjmp/longjmp may be a red herring, or simply compounding the problem of debugging an issue which is ultimately to do with the limitations of MMBasic error handling in the context of the function/subroutine "stack" as alluded to by @robert.rozee (a link to the earlier thread would be helpful if anyone finds it). I will need to investigate further. Best wishes, Tom Edited 2023-01-11 02:33 by thwill MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
OK, The monkey may know what is going on ... but remember he is still a monkey so this might be a load of primate poo: I believe the problem is that the ErrNext jump buffer is getting messed up by the recursive calls to ExecuteProgram(). As it currently stands MMBasic can call longjmp() from a fun/sub at depth (LocalIndex) N after ErrNext has been overwritten at depth N+1 (possibly N+M). This is from the PicoMite MMBasic.c file, but the same code exists in other MMBasic ports, my suggested changes and comments are marked in red. void __not_in_flash_func(ExecuteProgram)(unsigned char *p) { int i, SaveLocalIndex = 0; jmp_buf SaveErrNext; memcpy(SaveErrNext, ErrNext, sizeof(jmp_buf)); // we call ExecuteProgram() recursively so we need to store/restore old jump buffer between calls skipspace(p); // just in case, skip any whitespace while(1) { if(*p == 0) p++; // step over the zero byte marking the beginning of a new element if(*p == T_NEWLINE) { CurrentLinePtr = p; // and pointer to the line for error reporting TraceBuff[TraceBuffIndex] = p; // used by TRACE LIST if(++TraceBuffIndex >= TRACE_BUFF_SIZE) TraceBuffIndex = 0; if(TraceOn && p < ProgMemory + Option.PROG_FLASH_SIZE) { inpbuf[0] = '['; IntToStr(inpbuf + 1, CountLines(p), 10); strcat(inpbuf, "]"); MMPrintString(inpbuf); uSec(1000); } p++; // and step over the token } if(*p == T_LINENBR) p += 3; // and step over the number skipspace(p); // and skip any trailing white space if(p[0] == T_LABEL) { // got a label p += p[1] + 2; // skip over the label skipspace(p); // and any following spaces } if(*p) { // if p is pointing to a command nextstmt = cmdline = p + 1; skipspace(cmdline); skipelement(nextstmt); if(*p && *p != '\'') { // ignore a comment line SaveLocalIndex = LocalIndex; // save this if we need to cleanup after an error // moved from within the setjmp() block, otherwise should be flagged volatile if(setjmp(ErrNext) == 0) { // return to the else leg of this if error and OPTION ERROR SKIP/IGNORE is in effect if(*(char*)p >= C_BASETOKEN && *(char*)p - C_BASETOKEN < CommandTableSize - 1 && (commandtbl[*(char*)p - C_BASETOKEN].type & T_CMD)) { cmdtoken = *(char*)p; targ = T_CMD; commandtbl[*(char*)p - C_BASETOKEN].fptr(); // execute the command } else { if(!isnamestart(*p)) error("Invalid character: @", (int)(*p)); i = FindSubFun(p, false); // it could be a defined command if(i >= 0) { // >= 0 means it is a user defined command DefinedSubFun(false, p, i, NULL, NULL, NULL, NULL); } else error("Unknown command"); } } else { LocalIndex = SaveLocalIndex; // restore so that we can clean up any memory leaks ClearTempMemory(); } if(OptionErrorSkip > 0) OptionErrorSkip--; // if OPTION ERROR SKIP decrement the count - we do not error if it is greater than zero if(TempMemoryIsChanged) ClearTempMemory(); // at the end of each command we need to clear any temporary string vars CheckAbort(); check_interrupt(); // check for an MMBasic interrupt or touch event and handle it } p = nextstmt; } if((p[0] == 0 && p[1] == 0) || (p[0] == 0xff && p[1] == 0xff)) break; // the end of the program is marked by TWO zero chars, empty flash by two 0xff } memcpy(ErrNext, SaveErrNext, sizeof(jmp_buf)); // restore old jump buffer } Note that gcc with the -Wclobbered warning thinks the 'p' parameter should be flagged volatile, but that isn't easy because it then insists all the functions it is passed into flag it as volatile, etc. I think it is raising this warning because of this: "If you have an automatic (function-local non-static) variable that's not declared volatile; and you change the value of the variable between setjmp and longjmp then after the longjmp the value of that variable becomes indeterminate." I don't have any suggestions here, it *currently* doesn't appear to be causing a problem. Best wishes, Tom Edited 2023-01-13 09:31 by thwill MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
JohnS Guru ![]() Joined: 18/11/2011 Location: United KingdomPosts: 4044 |
Is gcc being fooled - I mean, 'p' isn't changed anywhere between setjmp & longjmp is it? John |
||||
matherp Guru ![]() Joined: 11/12/2012 Location: United KingdomPosts: 10310 |
Tom Is this tested code or just proposed? Isn't there a lot more wrong with the whole ON ERROR SKIP concept than just this? How does the line count survive subroutine calls etc? Is it worth trying to fix or is it better to accept its limitations? i.e.reserve for simple cases like file open fail etc. |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
Hi Peter, It is tested on MMB4L which has practically an identical ExecuteProgram() function to the PicoMite. I plan to do some more exploratory testing over the next couple of evenings though. Very possibly, it's certainly not state-of-the-art, but I don't have unit-tests demonstrating any other issues (yet). If you'd care to provide me with a piece of code demonstrating then I am happy to investigate (in the context of MMB4L). That's up to you, but I think this is just an honest to goodness bug (actually 2 bugs) rather than a limitation: 1. Before my change SaveLocalIndex is a non-volatile local variable being modified between setjmp() and longjmp(), and then if you make it volatile it starts containing very "weird" values after an error which are due to bug 2. Note that I believe @JohnS is correct and the warning about the possibility of 'p' being clobbered is spurious, and in any case probably irrelevant even if it was clobbered because its value is overwritten on exiting the setjmp() block by p = nextstmt; 2. When an MMBasic function call occurs ErrNext is overwritten but not restored - I'm finding it really difficult to explain, but here is my best effort: Given: a$ = foo%() Then: When encountering this (implicit LET) command ExecuteProgram() sets the state of ErrNext to 'stateX' and in processing the LET command performs a recursive call to ExecuteProgram() which runs the commands in foo%() overwriting the state of ErrNext with 'stateY'. When the recursive call returns MMBasic encounters a type-mismatch error and does a longjmp(), however before my change ErrNext would still be 'stateY' instead of the 'stateX' it should have been for processing the LET command. You can see the issue with something as simple as: Option Base 0 Option Default None Option Explicit On Dim s$ ? "Expect 0, found" Mm.Info(CallDepth) On Error Skip 3 s$ = bar%() ? "Expect 0, found" Mm.Info(CallDepth) Print "Caught: " Mm.ErrMsg$ End Function bar%() ? "Expect 1, found" Mm.Info(CallDepth) End Function Though first you have to implement MM.INFO(CALLDEPTH) - it just returns LocalIndex. Best wishes, Tom Edited 2023-01-13 20:20 by thwill MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
IMO the case I encountered it in was simple, I was trying to write a unit-test for MMBasic behaviour when there is a mismatch in type between the return value of a function and the type expected: Sub test_call_fn_with_wrong_type() On Error Skip 2 Local s$ = fun_b%() assert_raw_error("Expected a string") End Sub Function fun_b%() End Function Best wishes, Tom Edited 2023-01-13 20:17 by thwill MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
matherp Guru ![]() Joined: 11/12/2012 Location: United KingdomPosts: 10310 |
What should this do? On error skip 5 splod fred b=6 Print b Sub fred a=10 c=12 d=13 e=15 f=16 End Sub or how about just this On error skip 5 splod fred b=6 Print b Sub fred End Sub Edited 2023-01-13 20:26 by matherp |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
Write " 6" to the console (which it does in MMB4L), why? what do you think it should do? what does it do on the PicoMite? Best wishes, Tom MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
matherp Guru ![]() Joined: 11/12/2012 Location: United KingdomPosts: 10310 |
Check the second example or set b in fred Edited 2023-01-13 20:28 by matherp |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
Still write " 6" to the console, which it does. Either I'm slow, or you're extra cryptic this morning ![]() EDIT: Ooo, set 'b' in fred(), that is fun ... wtf! EDIT 2: No, just confused myself, it should still write " 6" and I still don't know what you are trying to tell me ;-) Best wishes, Tom Edited 2023-01-13 20:34 by thwill MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
matherp Guru ![]() Joined: 11/12/2012 Location: United KingdomPosts: 10310 |
Why if the function is empty is b set to 6? surely the assignment should have been skipped as well as the print. Edited 2023-01-13 20:47 by matherp |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
Why ? ... ON ERROR SKIP 5 means ignore errors reported by the next 5 commands. Manual: "ON ERROR SKIP will ignore an error in a number of commands (specified by the number 'nn') executed following this command." Have you run out of dried frog pills ? Best wishes, Tom Edited 2023-01-13 20:49 by thwill MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
matherp Guru ![]() Joined: 11/12/2012 Location: United KingdomPosts: 10310 |
Shows why I never use ON ERROR. I thought it was supposed to skip the next five commands after an error Edited 2023-01-13 20:54 by matherp |
||||
Mixtel90![]() Guru ![]() Joined: 05/10/2019 Location: United KingdomPosts: 7937 |
You aren't alone, Peter. I thought that too. :) I get confused with ON ERROR IGNORE, but that applies to *all* errors, I think. Edited 2023-01-13 21:25 by Mixtel90 Mick Zilog Inside! nascom.info for Nascom & Gemini Preliminary MMBasic docs & my PCB designs |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
I'm saying nothing ... dried frog pill anyone ? Best wishes, Tom MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
Mixtel90![]() Guru ![]() Joined: 05/10/2019 Location: United KingdomPosts: 7937 |
Well - I think I've used it once in the last ten years.... I live in a world of WHAT, HOW and SORRY, not this new-fangled stuff. Mick Zilog Inside! nascom.info for Nascom & Gemini Preliminary MMBasic docs & my PCB designs |
||||
thwill![]() Guru ![]() Joined: 16/09/2019 Location: United KingdomPosts: 4311 |
TRS-80 Level I BASIC ;-) The 13 Other Greatest Error Messages of All Time Actually I pretty much only use ON ERROR SKIP for catching errors with the file handling commands/functions or coping with a missing command in a particular MMBasic dialect (if I didn't use MM.DEVICE$ instead), however I have been writing unit-tests for them so I can have confidence I'm not breaking something when refactoring some of the inner workings of MMB4L. Best wishes, Tom MMBasic for Linux, Game*Mite, CMM2 Welcome Tape, Creaky old text adventures |
||||
Page 1 of 2 ![]() ![]() |
![]() |
![]() |
The Back Shed's forum code is written, and hosted, in Australia. | © JAQ Software 2025 |