From 61221dcc8d5a08ebbbad90c19ca98b17d139caa2 Mon Sep 17 00:00:00 2001 From: Architector #4 <23612841+Architector4@users.noreply.github.com> Date: Wed, 5 Nov 2025 19:07:37 +0300 Subject: [PATCH 1/2] AHuman - unset m_pItemInReach if set to delete This hopefully fixes a random use-after-free error I've just had. As far as I can tell, the scenario of the crash is as follows. 1. An item is set as item in reach of a character at some point before 2. It gets marked to delete, probably during `MovableMan::Travel()` functions, i.e. when shot hard or whatever 3. It is *not* unset as the item in reach in `MovableMan::PreControllerUpdate()` which is run after `Travel()` 4. It gets properly deleted later in the same `MovableMan::Update()` call 5. `RunGameLoop()` later calls `FrameMan::Draw()` which eventually calls `RTE::AHuman::DrawHUD` which renders the **deleted** item in reach's name. This fix is *not* perfect, I think. Something *between* `PreControllerUpdate()` and the eventual deletion could still catch it. I wonder if this `MOID` thing could solve this by making the relevant code have to figure out if the entity still exists or not reliably lol
Address Sanitizer's complaint it gave me ``` ================================================================= ==37450==ERROR: AddressSanitizer: heap-use-after-free on address 0x7c2ffd5a8e50 at pc 0x7ffff787aa64 bp 0x7fffffffa6f0 sp 0x7fffffff9e68 READ of size 4 at 0x7c2ffd5a8e50 thread T0 #0 0x7ffff787aa63 in printf_common /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:563 #1 0x7ffff789f956 in vsnprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1707 #2 0x7ffff78a24dc in snprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1778 #3 0x555555c6e9a7 in RTE::AHuman::DrawHUD(BITMAP*, RTE::Vector const&, int, bool) ../Source/Entities/AHuman.cpp:2935 #4 0x55555646def3 in RTE::MovableMan::DrawHUD(BITMAP*, RTE::Vector const&, int, bool) ../Source/Managers/MovableMan.cpp:1956 #5 0x555556539ab6 in RTE::SceneMan::Draw(BITMAP*, BITMAP*, RTE::Vector const&, bool, bool) ../Source/Managers/SceneMan.cpp:2630 #6 0x5555563b28ad in RTE::FrameMan::Draw() ../Source/Managers/FrameMan.cpp:870 #7 0x555555a4212e in RunGameLoop() ../Source/Main.cpp:395 #8 0x555555a42dc9 in main ../Source/Main.cpp:468 #9 0x7ffff6227b8a (/usr/lib/libc.so.6+0x27b8a) (BuildId: 3fb5bf3586fec17ba65a16ec9a3132455897d306) #10 0x7ffff6227c4a in __libc_start_main (/usr/lib/libc.so.6+0x27c4a) (BuildId: 3fb5bf3586fec17ba65a16ec9a3132455897d306) #11 0x555555a3ee34 in _start (/media/ext_hdd/nobackup/architector4/Downloads/Cortex-Command-Community-Project/builddebug/CortexCommand+0x4eae34) (BuildId: 47c8b8b068e428fe0a2bb7632a0eb2dabd939356) 0x7c2ffd5a8e50 is located 0 bytes inside of 31-byte region [0x7c2ffd5a8e50,0x7c2ffd5a8e6f) freed by thread T0 here: #0 0x7ffff7954e8d in operator delete(void*, unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:155 #1 0x555555a543de in std::__new_allocator::deallocate(char*, unsigned long) /usr/include/c++/15.2.1/bits/new_allocator.h:172 #2 0x555555a4c495 in std::allocator::deallocate(char*, unsigned long) /usr/include/c++/15.2.1/bits/allocator.h:215 #3 0x555555a4c495 in std::allocator_traits >::deallocate(std::allocator&, char*, unsigned long) /usr/include/c++/15.2.1/bits/alloc_traits.h:649 #4 0x555555a4c495 in std::__cxx11::basic_string, std::allocator >::_M_destroy(unsigned long) /usr/include/c++/15.2.1/bits/basic_string.h:305 #5 0x555555a49154 in std::__cxx11::basic_string, std::allocator >::_M_dispose() /usr/include/c++/15.2.1/bits/basic_string.h:299 #6 0x555555a48da3 in std::__cxx11::basic_string, std::allocator >::~basic_string() /usr/include/c++/15.2.1/bits/basic_string.h:896 #7 0x5555568f6aa3 in RTE::Entity::~Entity() ../Source/System/Entity.cpp:18 #8 0x555555f6150d in RTE::SceneObject::~SceneObject() ../Source/Entities/SceneObject.cpp:18 #9 0x555555e0bd63 in RTE::MovableObject::~MovableObject() ../Source/Entities/MovableObject.cpp:35 #10 0x555555dc24ab in RTE::MOSprite::~MOSprite() ../Source/Entities/MOSprite.cpp:19 #11 0x555555dd209d in RTE::MOSRotating::~MOSRotating() ../Source/Entities/MOSRotating.cpp:44 #12 0x555555ce5353 in RTE::Attachable::~Attachable() ../Source/Entities/Attachable.cpp:21 #13 0x555555d70bef in RTE::HeldDevice::~HeldDevice() ../Source/Entities/HeldDevice.cpp:27 #14 0x555555d55995 in RTE::HDFirearm::~HDFirearm() ../Source/Entities/HDFirearm.cpp:25 #15 0x555555d559b1 in RTE::HDFirearm::~HDFirearm() ../Source/Entities/HDFirearm.cpp:25 #16 0x55555644dd39 in RTE::MovableMan::Update() ../Source/Managers/MovableMan.cpp:1618 #17 0x555555a41896 in RunGameLoop() ../Source/Main.cpp:354 #18 0x555555a42dc9 in main ../Source/Main.cpp:468 #19 0x7ffff6227b8a (/usr/lib/libc.so.6+0x27b8a) (BuildId: 3fb5bf3586fec17ba65a16ec9a3132455897d306) previously allocated by thread T0 here: #0 0x7ffff7953d2d in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:86 #1 0x555555a491a7 in std::__new_allocator::allocate(unsigned long, void const*) /usr/include/c++/15.2.1/bits/new_allocator.h:151 #2 0x555555a4689a in std::allocator::allocate(unsigned long) /usr/include/c++/15.2.1/bits/allocator.h:203 #3 0x555555a4689a in std::allocator_traits >::allocate(std::allocator&, unsigned long) /usr/include/c++/15.2.1/bits/alloc_traits.h:614 #4 0x555555a4689a in std::__cxx11::basic_string, std::allocator >::_S_allocate(std::allocator&, unsigned long) /usr/include/c++/15.2.1/bits/basic_string.h:142 #5 0x555555a4654d in std::__cxx11::basic_string, std::allocator >::_M_create(unsigned long&, unsigned long) /usr/include/c++/15.2.1/bits/basic_string.tcc:164 #6 0x555555a4d0d6 in std::__cxx11::basic_string, std::allocator >::_M_assign(std::__cxx11::basic_string, std::allocator > const&) /usr/include/c++/15.2.1/bits/basic_string.tcc:319 #7 0x555555a4d692 in std::__cxx11::basic_string, std::allocator >::assign(std::__cxx11::basic_string, std::allocator > const&) /usr/include/c++/15.2.1/bits/basic_string.h:1771 #8 0x555555a49a7a in std::__cxx11::basic_string, std::allocator >::operator=(std::__cxx11::basic_string, std::allocator > const&) /usr/include/c++/15.2.1/bits/basic_string.h:906 #9 0x5555568f6cc1 in RTE::Entity::Create(RTE::Entity const&) ../Source/System/Entity.cpp:34 #10 0x555555f63fb2 in RTE::SceneObject::Create(RTE::SceneObject const&) ../Source/Entities/SceneObject.cpp:159 #11 0x555555e0d6f5 in RTE::MovableObject::Create(RTE::MovableObject const&) ../Source/Entities/MovableObject.cpp:193 #12 0x555555dc3b61 in RTE::MOSprite::Create(RTE::MOSprite const&) ../Source/Entities/MOSprite.cpp:102 #13 0x555555dd415d in RTE::MOSRotating::Create(RTE::MOSRotating const&) ../Source/Entities/MOSRotating.cpp:194 #14 0x555555ce5c08 in RTE::Attachable::Create(RTE::Attachable const&) ../Source/Entities/Attachable.cpp:75 #15 0x555555d7227a in RTE::HeldDevice::Create(RTE::HeldDevice const&) ../Source/Entities/HeldDevice.cpp:115 #16 0x555555d56897 in RTE::HDFirearm::Create(RTE::HDFirearm const&) ../Source/Entities/HDFirearm.cpp:100 #17 0x555555d6ed11 in RTE::HDFirearm::Clone(RTE::Entity*) const ../Source/Entities/HDFirearm.h:20 #18 0x55555723e9ba in RTE::LuaAdaptersEntityCreate::RandomHDFirearm(std::__cxx11::basic_string, std::allocator >, int) ../Source/Lua/LuaAdapters.cpp:79 #19 0x5555570c73d0 in int luabind::detail::free_functions::returns::call, luabind::detail::null_type>, std::__cxx11::basic_string, std::allocator >, int>(RTE::HDFirearm* (*)(std::__cxx11::basic_string, std::allocator >, int), lua_State*, luabind::detail::policy_cons, luabind::detail::null_type> const*) ../external/sources/luabind-0.7.1/luabind/function.hpp:347 #20 0x5555570adb9e in int luabind::detail::free_functions::call, luabind::detail::null_type>, RTE::HDFirearm*, std::__cxx11::basic_string, std::allocator >, int>(RTE::HDFirearm* (*)(std::__cxx11::basic_string, std::allocator >, int), lua_State*, luabind::detail::policy_cons, luabind::detail::null_type> const*) ../external/sources/luabind-0.7.1/luabind/function.hpp:403 #21 0x55555708ffbb in luabind::detail::free_functions::function_callback_s, std::allocator >, int), luabind::detail::policy_cons, luabind::detail::null_type> >::apply(lua_State*, void (*)()) ../external/sources/luabind-0.7.1/luabind/function.hpp:176 #22 0x555557fab537 in luabind::detail::free_functions::overload_rep::call(lua_State*, void (*)()) const ../external/sources/luabind-0.7.1/luabind/function.hpp:75 #23 0x555557fab0f4 in luabind::detail::free_functions::function_dispatcher(lua_State*) ../external/sources/luabind-0.7.1/src/function.cpp:142 #24 0x555556ef05f5 in lj_BC_FUNCC external/sources/LuaJIT-2.1/src/lj_vm.s:857 SUMMARY: AddressSanitizer: heap-use-after-free ../Source/Entities/AHuman.cpp:2935 in RTE::AHuman::DrawHUD(BITMAP*, RTE::Vector const&, int, bool) Shadow bytes around the buggy address: 0x7c2ffd5a8b80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7c2ffd5a8c00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd 0x7c2ffd5a8c80: fd fa fa fa fa fa fa fa fa fa fd fd fd fa fa fa 0x7c2ffd5a8d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7c2ffd5a8d80: fa fa fd fd fd fa fa fa fa fa fa fa fa fa fd fd =>0x7c2ffd5a8e00: fd fa fa fa fa fa fa fa fa fa[fd]fd fd fd fa fa 0x7c2ffd5a8e80: fd fd fd fa fa fa fa fa fa fa fa fa fd fd fd fa 0x7c2ffd5a8f00: fa fa fd fd fd fa fa fa fd fd fd fd fa fa fd fd 0x7c2ffd5a8f80: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa 0x7c2ffd5a9000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7c2ffd5a9080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==37450==ABORTING ```
--- Source/Entities/AHuman.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Source/Entities/AHuman.cpp b/Source/Entities/AHuman.cpp index 56866a98ec..51881d0771 100644 --- a/Source/Entities/AHuman.cpp +++ b/Source/Entities/AHuman.cpp @@ -21,7 +21,9 @@ #include "PrimitiveMan.h" +#include "TimerMan.h" #include "tracy/Tracy.hpp" +#include using namespace RTE; @@ -2057,7 +2059,7 @@ void AHuman::PreControllerUpdate() { } // Item currently set to be within reach has expired or is now out of range - if (m_pItemInReach && (!m_pItemInReach->IsPickupableBy(this) || !g_MovableMan.IsDevice(m_pItemInReach) || g_SceneMan.ShortestDistance(reachPoint, m_pItemInReach->GetPos(), g_SceneMan.SceneWrapsX()).MagnitudeIsGreaterThan(reach + m_pItemInReach->GetRadius()))) { + if (m_pItemInReach && (m_pItemInReach->ToDelete() || !m_pItemInReach->IsPickupableBy(this) || !g_MovableMan.IsDevice(m_pItemInReach) || g_SceneMan.ShortestDistance(reachPoint, m_pItemInReach->GetPos(), g_SceneMan.SceneWrapsX()).MagnitudeIsGreaterThan(reach + m_pItemInReach->GetRadius()))) { m_pItemInReach = nullptr; } From cf50f40cfcb058b254d31fee67fac79953c63301 Mon Sep 17 00:00:00 2001 From: Architector #4 <23612841+Architector4@users.noreply.github.com> Date: Wed, 5 Nov 2025 19:09:19 +0300 Subject: [PATCH 2/2] AHuman.cpp - remove accidental needless includes --- Source/Entities/AHuman.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/Source/Entities/AHuman.cpp b/Source/Entities/AHuman.cpp index 51881d0771..71c5e885a7 100644 --- a/Source/Entities/AHuman.cpp +++ b/Source/Entities/AHuman.cpp @@ -21,9 +21,7 @@ #include "PrimitiveMan.h" -#include "TimerMan.h" #include "tracy/Tracy.hpp" -#include using namespace RTE;