mgbdis icon indicating copy to clipboard operation
mgbdis copied to clipboard

Garbage function calls break valid ASM instructions

Open Mallos31 opened this issue 3 years ago • 3 comments

I have an example of the issue. Explanation: A garbage function call from a data section of the ROM broke what should have been a 3-byte instruction. Screenshot shows comparison in Ghidra. image

Mallos31 avatar Jul 01 '21 01:07 Mallos31

"Garbage" label creation is fairly widespread (jr targets next to data blocks do the same), but needs to be handled somehow, the target cannot be simply omitted, as the instruction needs a target.

Sometimes it's intentional, too:

    jr nz, .notZero
    db $3E ; ld a, $AF
.notZero
    xor a
    ld [wFlag], a ; Write $AF or $00

ISSOtm avatar Jul 01 '21 13:07 ISSOtm

I'm not sure how this could be fixed, beyond adding some sort of data section recognition. There might be some heuristics that could be applied, like loading a register with two different values right after each other, etc. But maybe it would suffice to be even clearer in the README that properly labeling data sections is very important to get a good disassembly? It already suggests doing a trace in an emulator.

tobiasvl avatar Feb 13 '23 21:02 tobiasvl

Without perfectly knowing what is code or data this will always an issue, in particular with bank 0. I believe originally mgbdis did not create labels in bank 0 for calls/jumps originating from other banks, however this changed while back.

We could go back to that old behaviour by default and add an option to enable labels to be generated in bank 0 from other banks (useful for 32KB ROMs).

Another thing that could improve it is to only generate labels in bank 0 from other banks when the call/jump originates in a known code block (from the sym file).

mattcurrie avatar Feb 13 '23 22:02 mattcurrie