AHuman - unset m_pItemInReach if set to delete #231
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
Contributor
Author
|
Huh? What? How did those extra includes get there??? Lemme just bonk lmao |
Causeless
approved these changes
Nov 14, 2025
Contributor
Causeless
left a comment
Causeless
left a comment
There was a problem hiding this comment.
Nice! I'll have a think, there may indeed be a safer place to do this more consistently. In any case, this is reasonable though
Merged
via the queue into
cortex-command-community:development
with commit Nov 14, 2025
e7f64c3
4 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
MovableMan::Travel()functions, i.e. when shot hard or whateverMovableMan::PreControllerUpdate()which is run afterTravel()MovableMan::Update()callRunGameLoop()later callsFrameMan::Draw()which eventually callsRTE::AHuman::DrawHUDwhich 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 thisMOIDthing could solve this by making the relevant code have to figure out if the entity still exists or not reliably lolAddress Sanitizer's complaint it gave me