diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 76b238f..8e8557b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -6,10 +6,6 @@ on: [ workflow_dispatch ] -env: - LLVM_MINGW_VERSION: llvm-mingw-20240619-msvcrt-ubuntu-20.04-x86_64 - LLVM_MINGW_DOWNLOAD: https://github.com/mstorsjo/llvm-mingw/releases/download/20240619/llvm-mingw-20240619-msvcrt-ubuntu-20.04-x86_64.tar.xz - jobs: build-windows: runs-on: windows-latest @@ -44,16 +40,13 @@ jobs: submodules: recursive - name: Configure CMake - working-directory: ${{ env.GITHUB_WORKSPACE }} run: cmake . --preset ${{ matrix.preset }} - name: Build - working-directory: ${{ env.GITHUB_WORKSPACE }} run: cmake --build out/build/${{ matrix.preset }} build-ubuntu: - # Require at least 24 for the mingw build - runs-on: ubuntu-24.04 + runs-on: ubuntu-latest strategy: fail-fast: false @@ -78,6 +71,13 @@ jobs: with: submodules: recursive + - name: Login to GitHub Container Registry + uses: docker/login-action@v2 + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Build uses: devcontainers/ci@v0.3 with: @@ -87,6 +87,9 @@ jobs: # The git watcher cmake thinks something's unsafe? Doesn't happen to me locally. runCmd: | git config --global --add safe.directory `pwd` + + set -e + cmake . --preset ${{ matrix.toolchain.preset }} -G Ninja cmake --build out/build/${{ matrix.toolchain.preset }} @@ -106,7 +109,6 @@ jobs: steps: - name: Setup Clang - if: startswith(matrix.preset, 'clang') uses: egor-tensin/setup-clang@v1 - name: Setup CMake and Ninja @@ -122,7 +124,6 @@ jobs: submodules: recursive - name: Configure CMake - working-directory: ${{ env.GITHUB_WORKSPACE }} run: cmake . --preset ${{ matrix.preset }} -DCMAKE_DISABLE_PRECOMPILE_HEADERS=On - name: Remove `.modmap`s from compile commands @@ -133,7 +134,6 @@ jobs: -Path "out\build\${{ matrix.preset }}\compile_commands.json" - name: Run clang-tidy - working-directory: ${{ env.GITHUB_WORKSPACE }} run: | python (Get-Command run-clang-tidy).Source ` -p "out\build\${{ matrix.preset }}" ` @@ -150,7 +150,6 @@ jobs: steps: - name: Setup Clang - if: startswith(matrix.preset, 'clang') uses: egor-tensin/setup-clang@v1 - name: Checkout repository diff --git a/changelog.md b/changelog.md index 72ac444..33fe361 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,13 @@ # Changelog +## Upcoming + +- Fixed that assigning an entire array, rather than getting the array and setting it's elements, + would likely cause memory corruption. This was most common when using an array of large structs, + and when assigning to one which was previously empty. + + [275bbc8b](https://github.com/bl-sdk/unrealsdk/commit/275bbc8b) + ## 1.8.0 - Added support for sending property changed events, via `UObject::post_edit_change_property` and diff --git a/src/unrealsdk/game/bl3/object.cpp b/src/unrealsdk/game/bl3/object.cpp index 30348f9..3f26aaf 100644 --- a/src/unrealsdk/game/bl3/object.cpp +++ b/src/unrealsdk/game/bl3/object.cpp @@ -17,7 +17,7 @@ namespace unrealsdk::game { namespace { -using construct_obj_func = UObject* (*)(UClass* cls, +using construct_obj_func = UObject* (*)(UClass * cls, UObject* obj, FName name, uint32_t flags, diff --git a/src/unrealsdk/unreal/classes/properties/uarrayproperty.cpp b/src/unrealsdk/unreal/classes/properties/uarrayproperty.cpp index 7587c29..0ca373d 100644 --- a/src/unrealsdk/unreal/classes/properties/uarrayproperty.cpp +++ b/src/unrealsdk/unreal/classes/properties/uarrayproperty.cpp @@ -49,9 +49,9 @@ void PropTraits::set(const UArrayProperty* prop, return; } - cast(inner, [prop, &arr, &value](const T* inner) { + cast(inner, [&arr, &value](const T* inner) { auto new_size = value.size(); - arr->resize(new_size, prop->ElementSize); + arr->resize(new_size, inner->ElementSize); for (size_t i = 0; i < new_size; i++) { set_property(inner, 0, diff --git a/src/unrealsdk/unreal/classes/uproperty.h b/src/unrealsdk/unreal/classes/uproperty.h index e955fee..645d443 100644 --- a/src/unrealsdk/unreal/classes/uproperty.h +++ b/src/unrealsdk/unreal/classes/uproperty.h @@ -119,7 +119,7 @@ class UProperty : public UField { template >> - FieldType read_field(FieldType PropertyType::*field) const { + FieldType read_field(FieldType PropertyType::* field) const { #ifdef UE4 return reinterpret_cast(this)->*field; #else diff --git a/src/unrealsdk/unreal/classes/ustruct.h b/src/unrealsdk/unreal/classes/ustruct.h index 30353eb..b37580d 100644 --- a/src/unrealsdk/unreal/classes/ustruct.h +++ b/src/unrealsdk/unreal/classes/ustruct.h @@ -105,7 +105,7 @@ class UStruct : public UField { template >> - [[nodiscard]] const FieldType& get_field(FieldType SubType::*field) const { + [[nodiscard]] const FieldType& get_field(FieldType SubType::* field) const { #ifdef UE4 return reinterpret_cast(this)->*field; #else @@ -117,7 +117,7 @@ class UStruct : public UField { template >> - FieldType& get_field(FieldType SubType::*field) { + FieldType& get_field(FieldType SubType::* field) { return const_cast(const_cast(this)->get_field(field)); } diff --git a/src/unrealsdk/unreal/structs/fstring.cpp b/src/unrealsdk/unreal/structs/fstring.cpp index 25321fc..7506984 100644 --- a/src/unrealsdk/unreal/structs/fstring.cpp +++ b/src/unrealsdk/unreal/structs/fstring.cpp @@ -43,9 +43,11 @@ UnmanagedFString::UnmanagedFString(std::string_view str) : UnmanagedFString(unrealsdk::utils::widen(str)) {} UnmanagedFString::UnmanagedFString(std::wstring_view str) : TArray{.data = nullptr, .count = 0, .max = 0} { - auto size = valid_size(str); - this->resize(size); - memcpy(this->data, str.data(), size * sizeof(*this->data)); + if (!str.empty()) { + auto size = valid_size(str); + this->resize(size); + memcpy(this->data, str.data(), size * sizeof(*this->data)); + } } UnmanagedFString::UnmanagedFString(UnmanagedFString&& other) noexcept : TArray{.data = std::exchange(other.data, nullptr), diff --git a/src/unrealsdk/unreal/structs/tarray_funcs.h b/src/unrealsdk/unreal/structs/tarray_funcs.h index b56ece9..41265c4 100644 --- a/src/unrealsdk/unreal/structs/tarray_funcs.h +++ b/src/unrealsdk/unreal/structs/tarray_funcs.h @@ -1,5 +1,5 @@ -#ifndef PYUNREALSDK_LIBS_UNREALSDK_SRC_UNREALSDK_UNREAL_STRUCTS_TARRAY_FUNCS_H -#define PYUNREALSDK_LIBS_UNREALSDK_SRC_UNREALSDK_UNREAL_STRUCTS_TARRAY_FUNCS_H +#ifndef UNREALSDK_UNREAL_STRUCTS_TARRAY_FUNCS_H +#define UNREALSDK_UNREAL_STRUCTS_TARRAY_FUNCS_H #include "unrealsdk/pch.h" @@ -23,31 +23,41 @@ void TArray::reserve(size_t new_cap, size_t element_size) { if (new_cap > MAX_CAPACITY) { throw std::length_error("Tried to increase TArray beyond max capacity!"); } - if (new_cap == this->capacity()) { + + auto old_cap = this->capacity(); + if (new_cap == old_cap) { return; } - size_t new_size = new_cap * element_size; + size_t new_cap_bytes = new_cap * element_size; /* If realloc fails, it'll return null, which we want to handle "gracefully" by throwing on our - thread and letting the game continue on as if nothing happened. + thread and letting the game continue on as if nothing happened. + We have a race condition between the realloc call finishing and us overwriting the data pointer - however, another thread could try access the now free'd memory. + however, another thread could try access the now free'd memory. + Since the realloc failure should be a lot rarer, overwrite the pointer ASAP, and only check if - it's valid after. + it's valid after. */ auto old_data = this->data; - this->data = (this->data == nullptr) ? unrealsdk::u_malloc(new_size) - : unrealsdk::u_realloc(this->data, new_size); + this->data = (this->data == nullptr) ? unrealsdk::u_malloc(new_cap_bytes) + : unrealsdk::u_realloc(this->data, new_cap_bytes); if (this->data == nullptr) { this->data = old_data; throw std::runtime_error("Failed to allocate memory to resize array!"); } + // If the array grew (and wasn't empty before), zero out any new bytes + if (0 < old_cap && old_cap < new_cap) { + auto u8_data = reinterpret_cast(this->data); + memset(u8_data + (old_cap * element_size), 0, (new_cap - old_cap) * element_size); + } + this->max = (decltype(max))new_cap; } } // namespace unrealsdk::unreal -#endif /* PYUNREALSDK_LIBS_UNREALSDK_SRC_UNREALSDK_UNREAL_STRUCTS_TARRAY_FUNCS_H */ +#endif /* UNREALSDK_UNREAL_STRUCTS_TARRAY_FUNCS_H */ diff --git a/src/unrealsdk/unrealsdk.h b/src/unrealsdk/unrealsdk.h index 966184c..1c3612c 100644 --- a/src/unrealsdk/unrealsdk.h +++ b/src/unrealsdk/unrealsdk.h @@ -76,6 +76,7 @@ bool init(const std::function(void)>& game_g /** * @brief Calls unreal's malloc function. + * @note This memory is guaranteed to be zero'd. * * @tparam T The type to cast the return to. * @param len The amount of bytes to allocate. @@ -89,6 +90,7 @@ template /** * @brief Calls unreal's realloc function. + * @note This *does not* zero bytes after the original allocation. * * @tparam T The type to cast the return to. * @param original The original memory to re-allocate.