diff --git a/docs/dev/data-identity.rst b/docs/dev/data-identity.rst index 855a6db26c..ae7076941d 100644 --- a/docs/dev/data-identity.rst +++ b/docs/dev/data-identity.rst @@ -123,18 +123,12 @@ Type identity object lifetime and mutability ============================================ *Most* instances of ``type_identity`` are statically constructed and immutable and are thus ``const static`` when constructed. -All ``type_identity`` pointers should be declared ``const``. -Due to the use of lazy initialization, ``compound_identity`` and its subclasses have a few fields that are marked as ``mutable`` -(so that the ``parent`` and ``child`` members can be updated as additional instances are constructed). -In addition, ``virtual_identity`` has two fields that are ``mutable`` due to the dual use of this type to implement -DFHack's vmethod interpose system. -Due to this dual purpose, it is important that there be at most one ``virtual_identity`` object per virtual class, -although this is not enforced at present. +All ``type_identity`` pointers should be declared ``const``. Due to ``virtual_identity``'s role in implementing +DFHack's vmethod interpose system, it is important that there be at most one ``virtual_identity`` object per virtual class. Having more than one ``struct_identity`` object for the same type might also potentially lead to misoperation. In general, there should be a one to one correspondence between ``type_identity`` objects and C++ types -(with the special case that ``global_identity`` has no corresponding type). -As far as we know, for any type other than ``virtual_identity``, +(with the special case that ``global_identity`` has no corresponding type). As far as we know, for any type other than ``virtual_identity``, violations of this constraint will not lead to misoperation, but this constraint should not be lightly violated. The Lua/C++ interface does, in a handful of places, assume that it can compare ``type_identity`` pointers to determine if they reference the same type, but as far as we know all of these instances will fall diff --git a/library/DataDefs.cpp b/library/DataDefs.cpp index 50be583459..5c187290f7 100644 --- a/library/DataDefs.cpp +++ b/library/DataDefs.cpp @@ -27,6 +27,7 @@ distribution. #include #include #include +#include #include "MemAccess.h" #include "Core.h" @@ -86,27 +87,43 @@ void *enum_identity::do_allocate() const { return p; } -/* The order of global object constructor calls is - * undefined between compilation units. Therefore, - * this list has to be plain data, so that it gets - * initialized by the loader in the initial mmap. - */ -compound_identity *compound_identity::list = NULL; -std::vector compound_identity::top_scope; +/****** COMPOUND IDENTITIES ******/ -compound_identity::compound_identity(size_t size, TAllocateFn alloc, - const compound_identity *scope_parent, const char *dfhack_name) - : constructed_identity(size, alloc), dfhack_name(dfhack_name), scope_parent(const_cast(scope_parent)) // fixme +decltype(compound_identity::list) compound_identity::list = nullptr; +decltype(compound_identity::parent_map) compound_identity::parent_map = nullptr; +decltype(compound_identity::children_map) compound_identity::children_map = nullptr; +decltype(compound_identity::top_scope) compound_identity::top_scope = nullptr; + +void compound_identity::ensure_compound_identity_init() { - next = list; list = this; + static std::once_flag compound_identity_init_flag; + std::call_once(compound_identity_init_flag, [] () { + list = new (std::remove_pointer::type)(); + parent_map = new (std::remove_pointer::type)(); + children_map = new (std::remove_pointer::type)(); + top_scope = new std::vector(); + }); } -void compound_identity::doInit(Core *) +compound_identity::compound_identity(size_t size, TAllocateFn alloc, + const compound_identity* scope_parent, const char* dfhack_name) + : constructed_identity(size, alloc), dfhack_name(dfhack_name), scope_parent(scope_parent) { + ensure_compound_identity_init(); + if (scope_parent) - scope_parent->scope_children.push_back(this); + { + (*parent_map)[this] = scope_parent; + (*children_map)[scope_parent].push_back(this); + } else - top_scope.push_back(this); + (*top_scope).push_back(this); + + list->push_back(this); +} + +void compound_identity::doInit(Core *) const +{ } const std::string compound_identity::getFullName() const @@ -124,10 +141,10 @@ void compound_identity::Init(Core *core) if (!known_mutex) known_mutex = new std::mutex(); - // This cannot be done in the constructors, because - // they are called in an undefined order. - for (compound_identity *p = list; p; p = p->next) - p->doInit(core); + // do any late initialization required for compound identities + + for (auto ci : *list) + ci->doInit(core); } bitfield_identity::bitfield_identity(size_t size, @@ -176,27 +193,44 @@ enum_identity::ComplexData::ComplexData(std::initializer_list values) } } +/****** STRUCT IDENTITIES ******/ + +decltype(struct_identity::parent_map) struct_identity::parent_map = nullptr; +decltype(struct_identity::children_map) struct_identity::children_map = nullptr; + +void struct_identity::ensure_struct_identity_init() +{ + static std::once_flag struct_identity_init_flag; + std::call_once(struct_identity_init_flag, []() { + parent_map = new (std::remove_pointer::type)(); + children_map = new (std::remove_pointer::type)(); + }); +} + + + struct_identity::struct_identity(size_t size, TAllocateFn alloc, const compound_identity *scope_parent, const char *dfhack_name, const struct_identity *parent, const struct_field_info *fields) : compound_identity(size, alloc, scope_parent, dfhack_name), - parent(const_cast(parent)), has_children(false), fields(fields) + fields(fields) { + ensure_struct_identity_init(); + if (parent) + { + (*parent_map)[this] = parent; + (*children_map)[parent].push_back(this); + } } -void struct_identity::doInit(Core *core) +void struct_identity::doInit(Core *core) const { compound_identity::doInit(core); - - if (parent) { - parent->children.push_back(this); - parent->has_children = true; - } } bool struct_identity::is_subclass(const struct_identity *actual) const { - if (!has_children && actual != this) + if (!hasChildren() && actual != this) return false; for (; actual; actual = actual->getParent()) @@ -238,13 +272,32 @@ union_identity::union_identity(size_t size, const TAllocateFn alloc, { } +/****** VIRTUAL IDENTITIES ******/ + +decltype(virtual_identity::name_lookup) virtual_identity::name_lookup = nullptr; +decltype(virtual_identity::known) virtual_identity::known = nullptr; +decltype(virtual_identity::vtable_ptr_map) virtual_identity::vtable_ptr_map = nullptr; +decltype(virtual_identity::interpose_list_map) virtual_identity::interpose_list_map = nullptr; + +void virtual_identity::ensure_virtual_identity_init() +{ + static std::once_flag virtual_identity_init_flag; + std::call_once(virtual_identity_init_flag, []() { + name_lookup = new (std::remove_pointer::type)(); + known = new (std::remove_pointer::type)(); + vtable_ptr_map = new (std::remove_pointer::type)(); + interpose_list_map = new (std::remove_pointer::type)(); + }); +} + virtual_identity::virtual_identity(size_t size, const TAllocateFn alloc, const char *dfhack_name, const char *original_name, const virtual_identity *parent, const struct_field_info *fields, bool is_plugin) : struct_identity(size, alloc, NULL, dfhack_name, parent, fields), original_name(original_name), - vtable_ptr(NULL), is_plugin(is_plugin) + is_plugin(is_plugin) { + ensure_virtual_identity_init(); // Plugins are initialized after Init was called, so they need to be added to the name table here if (is_plugin) { @@ -252,15 +305,10 @@ virtual_identity::virtual_identity(size_t size, const TAllocateFn alloc, } } -/* Vtable name to identity lookup. */ -static std::map name_lookup; - -/* Vtable pointer to identity lookup. */ -std::map virtual_identity::known; - virtual_identity::~virtual_identity() { // Remove interpose entries, so that they don't try accessing this object later + auto& interpose_list = (*interpose_list_map)[this]; for (auto it = interpose_list.begin(); it != interpose_list.end(); ++it) if (it->second) it->second->on_host_delete(this); @@ -269,75 +317,79 @@ virtual_identity::~virtual_identity() // Remove global lookup table entries if we're from a plugin if (is_plugin) { - name_lookup.erase(getOriginalName()); + (*name_lookup).erase(getOriginalName()); - if (vtable_ptr) - known.erase(vtable_ptr); + if (vtable_ptr()) + (*known).erase(vtable_ptr()); } } -void virtual_identity::doInit(Core *core) +void virtual_identity::doInit(Core *core) const { struct_identity::doInit(core); auto vtname = getOriginalName(); - name_lookup[vtname] = this; + (*name_lookup)[vtname] = this; - vtable_ptr = core->vinfo->getVTable(vtname); + auto vtable_ptr = core->vinfo->getVTable(vtname); if (vtable_ptr) - known[vtable_ptr] = this; + { + (*known)[vtable_ptr] = this; + (*vtable_ptr_map)[this] = vtable_ptr; + } } -virtual_identity *virtual_identity::find(const std::string &name) +const virtual_identity *virtual_identity::find(std::string_view name) { - auto name_it = name_lookup.find(name); + auto name_it = (*name_lookup).find(name); - return (name_it != name_lookup.end()) ? name_it->second : NULL; + return (name_it != (*name_lookup).end()) ? name_it->second : nullptr; } -virtual_identity *virtual_identity::get(virtual_ptr instance_ptr) +const virtual_identity *virtual_identity::get(virtual_ptr instance_ptr) { - if (!instance_ptr) return NULL; + if (!instance_ptr) return nullptr; return find(get_vtable(instance_ptr)); } -virtual_identity *virtual_identity::find(void *vtable) +const virtual_identity *virtual_identity::find(void *vtable) { if (!vtable || !known_mutex) - return NULL; + return nullptr; + + ensure_virtual_identity_init(); // Actually, a reader/writer lock would be sufficient, // since the table is only written once per class. std::lock_guard lock(*known_mutex); - std::map::iterator it = known.find(vtable); - - if (it != known.end()) + auto it = (*known).find(vtable); + if (it != (*known).end()) return it->second; // If using a reader/writer lock, re-grab as write here, and recheck Core &core = Core::getInstance(); std::string name = core.p->doReadClassName(vtable); - auto name_it = name_lookup.find(name); - if (name_it != name_lookup.end()) { - virtual_identity *p = name_it->second; + auto name_it = (*name_lookup).find(name); + if (name_it != (*name_lookup).end()) { + const virtual_identity *p = name_it->second; - if (p->vtable_ptr && p->vtable_ptr != vtable) { + if (p->vtable_ptr() && p->vtable_ptr() != vtable) { std::cerr << "Conflicting vtable ptr for class '" << p->getName() << "': found 0x" << std::hex << uintptr_t(vtable) - << ", previous 0x" << uintptr_t(p->vtable_ptr) << std::dec << std::endl; + << ", previous 0x" << uintptr_t(p->vtable_ptr()) << std::dec << std::endl; abort(); - } else if (!p->vtable_ptr) { + } else if (!p->vtable_ptr()) { uintptr_t pv = uintptr_t(vtable); pv -= Core::getInstance().vinfo->getRebaseDelta(); std::cerr << "" << std::endl; } - known[vtable] = p; - p->vtable_ptr = vtable; + (*known)[vtable] = p; + (*vtable_ptr_map)[p] = vtable; return p; } @@ -346,14 +398,15 @@ virtual_identity *virtual_identity::find(void *vtable) << std::hex << uintptr_t(vtable) << std::dec << std::endl; } - known[vtable] = NULL; - return NULL; + (*known).erase(vtable); + return nullptr; } void virtual_identity::adjust_vtable(virtual_ptr obj, const virtual_identity *main) const { - if (vtable_ptr) { - *(void**)obj = vtable_ptr; + auto vp = vtable_ptr(); + if (vp) { + *(void**)obj = vp; return; } @@ -366,10 +419,10 @@ void virtual_identity::adjust_vtable(virtual_ptr obj, const virtual_identity *ma virtual_ptr virtual_identity::clone(virtual_ptr obj) { - virtual_identity *id = get(obj); - if (!id) return NULL; + const virtual_identity *id = get(obj); + if (!id) return nullptr; virtual_ptr copy = id->instantiate(); - if (!copy) return NULL; + if (!copy) return nullptr; id->do_copy(copy, obj); return copy; } diff --git a/library/LuaWrapper.cpp b/library/LuaWrapper.cpp index ef37bd61ee..823d542b79 100644 --- a/library/LuaWrapper.cpp +++ b/library/LuaWrapper.cpp @@ -221,7 +221,7 @@ void LuaWrapper::push_object_internal(lua_State *state, const type_identity *typ if (type->type() == IDTYPE_CLASS) { - virtual_identity *class_vid = virtual_identity::get(virtual_ptr(ptr)); + const virtual_identity *class_vid = virtual_identity::get(virtual_ptr(ptr)); if (class_vid) type = class_vid; } diff --git a/library/VTableInterpose.cpp b/library/VTableInterpose.cpp index 797161240e..3c2a4a50fc 100644 --- a/library/VTableInterpose.cpp +++ b/library/VTableInterpose.cpp @@ -210,7 +210,7 @@ void DFHack::addr_to_method_pointer_(void *pptr, void *addr) void *virtual_identity::get_vmethod_ptr(int idx) const { assert(idx >= 0); - void **vtable = (void**)vtable_ptr; + void **vtable = (void**)vtable_ptr(); if (!vtable) return NULL; return vtable[idx]; } @@ -218,7 +218,7 @@ void *virtual_identity::get_vmethod_ptr(int idx) const bool virtual_identity::set_vmethod_ptr(MemoryPatcher &patcher, int idx, void *ptr) const { assert(idx >= 0); - void **vtable = (void**)vtable_ptr; + void **vtable = (void**)vtable_ptr(); if (!vtable) return NULL; return patcher.write(&vtable[idx], &ptr, sizeof(void*)); } @@ -326,16 +326,13 @@ VMethodInterposeLinkBase::~VMethodInterposeLinkBase() VMethodInterposeLinkBase *VMethodInterposeLinkBase::get_first_interpose(const virtual_identity *id) { - auto pitem = id->interpose_list.find(vmethod_idx); - if (pitem == id->interpose_list.end()) - return NULL; - auto item = pitem->second; + auto item = id->get_interpose(vmethod_idx); if (!item) - return NULL; + return nullptr; if (item->host != id) - return NULL; + return nullptr; while (item->prev && item->prev->host == id) item = item->prev; @@ -364,7 +361,7 @@ bool VMethodInterposeLinkBase::find_child_hosts(const virtual_identity *cur, voi child_next.insert(base); found = true; } - else if (child->vtable_ptr) + else if (child->vtable_ptr()) { void *cptr = child->get_vmethod_ptr(vmethod_idx); if (cptr != vmptr) @@ -401,7 +398,7 @@ void VMethodInterposeLinkBase::on_host_delete(const virtual_identity *from) { // Otherwise, drop the link to that child: assert(child_hosts.count(from) != 0 && - from->interpose_list[vmethod_idx] == this); // while mutating this gets cleaned up below so machts nichts + from->get_interpose(vmethod_idx) == this); // while mutating this gets cleaned up below so machts nichts // Find and restore the original vmethod ptr auto last = this; @@ -413,7 +410,7 @@ void VMethodInterposeLinkBase::on_host_delete(const virtual_identity *from) // Unlink the chains child_hosts.erase(from); - from->interpose_list.erase(vmethod_idx); + from->set_interpose(vmethod_idx,nullptr); } } @@ -427,7 +424,7 @@ bool VMethodInterposeLinkBase::apply(bool enable) if (is_applied()) return true; - if (!host->vtable_ptr) + if (!host->vtable_ptr()) { std::cerr << "VMethodInterposeLinkBase::apply(" << enable << "): " << name() << ": no vtable pointer: " << host->getName() << endl; @@ -435,10 +432,10 @@ bool VMethodInterposeLinkBase::apply(bool enable) } // Retrieve the current vtable entry - auto l = host->interpose_list.find(vmethod_idx); + auto l = host->get_interpose(vmethod_idx); - VMethodInterposeLinkBase* old_link = (l != host->interpose_list.end()) ? (l->second) : nullptr; - VMethodInterposeLinkBase* next_link = NULL; + VMethodInterposeLinkBase* old_link = (l != nullptr) ? l : nullptr; + VMethodInterposeLinkBase* next_link = nullptr; while (old_link && old_link->host == host && old_link->priority > priority) { @@ -473,7 +470,7 @@ bool VMethodInterposeLinkBase::apply(bool enable) if (next_link) next_link->prev = this; else - host->interpose_list[vmethod_idx] = this; + host->set_interpose(vmethod_idx,this); child_hosts.clear(); child_next.clear(); @@ -532,9 +529,9 @@ bool VMethodInterposeLinkBase::apply(bool enable) for (auto it = child_hosts.begin(); it != child_hosts.end(); ++it) { auto nhost = *it; - assert(nhost->interpose_list[vmethod_idx] == old_link); // acceptable due to assign below + assert(nhost->get_interpose(vmethod_idx) == old_link); // acceptable due to assign below nhost->set_vmethod_ptr(patcher, vmethod_idx, interpose_method); - nhost->interpose_list[vmethod_idx] = this; + nhost->set_interpose(vmethod_idx, this); } return true; @@ -573,7 +570,7 @@ void VMethodInterposeLinkBase::remove() MemoryPatcher patcher; // Remove from the list in the identity and vtable - host->interpose_list[vmethod_idx] = prev; + host->set_interpose(vmethod_idx, prev); host->set_vmethod_ptr(patcher, vmethod_idx, saved_chain); for (auto it = child_next.begin(); it != child_next.end(); ++it) @@ -589,8 +586,8 @@ void VMethodInterposeLinkBase::remove() for (auto it = child_hosts.begin(); it != child_hosts.end(); ++it) { auto nhost = *it; - assert(nhost->interpose_list[vmethod_idx] == this); // acceptable due to assign below - nhost->interpose_list[vmethod_idx] = prev; + assert(nhost->get_interpose(vmethod_idx) == this); // acceptable due to assign below + nhost->set_interpose(vmethod_idx,prev); nhost->set_vmethod_ptr(patcher, vmethod_idx, saved_chain); if (prev) prev->child_hosts.insert(nhost); diff --git a/library/include/DataDefs.h b/library/include/DataDefs.h index 6a5695809d..9d1eb4c664 100644 --- a/library/include/DataDefs.h +++ b/library/include/DataDefs.h @@ -24,6 +24,7 @@ distribution. #pragma once +#include #include #include #include @@ -134,28 +135,30 @@ namespace DFHack }; class DFHACK_EXPORT compound_identity : public constructed_identity { - static compound_identity *list; - mutable compound_identity *next; + static std::list* list; + static std::map* parent_map; + static std::map>* children_map; + static std::vector* top_scope; const char *dfhack_name; - mutable compound_identity *scope_parent; - mutable std::vector scope_children; - static std::vector top_scope; + const compound_identity *const scope_parent; + + static void ensure_compound_identity_init(); protected: compound_identity(size_t size, TAllocateFn alloc, const compound_identity *scope_parent, const char *dfhack_name); - virtual void doInit(Core *core); + virtual void doInit(Core *core) const; public: const char *getName() const { return dfhack_name; } virtual const std::string getFullName() const; - const compound_identity *getScopeParent() const { return scope_parent; } - const std::vector &getScopeChildren() const { return scope_children; } - static const std::vector &getTopScope() { return top_scope; } + const compound_identity *getScopeParent() const { return (*parent_map)[this]; } + const std::vector &getScopeChildren() const { return (*children_map)[this]; } + static const std::vector &getTopScope() { return *top_scope; } static void Init(Core *core); }; @@ -286,14 +289,15 @@ namespace DFHack }; class DFHACK_EXPORT struct_identity : public compound_identity { - mutable struct_identity *parent; - mutable std::vector children; - bool has_children; + static std::map* parent_map; + static std::map>* children_map; const struct_field_info *fields; + static void ensure_struct_identity_init(); + protected: - virtual void doInit(Core *core); + virtual void doInit(Core *core) const override; public: struct_identity(size_t size, TAllocateFn alloc, @@ -302,9 +306,9 @@ namespace DFHack virtual identity_type type() const { return IDTYPE_STRUCT; } - const struct_identity *getParent() const { return parent; } - const std::vector &getChildren() const { return children; } - bool hasChildren() const { return has_children; } + const struct_identity *getParent() const { return (*parent_map)[this]; } + const std::vector &getChildren() const { return (*children_map)[this]; } + bool hasChildren() const { return (*children_map)[this].size() > 0; } const struct_field_info *getFields() const { return fields; } @@ -361,27 +365,55 @@ namespace DFHack class MemoryPatcher; class DFHACK_EXPORT virtual_identity : public struct_identity { - static std::map known; + public: + using interpose_t = VMethodInterposeLinkBase*; + using interpose_list_t = std::map; - const char *original_name; + private: + static std::map> *name_lookup; + static std::map* known; + static std::map* vtable_ptr_map; + static std::map* interpose_list_map; - mutable void *vtable_ptr; + const char *original_name; bool is_plugin; friend class VMethodInterposeLinkBase; - mutable std::map interpose_list; + + static void ensure_virtual_identity_init(); protected: - virtual void doInit(Core *core); + virtual void doInit(Core *core) const override; static void *get_vtable(virtual_ptr instance_ptr) { return *(void**)instance_ptr; } - bool can_allocate() const { return struct_identity::can_allocate() && (vtable_ptr != NULL); } + void* vtable_ptr() const + { + auto& lst = (*vtable_ptr_map); + auto it = lst.find(this); + return it != lst.end() ? it->second : nullptr; + } + + bool can_allocate() const { return struct_identity::can_allocate() && (vtable_ptr() != nullptr); } void *get_vmethod_ptr(int index) const; bool set_vmethod_ptr(MemoryPatcher &patcher, int index, void *ptr) const; + interpose_list_t& get_interpose_list() const { return (*interpose_list_map)[this]; } + interpose_t get_interpose(int index) const { + auto &lst = get_interpose_list(); + auto it = lst.find(index); + return it != lst.end() ? it->second : nullptr; + } + void set_interpose(int index, interpose_t link) const { + auto &lst = get_interpose_list(); + if (link) + lst[index] = link; + else + lst.erase(index); + } + public: virtual_identity(size_t size, const TAllocateFn alloc, const char *dfhack_name, const char *original_name, @@ -394,16 +426,16 @@ namespace DFHack const char *getOriginalName() const { return original_name ? original_name : getName(); } public: - static virtual_identity *get(virtual_ptr instance_ptr); + static const virtual_identity *get(virtual_ptr instance_ptr); - static virtual_identity *find(void *vtable); - static virtual_identity *find(const std::string &name); + static const virtual_identity *find(void *vtable); + static const virtual_identity *find(std::string_view name); bool is_instance(virtual_ptr instance_ptr) const { if (!instance_ptr) return false; - if (vtable_ptr) { + if (vtable_ptr()) { void *vtable = get_vtable(instance_ptr); - if (vtable == vtable_ptr) return true; + if (vtable == vtable_ptr()) return true; if (!hasChildren()) return false; } return is_subclass(get(instance_ptr)); @@ -411,8 +443,8 @@ namespace DFHack bool is_direct_instance(virtual_ptr instance_ptr) const { if (!instance_ptr) return false; - return vtable_ptr ? (vtable_ptr == get_vtable(instance_ptr)) - : (this == get(instance_ptr)); + auto vp = vtable_ptr(); + return vp ? (vp == get_vtable(instance_ptr)) : (this == get(instance_ptr)); } template static P get_vmethod_ptr(P selector); diff --git a/library/modules/Gui.cpp b/library/modules/Gui.cpp index b5df3ca990..3808fa0271 100644 --- a/library/modules/Gui.cpp +++ b/library/modules/Gui.cpp @@ -135,7 +135,7 @@ static df::layer_object_listst *getLayerList(df::viewscreen_layer *layer, int id } */ -static std::string getNameChunk(virtual_identity *id, int start, int end) +static std::string getNameChunk(const virtual_identity *id, int start, int end) { if (!id) return "UNKNOWN"; @@ -970,7 +970,7 @@ std::vector Gui::getFocusStrings(df::viewscreen* top) } } - if (virtual_identity *id = virtual_identity::get(top)) + if (const virtual_identity *id = virtual_identity::get(top)) { std::string name = getNameChunk(id, 11, 2);