buildingplan: crash when attempting to construct building after save/load cycle

[lethosor]

To reproduce:

1. Load a game
2. Start planning a building, e.g. beds, with b-b-P
3. Unload the game (either by saving or with :lua df.global.save_on_exit = false)
4. Load another game
5. Start planning the same type of building as in (2) - b-b should result in a crash

I enabled debug symbols for buildingplan(-lib) with

diff --git a/plugins/CMakeLists.txt b/plugins/CMakeLists.txt
index 9f5b2c67..0cb27d85 100644
--- a/plugins/CMakeLists.txt
+++ b/plugins/CMakeLists.txt
@@ -76,6 +76,7 @@ set_source_files_properties( Brushes.h PROPERTIES HEADER_FILE_ONLY TRUE )

 add_library(buildingplan-lib STATIC buildingplan-lib.cpp buildingplan-planner.cpp buildingplan-rooms.cpp)
 target_link_libraries(buildingplan-lib dfhack)
+target_compile_options(buildingplan-lib PUBLIC -O0 -ggdb3)

 # Plugins
 option(BUILD_SUPPORTED "Build the supported plugins (reveal, probe, etc.)." ON)

and got the following backtrace:

#0  0x00007ffff7bdf477 in DFHack::MaterialInfo::toString(unsigned short, bool) const () at ./hack/libdfhack.so
#1  0x00007fffe8b21e2c in material_to_string_fn(DFHack::MaterialInfo const&) (m=...) at ../plugins/buildingplan-planner.cpp:166
#2  0x00007fffe8b2abe7 in std::transform<__gnu_cxx::__normal_iterator<DFHack::MaterialInfo const*, std::vector<DFHack::MaterialInfo, std::allocator<DFHack::MaterialInfo> > >, std::back_insert_iterator<std::vector<std::string, std::allocator<std::string> > >, std::string (*)(DFHack::MaterialInfo const&)>(__gnu_cxx::__normal_iterator<DFHack::MaterialInfo const*, std::vector<DFHack::MaterialInfo, std::allocator<DFHack::MaterialInfo> > >, __gnu_cxx::__normal_iterator<DFHack::MaterialInfo const*, std::vector<DFHack::MaterialInfo, std::allocator<DFHack::MaterialInfo> > >, std::back_insert_iterator<std::vector<std::string, std::allocator<std::string> > >, std::string (*)(DFHack::MaterialInfo const&)) (__first={static NUM_BUILTIN = 19, static GROUP_SIZE = 200, static CREATURE_BASE = 19, static FIGURE_BASE = 219, static PLANT_BASE = 419, static END_BASE = 619, type = 25956, index = 0, material = 0x31, mode = 3639306752, subtype = 32767, inorganic = 0x7fffac5ee680, creature = 0x2d2d2d2d2d2d2d2d, plant = 0x2d2d2d2d2d2d2d2d, figure = 0x30}, __last=<error reading variable: Cannot access memory at address 0x2d2d2d2d2d2d2d2d>, __result=..., __unary_op=0x7fffe8b21def <material_to_string_fn(DFHack::MaterialInfo const&)>) at /usr/include/c++/10/bits/stl_algo.h:4313
#3  0x00007fffe8b26b69 in transform_<DFHack::MaterialInfo, std::basic_string<char>, std::basic_string<char> (*)(DFHack::MaterialInfo const&)>(std::vector<DFHack::MaterialInfo, std::allocator<DFHack::MaterialInfo> > const&, std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > (*)(DFHack::MaterialInfo const&)) (src=std::vector of length 58127982886225979, capacity 58127982886225979 = {...}, dst=std::vector of length 0, capacity 0, func=0x7fffe8b21def <material_to_string_fn(DFHack::MaterialInfo const&)>) at ../plugins/uicommon.h:84
#4  0x00007fffe8b21ea8 in ItemFilter::getMaterials() const (this=0x7fffd8eb6a10) at ../plugins/buildingplan-planner.cpp:173
#5  0x00007fffe8b0b833 in buildingplan_place_hook::interpose_fn_render() (this=0x7fffb2673840) at ../plugins/buildingplan.cpp:787
#6  0x00007fffe8b0c0d1 in buildingplan_room_hook::interpose_fn_render() (this=0x7fffb2673840) at ../plugins/buildingplan.cpp:874
#7  0x00007fffe83fea84 in resume_hook::interpose_fn_render() () at hack/plugins/resume.plug.so
#8  0x00007fffe8fe58ee in trackstop_hook::interpose_fn_render() () at hack/plugins/trackstop.plug.so
#9  0x00007fffe8fe50fe in roller_hook::interpose_fn_render() () at hack/plugins/trackstop.plug.so
#10 0x00007fffe8ea9f7e in zone_hook::interpose_fn_render() () at hack/plugins/zone.plug.so
#11 0x00007fffe8ca9e5c in stocks_stockpile_hook::interpose_fn_render() () at hack/plugins/stocks.plug.so
#12 0x00007fffe8d0d5ac in stockpiles_import_hook::interpose_fn_render() () at hack/plugins/stockpiles.plug.so
#13 0x00007fffe86ddca9 in mousequery_hook::interpose_fn_render() () at hack/plugins/mousequery.plug.so
#14 0x00007fffe868899a in autochop_hook::interpose_fn_render() () at hack/plugins/autochop.plug.so
#15 0x00007ffff6e132f6 in render_things() () at libs/libgraphics.so
#16 0x00007ffff6e02407 in enablerst::async_loop() () at libs/libgraphics.so
#17 0x00007ffff6e024e0 in call_loop(void*) () at libs/libgraphics.so
#18 0x00007ffff7412f3c in  () at /lib/x86_64-linux-gnu/libSDL-1.2.so.0
#19 0x00007ffff7452baf in  () at /lib/x86_64-linux-gnu/libSDL-1.2.so.0
#20 0x00007ffff65f2590 in start_thread (arg=0x7ffff2bf7640) at pthread_create.c:463
#21 0x00007ffff67a0223 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

What I've found so far:

• materials is corrupted here (.size() gives an extremely large value): https://github.com/DFHack/dfhack/blob/923b1b14f33bf6c013c1bcc49270139242ec960b/plugins/buildingplan-planner.cpp#L173
• filter is assigned here, using an iterator: https://github.com/DFHack/dfhack/blob/923b1b14f33bf6c013c1bcc49270139242ec960b/plugins/buildingplan.cpp#L580 I suspect that this iterator may be invalidated when the loaded world changes, but I haven't figured out where that happens yet. Given the check at https://github.com/DFHack/dfhack/blob/923b1b14f33bf6c013c1bcc49270139242ec960b/plugins/buildingplan.cpp#L574, I suspect that this crash may not occur if using different building types in steps (2) and (5) above.

I'll try to continue investigating tomorrow, but @myk002, if you're able to take a look, that would certainly be appreciated. Unsure if this occurs in r3 or just on develop.

[myk002]

Oh, statics, you bite me dearly. No wonder they name you "antipattern".

What I suspect is happening here is that the ItemFilter vector is cleared and repopulated when the state is reset on map load, but the iterator in the static still points to the cleared vector data. And since you selected the same building type as the cached iterator, it did not get reset before it was used. Excellent excellent catch. Thank you!

[myk002]

yup, reproduced the issue and verified the fix. thank you again for finding this.