AHuman - unset m_pItemInReach if set to delete

Architector4 · web-flow · commit 61221dcc8d5a · 2025-11-05T19:07:37.000+03:00
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 <details> <summary>Address Sanitizer's complaint it gave me</summary> ``` ================================================================= ==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 cortex-command-community#1 0x7ffff789f956 in vsnprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1707 cortex-command-community#2 0x7ffff78a24dc in snprintf /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1778 cortex-command-community#3 0x555555c6e9a7 in RTE::AHuman::DrawHUD(BITMAP*, RTE::Vector const&, int, bool) ../Source/Entities/AHuman.cpp:2935 cortex-command-community#4 0x55555646def3 in RTE::MovableMan::DrawHUD(BITMAP*, RTE::Vector const&, int, bool) ../Source/Managers/MovableMan.cpp:1956 cortex-command-community#5 0x555556539ab6 in RTE::SceneMan::Draw(BITMAP*, BITMAP*, RTE::Vector const&, bool, bool) ../Source/Managers/SceneMan.cpp:2630 cortex-command-community#6 0x5555563b28ad in RTE::FrameMan::Draw() ../Source/Managers/FrameMan.cpp:870 cortex-command-community#7 0x555555a4212e in RunGameLoop() ../Source/Main.cpp:395 cortex-command-community#8 0x555555a42dc9 in main ../Source/Main.cpp:468 cortex-command-community#9 0x7ffff6227b8a (/usr/lib/libc.so.6+0x27b8a) (BuildId: 3fb5bf3586fec17ba65a16ec9a3132455897d306) cortex-command-community#10 0x7ffff6227c4a in __libc_start_main (/usr/lib/libc.so.6+0x27c4a) (BuildId: 3fb5bf3586fec17ba65a16ec9a3132455897d306) cortex-command-community#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 cortex-command-community#1 0x555555a543de in std::__new_allocator<char>::deallocate(char*, unsigned long) /usr/include/c++/15.2.1/bits/new_allocator.h:172 cortex-command-community#2 0x555555a4c495 in std::allocator<char>::deallocate(char*, unsigned long) /usr/include/c++/15.2.1/bits/allocator.h:215 cortex-command-community#3 0x555555a4c495 in std::allocator_traits<std::allocator<char> >::deallocate(std::allocator<char>&, char*, unsigned long) /usr/include/c++/15.2.1/bits/alloc_traits.h:649 cortex-command-community#4 0x555555a4c495 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_destroy(unsigned long) /usr/include/c++/15.2.1/bits/basic_string.h:305 cortex-command-community#5 0x555555a49154 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose() /usr/include/c++/15.2.1/bits/basic_string.h:299 cortex-command-community#6 0x555555a48da3 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() /usr/include/c++/15.2.1/bits/basic_string.h:896 cortex-command-community#7 0x5555568f6aa3 in RTE::Entity::~Entity() ../Source/System/Entity.cpp:18 cortex-command-community#8 0x555555f6150d in RTE::SceneObject::~SceneObject() ../Source/Entities/SceneObject.cpp:18 cortex-command-community#9 0x555555e0bd63 in RTE::MovableObject::~MovableObject() ../Source/Entities/MovableObject.cpp:35 cortex-command-community#10 0x555555dc24ab in RTE::MOSprite::~MOSprite() ../Source/Entities/MOSprite.cpp:19 cortex-command-community#11 0x555555dd209d in RTE::MOSRotating::~MOSRotating() ../Source/Entities/MOSRotating.cpp:44 cortex-command-community#12 0x555555ce5353 in RTE::Attachable::~Attachable() ../Source/Entities/Attachable.cpp:21 cortex-command-community#13 0x555555d70bef in RTE::HeldDevice::~HeldDevice() ../Source/Entities/HeldDevice.cpp:27 cortex-command-community#14 0x555555d55995 in RTE::HDFirearm::~HDFirearm() ../Source/Entities/HDFirearm.cpp:25 cortex-command-community#15 0x555555d559b1 in RTE::HDFirearm::~HDFirearm() ../Source/Entities/HDFirearm.cpp:25 cortex-command-community#16 0x55555644dd39 in RTE::MovableMan::Update() ../Source/Managers/MovableMan.cpp:1618 cortex-command-community#17 0x555555a41896 in RunGameLoop() ../Source/Main.cpp:354 cortex-command-community#18 0x555555a42dc9 in main ../Source/Main.cpp:468 cortex-command-community#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 cortex-command-community#1 0x555555a491a7 in std::__new_allocator<char>::allocate(unsigned long, void const*) /usr/include/c++/15.2.1/bits/new_allocator.h:151 cortex-command-community#2 0x555555a4689a in std::allocator<char>::allocate(unsigned long) /usr/include/c++/15.2.1/bits/allocator.h:203 cortex-command-community#3 0x555555a4689a in std::allocator_traits<std::allocator<char> >::allocate(std::allocator<char>&, unsigned long) /usr/include/c++/15.2.1/bits/alloc_traits.h:614 cortex-command-community#4 0x555555a4689a in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_allocate(std::allocator<char>&, unsigned long) /usr/include/c++/15.2.1/bits/basic_string.h:142 cortex-command-community#5 0x555555a4654d in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_create(unsigned long&, unsigned long) /usr/include/c++/15.2.1/bits/basic_string.tcc:164 cortex-command-community#6 0x555555a4d0d6 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_assign(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /usr/include/c++/15.2.1/bits/basic_string.tcc:319 cortex-command-community#7 0x555555a4d692 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::assign(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /usr/include/c++/15.2.1/bits/basic_string.h:1771 cortex-command-community#8 0x555555a49a7a in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator=(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /usr/include/c++/15.2.1/bits/basic_string.h:906 cortex-command-community#9 0x5555568f6cc1 in RTE::Entity::Create(RTE::Entity const&) ../Source/System/Entity.cpp:34 cortex-command-community#10 0x555555f63fb2 in RTE::SceneObject::Create(RTE::SceneObject const&) ../Source/Entities/SceneObject.cpp:159 cortex-command-community#11 0x555555e0d6f5 in RTE::MovableObject::Create(RTE::MovableObject const&) ../Source/Entities/MovableObject.cpp:193 cortex-command-community#12 0x555555dc3b61 in RTE::MOSprite::Create(RTE::MOSprite const&) ../Source/Entities/MOSprite.cpp:102 cortex-command-community#13 0x555555dd415d in RTE::MOSRotating::Create(RTE::MOSRotating const&) ../Source/Entities/MOSRotating.cpp:194 cortex-command-community#14 0x555555ce5c08 in RTE::Attachable::Create(RTE::Attachable const&) ../Source/Entities/Attachable.cpp:75 cortex-command-community#15 0x555555d7227a in RTE::HeldDevice::Create(RTE::HeldDevice const&) ../Source/Entities/HeldDevice.cpp:115 cortex-command-community#16 0x555555d56897 in RTE::HDFirearm::Create(RTE::HDFirearm const&) ../Source/Entities/HDFirearm.cpp:100 cortex-command-community#17 0x555555d6ed11 in RTE::HDFirearm::Clone(RTE::Entity*) const ../Source/Entities/HDFirearm.h:20 cortex-command-community#18 0x55555723e9ba in RTE::LuaAdaptersEntityCreate::RandomHDFirearm(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int) ../Source/Lua/LuaAdapters.cpp:79 cortex-command-community#19 0x5555570c73d0 in int luabind::detail::free_functions::returns<RTE::HDFirearm*>::call<luabind::detail::policy_cons<luabind::detail::adopt_policy<0>, luabind::detail::null_type>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>(RTE::HDFirearm* (*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int), lua_State*, luabind::detail::policy_cons<luabind::detail::adopt_policy<0>, luabind::detail::null_type> const*) ../external/sources/luabind-0.7.1/luabind/function.hpp:347 cortex-command-community#20 0x5555570adb9e in int luabind::detail::free_functions::call<luabind::detail::policy_cons<luabind::detail::adopt_policy<0>, luabind::detail::null_type>, RTE::HDFirearm*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>(RTE::HDFirearm* (*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int), lua_State*, luabind::detail::policy_cons<luabind::detail::adopt_policy<0>, luabind::detail::null_type> const*) ../external/sources/luabind-0.7.1/luabind/function.hpp:403 cortex-command-community#21 0x55555708ffbb in luabind::detail::free_functions::function_callback_s<RTE::HDFirearm* (*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int), luabind::detail::policy_cons<luabind::detail::adopt_policy<0>, luabind::detail::null_type> >::apply(lua_State*, void (*)()) ../external/sources/luabind-0.7.1/luabind/function.hpp:176 cortex-command-community#22 0x555557fab537 in luabind::detail::free_functions::overload_rep::call(lua_State*, void (*)()) const ../external/sources/luabind-0.7.1/luabind/function.hpp:75 cortex-command-community#23 0x555557fab0f4 in luabind::detail::free_functions::function_dispatcher(lua_State*) ../external/sources/luabind-0.7.1/src/function.cpp:142 cortex-command-community#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 ``` </details>
diff --git 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 <limits>
 
 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;
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,9 @@`
`21`	`21`
`22`	`22`	`#include "PrimitiveMan.h"`
`23`	`23`
	`24`	`+#include "TimerMan.h"`
`24`	`25`	`#include "tracy/Tracy.hpp"`
	`26`	`+#include <limits>`
`25`	`27`
`26`	`28`	`using namespace RTE;`
`27`	`29`
`@@ -2057,7 +2059,7 @@ void AHuman::PreControllerUpdate() {`
`2057`	`2059`	`}`
`2058`	`2060`
`2059`	`2061`	`// Item currently set to be within reach has expired or is now out of range`
`2060`		`- 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()))) {`
	`2062`	`+ 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()))) {`
`2061`	`2063`	`m_pItemInReach = nullptr;`
`2062`	`2064`	`}`
`2063`	`2065`