Skip to content

Commit 042c62d

Browse files
committed
chip8: fix branch reference operands losing instruction-specific formatting
1 parent aea4baa commit 042c62d

6 files changed

Lines changed: 101 additions & 1 deletion

File tree

internal/arch/chip8/chip8.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,27 @@ func (c *Chip8) formatOffsetCode(offsetInfo *offset.DisasmOffset, instruction in
172172
offsetInfo.Code = name
173173
}
174174

175+
// FormatBranchReference formats an instruction with a branch or data destination label.
176+
func (c *Chip8) FormatBranchReference(offsetInfo *offset.DisasmOffset, label string) string {
177+
opcode, ok := decodeOpcode(offsetInfo.Data)
178+
if !ok {
179+
return offset.FormatBranchReference(offsetInfo, label)
180+
}
181+
182+
switch opcode & 0xF000 {
183+
case 0x1000:
184+
return fmt.Sprintf("%s %s", chip8.Jp.Name, label)
185+
case 0x2000:
186+
return fmt.Sprintf("%s %s", chip8.Call.Name, label)
187+
case 0xA000:
188+
return fmt.Sprintf("%s I, %s", chip8.Ld.Name, label)
189+
case 0xB000:
190+
return fmt.Sprintf("%s V0, %s", chip8.Jp.Name, label)
191+
default:
192+
return offset.FormatBranchReference(offsetInfo, label)
193+
}
194+
}
195+
175196
// handleControlFlow processes control flow based on instruction type
176197
func (c *Chip8) handleControlFlow(address uint16, offsetInfo *offset.DisasmOffset, instruction instruction.Instruction, instr Instruction) {
177198
pc := c.dis.ProgramCounter()

internal/arch/m6502/format.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package m6502
2+
3+
import (
4+
"github.com/retroenv/retrodisasm/internal/offset"
5+
)
6+
7+
// FormatBranchReference formats an instruction with a branch destination label.
8+
func (ar *Arch6502) FormatBranchReference(offsetInfo *offset.DisasmOffset, label string) string {
9+
return offset.FormatBranchReference(offsetInfo, label)
10+
}

internal/disasm/code.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ func (dis *Disasm) processJumpDestinations() {
5252

5353
// reference can be a function address of a jump engine
5454
if offsetInfo.IsType(program.CodeOffset) {
55-
offsetInfo.Code = offsetInfo.Opcode.Instruction().Name()
55+
offsetInfo.Code = dis.arch.FormatBranchReference(offsetInfo, name)
56+
offsetInfo.BranchingTo = ""
5657
}
5758
}
5859
}

internal/disasm/disasm.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ type architecture interface {
3636
BankWindowSize(cart *cartridge.Cartridge) int
3737
// Constants returns the constants translation map.
3838
Constants() (map[uint16]consts.Constant, error)
39+
// FormatBranchReference formats an instruction that references a branch or data label.
40+
FormatBranchReference(offsetInfo *offset.DisasmOffset, label string) string
3941
// HandleDisambiguousInstructions translates disambiguous instructions into data bytes.
4042
HandleDisambiguousInstructions(address uint16, offsetInfo *offset.DisasmOffset) bool
4143
// Initialize the architecture.

internal/disasm/disasm_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ import (
88
"strings"
99
"testing"
1010

11+
chip8arch "github.com/retroenv/retrodisasm/internal/arch/chip8"
1112
"github.com/retroenv/retrodisasm/internal/arch/m6502"
1213
"github.com/retroenv/retrodisasm/internal/assembler"
1314
"github.com/retroenv/retrodisasm/internal/assembler/ca65"
15+
"github.com/retroenv/retrodisasm/internal/assembler/retroasm"
1416
"github.com/retroenv/retrodisasm/internal/options"
1517
"github.com/retroenv/retrogolib/arch"
1618
"github.com/retroenv/retrogolib/arch/system/nes/cartridge"
@@ -19,6 +21,24 @@ import (
1921
"github.com/retroenv/retrogolib/log"
2022
)
2123

24+
func TestDisasmChip8BranchReferenceOperands(t *testing.T) {
25+
input := []byte{
26+
0xa2, 0x08, // ld I, $208
27+
0xb2, 0x0a, // jp V0, $20a
28+
0x00, 0x00,
29+
0x00, 0x00,
30+
0x12, 0x34, // labeled data referenced by ld I
31+
0x00, 0xee, // labeled ret target referenced by jp V0
32+
}
33+
34+
output := runChip8Disasm(t, input)
35+
36+
assert.True(t, strings.Contains(output, "ld I, _label_0208"))
37+
assert.True(t, strings.Contains(output, "jp V0, _label_020a"))
38+
assert.False(t, strings.Contains(output, "ld _label_0208"))
39+
assert.False(t, strings.Contains(output, "jp _label_020a"))
40+
}
41+
2242
func TestDisasmZeroDataReference(t *testing.T) {
2343
input := []byte{
2444
0xad, 0x20, 0x80, // lda a:$8020
@@ -524,6 +544,36 @@ func testProgram(t *testing.T, opts options.Disassembler, cart *cartridge.Cartri
524544
return disasm
525545
}
526546

547+
func runChip8Disasm(t *testing.T, input []byte) string {
548+
t.Helper()
549+
550+
opts := options.NewDisassembler(assembler.Retroasm, string(arch.CHIP8System))
551+
opts.OffsetComments = false
552+
opts.HexComments = false
553+
554+
cart := cartridge.New()
555+
cart.PRG = make([]byte, len(input))
556+
copy(cart.PRG, input)
557+
558+
logger := log.NewTestLogger(t)
559+
disasm, err := New(logger, chip8arch.New(), cart, opts, retroasm.New)
560+
assert.NoError(t, err)
561+
562+
var buffer bytes.Buffer
563+
writer := bufio.NewWriter(&buffer)
564+
565+
newBankWriter := func(_ string) (io.WriteCloser, error) {
566+
return nopWriteCloser{writer}, nil
567+
}
568+
569+
app, err := disasm.Process(context.Background(), writer, newBankWriter)
570+
assert.NoError(t, err)
571+
assert.True(t, app != nil, "app should not be nil")
572+
assert.NoError(t, writer.Flush())
573+
574+
return trimStringList(buffer.String())
575+
}
576+
527577
func trimStringList(s string) string {
528578
sl := strings.Split(s, "\n")
529579
for i, s := range sl {

internal/offset/offset.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
package offset
33

44
import (
5+
"fmt"
6+
57
"github.com/retroenv/retrodisasm/internal/instruction"
68
"github.com/retroenv/retrodisasm/internal/program"
79
)
@@ -32,6 +34,20 @@ type BankReference struct {
3234
Index uint16 // index in the bank
3335
}
3436

37+
// FormatBranchReference formats an instruction with a branch destination label.
38+
func FormatBranchReference(offsetInfo *DisasmOffset, label string) string {
39+
if offsetInfo.Opcode == nil {
40+
return fmt.Sprintf("%s %s", offsetInfo.Code, label)
41+
}
42+
43+
instr := offsetInfo.Opcode.Instruction()
44+
if instr == nil || instr.IsNil() {
45+
return fmt.Sprintf("%s %s", offsetInfo.Code, label)
46+
}
47+
48+
return fmt.Sprintf("%s %s", instr.Name(), label)
49+
}
50+
3551
// Mapper provides a mapper manager interface for architecture code.
3652
type Mapper interface {
3753
MappedBank(address uint16) MappedBank

0 commit comments

Comments
 (0)