fix corrupting memory when assigning entire array

.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/[email protected]
         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

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

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,

src/unrealsdk/unreal/classes/properties/uarrayproperty.cpp

@@ -49,9 +49,9 @@ void PropTraits<UArrayProperty>::set(const UArrayProperty* prop,
         return;
     }
 
-    cast(inner, [prop, &arr, &value]<typename T>(const T* inner) {
+    cast(inner, [&arr, &value]<typename T>(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<T>(inner, 0,

src/unrealsdk/unreal/classes/uproperty.h

@@ -119,7 +119,7 @@ class UProperty : public UField {
     template <typename PropertyType,
               typename FieldType,
               typename = std::enable_if_t<std::is_base_of_v<UProperty, PropertyType>>>
-    FieldType read_field(FieldType PropertyType::*field) const {
+    FieldType read_field(FieldType PropertyType::* field) const {
 #ifdef UE4
         return reinterpret_cast<const PropertyType*>(this)->*field;
 #else

src/unrealsdk/unreal/classes/ustruct.h

@@ -105,7 +105,7 @@ class UStruct : public UField {
     template <typename SubType,
               typename FieldType,
               typename = std::enable_if_t<std::is_base_of_v<UStruct, SubType>>>
-    [[nodiscard]] const FieldType& get_field(FieldType SubType::*field) const {
+    [[nodiscard]] const FieldType& get_field(FieldType SubType::* field) const {
 #ifdef UE4
         return reinterpret_cast<const SubType*>(this)->*field;
 #else
@@ -117,7 +117,7 @@ class UStruct : public UField {
     template <typename SubType,
               typename FieldType,
               typename = std::enable_if_t<std::is_base_of_v<UStruct, SubType>>>
-    FieldType& get_field(FieldType SubType::*field) {
+    FieldType& get_field(FieldType SubType::* field) {
         return const_cast<FieldType&>(const_cast<const UStruct*>(this)->get_field(field));
     }
 

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),

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<T>::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<T>(new_size)
-                                         : unrealsdk::u_realloc<T>(this->data, new_size);
+    this->data = (this->data == nullptr) ? unrealsdk::u_malloc<T>(new_cap_bytes)
+                                         : unrealsdk::u_realloc<T>(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<uint8_t*>(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 */

src/unrealsdk/unrealsdk.h

@@ -76,6 +76,7 @@ bool init(const std::function<std::unique_ptr<game::AbstractHook>(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 <typename T>
 
 /**
  * @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.