Home
JAQForum Ver 24.01
Log In or Join  
Active Topics
Local Time 10:36 01 Aug 2025 Privacy Policy
Jump to

Notice. New forum software under development. It's going to miss a few functions and look a bit ugly for a while, but I'm working on it full time now as the old forum was too unstable. Couple days, all good. If you notice any issues, please contact me.

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 Kingdom
Posts: 4311
Posted: 12:30pm 10 Jan 2023
Copy link to clipboard 
Print this post

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 Zealand
Posts: 2442
Posted: 01:51pm 10 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 4044
Posted: 03:40pm 10 Jan 2023
Copy link to clipboard 
Print this post

I wonder if making it (SaveLocalIndex) volatile would be enough?

John
 
thwill

Guru

Joined: 16/09/2019
Location: United Kingdom
Posts: 4311
Posted: 04:05pm 10 Jan 2023
Copy link to clipboard 
Print this post

  JohnS said  I wonder if making it (SaveLocalIndex) volatile would be enough?


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 Kingdom
Posts: 4311
Posted: 10:51pm 12 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 4044
Posted: 11:45pm 12 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 10310
Posted: 09:02am 13 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 4311
Posted: 10:08am 13 Jan 2023
Copy link to clipboard 
Print this post

Hi Peter,

  matherp said  Tom

Is this tested code or just proposed?


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.

  matherp said   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?


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).

  matherp said  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.


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 Kingdom
Posts: 4311
Posted: 10:17am 13 Jan 2023
Copy link to clipboard 
Print this post

  matherp said  i.e.reserve for simple cases like file open fail etc.


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 Kingdom
Posts: 10310
Posted: 10:20am 13 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 4311
Posted: 10:26am 13 Jan 2023
Copy link to clipboard 
Print this post

  matherp said  What should this do? ...


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 Kingdom
Posts: 10310
Posted: 10:27am 13 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 4311
Posted: 10:31am 13 Jan 2023
Copy link to clipboard 
Print this post

  matherp said  Check the second example or set b in fred


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 Kingdom
Posts: 10310
Posted: 10:46am 13 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 4311
Posted: 10:48am 13 Jan 2023
Copy link to clipboard 
Print this post

  matherp said  Why if the function is empty is b set to 6? surely the assignment should have been skipped.


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 Kingdom
Posts: 10310
Posted: 10:52am 13 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 7937
Posted: 11:22am 13 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 4311
Posted: 11:25am 13 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 7937
Posted: 11:27am 13 Jan 2023
Copy link to clipboard 
Print this post

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 Kingdom
Posts: 4311
Posted: 11:34am 13 Jan 2023
Copy link to clipboard 
Print this post

  Mixtel90 said  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.


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    
Print this page
The Back Shed's forum code is written, and hosted, in Australia.
© JAQ Software 2025