From 779d75ea97b68d217bd8c1507375360bac0a8f01 Mon Sep 17 00:00:00 2001 From: apple1417 Date: Sun, 19 Jan 2025 13:09:28 +1300 Subject: [PATCH 1/3] `unrealsdk::unreal::cast` now copies the constness of it's imput object --- src/unrealsdk/unreal/cast.h | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/unrealsdk/unreal/cast.h b/src/unrealsdk/unreal/cast.h index ddc86ca..9662a5e 100644 --- a/src/unrealsdk/unreal/cast.h +++ b/src/unrealsdk/unreal/cast.h @@ -125,7 +125,7 @@ template -void cast_impl(const InputType* obj, +void cast_impl(InputType* obj, const UStruct* working_class, const Function& func, const Fallback& fallback) { @@ -149,12 +149,17 @@ void cast_impl(const InputType* obj, using cls = std::tuple_element_t; // If this class inherits from the input type - if constexpr (std::is_base_of_v - && (include_input_type || !std::is_same_v)) { + if constexpr (std::is_base_of_v, cls> + && (include_input_type + || !std::is_same_v, cls>)) { // If the class name matches if (working_class->Name == cls_fname()) { // Run the callback - return func.template operator()(reinterpret_cast(obj)); + if constexpr (std::is_const_v) { + return func.template operator()(reinterpret_cast(obj)); + } else { + return func.template operator()(reinterpret_cast(obj)); + } } } @@ -222,10 +227,11 @@ struct cast_options { * * @tparam Options The options to use for this cast. * @tparam InputType The type of the input object, the least derived type which may be cast to. - * @tparam Function The type of the callback function. The signature should be `void(const T* obj)`, - * where `T` is a template arg which will derive from (or be equal to) `InputType`. + * @tparam Function The type of the callback function. The signature should be `void(T* obj)` (or + * `void(const T* obj)` if `InputType` is const), where `T` is a template arg which + * will derive from (or be equal to) `std::remove_const_t`. * @tparam Fallback The fallback function's type, only templated for auto deduction. The signature - should be `void(const InputType* obj)`. + should be `void(InputType* obj)` or `void(const InputType* obj)`. * @param obj The object to cast. * @param func The templated callback function to call. * @param fallback A function to call when casting fails. Defaults to throwing a runtime error. @@ -233,11 +239,10 @@ struct cast_options { template , typename InputType, typename Function, - typename Fallback = std::function, - typename = std::enable_if_t>> -void cast(const InputType* obj, - const Function& func, - const Fallback& fallback = default_cast_fallback) { + typename Fallback = std::function> +void cast(InputType* obj, const Function& func, const Fallback& fallback = default_cast_fallback) + requires(std::is_base_of_v) +{ if (obj == nullptr) { throw std::invalid_argument("Tried to cast null object!"); } From 49bff4a47ddab44e2de657b4d37eba5871de5e97 Mon Sep 17 00:00:00 2001 From: apple1417 Date: Sun, 19 Jan 2025 13:11:17 +1300 Subject: [PATCH 2/3] make `UnrealPointer` support pointers to properties, rework `PropertyProxy` to use it this prevents some use after frees if you kept a reference the return value --- .../unreal/wrappers/property_proxy.cpp | 88 +++---------------- .../unreal/wrappers/property_proxy.h | 50 +++++------ .../unreal/wrappers/unreal_pointer.cpp | 38 +++++++- .../unreal/wrappers/unreal_pointer.h | 59 +++++++++---- .../unreal/wrappers/unreal_pointer_funcs.h | 51 ++++++++--- 5 files changed, 157 insertions(+), 129 deletions(-) diff --git a/src/unrealsdk/unreal/wrappers/property_proxy.cpp b/src/unrealsdk/unreal/wrappers/property_proxy.cpp index 08d1025..8f1be51 100644 --- a/src/unrealsdk/unreal/wrappers/property_proxy.cpp +++ b/src/unrealsdk/unreal/wrappers/property_proxy.cpp @@ -10,26 +10,9 @@ namespace unrealsdk::unreal { -/* -Property proxies are primarily intended to hold hook return values. -We generally expect these to be small values, such as bools, enums/ints, or object pointers, which -would be a bit off a waste to allocate space on the heap for. -Instead, we take inspiration from short string optimization, and store short properties inside the -value pointer. -We need to access the property to get it's offset whenever we try read/write the value anyway, so -also checking size doesn't even add any extra pointer accesses. -*/ - -PropertyProxy::PropertyProxy(UProperty* prop) : prop(prop), value(nullptr), been_set(false) { - if (prop != nullptr) { - const size_t size = this->prop->ElementSize * this->prop->ArrayDim; - if (size > sizeof(void*)) { - this->value = unrealsdk::u_malloc(size); - } - } -} -PropertyProxy::PropertyProxy(const PropertyProxy& other) : PropertyProxy(other.prop) { - if (other.been_set) { +PropertyProxy::PropertyProxy(UProperty* prop) : prop(prop), ptr(nullptr) {} +PropertyProxy::PropertyProxy(const PropertyProxy& other) : prop(other.prop), ptr(nullptr) { + if (this->prop != nullptr && other.has_value()) { cast(this->prop, [this, &other](const T* prop) { for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) { this->set(i, other.get(i)); @@ -37,11 +20,6 @@ PropertyProxy::PropertyProxy(const PropertyProxy& other) : PropertyProxy(other.p }); } } -PropertyProxy::PropertyProxy(PropertyProxy&& other) noexcept - : prop(std::exchange(other.prop, nullptr)), - value(std::exchange(other.value, nullptr)), - been_set(std::exchange(other.been_set, false)) {} - PropertyProxy& PropertyProxy::operator=(const PropertyProxy& other) { if (other.prop != this->prop) { throw std::runtime_error("Property proxy is not instance of " @@ -56,64 +34,26 @@ PropertyProxy& PropertyProxy::operator=(const PropertyProxy& other) { } return *this; } -PropertyProxy& PropertyProxy::operator=(PropertyProxy&& other) noexcept { - std::swap(this->prop, other.prop); - std::swap(this->value, other.value); - std::swap(this->been_set, other.been_set); - return *this; -} - -PropertyProxy::~PropertyProxy() { - if (this->prop == nullptr) { - // Nothing to do - return; - } - - auto addr = this->get_value_addr(); - cast(this->prop, [addr](const T* prop) { - for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) { - destroy_property(prop, i, addr); - } - }); - - const size_t size = this->prop->ElementSize * this->prop->ArrayDim; - if (size > sizeof(void*)) { - unrealsdk::u_free(this->value); - } -} - -uintptr_t PropertyProxy::get_value_addr(void) const { - if (this->prop == nullptr) { - throw std::runtime_error("Property does not exist!"); - } - const size_t size = this->prop->ElementSize * this->prop->ArrayDim; - - return (size > sizeof(void*) ? reinterpret_cast(this->value) - : reinterpret_cast(&this->value)) - - this->prop->Offset_Internal; -} bool PropertyProxy::has_value(void) const { - return this->been_set; + return this->ptr.get() != nullptr; } void PropertyProxy::destroy(void) { - // We're lying a little here, we'll simply act as if we don't have a value set, and only - // actually destroy it in the destructor - // In case of fixed arrays, just going to consider uninitialized entries to have an undefined - // value, so doing `destroy(); set(0, ...); get(1);` is your own fault. - this->been_set = false; + this->ptr = UnrealPointer{nullptr}; } -void PropertyProxy::copy_to(uintptr_t addr) { - if (!this->been_set) { +void PropertyProxy::copy_to(uintptr_t addr) const { + if (!this->has_value()) { throw std::runtime_error("Cannot copy empty property!"); } - cast(this->prop, [this, addr](const T* prop) { - for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) { - set_property(prop, i, addr, this->get(i)); - } - }); + if (this->prop != nullptr) { + cast(this->prop, [this, addr](const T* prop) { + for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) { + set_property(prop, i, addr, this->get(i)); + } + }); + } } void PropertyProxy::copy_from(uintptr_t addr) { diff --git a/src/unrealsdk/unreal/wrappers/property_proxy.h b/src/unrealsdk/unreal/wrappers/property_proxy.h index a498560..c39ba1a 100644 --- a/src/unrealsdk/unreal/wrappers/property_proxy.h +++ b/src/unrealsdk/unreal/wrappers/property_proxy.h @@ -5,28 +5,19 @@ #include "unrealsdk/unreal/class_name.h" #include "unrealsdk/unreal/prop_traits.h" +#include "unrealsdk/unreal/wrappers/unreal_pointer.h" +#include "unrealsdk/unreal/wrappers/unreal_pointer_funcs.h" #include "unrealsdk/unrealsdk.h" namespace unrealsdk::unreal { class UProperty; -class PropertyProxy { +struct PropertyProxy { public: UProperty* prop; + UnrealPointer ptr; - private: - void* value; - bool been_set; - - /** - * @brief Get the address access to the value should use. - * - * @return The address access to the value should use. - */ - [[nodiscard]] uintptr_t get_value_addr(void) const; - - public: /** * @brief Constructs a new property proxy. * @@ -35,7 +26,7 @@ class PropertyProxy { */ PropertyProxy(UProperty* prop); PropertyProxy(const PropertyProxy& other); - PropertyProxy(PropertyProxy&& other) noexcept; + PropertyProxy(PropertyProxy&& other) noexcept = default; /** * @brief Assigns to the property proxy. @@ -45,12 +36,12 @@ class PropertyProxy { * @return A reference to this property proxy. */ PropertyProxy& operator=(const PropertyProxy& other); - PropertyProxy& operator=(PropertyProxy&& other) noexcept; + PropertyProxy& operator=(PropertyProxy&& other) noexcept = default; /** * @brief Destroy the property proxy. */ - ~PropertyProxy(); + ~PropertyProxy() = default; /** * @brief Checks if we have a value stored. @@ -68,12 +59,13 @@ class PropertyProxy { */ template [[nodiscard]] typename PropTraits::Value get(size_t idx = 0) const { - if (!this->been_set) { + if (!this->has_value()) { throw std::runtime_error( "Tried to get value of a property proxy which is yet to be set!"); } - return get_property(validate_type(this->prop), idx, this->get_value_addr()); + return get_property(validate_type(this->prop), idx, + reinterpret_cast(this->ptr.get()), this->ptr); } /** @@ -81,20 +73,23 @@ class PropertyProxy { * * @tparam T The property type. * @param idx The fixed array index to get the value at. Defaults to 0. - * @param value The new stored value. + * @param new_value The new stored value. */ template - void set(const typename PropTraits::Value& value) { - this->set(0, value); + void set(const typename PropTraits::Value& new_value) { + this->set(0, new_value); } template - void set(size_t idx, const typename PropTraits::Value& value) { + void set(size_t idx, const typename PropTraits::Value& new_value) { if (this->prop == nullptr) { - throw std::runtime_error("Property does not exist!"); + throw std::runtime_error("Cannot set null property!"); } - set_property(validate_type(this->prop), idx, this->get_value_addr(), value); - this->been_set = true; + if (!this->has_value()) { + this->ptr = UnrealPointer{this->prop}; + } + set_property(validate_type(this->prop), idx, + reinterpret_cast(this->ptr.get()), new_value); } /** @@ -102,12 +97,15 @@ class PropertyProxy { */ void destroy(void); + // The following functions aren't too useful for user code, but sdk internals still find them + // handy to have available. + /** * @brief Copies the stored property to another address. * * @param addr The address to copy to. */ - void copy_to(uintptr_t addr); + void copy_to(uintptr_t addr) const; /** * @brief Copies the value of another address to the stored property. diff --git a/src/unrealsdk/unreal/wrappers/unreal_pointer.cpp b/src/unrealsdk/unreal/wrappers/unreal_pointer.cpp index 1dfd5ec..ba06e1f 100644 --- a/src/unrealsdk/unreal/wrappers/unreal_pointer.cpp +++ b/src/unrealsdk/unreal/wrappers/unreal_pointer.cpp @@ -1,4 +1,8 @@ -#include "unrealsdk/unreal/wrappers/unreal_pointer.h" +#include "unrealsdk/pch.h" +#include "unrealsdk/unreal/cast.h" +#include "unrealsdk/unreal/classes/uproperty.h" +#include "unrealsdk/unreal/classes/ustruct.h" +#include "unrealsdk/unrealsdk.h" namespace unrealsdk::unreal::impl { @@ -17,4 +21,36 @@ size_t UnrealPointerControl::dec_ref(void) { return --this->refs; } +void UnrealPointerControl::destroy_object(void) { + switch (this->pointer_type) { + case PointerType::STRUCT: { + auto struct_type = this->metadata.struct_type; + if (struct_type != nullptr) { + // Need to reconstruct the base address, since our pointer may be elsewhere + auto struct_base = + reinterpret_cast(this) + sizeof(impl::UnrealPointerControl); + + destroy_struct(struct_type, struct_base); + } + return; + } + + case PointerType::PROPERTY: { + auto prop = this->metadata.prop; + if (prop != nullptr) { + // Need to reconstruct the base address, since our pointer may be elsewhere + auto prop_base = reinterpret_cast(this) + + sizeof(impl::UnrealPointerControl) - prop->Offset_Internal; + + cast(prop, [prop_base](const T* prop) { + for (size_t i = 0; i < (size_t)prop->ArrayDim; i++) { + destroy_property(prop, i, prop_base); + } + }); + } + return; + } + } +} + } // namespace unrealsdk::unreal::impl diff --git a/src/unrealsdk/unreal/wrappers/unreal_pointer.h b/src/unrealsdk/unreal/wrappers/unreal_pointer.h index d375d37..ecbe938 100644 --- a/src/unrealsdk/unreal/wrappers/unreal_pointer.h +++ b/src/unrealsdk/unreal/wrappers/unreal_pointer.h @@ -6,6 +6,7 @@ namespace unrealsdk::unreal { class UStruct; +class UProperty; namespace impl { @@ -29,19 +30,28 @@ class UnrealPointerControl { && alignof(std::atomic) == alignof(size_t), "atomic size_t may not be safe to cross dll boundaries"); - public: - // The only way get a pointer owned by the sdk (excluding calling u_malloc) is by making a new - // struct - we have a specific constructor on the just for it. - // Structs need to run custom deleter. Because all sdk-owned pointers are structs, the only - // deleter metadata we need is the struct type, we can get away with just storing that. - // If we add we add more ways to allocate new memory, this will have to turn into a more - // comprehensive custom deleter. - const UStruct* deleter_struct; + // We need a bit of metadata for UnrealPointer::release to know how to safely delete us. + enum class PointerType : uint8_t { + STRUCT, + PROPERTY, + } pointer_type; + + // Deliberately putting pointer type first so the padding's here, in the middle, meaning this + // type ends up naturally aligned. Yes this is probably a bit fragile. + + union { + const UStruct* struct_type; + const UProperty* prop; + } metadata; + public: /** - * @brief Construct a new control block. + * @brief Constructs a new control block. */ - UnrealPointerControl(const UStruct* deleter_struct) : refs(0), deleter_struct(deleter_struct) {} + UnrealPointerControl(const UStruct* struct_type) + : refs(0), pointer_type(PointerType::STRUCT), metadata{.struct_type = struct_type} {} + UnrealPointerControl(const UProperty* prop) + : refs(0), pointer_type(PointerType::PROPERTY), metadata{.prop = prop} {} /** * @brief Destroys the control block. @@ -63,6 +73,11 @@ class UnrealPointerControl { */ virtual size_t dec_ref(void); + /** + * @brief Destroys the object this control block is for. + */ + void destroy_object(void); + UnrealPointerControl(const UnrealPointerControl& other) = delete; UnrealPointerControl(UnrealPointerControl&& other) noexcept = delete; UnrealPointerControl& operator=(const UnrealPointerControl& other) = delete; @@ -119,11 +134,22 @@ class UnrealPointer { UnrealPointer(std::nullptr_t) : control(nullptr), ptr(nullptr) {} /** - * @brief Constructs a pointer to a new block of memory holding a specific struct. + * @brief Constructs a pointer to a new, owned, block of memory holding a specific struct. + * + * @param struct_type The struct to hold. + */ + explicit UnrealPointer(const UStruct* struct_type) + requires std::is_void_v; + + /** + * @brief Constructs a pointer to a new, owned, block of memory holding a single property. + * @note The pointer is offset by -prop->Offset_Internal, which allows passing it directly to + * get_property/set_property. * - * @param struct_type The type of struct to construct. + * @param prop The property to hold. */ - explicit UnrealPointer(const UStruct* struct_type); + explicit UnrealPointer(const UProperty* prop) + requires std::is_void_v; /** * @brief Construct a new pointer pointing at memory owned by another pointer. @@ -189,9 +215,10 @@ class UnrealPointer { * * @return The value the pointer points to */ - template && std::negation_v>>> - U& operator*() const noexcept { + template + U& operator*() const noexcept + requires std::is_same_v && std::negation_v> + { return *this->ptr; } T* operator->() const noexcept { return this->ptr; } diff --git a/src/unrealsdk/unreal/wrappers/unreal_pointer_funcs.h b/src/unrealsdk/unreal/wrappers/unreal_pointer_funcs.h index 9e5783b..fc51fac 100644 --- a/src/unrealsdk/unreal/wrappers/unreal_pointer_funcs.h +++ b/src/unrealsdk/unreal/wrappers/unreal_pointer_funcs.h @@ -2,8 +2,9 @@ #define UNREALSDK_UNREAL_WRAPPERS_UNREAL_POINTER_FUNCS_H #include "unrealsdk/pch.h" +#include "unrealsdk/unreal/classes/uproperty.h" +#include "unrealsdk/unreal/classes/ustruct.h" #include "unrealsdk/unreal/wrappers/unreal_pointer.h" -#include "unrealsdk/unreal/wrappers/wrapped_struct.h" #include "unrealsdk/unrealsdk.h" namespace unrealsdk::unreal { @@ -14,21 +15,16 @@ void UnrealPointer::release(void) { this->control = nullptr; this->ptr = nullptr; + // If we had a control block, we owned this pointer, and need to delete it // If dec_ref throws, we can't be sure there aren't other references to the control block, // so we can't really do anything, better to potentially leak than free something used // elsewhere if (old_control != nullptr && old_control->dec_ref() == 0) { // If the destructors throw, we still want to free the memory try { - // Destroy the struct first, since we know it's less catastrophic to miss the control - // block destructor - if (old_control->deleter_struct != nullptr) { - // Need to reconstruct the struct base, since our pointer may be somewhere else - auto struct_base = - reinterpret_cast(old_control) + sizeof(impl::UnrealPointerControl); - - destroy_struct(old_control->deleter_struct, struct_base); - } + // Destroy the object first, since it might allocate more memory, we know it's less + // catastrophic to miss the control block destructor + old_control->destroy_object(); // Since we're using placement new, we need to manually call the destructor old_control->~UnrealPointerControl(); @@ -46,7 +42,9 @@ void UnrealPointer::release(void) { } template -UnrealPointer::UnrealPointer(const UStruct* struct_type) : control(nullptr), ptr(nullptr) { +UnrealPointer::UnrealPointer(const UStruct* struct_type) + requires std::is_void_v + : control(nullptr), ptr(nullptr) { // If malloc throws, it should have handled freeing memory if required auto buf = unrealsdk::u_malloc(struct_type->get_struct_size() + sizeof(impl::UnrealPointerControl)); @@ -56,7 +54,36 @@ UnrealPointer::UnrealPointer(const UStruct* struct_type) : control(nullptr), // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) this->control = new (buf) impl::UnrealPointerControl(struct_type); - this->ptr = reinterpret_cast(this->control + 1); + this->ptr = reinterpret_cast(this->control + 1); + } catch (const std::exception& ex) { + unrealsdk::u_free(buf); + LOG(ERROR, "Exception in unreal pointer constructor: {}", ex.what()); + throw; + } catch (...) { + unrealsdk::u_free(buf); + LOG(ERROR, "Unknown exception in unreal pointer constructor"); + throw; + } + + // Add our ref + this->attach(); +} + +template +UnrealPointer::UnrealPointer(const UProperty* prop) + requires std::is_void_v + : control(nullptr), ptr(nullptr) { + // If malloc throws, it should have handled freeing memory if required + auto buf = unrealsdk::u_malloc((prop->ElementSize * prop->ArrayDim) + + sizeof(impl::UnrealPointerControl)); + + // Otherwise, if we throw during initialization we need to free manually + try { + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + this->control = new (buf) impl::UnrealPointerControl(prop); + + this->ptr = reinterpret_cast(reinterpret_cast(this->control + 1) + - prop->Offset_Internal); } catch (const std::exception& ex) { unrealsdk::u_free(buf); LOG(ERROR, "Exception in unreal pointer constructor: {}", ex.what()); From 69e6dc7ec2c1d34eeff887e3220a1d9e31ae0733 Mon Sep 17 00:00:00 2001 From: apple1417 Date: Sun, 19 Jan 2025 13:24:43 +1300 Subject: [PATCH 3/3] update changelog/bump version number --- CMakeLists.txt | 2 +- changelog.md | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d927f5d..e9dd1da 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.25) -project(unrealsdk VERSION 1.6.1) +project(unrealsdk VERSION 1.7.0) set(UNREALSDK_UE_VERSION "UE4" CACHE STRING "The unreal engine version to build the SDK for. One of 'UE3' or 'UE4'.") set(UNREALSDK_ARCH "x64" CACHE STRING "The architecture to build the sdk for. One of 'x86' or 'x64'.") diff --git a/changelog.md b/changelog.md index 0383fcf..123d574 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,19 @@ # Changelog +## 1.7.0 + +- `unrealsdk::unreal::cast` now copies the const-ness of its input object to its callbacks. + + [779d75ea](https://github.com/bl-sdk/unrealsdk/commit/779d75ea) + +- Reworked `PropertyProxy` to be based on `UnrealPointer` (and reworked it too). This fixes some + issues with ownership and possible use after frees. + + *This breaks binary compatibility*, though existing code should work pretty much as is after a + recompile. + + [49bff4a4](https://github.com/bl-sdk/unrealsdk/commit/49bff4a4) + ## v1.6.1 - Handled `UClass::Interfaces` also having a different offset between BL2 and TPS.