From de31dfcf585a4eb4d4eb7565f2c61b831188ce52 Mon Sep 17 00:00:00 2001 From: AJR Date: Sat, 19 Dec 2015 18:22:19 -0500 Subject: [PATCH] Refactoring memory map validity checking --- src/emu/addrmap.cpp | 153 +++++++++++++++++++++++++++++++++++++++++- src/emu/addrmap.h | 7 ++ src/emu/devdelegate.h | 5 +- src/emu/dimemory.cpp | 129 ++--------------------------------- 4 files changed, 168 insertions(+), 126 deletions(-) diff --git a/src/emu/addrmap.cpp b/src/emu/addrmap.cpp index 708caf5d3ee..2ae44142ac1 100644 --- a/src/emu/addrmap.cpp +++ b/src/emu/addrmap.cpp @@ -9,6 +9,15 @@ ***************************************************************************/ #include "emu.h" +#include "validity.h" + + +//************************************************************************** +// PARAMETERS +//************************************************************************** + +#define DETECT_OVERLAPPING_MEMORY (0) + //************************************************************************** @@ -332,7 +341,7 @@ address_map::address_map(device_t &device, address_spacenum spacenum) // and then the configuration for the current address space const address_space_config *spaceconfig = memintf->space_config(spacenum); - if (!device.interface(memintf)) + if (spaceconfig == nullptr) throw emu_fatalerror("No memory address space configuration found for device '%s', space %d\n", device.tag(), spacenum); // construct the internal device map (first so it takes priority) @@ -614,3 +623,145 @@ void address_map::uplift_submaps(running_machine &machine, device_t &device, dev } } } + + +//------------------------------------------------- +// map_validity_check - perform validity checks on +// one of the device's address maps +//------------------------------------------------- + +void address_map::map_validity_check(validity_checker &valid, const device_t &device, address_spacenum spacenum) const +{ + // it's safe to assume here that the device has a memory interface and a config for this space + const address_space_config &spaceconfig = *device.memory().space_config(spacenum); + int datawidth = spaceconfig.m_databus_width; + int alignunit = datawidth / 8; + + bool detected_overlap = DETECT_OVERLAPPING_MEMORY ? false : true; + + // if this is an empty map, just ignore it + if (m_entrylist.first() == nullptr) + return; + + // validate the global map parameters + if (m_spacenum != spacenum) + osd_printf_error("Space %d has address space %d handlers!\n", spacenum, m_spacenum); + if (m_databits != datawidth) + osd_printf_error("Wrong memory handlers provided for %s space! (width = %d, memory = %08x)\n", spaceconfig.m_name, datawidth, m_databits); + + // loop over entries and look for errors + for (address_map_entry *entry = m_entrylist.first(); entry != nullptr; entry = entry->next()) + { + UINT32 bytestart = spaceconfig.addr2byte(entry->m_addrstart); + UINT32 byteend = spaceconfig.addr2byte_end(entry->m_addrend); + + // look for overlapping entries + if (!detected_overlap) + { + for (address_map_entry *scan = m_entrylist.first(); scan != entry; scan = scan->next()) + if (entry->m_addrstart <= scan->m_addrend && entry->m_addrend >= scan->m_addrstart && + ((entry->m_read.m_type != AMH_NONE && scan->m_read.m_type != AMH_NONE) || + (entry->m_write.m_type != AMH_NONE && scan->m_write.m_type != AMH_NONE))) + { + osd_printf_warning("%s space has overlapping memory (%X-%X,%d,%d) vs (%X-%X,%d,%d)\n", spaceconfig.m_name, entry->m_addrstart, entry->m_addrend, entry->m_read.m_type, entry->m_write.m_type, scan->m_addrstart, scan->m_addrend, scan->m_read.m_type, scan->m_write.m_type); + detected_overlap = true; + break; + } + } + + // look for inverted start/end pairs + if (byteend < bytestart) + osd_printf_error("Wrong %s memory read handler start = %08x > end = %08x\n", spaceconfig.m_name, entry->m_addrstart, entry->m_addrend); + + // look for misaligned entries + if ((bytestart & (alignunit - 1)) != 0 || (byteend & (alignunit - 1)) != (alignunit - 1)) + osd_printf_error("Wrong %s memory read handler start = %08x, end = %08x ALIGN = %d\n", spaceconfig.m_name, entry->m_addrstart, entry->m_addrend, alignunit); + + // if this is a program space, auto-assign implicit ROM entries + if (entry->m_read.m_type == AMH_ROM && entry->m_region == nullptr) + { + entry->m_region = device.tag(); + entry->m_rgnoffs = entry->m_addrstart; + } + + // if this entry references a memory region, validate it + if (entry->m_region != nullptr && entry->m_share == nullptr) + { + // make sure we can resolve the full path to the region + bool found = false; + std::string entry_region = entry->m_devbase.subtag(entry->m_region); + + // look for the region + device_iterator deviter(device.mconfig().root_device()); + for (device_t *dev = deviter.first(); dev != nullptr; dev = deviter.next()) + for (const rom_entry *romp = rom_first_region(*dev); romp != nullptr && !found; romp = rom_next_region(romp)) + { + if (rom_region_name(*dev, romp) == entry_region) + { + // verify the address range is within the region's bounds + offs_t length = ROMREGION_GETLENGTH(romp); + if (entry->m_rgnoffs + (byteend - bytestart + 1) > length) + osd_printf_error("%s space memory map entry %X-%X extends beyond region '%s' size (%X)\n", spaceconfig.m_name, entry->m_addrstart, entry->m_addrend, entry->m_region, length); + found = true; + } + } + + // error if not found + if (!found) + osd_printf_error("%s space memory map entry %X-%X references non-existant region '%s'\n", spaceconfig.m_name, entry->m_addrstart, entry->m_addrend, entry->m_region); + } + + // make sure all devices exist + if (entry->m_read.m_type == AMH_DEVICE_DELEGATE) + { + // extract the device tag from the proto-delegate + const char *devtag = nullptr; + switch (entry->m_read.m_bits) + { + case 8: devtag = entry->m_rproto8.device_name(); break; + case 16: devtag = entry->m_rproto16.device_name(); break; + case 32: devtag = entry->m_rproto32.device_name(); break; + case 64: devtag = entry->m_rproto64.device_name(); break; + } + if (entry->m_devbase.subdevice(devtag) == nullptr) + osd_printf_error("%s space memory map entry reads from nonexistant device '%s'\n", spaceconfig.m_name, + devtag != nullptr ? devtag : ""); + } + if (entry->m_write.m_type == AMH_DEVICE_DELEGATE) + { + // extract the device tag from the proto-delegate + const char *devtag = nullptr; + switch (entry->m_write.m_bits) + { + case 8: devtag = entry->m_wproto8.device_name(); break; + case 16: devtag = entry->m_wproto16.device_name(); break; + case 32: devtag = entry->m_wproto32.device_name(); break; + case 64: devtag = entry->m_wproto64.device_name(); break; + } + if (entry->m_devbase.subdevice(devtag) == nullptr) + osd_printf_error("%s space memory map entry writes to nonexistant device '%s'\n", spaceconfig.m_name, + devtag != nullptr ? devtag : ""); + } + if (entry->m_setoffsethd.m_type == AMH_DEVICE_DELEGATE) + { + // extract the device tag from the proto-delegate + const char *devtag = entry->m_soproto.device_name(); + if (entry->m_devbase.subdevice(devtag) == nullptr) + osd_printf_error("%s space memory map entry references nonexistant device '%s'\n", spaceconfig.m_name, + devtag != nullptr ? devtag : ""); + } + + // make sure ports exist +// if ((entry->m_read.m_type == AMH_PORT && entry->m_read.m_tag != NULL && portlist.find(entry->m_read.m_tag) == NULL) || +// (entry->m_write.m_type == AMH_PORT && entry->m_write.m_tag != NULL && portlist.find(entry->m_write.m_tag) == NULL)) +// osd_printf_error("%s space memory map entry references nonexistant port tag '%s'\n", spaceconfig.m_name, entry->m_read.m_tag); + + // validate bank and share tags + if (entry->m_read.m_type == AMH_BANK) + valid.validate_tag(entry->m_read.m_tag); + if (entry->m_write.m_type == AMH_BANK) + valid.validate_tag(entry->m_write.m_tag); + if (entry->m_share != nullptr) + valid.validate_tag(entry->m_share); + } +} diff --git a/src/emu/addrmap.h b/src/emu/addrmap.h index 0e767570b05..492ad21b03b 100644 --- a/src/emu/addrmap.h +++ b/src/emu/addrmap.h @@ -43,6 +43,10 @@ enum map_handler_type // TYPE DEFINITIONS //************************************************************************** +// forward declarations +class validity_checker; + + // address map handler data class map_handler_data { @@ -289,6 +293,7 @@ public: simple_list m_entrylist; // list of entries void uplift_submaps(running_machine &machine, device_t &device, device_t &owner, endianness_t endian); + void map_validity_check(validity_checker &valid, const device_t &device, address_spacenum spacenum) const; }; @@ -309,6 +314,7 @@ void ADDRESS_MAP_NAME(_name)(address_map &map, device_t &device) \ typedef write##_bits##_delegate write_delegate ATTR_UNUSED; \ address_map_entry##_bits *curentry = NULL; \ (void)curentry; \ + assert(&device != nullptr); \ map.configure(_space, _bits); \ typedef _class drivdata_class ATTR_UNUSED; #define DEVICE_ADDRESS_MAP_START(_name, _bits, _class) \ @@ -318,6 +324,7 @@ void _class :: _name(::address_map &map, device_t &device) \ typedef write##_bits##_delegate write_delegate ATTR_UNUSED; \ address_map_entry##_bits *curentry = NULL; \ (void)curentry; \ + assert(&device != nullptr); \ map.configure(AS_PROGRAM, _bits); \ typedef _class drivdata_class ATTR_UNUSED; #define ADDRESS_MAP_END \ diff --git a/src/emu/devdelegate.h b/src/emu/devdelegate.h index 531bea5bd44..b688cd31261 100644 --- a/src/emu/devdelegate.h +++ b/src/emu/devdelegate.h @@ -74,7 +74,10 @@ public: device_delegate(const thistype &src, device_t &search_root) : basetype(src), device_delegate_helper(src.m_device_name) { bind_relative_to(search_root); } // perform the binding - void bind_relative_to(device_t &search_root) { if (!basetype::isnull()) basetype::late_bind(bound_object(search_root)); } + void bind_relative_to(device_t &search_root) { assert(&search_root != nullptr); if (!basetype::isnull()) basetype::late_bind(bound_object(search_root)); } + + // getter (for validation purposes) + const char *device_name() const { return m_device_name; } }; diff --git a/src/emu/dimemory.cpp b/src/emu/dimemory.cpp index 0650ebfab70..b245d51a0b9 100644 --- a/src/emu/dimemory.cpp +++ b/src/emu/dimemory.cpp @@ -12,14 +12,6 @@ #include "validity.h" -//************************************************************************** -// PARAMETERS -//************************************************************************** - -#define DETECT_OVERLAPPING_MEMORY (0) - - - //************************************************************************** // CONSTANTS //************************************************************************** @@ -238,127 +230,16 @@ bool device_memory_interface::memory_readop(offs_t offset, int size, UINT64 &val void device_memory_interface::interface_validity_check(validity_checker &valid) const { - bool detected_overlap = DETECT_OVERLAPPING_MEMORY ? false : true; - // loop over all address spaces for (address_spacenum spacenum = AS_0; spacenum < ADDRESS_SPACES; ++spacenum) { - const address_space_config *spaceconfig = space_config(spacenum); - if (spaceconfig != nullptr) + if (space_config(spacenum) != nullptr) { - int datawidth = spaceconfig->m_databus_width; - int alignunit = datawidth / 8; + // construct the map + ::address_map addrmap(const_cast(device()), spacenum); - // construct the maps - auto map = global_alloc(::address_map(const_cast(device()), spacenum)); - - // if this is an empty map, just skip it - if (map->m_entrylist.first() == nullptr) - { - global_free(map); - continue; - } - - // validate the global map parameters - if (map->m_spacenum != spacenum) - osd_printf_error("Space %d has address space %d handlers!\n", spacenum, map->m_spacenum); - if (map->m_databits != datawidth) - osd_printf_error("Wrong memory handlers provided for %s space! (width = %d, memory = %08x)\n", spaceconfig->m_name, datawidth, map->m_databits); - - // loop over entries and look for errors - for (address_map_entry *entry = map->m_entrylist.first(); entry != nullptr; entry = entry->next()) - { - UINT32 bytestart = spaceconfig->addr2byte(entry->m_addrstart); - UINT32 byteend = spaceconfig->addr2byte_end(entry->m_addrend); - - // look for overlapping entries - if (!detected_overlap) - { - address_map_entry *scan; - for (scan = map->m_entrylist.first(); scan != entry; scan = scan->next()) - if (entry->m_addrstart <= scan->m_addrend && entry->m_addrend >= scan->m_addrstart && - ((entry->m_read.m_type != AMH_NONE && scan->m_read.m_type != AMH_NONE) || - (entry->m_write.m_type != AMH_NONE && scan->m_write.m_type != AMH_NONE))) - { - osd_printf_warning("%s space has overlapping memory (%X-%X,%d,%d) vs (%X-%X,%d,%d)\n", spaceconfig->m_name, entry->m_addrstart, entry->m_addrend, entry->m_read.m_type, entry->m_write.m_type, scan->m_addrstart, scan->m_addrend, scan->m_read.m_type, scan->m_write.m_type); - detected_overlap = true; - break; - } - } - - // look for inverted start/end pairs - if (byteend < bytestart) - osd_printf_error("Wrong %s memory read handler start = %08x > end = %08x\n", spaceconfig->m_name, entry->m_addrstart, entry->m_addrend); - - // look for misaligned entries - if ((bytestart & (alignunit - 1)) != 0 || (byteend & (alignunit - 1)) != (alignunit - 1)) - osd_printf_error("Wrong %s memory read handler start = %08x, end = %08x ALIGN = %d\n", spaceconfig->m_name, entry->m_addrstart, entry->m_addrend, alignunit); - - // if this is a program space, auto-assign implicit ROM entries - if (entry->m_read.m_type == AMH_ROM && entry->m_region == nullptr) - { - entry->m_region = device().tag(); - entry->m_rgnoffs = entry->m_addrstart; - } - - // if this entry references a memory region, validate it - if (entry->m_region != nullptr && entry->m_share == nullptr) - { - // make sure we can resolve the full path to the region - bool found = false; - std::string entry_region = entry->m_devbase.subtag(entry->m_region); - - // look for the region - device_iterator deviter(device().mconfig().root_device()); - for (device_t *device = deviter.first(); device != nullptr; device = deviter.next()) - for (const rom_entry *romp = rom_first_region(*device); romp != nullptr && !found; romp = rom_next_region(romp)) - { - if (rom_region_name(*device, romp) == entry_region) - { - // verify the address range is within the region's bounds - offs_t length = ROMREGION_GETLENGTH(romp); - if (entry->m_rgnoffs + (byteend - bytestart + 1) > length) - osd_printf_error("%s space memory map entry %X-%X extends beyond region '%s' size (%X)\n", spaceconfig->m_name, entry->m_addrstart, entry->m_addrend, entry->m_region, length); - found = true; - } - } - - // error if not found - if (!found) - osd_printf_error("%s space memory map entry %X-%X references non-existant region '%s'\n", spaceconfig->m_name, entry->m_addrstart, entry->m_addrend, entry->m_region); - } - - // make sure all devices exist - // FIXME: This doesn't work! AMH_DEVICE_DELEGATE entries don't even set m_tag, the device tag is inside the proto-delegate - if (entry->m_read.m_type == AMH_DEVICE_DELEGATE && entry->m_read.m_tag != nullptr) - { - std::string temp(entry->m_read.m_tag); - if (device().siblingdevice(temp.c_str()) == nullptr) - osd_printf_error("%s space memory map entry references nonexistant device '%s'\n", spaceconfig->m_name, entry->m_read.m_tag); - } - if (entry->m_write.m_type == AMH_DEVICE_DELEGATE && entry->m_write.m_tag != nullptr) - { - std::string temp(entry->m_write.m_tag); - if (device().siblingdevice(temp.c_str()) == nullptr) - osd_printf_error("%s space memory map entry references nonexistant device '%s'\n", spaceconfig->m_name, entry->m_write.m_tag); - } - - // make sure ports exist -// if ((entry->m_read.m_type == AMH_PORT && entry->m_read.m_tag != NULL && portlist.find(entry->m_read.m_tag) == NULL) || -// (entry->m_write.m_type == AMH_PORT && entry->m_write.m_tag != NULL && portlist.find(entry->m_write.m_tag) == NULL)) -// osd_printf_error("%s space memory map entry references nonexistant port tag '%s'\n", spaceconfig->m_name, entry->m_read.m_tag); - - // validate bank and share tags - if (entry->m_read.m_type == AMH_BANK) - valid.validate_tag(entry->m_read.m_tag); - if (entry->m_write.m_type == AMH_BANK) - valid.validate_tag(entry->m_write.m_tag); - if (entry->m_share != nullptr) - valid.validate_tag(entry->m_share); - } - - // release the address map - global_free(map); + // let the map check itself + addrmap.map_validity_check(valid, device(), spacenum); } } }