Skip to content

Appliance: drop cables when taking down appliance#60621

Merged
Fris0uman merged 2 commits intoCleverRaven:masterfrom
dseguin:app_takedown_drop_cables
Sep 1, 2022
Merged

Appliance: drop cables when taking down appliance#60621
Fris0uman merged 2 commits intoCleverRaven:masterfrom
dseguin:app_takedown_drop_cables

Conversation

@dseguin
Copy link
Member

@dseguin dseguin commented Aug 31, 2022

Summary

None

Purpose of change

Describe the solution

When taking down an appliance, call vehicle::shed_loose_parts to handle dropping any power connections. Any items used to connect two or more appliances and/or vehicles will be dropped at the appliances position.

Describe alternatives you've considered

N/A

Testing

Taking down a standing lamp connected in various ways to other appliances and a vehicle:

2022-08-31.01-03-24.mp4

Additional context

While testing this originally, I ran into a bug where an appliance had a fake part while being taken down. So instead of calling map::destroy_vehicle, the parts list gets accessed after the real part is removed, causing this use-after-free output from ASAN:

(snip)
==48386==ERROR: AddressSanitizer: heap-use-after-free on address 0x61c00019ac20 at pc 0x55555eddea45 bp 0x7fffffff91d0 sp 0x7fffffff91c0
READ of size 8 at 0x61c00019ac20 thread T0
    #0 0x55555eddea44 in std::_Rb_tree<point, std::pair<point const, std::vector<int, std::allocator<int> > >, std::_Select1st<std::pair<point const, std::vector<int, std::allocator<int> > > >, std::less<point>, std::allocator<std::pair<point const, std::vector<int, std::allocator<int> > > > >::_M_begin() const /usr/include/c++/9/bits/stl_tree.h:756
    #1 0x55555edd4b93 in std::_Rb_tree<point, std::pair<point const, std::vector<int, std::allocator<int> > >, std::_Select1st<std::pair<point const, std::vector<int, std::allocator<int> > > >, std::less<point>, std::allocator<std::pair<point const, std::vector<int, std::allocator<int> > > > >::find(point const&) const /usr/include/c++/9/bits/stl_tree.h:2575
    #2 0x55555edc1862 in std::map<point, std::vector<int, std::allocator<int> >, std::less<point>, std::allocator<std::pair<point const, std::vector<int, std::allocator<int> > > > >::find(point const&) const /usr/include/c++/9/bits/stl_map.h:1194
    #3 0x55555ed25418 in vehicle::parts_at_relative(point const&, bool, bool) const src/vehicle.cpp:2418
    #4 0x55555ebfd3e9 in veh_interact::complete_vehicle(Character&) src/veh_interact.cpp:3514
    #5 0x5555595549fc in activity_handlers::vehicle_finish(player_activity*, Character*) src/activity_handlers.cpp:1863
    #6 0x5555595a33d9 in std::_Function_handler<void (player_activity*, Character*), void (*)(player_activity*, Character*)>::_M_invoke(std::_Any_data const&, player_activity*&&, Character*&&) /usr/include/c++/9/bits/std_function.h:300
    #7 0x5555596c65ff in std::function<void (player_activity*, Character*)>::operator()(player_activity*, Character*) const /usr/include/c++/9/bits/std_function.h:688
    #8 0x5555596c19b5 in activity_type::call_finish(player_activity*, Character*) const src/activity_type.cpp:118
    #9 0x55555de69d15 in player_activity::do_turn(Character&) src/player_activity.cpp:378
    #10 0x55555abfe103 in do_turn() src/do_turn.cpp:665
    #11 0x55555c3e20d4 in main src/main.cpp:789
    #12 0x7ffff6e7d082 in __libc_start_main ../csu/libc-start.c:308
    #13 0x5555592498bd in _start (/home/dtsadmin/Builds/Cataclysm-DDA/cataclysm-tiles+0x3cf58bd)

0x61c00019ac20 is located 928 bytes inside of 1792-byte region [0x61c00019a880,0x61c00019af80)
freed by thread T0 here:
    #0 0x7ffff768ec65 in operator delete(void*, unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cc:177
    #1 0x55555acc7c40 in std::default_delete<vehicle>::operator()(vehicle*) const /usr/include/c++/9/bits/unique_ptr.h:81
    #2 0x55555acbc7ac in std::unique_ptr<vehicle, std::default_delete<vehicle> >::~unique_ptr() /usr/include/c++/9/bits/unique_ptr.h:292
    #3 0x55555c42f35a in map::destroy_vehicle(vehicle*) src/map.cpp:515
    #4 0x55555ed1b923 in vehicle::part_removal_cleanup() src/vehicle.cpp:1925
    #5 0x55555ebfd340 in veh_interact::complete_vehicle(Character&) src/veh_interact.cpp:3513
    #6 0x5555595549fc in activity_handlers::vehicle_finish(player_activity*, Character*) src/activity_handlers.cpp:1863
    #7 0x5555595a33d9 in std::_Function_handler<void (player_activity*, Character*), void (*)(player_activity*, Character*)>::_M_invoke(std::_Any_data const&, player_activity*&&, Character*&&) /usr/include/c++/9/bits/std_function.h:300
    #8 0x5555596c65ff in std::function<void (player_activity*, Character*)>::operator()(player_activity*, Character*) const /usr/include/c++/9/bits/std_function.h:688
    #9 0x5555596c19b5 in activity_type::call_finish(player_activity*, Character*) const src/activity_type.cpp:118
    #10 0x55555de69d15 in player_activity::do_turn(Character&) src/player_activity.cpp:378
    #11 0x55555abfe103 in do_turn() src/do_turn.cpp:665
    #12 0x55555c3e20d4 in main src/main.cpp:789
    #13 0x7ffff6e7d082 in __libc_start_main ../csu/libc-start.c:308

previously allocated by thread T0 here:
    #0 0x7ffff768d587 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cc:104
    #1 0x55555e4d5ce7 in std::_MakeUniq<vehicle>::__single_object std::make_unique<vehicle>() /usr/include/c++/9/bits/unique_ptr.h:857
    #2 0x55555e49e3a9 in submap::load(JsonIn&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int) src/savegame_json.cpp:5033
    #3 0x55555c67c994 in mapbuffer::deserialize(JsonIn&) src/mapbuffer.cpp:306
    #4 0x55555c67bb4c in operator() src/mapbuffer.cpp:276
    #5 0x55555c67d4b9 in _M_invoke /usr/include/c++/9/bits/std_function.h:300
    #6 0x555559eb0ac8 in std::function<void (JsonIn&)>::operator()(JsonIn&) const /usr/include/c++/9/bits/std_function.h:688
    #7 0x555559ea8473 in operator() src/cata_utility.cpp:436
    #8 0x555559eac44e in _M_invoke /usr/include/c++/9/bits/std_function.h:300
    #9 0x555559eb0a48 in std::function<void (std::istream&)>::operator()(std::istream&) const /usr/include/c++/9/bits/std_function.h:688
    #10 0x555559ea7628 in read_from_file(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void (std::istream&)> const&) src/cata_utility.cpp:392
    #11 0x555559ea8341 in read_from_file_optional(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void (std::istream&)> const&) src/cata_utility.cpp:428
    #12 0x555559ea85f5 in read_from_file_optional_json(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void (JsonIn&)> const&) src/cata_utility.cpp:434
    #13 0x55555c67c0c9 in mapbuffer::unserialize_submaps(coords::coord_point<tripoint, (coords::origin)1, (coords::scale)1> const&) src/mapbuffer.cpp:275
    #14 0x55555c67968a in mapbuffer::lookup_submap(coords::coord_point<tripoint, (coords::origin)1, (coords::scale)1> const&) src/mapbuffer.cpp:112
    #15 0x55555c4a59e2 in map::loadn(tripoint const&, bool, bool) src/map.cpp:7555
    #16 0x55555c4a704e in map::loadn(point const&, bool, bool) src/map.cpp:7651
    #17 0x55555c4a0952 in map::load(coords::coord_point<tripoint, (coords::origin)1, (coords::scale)1> const&, bool, bool) src/map.cpp:7230
    #18 0x55555b12bf5b in game::load_map(coords::coord_point<tripoint, (coords::origin)1, (coords::scale)1> const&, bool) src/game.cpp:799
    #19 0x55555e38d6ee in game::unserialize(std::istream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/savegame.cpp:236
    #20 0x55555b15cdf1 in operator() src/game.cpp:2757
    #21 0x55555b25f361 in _M_invoke /usr/include/c++/9/bits/std_function.h:300
    #22 0x555559eb0a48 in std::function<void (std::istream&)>::operator()(std::istream&) const /usr/include/c++/9/bits/std_function.h:688
    #23 0x555559ea7628 in read_from_file(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void (std::istream&)> const&) src/cata_utility.cpp:392
    #24 0x55555b15db5e in game::load(save_t const&) src/game.cpp:2755
    #25 0x55555c40b387 in main_menu::load_character_tab(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/main_menu.cpp:1015
    #26 0x55555c405f78 in main_menu::opening_screen() src/main_menu.cpp:819
    #27 0x55555c3e2017 in main src/main.cpp:783
    #28 0x7ffff6e7d082 in __libc_start_main ../csu/libc-start.c:308

SUMMARY: AddressSanitizer: heap-use-after-free /usr/include/c++/9/bits/stl_tree.h:756 in std::_Rb_tree<point, std::pair<point const, std::vector<int, std::allocator<int> > >, std::_Select1st<std::pair<point const, std::vector<int, std::allocator<int> > > >, std::less<point>, std::allocator<std::pair<point const, std::vector<int, std::allocator<int> > > > >::_M_begin() const
Shadow bytes around the buggy address:
  0x0c388002b530: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b540: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b550: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b560: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b570: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c388002b580: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b590: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b5a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b5b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b5c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c388002b5d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd 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
  Shadow gap:              cc
==48386==ABORTING

The fix I used was to skip fake parts when checking the remaining part count (2604d39)

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Vehicles Vehicles, parts, mechanics & interactions <Bugfix> This is a fix for a bug (or closes open issue) astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Aug 31, 2022
@Fris0uman Fris0uman merged commit 7e5f5bf into CleverRaven:master Sep 1, 2022
@dseguin dseguin deleted the app_takedown_drop_cables branch September 3, 2022 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions Vehicles Vehicles, parts, mechanics & interactions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extension cord disappears when taking down appliance

2 participants