mre / mos6502

MOS 6502 emulator written in Rust
BSD 3-Clause "New" or "Revised" License
82 stars 15 forks source link

Possible carry bug with SBC #105

Closed pimlu closed 1 month ago

pimlu commented 1 month ago

Hello, I encountered a program where mos6502 deviates from the behavior I expected from SBC. I was trying to use this table lookup multiply from a "C64 old timer" and found that it worked in masswerk's emulator but not in mos6502. I narrowed it down to this difference in behavior with SBC:

use mos6502::memory::Bus;
use mos6502::memory::Memory;
use mos6502::instruction::Nmos6502;
use mos6502::cpu;
use mos6502::registers::Status;
use mos6502::registers::StatusArgs;

fn main() {
    let mut cpu = cpu::CPU::new(Memory::new(), Nmos6502);

    // sbc #0
    cpu.memory.set_byte(0, 0xe9);
    cpu.registers.status = Status::new(
        StatusArgs {
            negative: false,
            overflow: false,
            unused: false,
            brk: false,
            decimal_mode: false,
            disable_interrupts: false,
            zero: true,
            carry: true,
        });

    println!("registers: {:?}", cpu.registers);
    cpu.single_step();
    println!("registers: {:?}", cpu.registers);

    // expected carry to be 1 here, instead is 0
}

I'm new to 6502 so correct me if this is one of those things that vary depending on 6502 chip bugs, but it looks to me that, since C=1, and all the registers are 0, A - M - C̅ should result in 0 without any carrying, so afterwards C=1. This is what happens in masswerk's emulator at least.

mre commented 1 month ago

Interesting. Can you try out this? https://github.com/mre/mos6502/pull/106

mre commented 1 month ago

I think this is fixed in master now. Will go out with the next release.

mre commented 1 month ago

@omarandlorraine, we should add release-plz for automating parts of the release process. It creates pull requests which create new releases automatically when merged. It's pretty much magic. And yes, it checks for breaking changes and respects semver.

pimlu commented 1 month ago

I just confirmed this fixes the multiplication routine for all possible inputs - thanks a ton!