Yearly Archives: 2018

Martyn Hill, while testing his brand new Tetroid SuperGoldCard clone, has discovered a strange bug where his QL crashed when he was loading a _scr file into the QL’s screen memory. It works fine without the SuperGoldCard. A lot of theories were put forward, like:

• It’s a problem with the SuperGoldCard: turns out it works fine when QDOS or a very early Minerva version is used instead
• It’s connected to the memory timing of the screen area: turns out is also happens in fast memory
• It’s connected to the 68020 processor: turns out is also happens on the 68000 processor
• The SuperGoldCard patches are wrong: I’ve analyzed all the patches and they all look fine. In fact the patch code is identical to Minerva’s code except for the timing loops. Still, it appeared to work with earlier versions of the patches, which makes it weird again.

Personally I couldn’t care less if Microdrives work or not and the fact that this bug was not discovered for so long strongly implies that most other people also don’t use Microdrives with SGC systems anymore. Still I have trouble resisting a good puzzle so I had a look. And then another one. And then ten more because this was a very elusive bug.

The problem is that not only is the bug located in ROM locations that are hard to debug anyway, the MDV code is so time critical that it’s impossible to step through the code. So how can one debug this? Hardware to the rescue! The SuperGoldCard very conveniently has a parallel port on board that can be accessed very simply by the software. By connecting a logic analyzer to all 8 data lines and sprinkling output commandos throughout the MDV code I can get a live trace of what the code is doing or output the contents of registers byte-by-byte.

In hindsight I should have found the problem much earlier because of the symptoms, but that’s easy to say after you know what’s going wrong. Let’s make a little quiz, here’s the code part with the problem and you can try to spot the bug:

        move.w  #blksize,d0     full block normally (512)
        cmp.b   lstblok(a0),d7  is it the last file block?
        bne.s   not_last        no - copy to end of it
        move.w  lstbyte(a0),d0  get the number of bytes in last block
not_last
        lsl.w   #blkbits-1,d7
        add.l   d7,d7           is it the first file block?
        lea     -md_deend(a1,d7.l),a5 where to put data normally
        bne.s   not_frst
        moveq   #md_deend,d1    yes - move on past the file header
        add.l   d1,a4
        add.l   d1,a5
        sub.w   d1,d0           copy fewer bytes
not_frst
        ror.l   #2,d0           long words, and save odd byte count
        bcc.s   unbf_dbr        carry means there is an odd word to do
        move.w  (a4)+,(a5)+     if so, do it
        bra.s   unbf_dbr
unbf_lop
        move.l  (a4)+,(a5)+     move a long word
unbf_dbr
        dbra    d0,unbf_lop
        add.l   d0,d0           see if there's an odd byte to go
        bpl.s   notabyte
        move.b  (a4)+,(a5)+     if so, do it
notabyte

Keep in mind that the code is not generally broken, it sometimes works! The problem also appears for the first block, so you can pretty much ignore the special handling of the last block. Found it? I’ll wait…

.

.

.

.

.

The “ror.l #2,d0” is a pretty clever way to both divide the register by 4 (because the copy loop does long words) while still remembering the odd bytes that also need to be copied. But it relies on one thing: that bits 16 and 17 of the register are zero. These bits are shifted into the lower 16-bits and are then used by the dbra instruction, which means that if they are set instead of copying 128 long words it will copy at least 16384. Talk about a buffer overflow!

You should now also see why it sometimes works. Remember earlier that I wrote “In fact the [MDV] patch code is identical to Minerva’s code except for the timing loops.”. The timing code in the original Minerva ROM is hidden behind a macro but in the end looks like this:

; For delays 5 to 19µs
        moveq #([us]*15-38)/4,d0        four byte sequence
        ror.l  d0,d0 

; For delays 21 to 309µs
        moveq   #([us]*15-50)/36,d0     six byte sequence
        dbra    d0,*

; ... etc, other variants are similar

For the (S)GC this is exchanged by loops like this:

        move.w  gmt_fgap,d0              ; 2800 us
        dbra    d0,*

I guess everybody can see the difference now, the original code clears the upper word of D0 and the replacement code does not. And as it happens the upper word is $FFFF from an earlier call to md_slave. All in all a very subtle difference, hard to spot by just looking at the code.

The solution I chose is it to exchange the “move.w #blksize,d0” line with “move.l”. That adds two bytes (every byte is precious in Minerva, there are not many left!), but this is the minimum I can get away with without relying on side effects of outside code, which is how this bug was introduced in the first place.

As this is the second fix from me for Minerva 1.98 I will have to think about how to mark these modified Minerva versions as increasing the version number is a no-go unfortunately. 1.99 is the last number we can use in theory before running into extended compatibility problems and I’m not even sure about this, 1.98 is still a safer choice.

Anyway, I hope this was interesting to read, leave a comment if you want. I will soon release a fixed binary version.