Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kill vending machines vol 2 #18

Conversation

GuardianDll
Copy link
Owner

Summary

Category "Brief description"

Purpose of change

Describe the solution

Describe alternatives you've considered

Testing

Additional context

GuardianDll and others added 2 commits September 10, 2024 16:08
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@GuardianDll GuardianDll merged commit e20f07c into GuardianDll:kill_vend_machines Sep 13, 2024
10 checks passed
@GuardianDll GuardianDll deleted the kill_vending_machines_vol_2 branch September 13, 2024 06:59
GuardianDll pushed a commit that referenced this pull request Oct 2, 2024
Prevents the crash listed below that previously happened when searching
for zones to add, but the search yielded no results:

```
Thread 1 "cataclysm-tiles" received signal SIGABRT, Aborted.
__pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44

(gdb) bt
 #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
 #1  0x00007ffff787840f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
 #2  0x00007ffff78294f2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
 #3  0x00007ffff78124ed in __GI_abort () at ./stdlib/abort.c:79
 #4  0x00007ffff7ad501e in std::__glibcxx_assert_fail(char const*, int, char const*, char const*) () from /lib/x86_64-linux-gnu/libstdc++.so.6
 #5  0x0000555555de2639 in std::vector<uilist_entry, std::allocator<uilist_entry> >::operator[] (this=<optimized out>, __n=<optimized out>) at /usr/include/c++/14/bits/stl_vector.h:1130
 #6  std::vector<uilist_entry, std::allocator<uilist_entry> >::operator[] (this=<optimized out>, __n=<optimized out>) at /usr/include/c++/14/bits/stl_vector.h:1128
 #7  0x000055555699deac in uilist_impl::draw_controls (this=0x555599a181d0) at src/ui.cpp:138
 #8  0x0000555555b7c312 in cataimgui::window::draw (this=0x555599a181d0) at src/cata_imgui.cpp:631
 #9  cataimgui::window::draw (this=0x555599a181d0) at src/cata_imgui.cpp:600
 #10 0x00005555569a2b1c in ui_adaptor::redraw_invalidated () at src/ui_manager.cpp:440
 #11 0x00005555569a2bd9 in ui_adaptor::redraw () at src/ui_manager.cpp:345
 #12 0x00005555569a2c00 in ui_manager::redraw () at src/ui_manager.cpp:508
 #13 0x000055555699bfa4 in uilist::inputfilter (this=this@entry=0x7fffffffb9c8) at src/ui.cpp:531
 #14 0x000055555699ead4 in uilist::query (this=this@entry=0x7fffffffb9c8, loop=loop@entry=true, timeout=timeout@entry=-1, allow_unfiltered_hotkeys=allow_unfiltered_hotkeys@entry=false) at src/ui.cpp:865
 #15 0x0000555555c610d4 in zone_manager::query_type (this=this@entry=0x555557274060 <zone_manager::get_manager()::manager>, personal=personal@entry=false) at src/clzones.cpp:621
 #16 0x0000555555ef3894 in game::zones_manager (this=this@entry=0x555558291db0) at src/game.cpp:6944
 #17 0x0000555555f5d279 in game::do_regular_action (this=this@entry=0x555558291db0, act=@0x7fffffffcfec: ACTION_ZONES, player_character=..., mouse_target=std::optional [no contained value]) at src/handle_action.cpp:2438
 #18 0x0000555555f60769 in game::handle_action (this=0x555558291db0) at src/handle_action.cpp:3172
 CleverRaven#19 0x0000555555dcf14d in do_turn () at src/do_turn.cpp:579
 CleverRaven#20 0x00005555557a1217 in main (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:873
```

gdb shows that `parent.selected` was negative:
```
(gdb) frame 7
138                                           parent.entries[parent.selected].desc.c_str()
(gdb) print parent.selected
$1 = -1
(gdb) print parent.entries.size()
$2 = 63
```
GuardianDll pushed a commit that referenced this pull request Oct 2, 2024
Prevents crashes that previously happened from using references to
`explosion_data` that had been invalidated.

The problem before happened when killing "unfolded impossibility" in
LIXA facility with a grenade, since this monster runs an eoc that
switches maps. That led to `explosion_handler::process_explosions` being
called twice recursively. First invocation iterates references, and the
second invocation might append to the vector, and most certaily will
clear the vector, thus invalidating references for the first invocation.

Example crash being fixed by this commit, notice how
`explosion_handler::process_explosions` occurs twice in the callstack:

```
Thread 1 "cataclysm-tiles" received signal SIGABRT, Aborted.
__pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44

(gdb) bt
 #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
 #1  0x00007ffff787840f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
 #2  0x00007ffff78294f2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
 #3  0x00007ffff78124ed in __GI_abort () at ./stdlib/abort.c:79
 #4  0x00007ffff7ad501e in std::__glibcxx_assert_fail(char const*, int, char const*, char const*) () from /lib/x86_64-linux-gnu/libstdc++.so.6
 #5  0x000055555688b471 in std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const*, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const*> >::operator[] (this=<optimized out>, __n=1967424379) at /usr/include/c++/14/bits/stl_vector.h:1128
 #6  std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const*, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const*> >::operator[] (this=<optimized out>, __n=1967424379) at /usr/include/c++/14/bits/stl_vector.h:1128
 #7  string_identity_static::get_interned_string[abi:cxx11](int) (id=1967424379) at src/string_id.cpp:51
 #8  0x0000555555fffed7 in string_identity_static::str[abi:cxx11]() const (this=<optimized out>) at src/string_id.h:140
 #9  0x0000555556057423 in string_id<itype>::c_str (this=this@entry=0x5555a126a718) at src/string_id.h:253
 #10 0x00005555560be56a in Item_factory::find_template (this=0x55555751cad0, id=...) at src/item_factory.cpp:2563
 #11 0x0000555555e4b0c5 in explosion_handler::_make_explosion (source=<optimized out>, p=..., ex=...) at /usr/include/c++/14/bits/unique_ptr.h:193
 #12 0x0000555555e4b704 in explosion_handler::process_explosions () at src/explosion.cpp:923
 #13 0x0000555556246e97 in map::actualize (this=this@entry=0x5555a12bf890, grid=...) at src/map.cpp:9168
 #14 0x00005555562472c0 in map::load (this=this@entry=0x5555a12bf890, w=..., update_vehicle=update_vehicle@entry=true, pump_events=pump_events@entry=false) at src/map.cpp:8387
 #15 0x0000555555de2546 in tinymap::load (this=this@entry=0x5555a12bf890, w=..., update_vehicles=update_vehicles@entry=true, pump_events=pump_events@entry=false) at src/map.h:2765
 #16 0x00005555562a7e6f in update_mapgen_function_json::update_map (this=0x55555a8ce8f0, omt_pos=..., args=..., offset=..., miss=miss@entry=0x0, verify=verify@entry=true, mirror_horizontal=false, mirror_vertical=false, rotation=0) at src/mapgen.cpp:8015
 #17 0x00005555562a81e3 in run_mapgen_update_func (update_mapgen_id=..., omt_pos=..., args=..., miss=miss@entry=0x0, cancel_on_collision=cancel_on_collision@entry=true, mirror_horizontal=mirror_horizontal@entry=false, mirror_vertical=false, rotation=0) at src/mapgen.cpp:8117
 #18 0x00005555565a0250 in operator() (__closure=<optimized out>, d=...) at src/npctalk.cpp:3964
 CleverRaven#19 0x000055555659c7a4 in talk_effect_t::apply (this=this@entry=0x55555d5211d8, d=...) at src/npctalk.cpp:6526
 CleverRaven#20 0x0000555555e0c9ba in effect_on_condition::activate (this=0x55555d521160, d=..., require_callstack_check=require_callstack_check@entry=true) at src/effect_on_condition.cpp:329
 CleverRaven#21 0x00005555561f9693 in spell_effect::effect_on_condition (sp=..., caster=..., target=...) at src/magic_spell_effect.cpp:1806
 CleverRaven#22 0x00005555561d101e in spell::cast_all_effects (this=0x7fffffffc788, source=..., target=...) at src/magic.cpp:1907
 CleverRaven#23 0x0000555556463bec in monster::die (this=0x55559c7fc0a0, nkiller=0x0) at src/monster.cpp:2941
 CleverRaven#24 0x0000555555d33386 in Creature::deal_projectile_attack (this=0x55559c7fc0a0, source=0x0, attack=..., print_messages=<optimized out>, wp_attack=...) at src/creature.cpp:1311
 CleverRaven#25 0x0000555556465049 in monster::deal_projectile_attack (this=this@entry=0x55559c7fc0a0, source=source@entry=0x0, attack=..., print_messages=print_messages@entry=false, wp_attack=...) at src/monster.cpp:2212
 CleverRaven#26 0x0000555555e4a9d9 in explosion_handler::shrapnel (range=-1, source=<optimized out>, src=..., power=<optimized out>, casing_mass=<optimized out>, per_fragment_mass=<optimized out>) at src/explosion.cpp:463
 CleverRaven#27 explosion_handler::_make_explosion (source=<optimized out>, p=..., ex=...) at src/explosion.cpp:536
 CleverRaven#28 0x0000555555e4b704 in explosion_handler::process_explosions () at src/explosion.cpp:923
 CleverRaven#29 0x0000555555dcf46f in do_turn () at src/do_turn.cpp:648
 CleverRaven#30 0x00005555557a1227 in main (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:873
```

In the crash above, the `explosion_data` has been invalidated:
```
(gdb) frame 11
193           pointer    _M_ptr() const noexcept { return std::get<0>(_M_t); }

(gdb) print ex
$1 = (const explosion_data &) @0x5555a126a6f8: {power = 1.75295132e+25, distance_factor = 2.76847299e+20, max_noise = 1667855474, fire = 117, shrapnel = {casing_mass = 1936026889, fragment_mass = 6.82915174e+22, recovery = -1584519120, drop = {_version = 13059389229367304, _cid = 2019155690, _id = {
        _id = 1967424379}}}}

(gdb) print ex.shrapnel.drop
$2 = {_version = 13059389229367304, _cid = 2019155690, _id = {_id = 1967424379}}
```
GuardianDll pushed a commit that referenced this pull request Oct 2, 2024
…aven#75521)

Prevents referencing `visibility_cache` using out-of-bounds array
indexes such as negative values. Using out-of-bound indexes for this
array previously caused crashes when compiled with
`-D_GLIBCXX_ASSERTIONS`.

The function `pixel_minimap::render_critters` is, for example, called
with `center=(64,59,-5)`, which gives `start=(4,-1)` and then
`p=(4,-1,-5)`, which previously crashed because
`visibility_cache[p.x][p.y]` then gives a negative array index. Such
values were seen when peeking using `X` at a submap boundary.

Gdb backtrace of previous crash being fixed:
```
(gdb) bt
 #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
 #1  0x00007ffff787840f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
 #2  0x00007ffff78294f2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
 #3  0x00007ffff78124ed in __GI_abort () at ./stdlib/abort.c:79
 #4  0x00007ffff7ad30be in std::__glibcxx_assert_fail(char const*, int, char const*, char const*) () from /lib/x86_64-linux-gnu/libstdc++.so.6
 #5  0x000055555668a04e in std::array<lit_level, 132ul>::operator[] (__n=18446744073709551615, this=0x555599f30348) at /usr/include/c++/13/array:211
 #6  0x000055555668ab7a in std::array<lit_level, 132ul>::operator[] (__n=18446744073709551615, this=0x555599f30348) at /usr/include/c++/13/array:211
 #7  pixel_minimap::render_critters (this=this@entry=0x555558108960, center=...) at src/pixel_minimap.cpp:521
 #8  0x000055555668ada2 in pixel_minimap::render (this=0x555558108960, center=...) at src/pixel_minimap.cpp:447
 #9  0x000055555668bb69 in pixel_minimap::draw (this=<optimized out>, screen_rect=..., center=...) at src/pixel_minimap.cpp:555
 #10 0x0000555555b3fd5c in cata_tiles::draw_minimap (this=this@entry=0x555558093020, dest=..., center=..., width=width@entry=352, height=height@entry=352) at src/cata_tiles.cpp:1919
 #11 0x00005555567f43d3 in cata_cursesport::curses_drawwindow (w=...) at src/sdltiles.cpp:1428
 #12 0x000055555666f5e2 in std::function<void(draw_args const&)>::operator() (__args#0=..., this=<optimized out>) at /usr/include/c++/13/bits/std_function.h:591
 #13 operator() (d=..., __closure=<optimized out>) at src/panels.cpp:54
 #14 std::__invoke_impl<int, window_panel::window_panel(const std::function<void(const draw_args&)>&, const std::string&, const translation&, int, int, bool, const std::function<bool()>&, bool)::<lambda(const draw_args&)>&, const draw_args&> (__f=...) at /usr/include/c++/13/bits/invoke.h:61
 #15 std::__invoke_r<int, window_panel::window_panel(const std::function<void(const draw_args&)>&, const std::string&, const translation&, int, int, bool, const std::function<bool()>&, bool)::<lambda(const draw_args&)>&, const draw_args&> (__fn=...) at /usr/include/c++/13/bits/invoke.h:114
 #16 std::_Function_handler<int(const draw_args&), window_panel::window_panel(const std::function<void(const draw_args&)>&, const std::string&, const translation&, int, int, bool, const std::function<bool()>&, bool)::<lambda(const draw_args&)> >::_M_invoke(const std::_Any_data &, const draw_args &) (__functor=...,     __args#0=...) at /usr/include/c++/13/bits/std_function.h:290
 #17 0x0000555555e7c9ec in std::function<int(draw_args const&)>::operator() (__args#0=..., this=0x55559461d638) at /usr/include/c++/13/bits/std_function.h:591
 #18 game::draw_panels (this=this@entry=0x555558276c40, force_draw=force_draw@entry=true) at src/game.cpp:4006
 CleverRaven#19 0x0000555555e984f8 in game::draw (this=0x555558276c40, ui=...) at src/game.cpp:3960
 CleverRaven#20 0x000055555695df35 in ui_adaptor::redraw_invalidated () at src/ui_manager.cpp:440
 CleverRaven#21 0x000055555695e039 in ui_adaptor::redraw () at src/ui_manager.cpp:345
 CleverRaven#22 0x000055555695e060 in ui_manager::redraw () at src/ui_manager.cpp:506
 CleverRaven#23 0x0000555555ea95cf in game::look_around (this=this@entry=0x555558276c40, show_window=show_window@entry=true, center=..., start_point=..., has_first_point=has_first_point@entry=false, select_zone=false, peeking=true, is_moving_zone=<optimized out>, end_point=..., change_lv=true) at src/game.cpp:7529
 CleverRaven#24 0x0000555555eadd32 in game::look_around (this=this@entry=0x555558276c40, looka_params=...) at src/game.cpp:7697
 CleverRaven#25 0x0000555555eadecb in game::peek (this=this@entry=0x555558276c40, p=...) at src/game.cpp:6071
 CleverRaven#26 0x0000555555ec441a in game::peek (this=this@entry=0x555558276c40) at src/game.cpp:6050
 CleverRaven#27 0x0000555555f124ec in game::do_regular_action (this=this@entry=0x555558276c40, act=@0x7fffffffd104: ACTION_PEEK, player_character=..., mouse_target=std::optional [no contained value]) at src/handle_action.cpp:2432
 CleverRaven#28 0x0000555555f15b09 in game::handle_action (this=0x555558276c40) at src/handle_action.cpp:3174
 CleverRaven#29 0x0000555555d8a108 in do_turn () at /usr/include/c++/13/bits/unique_ptr.h:199
 CleverRaven#30 0x0000555555781b4e in main (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:873

(gdb) frame 7
(gdb) print p
$1 = {static dimension = 3, x = 4, y = -1, z = -5}
(gdb) print center
$2 = (const tripoint &) @0x7fffffffbda8: {static dimension = 3, x = 64, y = 59, z = -5}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant