From 23fbe1cd8abaf8002f7e117daace5264c9925c47 Mon Sep 17 00:00:00 2001 From: cornelk Date: Mon, 23 Dec 2024 00:02:09 -0600 Subject: [PATCH] refactor constants manager to own pkg --- internal/arch/arch.go | 7 +- internal/arch/disasm.go | 11 ++- internal/arch/m6502/const.go | 6 +- internal/arch/m6502/parser.go | 3 +- internal/bank.go | 47 +++++++++++-- internal/const.go | 34 --------- internal/consts/consts.go | 128 ++++++++++++++++++++++++++++++++++ internal/disasm.go | 73 ++++--------------- internal/vars.go | 25 ------- 9 files changed, 201 insertions(+), 133 deletions(-) delete mode 100644 internal/const.go create mode 100644 internal/consts/consts.go diff --git a/internal/arch/arch.go b/internal/arch/arch.go index 32c8f9b..687ed2a 100644 --- a/internal/arch/arch.go +++ b/internal/arch/arch.go @@ -5,7 +5,7 @@ package arch // Architecture contains architecture specific information. type Architecture interface { // Constants returns the constants translation map. - Constants() (map[uint16]ConstTranslation, error) + Constants() (map[uint16]Constant, error) // GetAddressingParam returns the address of the param if it references an address. GetAddressingParam(param any) (uint16, bool) // HandleDisambiguousInstructions translates disambiguous instructions into data bytes as it @@ -28,8 +28,9 @@ type Architecture interface { ReadOpParam(dis Disasm, addressing int, address uint16) (any, []byte, error) } -// ConstTranslation represents a constant translation from a read and write operation to a name. -type ConstTranslation struct { +// Constant represents a constant translation from a read and write operation to a name. +// This is used to replace the parameter of an instruction by a constant name. +type Constant struct { Address uint16 Read string diff --git a/internal/arch/disasm.go b/internal/arch/disasm.go index b2727d9..6f18378 100644 --- a/internal/arch/disasm.go +++ b/internal/arch/disasm.go @@ -21,6 +21,8 @@ type Disasm interface { // ChangeAddressRangeToCodeAsData sets a range of code address to code as // data types. It combines all data bytes that are not split by a label. ChangeAddressRangeToCodeAsData(address uint16, data []byte) + // Constants returns the constants manager. + Constants() ConstantManager // Logger returns the logger. Logger() *log.Logger // OffsetInfo returns the offset information for the given address. @@ -33,9 +35,6 @@ type Disasm interface { ReadMemory(address uint16) (byte, error) // ReadMemoryWord reads a word from the memory at the given address. ReadMemoryWord(address uint16) (uint16, error) - // ReplaceParamByConstant replaces the parameter of an instruction by a constant name - // if the address of the instruction is found in the constants map. - ReplaceParamByConstant(address uint16, opcode Opcode, paramAsString string) (string, bool) // SetCodeBaseAddress sets the code base address. SetCodeBaseAddress(address uint16) // SetHandlers sets the program vector handlers. @@ -44,6 +43,12 @@ type Disasm interface { SetVectorsStartAddress(address uint16) } +type ConstantManager interface { + // ReplaceParameter replaces the parameter of an instruction by a constant name + // if the address of the instruction is found in the constants map. + ReplaceParameter(address uint16, opcode Opcode, paramAsString string) (string, bool) +} + // JumpEngine contains jump engine related helper. type JumpEngine interface { // AddJumpEngine adds a jump engine function address to the list of jump engines. diff --git a/internal/arch/m6502/const.go b/internal/arch/m6502/const.go index f5febbe..36c39db 100644 --- a/internal/arch/m6502/const.go +++ b/internal/arch/m6502/const.go @@ -10,8 +10,8 @@ import ( // Constants builds the map of all known NES constants from all // modules that maps an address to a constant name. -func (ar *Arch6502) Constants() (map[uint16]arch.ConstTranslation, error) { - m := map[uint16]arch.ConstTranslation{} +func (ar *Arch6502) Constants() (map[uint16]arch.Constant, error) { + m := map[uint16]arch.Constant{} if err := mergeConstantsMaps(m, register.APUAddressToName); err != nil { return nil, fmt.Errorf("processing apu constants: %w", err) } @@ -24,7 +24,7 @@ func (ar *Arch6502) Constants() (map[uint16]arch.ConstTranslation, error) { return m, nil } -func mergeConstantsMaps(destination map[uint16]arch.ConstTranslation, source map[uint16]m6502.AccessModeConstant) error { +func mergeConstantsMaps(destination map[uint16]arch.Constant, source map[uint16]m6502.AccessModeConstant) error { for address, constantInfo := range source { translation := destination[address] translation.Address = address diff --git a/internal/arch/m6502/parser.go b/internal/arch/m6502/parser.go index 54d6bf6..72de030 100644 --- a/internal/arch/m6502/parser.go +++ b/internal/arch/m6502/parser.go @@ -113,7 +113,8 @@ func (ar *Arch6502) replaceParamByAlias(dis arch.Disasm, address uint16, opcode } } - changedParamAsString, ok := dis.ReplaceParamByConstant(addressReference, opcode, paramAsString) + consts := dis.Constants() + changedParamAsString, ok := consts.ReplaceParameter(addressReference, opcode, paramAsString) if ok { return changedParamAsString } diff --git a/internal/bank.go b/internal/bank.go index af3181d..f15753c 100644 --- a/internal/bank.go +++ b/internal/bank.go @@ -1,6 +1,10 @@ package disasm -import "github.com/retroenv/nesgodisasm/internal/arch" +import ( + "fmt" + + "github.com/retroenv/nesgodisasm/internal/program" +) const ( singleBankName = "CODE" @@ -10,8 +14,6 @@ const ( type bank struct { prg []byte - constants map[uint16]arch.ConstTranslation - usedConstants map[uint16]arch.ConstTranslation variables map[uint16]*variable usedVariables map[uint16]struct{} @@ -27,10 +29,45 @@ type bankReference struct { func newBank(prg []byte) *bank { return &bank{ prg: prg, - constants: map[uint16]arch.ConstTranslation{}, - usedConstants: map[uint16]arch.ConstTranslation{}, variables: map[uint16]*variable{}, usedVariables: map[uint16]struct{}{}, offsets: make([]offset, len(prg)), } } + +func (dis *Disasm) initializeBanks(prg []byte) { + for i := 0; i < len(prg); { + size := len(prg) - i + if size > 0x8000 { + size = 0x8000 + } + + b := prg[i : i+size] + bnk := newBank(b) + dis.banks = append(dis.banks, bnk) + i += size + + dis.constants.AddBank() + } +} + +func setBankVectors(bnk *bank, prgBank *program.PRGBank) { + idx := len(bnk.prg) - 6 + for i := range 3 { + b1 := bnk.prg[idx] + idx++ + b2 := bnk.prg[idx] + idx++ + addr := uint16(b2)<<8 | uint16(b1) + prgBank.Vectors[i] = addr + } +} + +func setBankName(prgBank *program.PRGBank, bnkIndex, numBanks int) { + if bnkIndex == 0 && numBanks == 1 { + prgBank.Name = singleBankName + return + } + + prgBank.Name = fmt.Sprintf(multiBankNameTemplate, bnkIndex) +} diff --git a/internal/const.go b/internal/const.go deleted file mode 100644 index c291766..0000000 --- a/internal/const.go +++ /dev/null @@ -1,34 +0,0 @@ -package disasm - -import ( - "strings" - - "github.com/retroenv/nesgodisasm/internal/arch" -) - -// ReplaceParamByConstant replaces the parameter of an instruction by a constant name -// if the address of the instruction is found in the constants map. -func (dis *Disasm) ReplaceParamByConstant(address uint16, opcode arch.Opcode, - paramAsString string) (string, bool) { - - constantInfo, ok := dis.constants[address] - if !ok { - return "", false - } - - // split parameter string in case of x/y indexing, only the first part will be replaced by a const name - paramParts := strings.Split(paramAsString, ",") - - if constantInfo.Read != "" && opcode.ReadsMemory() { - dis.usedConstants[address] = constantInfo - paramParts[0] = constantInfo.Read - return strings.Join(paramParts, ","), true - } - if constantInfo.Write != "" && opcode.WritesMemory() { - dis.usedConstants[address] = constantInfo - paramParts[0] = constantInfo.Write - return strings.Join(paramParts, ","), true - } - - return paramAsString, true -} diff --git a/internal/consts/consts.go b/internal/consts/consts.go new file mode 100644 index 0000000..6c854ec --- /dev/null +++ b/internal/consts/consts.go @@ -0,0 +1,128 @@ +// Package consts manages constants in the disassembled program. +package consts + +import ( + "fmt" + "sort" + "strings" + + "github.com/retroenv/nesgodisasm/internal/arch" + "github.com/retroenv/nesgodisasm/internal/program" +) + +var _ arch.ConstantManager = &Consts{} + +// Consts manages constants in the disassembled program. +type Consts struct { + banks []*bank + + constants map[uint16]arch.Constant + usedConstants map[uint16]arch.Constant +} + +type bank struct { + constants map[uint16]arch.Constant + usedConstants map[uint16]arch.Constant +} + +type architecture interface { + Constants() (map[uint16]arch.Constant, error) +} + +// New creates a new constants manager. +func New(ar architecture) (*Consts, error) { + constants, err := ar.Constants() + if err != nil { + return nil, fmt.Errorf("getting constants: %w", err) + } + + return &Consts{ + constants: constants, + usedConstants: make(map[uint16]arch.Constant), + }, nil +} + +// AddBank adds a new bank to the constants manager. +func (c *Consts) AddBank() { + c.banks = append(c.banks, &bank{ + constants: make(map[uint16]arch.Constant), + usedConstants: make(map[uint16]arch.Constant), + }) +} + +// ReplaceParameter replaces the parameter of an instruction by a constant name +// if the address of the instruction is found in the constants map. +func (c *Consts) ReplaceParameter(address uint16, opcode arch.Opcode, paramAsString string) (string, bool) { + constantInfo, ok := c.constants[address] + if !ok { + return "", false + } + + // split parameter string in case of x/y indexing, only the first part will be replaced by a const name + paramParts := strings.Split(paramAsString, ",") + + if constantInfo.Read != "" && opcode.ReadsMemory() { + c.usedConstants[address] = constantInfo + paramParts[0] = constantInfo.Read + return strings.Join(paramParts, ","), true + } + if constantInfo.Write != "" && opcode.WritesMemory() { + c.usedConstants[address] = constantInfo + paramParts[0] = constantInfo.Write + return strings.Join(paramParts, ","), true + } + + return paramAsString, true +} + +// ProcessConstants processes all constants and updates all banks with the used ones. There is currently no tracking +// for in which bank a constant is used, it will be added to all banks for now. +// TODO fix constants to only output in used banks +func (c *Consts) ProcessConstants() { + constants := make([]arch.Constant, 0, len(c.constants)) + for _, translation := range c.constants { + constants = append(constants, translation) + } + sort.Slice(constants, func(i, j int) bool { + return constants[i].Address < constants[j].Address + }) + + for _, constInfo := range constants { + _, used := c.usedConstants[constInfo.Address] + if !used { + continue + } + + for _, bnk := range c.banks { + bnk.constants[constInfo.Address] = constInfo + bnk.usedConstants[constInfo.Address] = constInfo + } + } +} + +// SetProgramConstants sets the used constants in the program for outputting. +func (c *Consts) SetProgramConstants(app *program.Program) { + for address := range c.usedConstants { + constantInfo := c.constants[address] + if constantInfo.Read != "" { + app.Constants[constantInfo.Read] = address + } + if constantInfo.Write != "" { + app.Constants[constantInfo.Write] = address + } + } +} + +// SetBankConstants sets the used constants in the bank for outputting. +func (c *Consts) SetBankConstants(bankID int, prgBank *program.PRGBank) { + bank := c.banks[bankID] + for address := range bank.usedConstants { + constantInfo := bank.constants[address] + if constantInfo.Read != "" { + prgBank.Constants[constantInfo.Read] = address + } + if constantInfo.Write != "" { + prgBank.Constants[constantInfo.Write] = address + } + } +} diff --git a/internal/disasm.go b/internal/disasm.go index 79aaff3..0e749de 100644 --- a/internal/disasm.go +++ b/internal/disasm.go @@ -9,6 +9,7 @@ import ( "github.com/retroenv/nesgodisasm/internal/arch" "github.com/retroenv/nesgodisasm/internal/assembler" + "github.com/retroenv/nesgodisasm/internal/consts" "github.com/retroenv/nesgodisasm/internal/options" "github.com/retroenv/nesgodisasm/internal/program" "github.com/retroenv/nesgodisasm/internal/writer" @@ -37,8 +38,7 @@ type Disasm struct { codeBaseAddress uint16 // codebase address of the cartridge, it is not always 0x8000 vectorsStartAddress uint16 - constants map[uint16]arch.ConstTranslation - usedConstants map[uint16]arch.ConstTranslation + constants *consts.Consts variables map[uint16]*variable usedVariables map[uint16]struct{} @@ -71,7 +71,6 @@ func New(ar arch.Architecture, logger *log.Logger, cart *cartridge.Cartridge, fileWriterConstructor: fileWriterConstructor, variables: map[uint16]*variable{}, usedVariables: map[uint16]struct{}{}, - usedConstants: map[uint16]arch.ConstTranslation{}, jumpEngineCallersAdded: map[uint16]*jumpEngineCaller{}, jumpEngines: map[uint16]struct{}{}, branchDestinations: map[uint16]struct{}{}, @@ -81,9 +80,9 @@ func New(ar arch.Architecture, logger *log.Logger, cart *cartridge.Cartridge, } var err error - dis.constants, err = ar.Constants() + dis.constants, err = consts.New(ar) if err != nil { - return nil, fmt.Errorf("getting constants: %w", err) + return nil, fmt.Errorf("creating constants: %w", err) } dis.initializeBanks(cart.PRG) @@ -114,7 +113,7 @@ func (dis *Disasm) Process(mainWriter io.Writer, newBankWriter assembler.NewBank if err := dis.processVariables(); err != nil { return nil, err } - dis.processConstants() + dis.constants.ProcessConstants() dis.processJumpDestinations() app, err := dis.convertToProgram() @@ -149,20 +148,6 @@ func (dis *Disasm) SetHandlers(handlers program.Handlers) { dis.handlers = handlers } -func (dis *Disasm) initializeBanks(prg []byte) { - for i := 0; i < len(prg); { - size := len(prg) - i - if size > 0x8000 { - size = 0x8000 - } - - b := prg[i : i+size] - bnk := newBank(b) - dis.banks = append(dis.banks, bnk) - i += size - } -} - func (dis *Disasm) SetCodeBaseAddress(address uint16) { dis.codeBaseAddress = address @@ -178,6 +163,11 @@ func (dis *Disasm) Options() options.Disassembler { return dis.options } +// Constants returns the constants manager. +func (dis *Disasm) Constants() arch.ConstantManager { + return dis.constants +} + // converts the internal disassembly representation to a program type that will be used by // the chosen assembler output instance to generate the asm file. func (dis *Disasm) convertToProgram() (*program.Program, error) { @@ -199,15 +189,8 @@ func (dis *Disasm) convertToProgram() (*program.Program, error) { prgBank.Offsets[i] = programOffsetInfo } - for address := range bnk.usedConstants { - constantInfo := bnk.constants[address] - if constantInfo.Read != "" { - prgBank.Constants[constantInfo.Read] = address - } - if constantInfo.Write != "" { - prgBank.Constants[constantInfo.Write] = address - } - } + dis.constants.SetBankConstants(bnkIndex, prgBank) + for address := range bnk.usedVariables { varInfo := bnk.variables[address] prgBank.Variables[varInfo.name] = address @@ -219,15 +202,8 @@ func (dis *Disasm) convertToProgram() (*program.Program, error) { app.PRG = append(app.PRG, prgBank) } - for address := range dis.usedConstants { - constantInfo := dis.constants[address] - if constantInfo.Read != "" { - app.Constants[constantInfo.Read] = address - } - if constantInfo.Write != "" { - app.Constants[constantInfo.Write] = address - } - } + dis.constants.SetProgramConstants(app) + for address := range dis.usedVariables { varInfo := dis.variables[address] app.Variables[varInfo.name] = address @@ -265,27 +241,6 @@ func (dis *Disasm) loadCodeDataLog() error { return nil } -func setBankName(prgBank *program.PRGBank, bnkIndex, numBanks int) { - if bnkIndex == 0 && numBanks == 1 { - prgBank.Name = singleBankName - return - } - - prgBank.Name = fmt.Sprintf(multiBankNameTemplate, bnkIndex) -} - -func setBankVectors(bnk *bank, prgBank *program.PRGBank) { - idx := len(bnk.prg) - 6 - for i := range 3 { - b1 := bnk.prg[idx] - idx++ - b2 := bnk.prg[idx] - idx++ - addr := uint16(b2)<<8 | uint16(b1) - prgBank.Vectors[i] = addr - } -} - func (dis *Disasm) getProgramOffset(address uint16, offsetInfo offset) (program.Offset, error) { programOffset := offsetInfo.Offset programOffset.Address = address diff --git a/internal/vars.go b/internal/vars.go index 40d7864..1faff85 100644 --- a/internal/vars.go +++ b/internal/vars.go @@ -118,31 +118,6 @@ func (dis *Disasm) processVariables() error { return nil } -// processConstants processes all constants and updates all banks with the used ones. There is currently no tracking -// for in which bank a constant is used, it will be added to all banks for now. -// TODO fix constants to only output in used banks -func (dis *Disasm) processConstants() { - constants := make([]arch.ConstTranslation, 0, len(dis.constants)) - for _, translation := range dis.constants { - constants = append(constants, translation) - } - sort.Slice(constants, func(i, j int) bool { - return constants[i].Address < constants[j].Address - }) - - for _, constInfo := range constants { - _, used := dis.usedConstants[constInfo.Address] - if !used { - continue - } - - for _, bnk := range dis.banks { - bnk.constants[constInfo.Address] = constInfo - bnk.usedConstants[constInfo.Address] = constInfo - } - } -} - // getOpcodeStart returns a reference to the opcode start of the given address. // In case it's in the first or second byte of an instruction, referencing the middle of an instruction will be // converted to a reference to the beginning of the instruction and optional address adjustment like +1 or +2.