dschmenk / PLASMA

Proto Language AsSeMbler for All (formerly Apple)
MIT License
194 stars 26 forks source link

PLASM compiler crashes using postfix array syntax #67

Closed tingtron closed 1 year ago

tingtron commented 1 year ago

Using both AppleWin and GSplus, the following program crashes the +PLASM compiler. The environment was unmodified disk image PLASMA2.2mg from GitHub.

Console output (AppleWin emulating Apple //e):

/PLASMA/BLD/:c                          
INC/                SAMPLES/            
PLASM+              CODEOPT+            
MY1.PLA             MY1+                
MY2.PLA             MY2+                
MY3.PLA                                 
/PLASMA/BLD/:+plasm my3.pla             
PLASMA Compiler, Version 2.0 ALPHA1     
Data+Code buffer size = 12353           

DATA:.....                              
11F5-    A=00 X=0D Y=73 P=37 S=C6       
*                                       

In context:

11F5-    A=00 X=0D Y=73 P=37 S=C6       
*11e8l                                  

11E8-   D0 EE       BNE   $11D8         
11EA-   E8          INX                 
11EB-   E8          INX                 
11EC-   E8          INX                 
11ED-   60          RTS                 
11EE-   E8          INX                 
11EF-   E8          INX                 
11F0-   E8          INX                 
11F1-   B5 CD       LDA   $CD,X         
11F3-   15 BD       ORA   $BD,X         
11F5-   F0 70       BEQ   $1267         
11F7-   B5 CE       LDA   $CE,X         
11F9-   D5 CF       CMP   $CF,X         
11FB-   B5 BE       LDA   $BE,X         
11FD-   F5 BF       SBC   $BF,X         
11FF-   90 2C       BCC   $122D         
1201-   B5 CF       LDA   $CF,X         
1203-   85 08       STA   $08           
1205-   B5 BF       LDA   $BF,X         
1207-   85 09       STA   $09           

Stack:


*1c0.1ff                                
01C0- 53 B5 53 FC A0 BA FD F8           
01C8- FE 84 FF 83 DC 0C 9B 95           
01D0- AD 83 DC 14 81 9D AD 83           
01D8- DC 3C 0C AC AD 83 DC 3E           
01E0- 42 FB AD 83 DC 43 3D 83           
01E8- DC 45 CE AD DC 1B 0B E5           
01F0- AE 59 DC 1E 16 00 AF 59           
01F8- DC 1E D5 59 DC 1F DF 05           

*0180.1c7                               
0180- FF FF 00 00 FF FF 00 00           
0188- FF FF 00 00 FF FF 00 00           
0190- FF FF 00 00 FF FF 00 00           
0198- FF FF 00 00 FF FF 00 00           
01A0- FF FF 00 00 FF FF 00 00           
01A8- FF FF 00 00 FF FF 00 00           
01B0- 0D 0D 00 00 0D 0D 00 17           
01B8- 05 CE 17 53 FC 53 FC 53           
01C0- 53 B5 53 FC A0 BA FD F8

In GSplus, it is reported slightly differently, but looks like in the same place:

/PLASMA/BLD/:+plasm my3.pla
PLASMA Compiler, Version 2.0 ALPHA1
Data+Code buffer size = 12353

DATA:.....
00/11F3: 00 60        BRK 60
 A=0100 X=000D Y=0073 S=01BD D=0000 P=B1
 B=00 K=00 M=24 Q=88 L=1 m=1 x=1 e=1

00/11EF: E8           INX
00/11F0: E8           INX
00/11F1: B5 CD        LDA CD,X
00/11F3: 15 BD        ORA BD,X
00/11F5: F0 70        BEQ 1267 {+70}
00/11F7: B5 CE        LDA CE,X
00/11F9: D5 CF        CMP CF,X
00/11FB: B5 BE        LDA BE,X

This sample code was taken from Kansasfest 2015, which was adapted using the PLASMA 2.0 Alpha1 syntax.

my3.pla.txt

// text screen

include "inc/cmdsys.plh"
include "inc/conio.plh"

const SCR_PTR = $0400
const SCR_SIZE = 1024

byte ScrSave[SRC_SIZE]
byte I
byte Name

// save text screen

memcpy(@ScrSave, SCR_PTR, SCR_SIZE)

conio:home()

for I=0 to 23
  conio:gotoxy(I,I)
  putc('A'+I)
  putc(' ')

  puti(I)
next

conio:gotoxy(10, 5)
puts("What is your name")
Name = gets('?'|$80)  // $BF

conio:gotoxy(12, 7)
puts("Nice to meet you, ")
puts(Name)

conio:gotoxy(16, 10)
puts("Press any key ...")
getc()

conio:gotoxy(0, 5)
memcpy(SCR_PTR, @ScrSave, SCR_SIZE)

💡 Workaround: use prefix array syntax. Instead of byte name[size], use byte[size] name.

dschmenk commented 1 year ago

Ok, I checked into this. I'm not sure why the version of the compiler you're using is crashing, but the latest should be pointing out the error in the provided source.

The working my3.pla looks like:

// text screen
include "inc/cmdsys.plh"
include "inc/conio.plh"
const SCR_PTR = $0400
const SCR_SIZE = 1024

byte ScrSave[SCR_SIZE]
byte I
word Name

// save text screen
memcpy(@ScrSave, SCR_PTR, SCR_SIZE)
conio:home()
for I=0 to 23
  conio:gotoxy(I,I)
  putc('A'+I)
  putc(' ')
  puti(I)
next
conio:gotoxy(10, 5)
puts("What is your name")
Name = gets('?'|$80)  // $BF
conio:gotoxy(12, 7)
puts("Nice to meet you, ")
puts(Name)
conio:gotoxy(16, 10)
puts("Press any key ...")
getc()
conio:gotoxy(0, 5)
memcpy(SCR_PTR, @ScrSave, SCR_SIZE)
dschmenk commented 1 year ago

So I did boot the plasma2.img and tried the my3.pla with the error. It reported it correctly using the Virtual ][ emulator. Not sure what is going on with the other emulators. I will try and get an Alpha 2 release out soon that hopefully fixes whatever issue you see

tingtron commented 1 year ago

Hi David. Thank you for looking into it.

The PLASMA2.2mg I was using was the latest from GitHub -- the Alpha 1 from Jul 4, 2020. It is still there.

Yes, indeed the two faults in the source are valid:

(*) In AppleWin it never reported an error -- it either crashed (with both byte Name and wrong array size) or just worked (with either changed), Interestingly, in GSPlus (there is a Mac version) -- it is now reporting "Error: invalid constant". (But at some point it crashed and I captured the monitor exit there. It may have been slightly different line order.)

Different behavior between AppleWin (//e) and GSPlus (IIgs) shows that machine configuration matters. I can try Virtual ][ on a Mac too.

I am looking forward to the Aplha 2. BTW, I couldn't find a Sandbox in Plasma 2 (at least in the PLASMA2.2mg). It was a very handy environment, since most of the examples on board would be small, and for large projects people would still prefer cross development or smaller modules. Since different configurations and memory strategies are already recognized, maybe it would be possible to even have specialized Sandboxes too -- resident for larger RAM and overlaid for smaller RAM. Also for IIc+ and IIgs, the cursor blinking rate in ED would be nice to be adjustable.

tingtron commented 1 year ago

Interesting, when using Virtual ][ on a Mac with similar configuration to AppleWin (//e with PLASMA2.2mg in OmniDisk), it reports the error without a crash. But when it's run, the screen corrupts when using byte Name, whereas on ApleWin, it runs normally. So definitely there is some different behavior between the emulators. Not sure if there are using exactly the same IIe ROM.

Can you tell where and what is that piece of code it crashes on in the original report? I could also provide the stack, if that's helpful.

PLASMA Editor, Version 2.0 DALPHA1
my3.pla:w
...
my3.pla:q
/PLASMA/BLD/:c
INC/                SAMPLES/
PLASM+              CODEOPT+
MY3.PLA
/PLASMA/BLD/:+plasm my3.pla
PLASMA Compiler, Version 2.0 ALPHA1
Data+Code buffer size = 12353

DATA:.....
Error:invalid constant
my3.pla[9]
BYTE SCRSAVE[SRC_SIZE]
             ^
/PLASMA/BLD/:
dschmenk commented 1 year ago

That's what's confusing. It seems like it is trying to execute code out of the auxiliary memory even though it should be executing it out of main memory. In GSPlus, you can see where it crashed on an instruction that clearly isn't there when listing the memory range. My guess it was trying to call memcpy() which exists in the cmd(jit).pla code (system routines in main memory). I'll try it on real hardware next and see if there is any different behavior.

tingtron commented 1 year ago

I was trying to run various machines (IIe, IIc, II+) on Virtual ][, but could not reproduce the issue - it reported the syntax error.

I put the stack contents in the original report for more context.

The ROMs between Virtual ][ and AppleWin, for the //e are somewhat different:

When running +plasm my3.pla with the original erroneous code on AppleWin as Apple IIe (non-enhanced), the screen freezes on DATA...... With the following are of code running: AppleWin - IIe (non-Enhanced)

When running as Apple II+, the screen freezes becomes filled with @@@@@... completely. With the following are of code running: AppleWin - IIplus

When the code is fixed and with no errors - on those other machines choices in AppleWin, everything runs fine. So it issues happen when error handling during compilation.

I wonder if it could be an issue or a bug with AppleWin. So if you have a chance to run AppleWin, it would be interesting to see what's going on. It has this nice colorful tracing facility.

dschmenk commented 1 year ago

Thanks for all your debugging. There is definitely a problem as I've run it on actual hardware and gotten an interesting mix of success and failure. I'm beginning to wonder if something related to 6502 vs 65C02 as my Apple ][+, unenhanced //e , and Apple /// all crash. My enhanced //e and // GS work. I've disabled the JIT compiler to no effect. Very perplexing.

It is likely something to do with the error reporting, so I'll focus my attention there. Thanks again for your help!

After further review of the compiler code, it may be that I'm not checking for the error condition of an invalid constant value in the array size declaration. A null pointer is dereferenced and many bad things ensue. I'll check out a fix shortly

tingtron commented 1 year ago

There a book with many details on //e vs //e Enhanced and 6502 vs 65C02:

dschmenk commented 1 year ago

I was able to verify that the problem lies with not properly handling the incorrect constant identifier in the array declaration. I also found the prefix array syntax isn't setting the array size correctly (zero!). As soon as I track that down I'll check in the fixes.

peterferrie commented 1 year ago

Virtual II and AppleWin also set initial memory patterns differently, which can account for different behaviour between the two. If there is variance on real hardware, then I suspect an uninitialised memory read.

dschmenk commented 1 year ago

I'm dumb. By using a byte size in the parsing of prefixed arrays the value was being truncated to 8 bits. Pretty bad if you have an array larger that 255. Both issues now fixed and I'll be checking in the fix ASAP.

dschmenk commented 1 year ago

I'm staging the ALPHA2 release. I've update the PLASMA2.2mg disk image with the latest build. Would you like to try it out? You will actually find my3.pla as /bld/samples/coniotst.pla and binary as /demos/coniotst. This is using the prefixed array size. Feel free to test out different array declarations.

And thanks again for all your diligence in helping track this down!

tingtron commented 1 year ago

It appears to be working now in all versions under AppleWin. Using the original syntax with errors, it's reporting correctly Error:invalid constant.

⚠️Note: the latest PLASMA2.2mg for some reason uses ProDOS 2.0.5, which requires Enhanced //e.

This is not necessary and the old ProDOS 1.9 works just fine and goes all the way back to the original Apple II (in AppleWin):

Original Apple II

dschmenk commented 1 year ago

Ah shoot. I inadvertently put the wrong PRODOS on there. I'll fix it. Thanks.