A curious bug in EhBasic

[SKO]

Hi everyone,

I recently encountered an issue with EhBasic, and I would like to hear opinions on this; in particular, do you agree that this is a bug in EhBasic, and if so, how can it best be fixed?

The issue is with the following code:

Code: Select all

; flush stack and clear continue flag

LAB_1491
    LDX #des_sk     ; set descriptor stack pointer
    STX next_s      ; save descriptor stack pointer
    PLA             ; pull return address low byte
    TAX             ; copy return address low byte
    PLA             ; pull return address high byte
    STX $1FE        ; save to cleared stack
    STA $1FF        ; save to cleared stack
    LDX #$FD        ; new stack pointer
    TXS             ; reset stack
    LDA #$00        ; clear byte
     .
     .
    RTS

I posit that this cannot work on a system where interrupts are being handled. If the stack pointer is $FF going into this code (or $FE, but $FF actually seems to be typical), and an interrupt occurs somewhere after STX $1FE and before TXS, then the interrupt sequence will overwrite the return address that is being put on the stack here. It does not matter what the interrupt service routine does or does not do; the issue would exist even if it is nothing more than an RTI, as the problem already happens during the hardware controlled modification of the stack. Nothing terrible will happen immediately after the ISR returns, but the moment the RTS is executed, things will go awry.

(Background story: I discovered this after implementing RTS/CTS flow control for the serial port on my self-designed system, with the RTS/CTS lines controlled by software, and this required going from polled serial input to an interrupt-driven mechanism with a small ring buffer. EhBasic was actually a very good use case for this new code, because this would allow me to paste BASIC programs for my 6502 computer directly into the PuTTY window on my PC. Without flow control, EhBasic is too slow to process the input even at 19200 baud, and characters will be lost. Unfortunately, as soon as I started testing my fancy new IRQ code, I began experiencing very disconcerting behaviour: I would paste a program, and suddenly EhBasic would start new at the memory prompt, or I would find myself at the monitor prompt, and similar things. I took me a while to nail it down to this piece of code.)

To test my theory, I fixed the issue very simply like this:

Code: Select all

    SEI
    STX LAB_SKFE    ; save to cleared stack
    STA LAB_SKFF    ; save to cleared stack
    LDX #$FD        ; new stack pointer
    TXS             ; reset stack
    CLI

And with this, my problems did indeed go away. A more sophisticated solution is not so easy to find. It seems like the right return address is already on the stack, just unfortunately in the wrong byte order. (Why this would be so, I have no idea. Incidentally I also have no idea why EhBasic goes through this kind of code when all I'm doing is entering a program.)

Now, if this were 65C02 code, you could simply do:

Code: Select all

    PLX
    PLA
    PHX
    PHA

But EhBasic is not 65C02 code, and I'd like to keep it that way. It's not clear to me if I'm allowed to modify Y at this point in the code; that would also make things easier.

What are your thoughts?

[barrym95838]

Welcome, SKO! I think that you did a fine sleuthing job and field repair, and you probably needn't provide it with any more real estate in your head. A large and complex object like EhBasic is likely to be a bit fragile, and if you start poking at it you might get poked back.

Lee passed away a number of years ago, but I believe that Klaus2m5 has generously volunteered to address bug fixes from time to time. Perhaps he can make your patch available to a wider audience?

[BigEd]

Indeed, welcome! Klaus maintains both the original and a patched version of EhBasic and a minimal monitor at
https://github.com/Klaus2m5/6502_EhBASIC_V2.22

As far as I can tell, your analysis is right, and SEI/CLI around the critical code would be a good fix. Maybe others are possible - it's certainly hard to think about code which manipulates the stack pointer this way.

[BigDumbDinosaur]

SKO wrote:

Hi everyone...

Welcome to 6502-land.

Quote:

I recently encountered an issue with EhBasic, and I would like to hear opinions on this; in particular, do you agree that this is a bug in EhBasic, and if so, how can it best be fixed?

I’m a little puzzled as to how your discovery went undiscovered all this time—EhBasic has been around for nearly 25 years.  Surely someone, somewhere, had to have been bitten by this little bit of stackrobatics faltering at the wrong time.  It’s possible it only happened once in a very long while and the victim thought that the problem was a design issue with his/her homebrew machine, or the effects of cosmic radiation. 

As Mike suggests, I’d be cautious about modifying that bit of code, other than wrapping it with Ed’s suggested fix.  I am not an EhBasic user, but do know that there are other things in the code that are unusual, possibly adding to the fragility Mike mentions.

BTW, I’m not sure I’d classify this as an outright bug.    A bug is anomalous behavior caused by code that doesn’t work as intended.  This little routine does work as intended...as long as an interrupt doesn’t hit at an inopportune moment.  Let’s call it a...wait for it...feature! 

[dmsc]

Hi!

SKO wrote:

Hi everyone,

The issue is with the following code:

Code: Select all

; flush stack and clear continue flag

LAB_1491
    LDX #des_sk     ; set descriptor stack pointer
    STX next_s      ; save descriptor stack pointer
    PLA             ; pull return address low byte
    TAX             ; copy return address low byte
    PLA             ; pull return address high byte
    STX $1FE        ; save to cleared stack
    STA $1FF        ; save to cleared stack
    LDX #$FD        ; new stack pointer
    TXS             ; reset stack
    LDA #$00        ; clear byte
     .
     .
    RTS

That code is in fact buggy - you should never write bellow the stack pointer, as any IRQ will write over your values. BUT, it will work whenever the label is called with the stack pointer lower than $FB, as you will be writing over the current stack pointer instead of bellow, and this is ok.

I think that the best fix is to use PHA to write to the stack, this is shorter and faster, and won't have this kind of problems:

Code: Select all

; flush stack and clear continue flag

LAB_1491
    LDX #des_sk     ; set descriptor stack pointer
    STX next_s      ; save descriptor stack pointer
    PLA             ; pull return address low byte
    TAY             ; copy return address low byte
    PLA             ; pull return address high byte
    LDX #$FF        ; new stack pointer
    TXS             ; reset stack
    PHA             ; push return address high byte
    TYA             ; copy return address low byte
    PHA             ; push return address low byte
    LDA #$00        ; clear byte
     .
     .
    RTS

This is 3 bytes shorter, so it will fit in the place of the original code.

Have Fun!

[barrym95838]

That seems to be a nice patch, but it assumes that the Y register can be stomped, which might need further investigation. SKO hinted at this in the initial post.

[dmsc]

Hi!

barrym95838 wrote:

That seems to be a nice patch, but it assumes that the Y register can be stomped, which might need further investigation. SKO hinted at this in the initial post.

Yes, this is true.

I scanned the usage of the above code a little, and it is only used on "CLEAR", "NEW" and "RUN", and in each case the Y register is reloaded with another value afterwards, so I assumed it is safe. The EHBasic code is not very optimized, so there is not many assumptions on the values of registers in the code.

Have Fun!

[floobydust]

Per the OP,

I've been using interrupt-driven code for years on my C02 Pocket SBC for the serial port and also a jiffy clock with the NXP (D)UARTs which includes a timer. I also implement 128-byte circular buffers for the serial port (used as a console) with RTS/CTS handshaking.

I also have used EhBasic quite a bit without such an issue as you're experiencing. However, I did a fair amount of work on EhBasic years ago and rewrote much of it taking advantage of the CMOS instructions and addressing modes. I also implemented all of Klaus' patches up to P5, which I still think is the last one he worked on.

You might want to give it a try... it can be found here:
viewtopic.php?f=5&t=5760#p73034

Beyond this, perhaps you might have something in your ISR handler that might be conflicting with the EhBasic code. You can find several versions of my BIOS code on my Github page that supports the 6551, 6522, SCC2691 and SC28L92 I/O chips, along with an IDE port supporting an IDE disk drive (and Compact Flash in IDE mode).

[SKO]

Thanks to everyone for the replies and the warm welcome. I have lurked on this forum for a while (or else been directed here by Google on occasion), and it has generally been an invaluable resource.

@dmsc : I've tested your suggested solution using the Y register, and I can report that it is also working fine. Using Y does not appear to be a problem.

I can only guess why I ran into this issue while others appear to be unaffected. It typically happens when I keep pasting the same program in quick succession into the console, trying to add input faster than EhBasic processes it. If you're not doing this sort of nonsense, you may by and large be safe (but not in principle! ). Incidentally, I am not ever issuing a RUN, NEW or CLEAR command. It seems this code is also called after each line that is added, presumably coming from LAB_1319. But there, too, Y is modified shortly after the call returns, and it's safe to "stomp" it.

[jgharston]

And if Y is always safe to clobber, than the original code would have been fine if the stack was set before writing to it:
LAB_1491
LDX #des_sk ; set descriptor stack pointer
STX next_s ; save descriptor stack pointer
PLA ; pull return address low byte
TAY ; copy return address low byte *USE Y*
PLA ; pull return address high byte
LDX #$FD ; new stack pointer
TXS ; reset stack *BEFORE WRITING TO IT*
STY $1FE ; save to cleared stack *USE Y*
STA $1FF ; save to cleared stack
LDA #$00 ; clear byte