DFHack / dfhack

Memory hacking library for Dwarf Fortress and a set of tools that use it
Other
1.84k stars 463 forks source link

`LuaWrapper::MakeMetatable` relies on the passed `type_identity` having static storage duration #4677

Open ab9rf opened 3 weeks ago

ab9rf commented 3 weeks ago

In general, there is an assumption that type_identity objects, throughout DFHack, have static storage duration and thus will exist for the lifetime of the program. However, for type_identity objects that are defined only in a plugin, this is not true: if the plugin is unloaded, the object goes away and pointers to it will point to unallocated memory.

As far as the C++ standard goes, this isn't allowed, which is why gcc marks objects with static storage duration whose addresses are taken using the & operator with a flag that prevents unloading the DLL, in order to preserve the standard-mandated guarantee of program lifetime duration of objects with static storage duration. (See also the -fno-gnu-unique flag and issue #4301.) MSVC, however, allows unloading DLLs, even those with static objects whose pointers are taken where those pointers might outlast the lifetime of the DLL, leaving it to the programmer to understand that unloading a DLL will break this guarantee.

Since we want to be able to unload DLLs as part of our plugin management approach, we need to ensure that type_identity objects originating from a plugin are never attached to a Lua lightuserdata object. Fortunately, there are only two places where we set the DFHACK_IDENTITY_FIELD_TOKEN property on lightuserdata, and only one of them relies on an externally provided type_identity object, that being LuaWrapper::MakeMetatable.

Therefore, what I'm proposing is that LuaWrapper::MakeMetatable needs to canonicalize incoming type_identity pointers, to a canonical version that is owned by the Lua core itself (which is guaranteed to have program lifetime duration as it is defined in the core library) and ensuring that only canonical type_identity objects are interned into Lua lightuserdata objects. The obvious approach is to add an unordered_map, with an appropriate type_identity::operator== and a hashing function that ensures that functionally identical type_identitys compare equal and hash equivalently; with this, we only need to guarantee that the type_identity pointers handed to MakeMetatable are valid only through the end of the function, rather than for as long as the associated lightuserdata object remains live somewhere in the Lua environment.

additional discussion on discord

ab9rf commented 3 weeks ago

i have a branch i've been working on that add a std::type_info reference to every type_identity; this would provide the necessary mechanics for a reliable operator== (by delegation to std::type_info::operator==) and hashing function (via std::type_info::hash_code or std::type_index) to satisfy the mechanics needed for canonicalization. however, this branch requires changes to codegen and needs further work before it's ready to roll