From 82c089cde49239af0d8fe296e874a4cce065624d Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 26 Feb 2017 21:24:54 -0500 Subject: [PATCH 1/9] Factored out paging type detection and, from that, took out 2k cartridge detection. Added an attempt at a CommaVid detector, which does not currently work. --- StaticAnalyser/Atari/StaticAnalyser.cpp | 96 +++++++++++++++++-------- StaticAnalyser/StaticAnalyser.hpp | 1 + 2 files changed, 69 insertions(+), 28 deletions(-) diff --git a/StaticAnalyser/Atari/StaticAnalyser.cpp b/StaticAnalyser/Atari/StaticAnalyser.cpp index 5e78faa5f..a0fa49ccf 100644 --- a/StaticAnalyser/Atari/StaticAnalyser.cpp +++ b/StaticAnalyser/Atari/StaticAnalyser.cpp @@ -12,6 +12,72 @@ using namespace StaticAnalyser::Atari; +static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const Storage::Cartridge::Cartridge::Segment &segment) +{ + // if this is a 2kb cartridge then it's definitely either unpaged or a CommaVid + uint16_t entry_address, break_address; + + entry_address = (uint16_t)(segment.data[0x7fc] | (segment.data[0x7fd] << 8)); + break_address = (uint16_t)(segment.data[0x7fe] | (segment.data[0x7ff] << 8)); + + // a CommaVid start address needs to be outside of its RAM + if(entry_address < 0x1800 || break_address < 0x1800) return; + + StaticAnalyser::MOS6502::Disassembly disassembly = + StaticAnalyser::MOS6502::Disassemble(segment.data, 0x1800, {entry_address, break_address}, 0x1fff); + std::set all_writes = disassembly.external_stores; + all_writes.insert(disassembly.external_modifies.begin(), disassembly.external_modifies.end()); + + // a CommaVid will use its RAM + if(all_writes.empty()) return; + + bool has_appropriate_writes = false; + for(uint16_t address : all_writes) + { + const uint16_t masked_address = address & 0x1fff; + if(masked_address >= 0x1400 && masked_address < 0x1800) + { + has_appropriate_writes = true; + break; + } + } + + // conclude that this is a CommaVid if it attempted to write something to the CommaVid RAM locations + if(has_appropriate_writes) target.atari.paging_model = StaticAnalyser::Atari2600PagingModel::CommaVid; +} + +static void DeterminePagingForCartridge(StaticAnalyser::Target &target, const Storage::Cartridge::Cartridge::Segment &segment) +{ + if(segment.data.size() == 2048) + { + DeterminePagingFor2kCartridge(target, segment); + return; + } + + uint16_t entry_address, break_address; + + entry_address = (uint16_t)(segment.data[0xffc] | (segment.data[0xffd] << 8)); + break_address = (uint16_t)(segment.data[0xffe] | (segment.data[0xfff] << 8)); + + StaticAnalyser::MOS6502::Disassembly disassembly = + StaticAnalyser::MOS6502::Disassemble(segment.data, 0x1000, {entry_address, break_address}, 0x1fff); + + // check for any sort of on-cartridge RAM; that might imply a Super Chip or else immediately tip the + // hat that this is a CBS RAM+ cartridge + if(!disassembly.internal_stores.empty()) + { + bool writes_above_128 = false; + for(uint16_t address : disassembly.internal_stores) + { + writes_above_128 |= ((address & 0x1fff) > 0x10ff) && ((address & 0x1fff) < 0x1200); + } + if(writes_above_128) + target.atari.paging_model = StaticAnalyser::Atari2600PagingModel::CBSRamPlus; + else + target.atari.uses_superchip = true; + } +} + void StaticAnalyser::Atari::AddTargets( const std::list> &disks, const std::list> &tapes, @@ -33,37 +99,11 @@ void StaticAnalyser::Atari::AddTargets( if(!cartridges.empty()) { const std::list &segments = cartridges.front()->get_segments(); + if(segments.size() == 1) { - uint16_t entry_address, break_address; const Storage::Cartridge::Cartridge::Segment &segment = segments.front(); - if(segment.data.size() < 4096) - { - entry_address = (uint16_t)(segment.data[0x7fc] | (segment.data[0x7fd] << 8)); - break_address = (uint16_t)(segment.data[0x7fe] | (segment.data[0x7ff] << 8)); - } - else - { - entry_address = (uint16_t)(segment.data[0xffc] | (segment.data[0xffd] << 8)); - break_address = (uint16_t)(segment.data[0xffe] | (segment.data[0xfff] << 8)); - } - StaticAnalyser::MOS6502::Disassembly disassembly = - StaticAnalyser::MOS6502::Disassemble(segment.data, 0x1000, {entry_address, break_address}, 0x1fff); - - // check for any sort of on-cartridge RAM; that might imply a Super Chip or else immediately tip the - // hat that this is a CBS RAM+ cartridge - if(!disassembly.internal_stores.empty()) - { - bool writes_above_128 = false; - for(uint16_t address : disassembly.internal_stores) - { - writes_above_128 |= ((address & 0x1fff) > 0x10ff) && ((address & 0x1fff) < 0x1200); - } - if(writes_above_128) - target.atari.paging_model = Atari2600PagingModel::CBSRamPlus; - else - target.atari.uses_superchip = true; - } + DeterminePagingForCartridge(target, segment); } } diff --git a/StaticAnalyser/StaticAnalyser.hpp b/StaticAnalyser/StaticAnalyser.hpp index eebaa1216..b7f739ab0 100644 --- a/StaticAnalyser/StaticAnalyser.hpp +++ b/StaticAnalyser/StaticAnalyser.hpp @@ -27,6 +27,7 @@ enum class Vic20MemoryModel { enum class Atari2600PagingModel { None, + CommaVid, Atari8k, Atari16k, Atari32k, From 0273860018874c14ff427cee9d794eb0956d3642 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 26 Feb 2017 21:58:09 -0500 Subject: [PATCH 2/9] Sought to weed out further false positives on CommaVid, partly by introducing a record of internal calls, to pair with the now confusingly named outward_calls. --- StaticAnalyser/Atari/StaticAnalyser.cpp | 20 +++++++++++++++++-- .../Disassembler/Disassembler6502.cpp | 1 + .../Disassembler/Disassembler6502.hpp | 1 + 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/StaticAnalyser/Atari/StaticAnalyser.cpp b/StaticAnalyser/Atari/StaticAnalyser.cpp index a0fa49ccf..03cc91ff8 100644 --- a/StaticAnalyser/Atari/StaticAnalyser.cpp +++ b/StaticAnalyser/Atari/StaticAnalyser.cpp @@ -17,14 +17,30 @@ static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const // if this is a 2kb cartridge then it's definitely either unpaged or a CommaVid uint16_t entry_address, break_address; - entry_address = (uint16_t)(segment.data[0x7fc] | (segment.data[0x7fd] << 8)); - break_address = (uint16_t)(segment.data[0x7fe] | (segment.data[0x7ff] << 8)); + entry_address = ((uint16_t)(segment.data[0x7fc] | (segment.data[0x7fd] << 8))) & 0x1fff; + break_address = ((uint16_t)(segment.data[0x7fe] | (segment.data[0x7ff] << 8))) & 0x1fff; // a CommaVid start address needs to be outside of its RAM if(entry_address < 0x1800 || break_address < 0x1800) return; StaticAnalyser::MOS6502::Disassembly disassembly = StaticAnalyser::MOS6502::Disassemble(segment.data, 0x1800, {entry_address, break_address}, 0x1fff); +// StaticAnalyser::MOS6502::Disassembly standard_disassembly = +// StaticAnalyser::MOS6502::Disassemble(segment.data, 0x1000, {entry_address, break_address}, 0x1fff); + + // if there are no subroutines in the top 2kb of memory then this isn't a CommaVid + bool has_subroutine_call = false; + for(uint16_t address : disassembly.internal_calls) + { + const uint16_t masked_address = address & 0x1fff; + if(masked_address >= 0x1800) + { + has_subroutine_call = true; + break; + } + } + if(!has_subroutine_call) return; + std::set all_writes = disassembly.external_stores; all_writes.insert(disassembly.external_modifies.begin(), disassembly.external_modifies.end()); diff --git a/StaticAnalyser/Disassembler/Disassembler6502.cpp b/StaticAnalyser/Disassembler/Disassembler6502.cpp index 5a1a66bed..9c3066769 100644 --- a/StaticAnalyser/Disassembler/Disassembler6502.cpp +++ b/StaticAnalyser/Disassembler/Disassembler6502.cpp @@ -18,6 +18,7 @@ struct PartialDisassembly { static void AddToDisassembly(PartialDisassembly &disassembly, const std::vector &memory, uint16_t start_address, uint16_t entry_point, uint16_t address_mask) { + disassembly.disassembly.internal_calls.insert(start_address); uint16_t address = entry_point & address_mask; while(1) { diff --git a/StaticAnalyser/Disassembler/Disassembler6502.hpp b/StaticAnalyser/Disassembler/Disassembler6502.hpp index 2204b095b..e973d110c 100644 --- a/StaticAnalyser/Disassembler/Disassembler6502.hpp +++ b/StaticAnalyser/Disassembler/Disassembler6502.hpp @@ -64,6 +64,7 @@ struct Instruction { struct Disassembly { std::map instructions_by_address; std::set outward_calls; + std::set internal_calls; std::set external_stores, external_loads, external_modifies; std::set internal_stores, internal_loads, internal_modifies; }; From b24cd00a39851db6100a94569dbc5dde168ac846 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 26 Feb 2017 21:58:43 -0500 Subject: [PATCH 3/9] Switched time of best-effort updater delegate setting, to avoid a callback before setupClockRate has happened, and therefore before it's clear what should be going on with audio. --- OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift b/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift index 995e35027..38a0f312e 100644 --- a/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift +++ b/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift @@ -56,7 +56,6 @@ class MachineDocument: self.machine.delegate = self self.bestEffortUpdater = CSBestEffortUpdater() - self.bestEffortUpdater.delegate = self // callbacks from the OpenGL may come on a different thread, immediately following the .delegate set; // hence the full setup of the best-effort updater prior to setting self as a delegate @@ -68,6 +67,9 @@ class MachineDocument: // bring OpenGL view-holding window on top of the options panel self.openGLView.window!.makeKeyAndOrderFront(self) + + // start accepting best effort updates + self.bestEffortUpdater.delegate = self } func machineDidChangeClockRate(_ machine: CSMachine!) { From 837216ee9a7e111cc73e4708a5aec1368ee86f56 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Feb 2017 07:49:33 -0500 Subject: [PATCH 4/9] Adjusted semantics to allow for more complicated mappings, e.g. whereby supplied data repeats itself within a range. --- .../Disassembler/Disassembler6502.cpp | 44 ++++++++++++------- .../Disassembler/Disassembler6502.hpp | 6 ++- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/StaticAnalyser/Disassembler/Disassembler6502.cpp b/StaticAnalyser/Disassembler/Disassembler6502.cpp index 9c3066769..6b6a2aed1 100644 --- a/StaticAnalyser/Disassembler/Disassembler6502.cpp +++ b/StaticAnalyser/Disassembler/Disassembler6502.cpp @@ -16,18 +16,18 @@ struct PartialDisassembly { std::vector remaining_entry_points; }; -static void AddToDisassembly(PartialDisassembly &disassembly, const std::vector &memory, uint16_t start_address, uint16_t entry_point, uint16_t address_mask) +static void AddToDisassembly(PartialDisassembly &disassembly, const std::vector &memory, const std::function &address_mapper, uint16_t entry_point) { - disassembly.disassembly.internal_calls.insert(start_address); - uint16_t address = entry_point & address_mask; + disassembly.disassembly.internal_calls.insert(entry_point); + uint16_t address = entry_point; while(1) { - uint16_t local_address = address - start_address; + size_t local_address = address_mapper(address); if(local_address >= memory.size()) return; struct Instruction instruction; instruction.address = address; - address = (address + 1) & address_mask; + address++; // get operation uint8_t operation = memory[local_address]; @@ -235,7 +235,7 @@ static void AddToDisassembly(PartialDisassembly &disassembly, const std::vector< case Instruction::IndexedIndirectX: case Instruction::IndirectIndexedY: case Instruction::Relative: { - uint16_t operand_address = address - start_address; + size_t operand_address = address_mapper(address); if(operand_address >= memory.size()) return; address++; @@ -247,11 +247,12 @@ static void AddToDisassembly(PartialDisassembly &disassembly, const std::vector< case Instruction::Absolute: case Instruction::AbsoluteX: case Instruction::AbsoluteY: case Instruction::Indirect: { - uint16_t operand_address = address - start_address; - if(operand_address >= memory.size()-1) return; + size_t low_operand_address = address_mapper(address); + size_t high_operand_address = address_mapper(address + 1); + if(low_operand_address >= memory.size() || high_operand_address >= memory.size()) return; address += 2; - instruction.operand = memory[operand_address] | (uint16_t)(memory[operand_address + 1] << 8); + instruction.operand = memory[low_operand_address] | (uint16_t)(memory[high_operand_address] << 8); } break; } @@ -262,7 +263,8 @@ static void AddToDisassembly(PartialDisassembly &disassembly, const std::vector< // TODO: something wider-ranging than this if(instruction.addressing_mode == Instruction::Absolute || instruction.addressing_mode == Instruction::ZeroPage) { - bool is_external = (instruction.operand&address_mask) < start_address || (instruction.operand&address_mask) >= start_address + memory.size(); + size_t mapped_address = address_mapper(instruction.operand); + bool is_external = mapped_address >= memory.size(); switch(instruction.operation) { @@ -297,23 +299,23 @@ static void AddToDisassembly(PartialDisassembly &disassembly, const std::vector< if(instruction.operation == Instruction::BRK) return; // TODO: check whether IRQ vector is within memory range if(instruction.operation == Instruction::JSR) { - disassembly.remaining_entry_points.push_back(instruction.operand & address_mask); + disassembly.remaining_entry_points.push_back(instruction.operand); } if(instruction.operation == Instruction::JMP) { if(instruction.addressing_mode == Instruction::Absolute) - disassembly.remaining_entry_points.push_back(instruction.operand & address_mask); + disassembly.remaining_entry_points.push_back(instruction.operand); return; } if(instruction.addressing_mode == Instruction::Relative) { uint16_t destination = (uint16_t)(address + (int8_t)instruction.operand); - disassembly.remaining_entry_points.push_back(destination & address_mask); + disassembly.remaining_entry_points.push_back(destination); } } } -Disassembly StaticAnalyser::MOS6502::Disassemble(const std::vector &memory, uint16_t start_address, std::vector entry_points, uint16_t address_mask) +Disassembly StaticAnalyser::MOS6502::Disassemble(const std::vector &memory, const std::function &address_mapper, std::vector entry_points) { PartialDisassembly partialDisassembly; partialDisassembly.remaining_entry_points = entry_points; @@ -321,18 +323,26 @@ Disassembly StaticAnalyser::MOS6502::Disassemble(const std::vector &mem while(!partialDisassembly.remaining_entry_points.empty()) { // pull the next entry point from the back of the vector - uint16_t next_entry_point = partialDisassembly.remaining_entry_points.back() & address_mask; + uint16_t next_entry_point = partialDisassembly.remaining_entry_points.back(); partialDisassembly.remaining_entry_points.pop_back(); // if that address has already bene visited, forget about it if(partialDisassembly.disassembly.instructions_by_address.find(next_entry_point) != partialDisassembly.disassembly.instructions_by_address.end()) continue; // if it's outgoing, log it as such and forget about it; otherwise disassemble - if(next_entry_point < start_address || next_entry_point >= start_address + memory.size()) + size_t mapped_entry_point = address_mapper(next_entry_point); + if(mapped_entry_point >= memory.size()) partialDisassembly.disassembly.outward_calls.insert(next_entry_point); else - AddToDisassembly(partialDisassembly, memory, start_address, next_entry_point, address_mask); + AddToDisassembly(partialDisassembly, memory, address_mapper, next_entry_point); } return std::move(partialDisassembly.disassembly); } + +std::function StaticAnalyser::MOS6502::OffsetMapper(uint16_t start_address) +{ + return [start_address](uint16_t argument) { + return (size_t)(argument - start_address); + }; +} diff --git a/StaticAnalyser/Disassembler/Disassembler6502.hpp b/StaticAnalyser/Disassembler/Disassembler6502.hpp index e973d110c..5026ededb 100644 --- a/StaticAnalyser/Disassembler/Disassembler6502.hpp +++ b/StaticAnalyser/Disassembler/Disassembler6502.hpp @@ -10,10 +10,11 @@ #define Disassembler6502_hpp #include +#include +#include #include #include #include -#include namespace StaticAnalyser { namespace MOS6502 { @@ -69,7 +70,8 @@ struct Disassembly { std::set internal_stores, internal_loads, internal_modifies; }; -Disassembly Disassemble(const std::vector &memory, uint16_t start_address, std::vector entry_points, uint16_t address_mask = 0xffff); +Disassembly Disassemble(const std::vector &memory, const std::function &address_mapper, std::vector entry_points); +std::function OffsetMapper(uint16_t start_address); } } From a3d339092ef148e6b73166310f472847fc663f6b Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Feb 2017 07:56:59 -0500 Subject: [PATCH 5/9] Corrected callers of the 6502 disassembler. --- StaticAnalyser/Atari/StaticAnalyser.cpp | 29 ++++++++++++++++++------- StaticAnalyser/Oric/StaticAnalyser.cpp | 2 +- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/StaticAnalyser/Atari/StaticAnalyser.cpp b/StaticAnalyser/Atari/StaticAnalyser.cpp index 03cc91ff8..1c97ca06e 100644 --- a/StaticAnalyser/Atari/StaticAnalyser.cpp +++ b/StaticAnalyser/Atari/StaticAnalyser.cpp @@ -23,14 +23,23 @@ static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const // a CommaVid start address needs to be outside of its RAM if(entry_address < 0x1800 || break_address < 0x1800) return; - StaticAnalyser::MOS6502::Disassembly disassembly = - StaticAnalyser::MOS6502::Disassemble(segment.data, 0x1800, {entry_address, break_address}, 0x1fff); -// StaticAnalyser::MOS6502::Disassembly standard_disassembly = -// StaticAnalyser::MOS6502::Disassemble(segment.data, 0x1000, {entry_address, break_address}, 0x1fff); + std::function high_location_mapper = [](uint16_t address) { + address &= 0x1fff; + return (size_t)(address - 0x1800); + }; + std::function full_range_mapper = [](uint16_t address) { + if(!(address & 0x1000)) return (size_t)-1; + return (size_t)(address & 0x7ff); + }; + + StaticAnalyser::MOS6502::Disassembly high_location_disassembly = + StaticAnalyser::MOS6502::Disassemble(segment.data, high_location_mapper, {entry_address, break_address}); + StaticAnalyser::MOS6502::Disassembly full_range_disassembly = + StaticAnalyser::MOS6502::Disassemble(segment.data, full_range_mapper, {entry_address, break_address}); // if there are no subroutines in the top 2kb of memory then this isn't a CommaVid bool has_subroutine_call = false; - for(uint16_t address : disassembly.internal_calls) + for(uint16_t address : high_location_disassembly.internal_calls) { const uint16_t masked_address = address & 0x1fff; if(masked_address >= 0x1800) @@ -41,8 +50,8 @@ static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const } if(!has_subroutine_call) return; - std::set all_writes = disassembly.external_stores; - all_writes.insert(disassembly.external_modifies.begin(), disassembly.external_modifies.end()); + std::set all_writes = high_location_disassembly.external_stores; + all_writes.insert(high_location_disassembly.external_modifies.begin(), high_location_disassembly.external_modifies.end()); // a CommaVid will use its RAM if(all_writes.empty()) return; @@ -75,8 +84,12 @@ static void DeterminePagingForCartridge(StaticAnalyser::Target &target, const St entry_address = (uint16_t)(segment.data[0xffc] | (segment.data[0xffd] << 8)); break_address = (uint16_t)(segment.data[0xffe] | (segment.data[0xfff] << 8)); + std::function address_mapper = [](uint16_t address) { + if(!(address & 0x1000)) return (size_t)-1; + return (size_t)(address & 0xfff); + }; StaticAnalyser::MOS6502::Disassembly disassembly = - StaticAnalyser::MOS6502::Disassemble(segment.data, 0x1000, {entry_address, break_address}, 0x1fff); + StaticAnalyser::MOS6502::Disassemble(segment.data, address_mapper, {entry_address, break_address}); // check for any sort of on-cartridge RAM; that might imply a Super Chip or else immediately tip the // hat that this is a CBS RAM+ cartridge diff --git a/StaticAnalyser/Oric/StaticAnalyser.cpp b/StaticAnalyser/Oric/StaticAnalyser.cpp index bbef75198..7ad354b41 100644 --- a/StaticAnalyser/Oric/StaticAnalyser.cpp +++ b/StaticAnalyser/Oric/StaticAnalyser.cpp @@ -99,7 +99,7 @@ void StaticAnalyser::Oric::AddTargets( { std::vector entry_points = {file.starting_address}; StaticAnalyser::MOS6502::Disassembly disassembly = - StaticAnalyser::MOS6502::Disassemble(file.data, file.starting_address, entry_points); + StaticAnalyser::MOS6502::Disassemble(file.data, StaticAnalyser::MOS6502::OffsetMapper(file.starting_address), entry_points); int basic10_score = Basic10Score(disassembly); int basic11_score = Basic11Score(disassembly); From fe07cd0248851a49d7966e4eeec1edadcb083ca4 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Feb 2017 08:06:57 -0500 Subject: [PATCH 6/9] Made another attempt to distinguish. --- StaticAnalyser/Atari/StaticAnalyser.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/StaticAnalyser/Atari/StaticAnalyser.cpp b/StaticAnalyser/Atari/StaticAnalyser.cpp index 1c97ca06e..5716bcff3 100644 --- a/StaticAnalyser/Atari/StaticAnalyser.cpp +++ b/StaticAnalyser/Atari/StaticAnalyser.cpp @@ -38,17 +38,17 @@ static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const StaticAnalyser::MOS6502::Disassemble(segment.data, full_range_mapper, {entry_address, break_address}); // if there are no subroutines in the top 2kb of memory then this isn't a CommaVid - bool has_subroutine_call = false; + bool has_appropriate_subroutine_calls = false; + bool has_inappropriate_subroutine_calls = false; for(uint16_t address : high_location_disassembly.internal_calls) { const uint16_t masked_address = address & 0x1fff; - if(masked_address >= 0x1800) - { - has_subroutine_call = true; - break; - } + has_appropriate_subroutine_calls |= (masked_address >= 0x1800); + has_inappropriate_subroutine_calls |= (masked_address < 0x1800); } - if(!has_subroutine_call) return; + + // assumption here: a CommaVid will never branch into RAM. Possibly unsafe: if it won't then what's the RAM for? + if(!has_appropriate_subroutine_calls || has_inappropriate_subroutine_calls) return; std::set all_writes = high_location_disassembly.external_stores; all_writes.insert(high_location_disassembly.external_modifies.begin(), high_location_disassembly.external_modifies.end()); @@ -68,7 +68,11 @@ static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const } // conclude that this is a CommaVid if it attempted to write something to the CommaVid RAM locations - if(has_appropriate_writes) target.atari.paging_model = StaticAnalyser::Atari2600PagingModel::CommaVid; + if(has_appropriate_writes) + { + target.atari.paging_model = StaticAnalyser::Atari2600PagingModel::CommaVid; + return; + } } static void DeterminePagingForCartridge(StaticAnalyser::Target &target, const Storage::Cartridge::Cartridge::Segment &segment) From 1af415a88edd08519967c3820f1f320ae878ece2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Feb 2017 08:39:53 -0500 Subject: [PATCH 7/9] Okay, this is a bit desperate, but worth investigation. --- StaticAnalyser/Atari/StaticAnalyser.cpp | 30 ++++++++++++++++++++----- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/StaticAnalyser/Atari/StaticAnalyser.cpp b/StaticAnalyser/Atari/StaticAnalyser.cpp index 5716bcff3..a9fd4787d 100644 --- a/StaticAnalyser/Atari/StaticAnalyser.cpp +++ b/StaticAnalyser/Atari/StaticAnalyser.cpp @@ -34,8 +34,8 @@ static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const StaticAnalyser::MOS6502::Disassembly high_location_disassembly = StaticAnalyser::MOS6502::Disassemble(segment.data, high_location_mapper, {entry_address, break_address}); - StaticAnalyser::MOS6502::Disassembly full_range_disassembly = - StaticAnalyser::MOS6502::Disassemble(segment.data, full_range_mapper, {entry_address, break_address}); +// StaticAnalyser::MOS6502::Disassembly full_range_disassembly = +// StaticAnalyser::MOS6502::Disassemble(segment.data, full_range_mapper, {entry_address, break_address}); // if there are no subroutines in the top 2kb of memory then this isn't a CommaVid bool has_appropriate_subroutine_calls = false; @@ -56,19 +56,37 @@ static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const // a CommaVid will use its RAM if(all_writes.empty()) return; - bool has_appropriate_writes = false; + bool has_appropriate_accesses = false; for(uint16_t address : all_writes) { const uint16_t masked_address = address & 0x1fff; if(masked_address >= 0x1400 && masked_address < 0x1800) { - has_appropriate_writes = true; + has_appropriate_accesses = true; break; } } - // conclude that this is a CommaVid if it attempted to write something to the CommaVid RAM locations - if(has_appropriate_writes) + // in desperation, accept any kind of store that looks likely to be intended for large amounts of memory + bool has_wide_area_store = false; + if(!has_appropriate_accesses) + { + for(std::map::value_type &entry : high_location_disassembly.instructions_by_address) + { + if(entry.second.operation == StaticAnalyser::MOS6502::Instruction::STA) + { + has_wide_area_store |= entry.second.addressing_mode == StaticAnalyser::MOS6502::Instruction::Indirect; + has_wide_area_store |= entry.second.addressing_mode == StaticAnalyser::MOS6502::Instruction::IndexedIndirectX; + has_wide_area_store |= entry.second.addressing_mode == StaticAnalyser::MOS6502::Instruction::IndirectIndexedY; + } + } + } + + // conclude that this is a CommaVid if it attempted to write something to the CommaVid RAM locations; + // caveat: false positives aren't likely to be problematic; a false positive is a 2KB ROM that always addresses + // itself so as to land in ROM even if mapped as a CommaVid and this code is on the fence as to whether it + // attempts to modify itself but it probably doesn't + if(has_appropriate_accesses || has_wide_area_store) { target.atari.paging_model = StaticAnalyser::Atari2600PagingModel::CommaVid; return; From 8f8b103224b498f51949598339175ff93953d52e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Feb 2017 08:42:43 -0500 Subject: [PATCH 8/9] Slightly tidier. Also in the interim: confirmed no remaining false positives or negatives from the existing published set. --- StaticAnalyser/Atari/StaticAnalyser.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/StaticAnalyser/Atari/StaticAnalyser.cpp b/StaticAnalyser/Atari/StaticAnalyser.cpp index a9fd4787d..d156e43b8 100644 --- a/StaticAnalyser/Atari/StaticAnalyser.cpp +++ b/StaticAnalyser/Atari/StaticAnalyser.cpp @@ -87,10 +87,7 @@ static void DeterminePagingFor2kCartridge(StaticAnalyser::Target &target, const // itself so as to land in ROM even if mapped as a CommaVid and this code is on the fence as to whether it // attempts to modify itself but it probably doesn't if(has_appropriate_accesses || has_wide_area_store) - { target.atari.paging_model = StaticAnalyser::Atari2600PagingModel::CommaVid; - return; - } } static void DeterminePagingForCartridge(StaticAnalyser::Target &target, const Storage::Cartridge::Cartridge::Segment &segment) From 184c8ae707af84bf22516cea3b9cfe1cc3b3ae99 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Feb 2017 20:50:59 -0500 Subject: [PATCH 9/9] Extended to emulate the CommaVid. --- Machines/Atari2600/Atari2600.cpp | 41 ++++++++++++++++++++++++-------- Machines/Atari2600/Atari2600.hpp | 5 ++-- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/Machines/Atari2600/Atari2600.cpp b/Machines/Atari2600/Atari2600.cpp index 054460596..2985e485d 100644 --- a/Machines/Atari2600/Atari2600.cpp +++ b/Machines/Atari2600/Atari2600.cpp @@ -90,17 +90,20 @@ unsigned int Machine::perform_bus_operation(CPU6502::BusOperation operation, uin uint16_t masked_address = address & 0x1fff; if(address&0x1000) { - if(isReadOperation(operation) && (!uses_superchip_ || masked_address > 0x10ff)) { - returnValue &= rom_pages_[(address >> 10)&3][address&1023]; + // check for a RAM access + bool was_ram_access = false; + if(has_ram_) { + if(masked_address >= ram_write_start_ && masked_address < ram_.size() + ram_write_start_) { + ram_[masked_address & ram_.size() - 1] = *value; + was_ram_access = true; + } else if(masked_address >= ram_read_start_ && masked_address < ram_.size() + ram_read_start_) { + returnValue &= ram_[masked_address & ram_.size() - 1]; + was_ram_access = true; + } } - // check for a Super Chip RAM access - if(uses_superchip_ && masked_address < 0x1100) { - if(masked_address < 0x1080) { - superchip_ram_[masked_address & 0x7f] = *value; - } else { - returnValue &= superchip_ram_[masked_address & 0x7f]; - } + if(isReadOperation(operation) && !was_ram_access) { + returnValue &= rom_pages_[(address >> 10)&3][address&1023]; } } @@ -277,7 +280,25 @@ void Machine::configure_as_target(const StaticAnalyser::Target &target) rom_pages_[2] = &rom_[2048 & romMask]; rom_pages_[3] = &rom_[3072 & romMask]; - uses_superchip_ = target.atari.uses_superchip; + switch(target.atari.paging_model) + { + default: + if(target.atari.uses_superchip) + { + ram_.resize(128); + has_ram_ = true; + ram_write_start_ = 0x1000; + ram_read_start_ = 0x1080; + } + break; + + case StaticAnalyser::Atari2600PagingModel::CommaVid: + ram_.resize(1024); + has_ram_ = true; + ram_write_start_ = 0x1400; + ram_read_start_ = 0x1000; + break; + } } #pragma mark - Audio and Video diff --git a/Machines/Atari2600/Atari2600.hpp b/Machines/Atari2600/Atari2600.hpp index c0ec017f1..68c45e97b 100644 --- a/Machines/Atari2600/Atari2600.hpp +++ b/Machines/Atari2600/Atari2600.hpp @@ -61,8 +61,9 @@ class Machine: size_t rom_size_; // cartridge RAM expansion store - uint8_t superchip_ram_[128]; - bool uses_superchip_; + std::vector ram_; + uint16_t ram_write_start_, ram_read_start_; + bool has_ram_; // the RIOT and TIA PIA mos6532_;