Philip, thank you, I'm sorry: I have sent this to upstream, but haven't heard anything from them. At least with a mailing list, I get feedback as to whether or not my mail was eaten by the void of the Internet. Also, if it gets into Debian, then the patches filter through to everything based on Debian.
Philip, your change to the patch looks right. Sorry, I based the patch off upstream. You do say it needs a better description, so I'm going to try to give you a sense of what's going on. What I think is happening is that, somewhere in the parser, "that an error occurred" is getting suppressed, and the parser continues to generate bytecode with the previous instruction incomplete, and then it tries to execute that. Sometimes, the bytecode reads an instruction while trying to read a reference. This appears to be most catastrophic in array handling. While what ought to be fixed is the code generation to not generate these erroneous references, it is easier to fix the bytecode interpreter to defend itself from them. To begin, starting in execute.c: for change one, it has read a label number, but then walks off the list looking for it. In change two, sometimes the function number is invalid. And change three protects from the string not being terminated. Looking at this again, if I had just added an 'else' to "if (ch != '\\')" then I could have made a less invasive change. Also: if you want to give any of these error messages better text, or if I've broken the internationalization with them, please change them to suite your preferences. What I gave you is better than the "DANGER, WILL ROBINSON!" that I had before. In storage.c, initializing 'v_next' is one of the things I consider a bug. Sometimes, it has a "valid" pointer in it. The next six changes are defensive error checks to ensure that the array being requested is plausible. The line "params++;" looks like a hold-over from an earlier version of the code, where the parameters were stored in an array. With the linked-list, the proper way to advance to the next parameter is "params = params->next;", which always occurs a few lines later. That leaves util.c. I think they were trying to save memory, at some point. Possibly: variable names are treated differently from array and function names, and I don't see the reason for that. What happens is that the value from lookup() is used to initialize av_name in nextarg(). Then, av_name is directly used to index v_names right above that removed "params++;" line. In this retrospective dive through the code, that may be it. The line in storage.c could be changed, I think, but, in my opinion, it is better to move the code so that it more consistently handles all types. In addition, while the line "if (id->v_name <= MAX_STORE)" is annoying in that it is different, it isn't guarding against an invalid access. Thank you again, Thomas DiModica
