-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PLASM compiler crashes if invoked with a space between "+" and PLASM #53
Comments
This needs tidying but seems to work well, and it may even be smaller - though I'd need to clean the code up and diff it with a pre-experimental-hacks commit to see. Noticed while testing this that doing "+ PLASM TEST.PLA" causes PLASM to crash in a nasty way but I tried it on Apple II and the same thing happens there (admittedly it kills the VM more on the Acorn, but I don't think it's that worrying; once you start scribbling over memory it's all just pot luck), have raised issue dschmenk#53 upstream for this. The basic idea here is that we leave the command line read from the user alone - this means we haven't stripped the leading command character or truncated it to just the module name - and so args.pla works without any changes from the upstream code. Most things don't care about stripping the leading command character (e.g. OSCLI quite happily accepts commands which have a "*" in them) and we've reworked + so obviously it doesn't care. We just copy the module name into a separate buffer; I've used SCRATCH for this at the moment, but may review that (though it's probably a sound choice with a small tweak or two). "*PLASMA +PLASM TEST.PLA" to launch VM and run the compiler now works without the append-a-CR hack I had in place at one point.
Actually I've been unfair in calling this a crash, it seems to generate an odd message but maybe that's fair enough given the invalid input. But it does seem a bit odd to have this parsing inconsistency. |
Hi Steve-
Yep, the command line parsing is pathetic at best. But, there are literally no bytes available to improve anything. It really wasn’t meant to be more than a way to automatically lunch modules at startup or run a better shell. There is no reason you have to suffer with it if you want to fix it for the Beeb.
As for looking at the JIT compiler, I’m going to have to re-populate all the neurons that either died or got repurposed ;-) I should get around to sometime in the near future - it will give me an opportunity to look at it with somewhat fresh eyes.
Dave…
P.S. Isn’t Will’s emulator incredible?
… On May 10, 2019, at 1:42 PM, ZornsLemma ***@***.***> wrote:
If I invoke the PLASM self-hosted compiler as "+ PLASM TEST.PLA", it crashes; here's a screenshot of this in an Apple II emulator:
<https://user-images.githubusercontent.com/7988240/57555333-e8722380-736b-11e9-83a3-30e951ea3f1e.png>
I think at least part of the issue is that the core VM strips away the space between the "+" and "PLASM", so it's able to successfully execute the PLASM module, but the argument parsing code in the ARGS module returns "+" and then "PLASM" as the first and second arguments.
It think the actual crash is caused by PLASM trying to load itself as source code, as if we'd typed "+PLASM PLASM", and crashing because the file is not text; I seem to be able to get a similar crash by typing that. I don't know if this is worth fixing; perhaps just changing the parsing so the core VM and ARGS are consistent in handling a space between the leading "+" and the module name would be adequate.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#53>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBOGJWGTJZE63VAZXYCYY3PUXM43ANCNFSM4HMGC66Q>.
|
Hi Dave, Thanks for the response, I didn't really expect one so quickly! I may be missing something, but one fix (untested, of course) would simply be to remove the last call to stripspaces() in parsecmd() in cmd.pla. That would shrink the core VM, not grow it, and I think it would make args.pla consistent with the core VM and get rid of this inconsistency - "+ PLASM" would then simply be invalid at the PLASMA prompt. As for the compiler itself, I overreacted by calling it a crash. On the Acorn VM it spews out some random text which happens to include the "turn printer on" control code, and then the output hangs because there's no printer connected and the printer buffer fills up. I misinterpreted this as the VM/compiler actually having crashed the machine and that primed my brain to misinterpret the even better behaviour of it on the Apple. The behaviour of the compiler is a little weird but it's being given weird input, and I think it actually copes pretty well, so there's probably no need to go fiddling with it. Not that I want to discourage you from having another look at it, I'm sure you'll come up with some cool stuff if you do look at it with fresh eyes! Cheers. Steve PS Yes, the emulator is a very nice piece of work - perfect too for someone like me who doesn't necessarily want the faff of installing an emulator on my machine but wants to play with something like PLASMA. |
PPS In terms of freeing up space in the core VM, one thing I've just done in the Acorn VM is that I don't copy the command line into a separate buffer for use later by args.pla before parsing it in cmd.pla. I have a single copy of it and the command parsing in cmd.pla has been tweaked to avoid modifying the command line, so it's left intact for args.pla to parse later. This avoids the need to set memory aside for a second copy of the command line. It may be you could do something similar on the Apple. (Or it may be there's a hidden flaw in this approach I simply haven't noticed yet. :-)) You can see - though it's perhaps a bit noisy - this change on my rather misnamed jit2 branch: ZornsLemma@245d506 |
The Apple II will blow away the input buffer as it loads files. It’s why I had to move it in the first place. 64K is such a hassle, it’s amazing we ever got anything done.
… On May 11, 2019, at 7:14 AM, ZornsLemma ***@***.***> wrote:
PPS In terms of freeing up space in the core VM, one thing I've just done in the Acorn VM is that I don't copy the command line into a separate buffer for use later by args.pla before parsing it in cmd.pla. I have a single copy of it and the command parsing in cmd.pla has been tweaked to avoid modifying the command line, so it's left intact for args.pla to parse later. This avoids the need to set memory aside for a second copy of the command line. It may be you could do something similar on the Apple. (Or it may be there's a hidden flaw in this approach I simply haven't noticed yet. :-)) You can see - though it's perhaps a bit noisy - this change
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#53 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBOGJR5T5WI4MPWUOQBCW3PU3IEZANCNFSM4HMGC66Q>.
|
Yeah, I guess that *would* be a problem... :-)
…On Sat, 11 May 2019, 15:27 David Schmenk, ***@***.***> wrote:
The Apple II will blow away the input buffer as it loads files. It’s why I
had to move it in the first place. 64K is such a hassle, it’s amazing we
ever got anything done.
> On May 11, 2019, at 7:14 AM, ZornsLemma ***@***.***>
wrote:
>
> PPS In terms of freeing up space in the core VM, one thing I've just
done in the Acorn VM is that I don't copy the command line into a separate
buffer for use later by args.pla before parsing it in cmd.pla. I have a
single copy of it and the command parsing in cmd.pla has been tweaked to
avoid modifying the command line, so it's left intact for args.pla to parse
later. This avoids the need to set memory aside for a second copy of the
command line. It may be you could do something similar on the Apple. (Or it
may be there's a hidden flaw in this approach I simply haven't noticed yet.
:-)) You can see - though it's perhaps a bit noisy - this change
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub <
#53 (comment)>, or
mute the thread <
https://github.com/notifications/unsubscribe-auth/ABBOGJR5T5WI4MPWUOQBCW3PU3IEZANCNFSM4HMGC66Q
>.
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#53 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AB46IEB22N7ZHE4BVI5OJXDPU3JUTANCNFSM4HMGC66Q>
.
|
If I invoke the PLASM self-hosted compiler as "+ PLASM TEST.PLA", it crashes; here's a screenshot of this in an Apple II emulator:

I think at least part of the issue is that the core VM strips away the space between the "+" and "PLASM", so it's able to successfully execute the PLASM module, but the argument parsing code in the ARGS module returns "+" and then "PLASM" as the first and second arguments.
I think the actual crash is caused by PLASM trying to load itself as source code, as if we'd typed "+PLASM PLASM", and crashing because the file is not text; I seem to be able to get a similar crash by typing that. I don't know if this is worth fixing; perhaps just changing the parsing so the core VM and ARGS are consistent in handling a space between the leading "+" and the module name would be adequate.
ETA: The commit in my repo which references this just notes that I found the problem; I am playing around with alternative implementation details on the Acorn port, that commit is not any kind of fix or suggestion for this issue and you should just ignore github being helpful and mentioning it...
The text was updated successfully, but these errors were encountered: