2xsaiko / HCTM

2xsaiko's tech mod
12 stars 4 forks source link

CPU related issue(Indirect addressing) #30

Open inseo-oh opened 5 years ago

inseo-oh commented 5 years ago

I was trying to write a program in assembly(I really wanted assembly programming, simply because it's much faster).
And I was using your assembler(https://github.com/therealfarfetchd/XC8010-Assembler) to compile my assembly code.

But I ran into a problem.

lda ($10), y

Whenever I tried to run this CPU instruction, random weird things happens and error light(in Computer block GUI) turns on. Here's what happened: image

Whenever I do LDA ($??), y, strange things like this happens, and in some cases, it randomly reboots and jumps to part of code which is something I did not want.

And here's computer GUI with error light turned on: image

What I tried to do is simply take start address of string from $10, and use Y as index to display string on screen:

; TargetTerminal runs MMU instruction to set bus target to terminal. This works fine.
; Print: Writes a message to screen
; $10: Message to write
Print:
  jsr TargetTerminal
  sep #$30        ; Set width to 8-bit
  ; Set screen row to cursor position
  lda #$01
  sta $FF00
  ; Initialize registers
  ldy #$00        ; Y = $00(Index)
Print_Loop:
  lda ($10), y    ; Get next character
  beq Print_End   ; Go to end if it's zero
  sta $FF10, y    ; Put character on screen
  iny             ; Increase index
  jmp Print_Loop
Print_End:
  rep #$30        ; Set width back to 16-bit
  rts

At first, I thought something was wrong with my code. But it was hard to believe that simple LDA instruction could crash the computer.
After lots of troubleshooting, I found something strange. I was looking at the result opcode in hex editor and looking at mod's source code, then I realized that what assembler outputs and what mod expects are different.
Here's assembler's output for LDA ($10), Y

B1 10

I believe this opcode is compatible with 6502 one and after a bit of googling, opcode B1 was correct for instruction LDA ($??), Y.
And in CPU code in your mod: https://github.com/therealfarfetchd/HCTM/blob/7ae61888b92629db5f146c3fcd9c98b12c4b6593/src/main/kotlin/therealfarfetchd/retrocomputers/common/cpu/Processor.kt#L475

It seems like this code handles LDA ($??), Y, and pc2IY clearly reads two bytes, not a single byte.

Furthermore, when I looked at bootldr.bin(which contains bootloader) in this mod, I found that it also has two bytes after opcode(it was same instruction with same operand value).

B1 10 00

NOTE: Of course, I checked(both mine and bootloader one) whether next opcode matched with next instruction in source code or not, and it matched.

So, I think what was happening was, it saw opcode for LDA ($??),Y(B1) without any issue, but then it read two bytes from memory, which not only results memory address that I did not want, but it also invalidates all subsequent instructions(and turning on error light later because of an undefined instruction error).

I also found that your assembler has been updated at some point to fix indirect addressing(https://github.com/therealfarfetchd/XC8010-Assembler/commit/c9da4589fa53f4db76822ce14b1f4dbf0d9b1ef1).

And it seems like other instructions that use indirect addressing also has same issue.

It seems like assembler was updated, but the mod was not.
I was going to try fixing this myself, but I was getting some build errors, so I decided to post an issue first.

2xsaiko commented 5 years ago

To work around this for now, you could insert a .db $00 after the lda instruction, just to insert that zero byte it expects (or use the older commit of the assembler that doesn't have the fix yet). This will be fixed in a future version of the mod. Thanks for reporting, this is a really detailed issue!

The build errors you're getting are most likely because my maven repository is now moved, I'm now using https://maven.amadorn.es for my stuff. Sadly you can't just replace it as is, since I've used + as the version for QuackLib in the build script, and some of the newer ones are broken/incompatible, as far as I know. So if you want to fix it, you'll have to find a version which works (which is probably going to be the same as the latest one on CurseForge)

inseo-oh commented 5 years ago

Thanks, adding db $00 immediately fixed my instruction issue(at least for now).

  lda ($10), y    ; Get next character
  db $00         ; **TEMPORARY FIX**
  beq Print_End   ; Go to end if it's zero
2xsaiko commented 5 years ago

Let's keep this open so that I know that I still have to properly fix this