-
Couldn't load subscription status.
- Fork 22
Description
There is an iterator invalidation problem in the for loop in StructureManager::updateStructures:
for (auto* structure : structures)
{
updateStructure(resources, population, *structure);
}The problem comes from how the MineFacility can dig new levels and create new MineShaft entries, which happens during iteration of the above loop. Any resize of the underlying collection caused by the new MineShaft will invalidate the for loop iterator, leading to undefined behavior upon the next iteration.
With knowledge of the current size() and capacity() of the mDeployedStructures collection, we can create the right number of Structure instances such that a new MineShaft will cause an allocation.
The problem was validated using valgrind:
==69052== Invalid read of size 8
==69052== at 0x1B91FC: StructureManager::updateStructures(StorableResources const&, PopulationPool&, std::vector<Structure*, std::allocator<Structure*> >&) (StructureManager.cpp:673)
==69052== by 0x1B9237: StructureManager::update(StorableResources const&, PopulationPool&) (StructureManager.cpp:636)
==69052== by 0x1AC512: MapViewState::nextTurn() (MapViewStateTurn.cpp:574)
==69052== by 0x19ACF9: MapViewState::onKeyDown(NAS2D::KeyCode, NAS2D::KeyModifier, bool) (MapViewState.cpp:523)
==69052== by 0x1E9058: NAS2D::DelegateX<void, NAS2D::KeyCode, NAS2D::KeyModifier, bool>::operator()(NAS2D::KeyCode, NAS2D::KeyModifier, bool) const (Delegate.h:411)
==69052== by 0x1E9A63: NAS2D::Signal<NAS2D::KeyCode, NAS2D::KeyModifier, bool>::emit(NAS2D::KeyCode, NAS2D::KeyModifier, bool) const (Signal.h:49)
==69052== by 0x1E8B64: operator() (Signal.h:53)
==69052== by 0x1E8B64: NAS2D::EventHandler::onMessage(SDL_Event const&) (EventHandler.cpp:599)
==69052== by 0x1E8DDC: NAS2D::EventHandler::pump() (EventHandler.cpp:585)
==69052== by 0x1D56D1: NAS2D::StateManager::update() (StateManager.cpp:71)
==69052== by 0x135E53: main (main.cpp:178)
==69052== Address 0x1caacaa0 is 64 bytes inside a block of size 1,024 free'd
==69052== at 0x484A61D: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==69052== by 0x12F795: std::__new_allocator<Structure*>::deallocate(Structure**, unsigned long) (new_allocator.h:172)
==69052== by 0x1B94BF: deallocate (allocator.h:210)
==69052== by 0x1B94BF: deallocate (alloc_traits.h:517)
==69052== by 0x1B94BF: _M_deallocate (stl_vector.h:390)
==69052== by 0x1B94BF: void std::vector<Structure*, std::allocator<Structure*> >::_M_realloc_insert<Structure*>(__gnu_cxx::__normal_iterator<Structure**, std::vector<Structure*, std::allocator<Structure*> > >, Structure*&&) (vector.tcc:519)
==69052== by 0x1B953C: Structure*& std::vector<Structure*, std::allocator<Structure*> >::emplace_back<Structure*>(Structure*&&) (vector.tcc:123)
==69052== by 0x1B6C56: push_back (stl_vector.h:1299)
==69052== by 0x1B6C56: StructureManager::addStructure(Structure&, Tile&) (StructureManager.cpp:151)
==69052== by 0x1B6CA8: StructureManager::create(StructureID, Tile&) (StructureManager.cpp:139)
==69052== by 0x188EC3: MapViewState::onMineFacilityExtend(MineFacility*) (MapViewStateEvent.cpp:264)
==69052== by 0x14DD9E: NAS2D::DelegateX<void, MineFacility*>::operator()(MineFacility*) const (Delegate.h:411)
==69052== by 0x14DC21: MineFacility::think() (MineFacility.cpp:79)
==69052== by 0x1B6AC3: StructureManager::updateStructure(StorableResources const&, PopulationPool&, Structure&) (StructureManager.cpp:749)
==69052== by 0x1B920C: StructureManager::updateStructures(StorableResources const&, PopulationPool&, std::vector<Structure*, std::allocator<Structure*> >&) (StructureManager.cpp:675)
==69052== by 0x1B9237: StructureManager::update(StorableResources const&, PopulationPool&) (StructureManager.cpp:636)
==69052== Block was alloc'd at
==69052== at 0x4846FA3: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==69052== by 0x12FBCD: std::__new_allocator<Structure*>::allocate(unsigned long, void const*) (new_allocator.h:151)
==69052== by 0x1B946E: allocate (allocator.h:198)
==69052== by 0x1B946E: allocate (alloc_traits.h:482)
==69052== by 0x1B946E: _M_allocate (stl_vector.h:381)
==69052== by 0x1B946E: void std::vector<Structure*, std::allocator<Structure*> >::_M_realloc_insert<Structure*>(__gnu_cxx::__normal_iterator<Structure**, std::vector<Structure*, std::allocator<Structure*> > >, Structure*&&) (vector.tcc:459)
==69052== by 0x1B953C: Structure*& std::vector<Structure*, std::allocator<Structure*> >::emplace_back<Structure*>(Structure*&&) (vector.tcc:123)
==69052== by 0x1B6C56: push_back (stl_vector.h:1299)
==69052== by 0x1B6C56: StructureManager::addStructure(Structure&, Tile&) (StructureManager.cpp:151)
==69052== by 0x1B6CA8: StructureManager::create(StructureID, Tile&) (StructureManager.cpp:139)
==69052== by 0x18F7F7: MapViewState::readStructures(NAS2D::Xml::XmlElement*) (MapViewStateIO.cpp:399)
==69052== by 0x1921F2: MapViewState::load(SavedGameFile&) (MapViewStateIO.cpp:267)
==69052== by 0x19B9AE: MapViewState::initialize() (MapViewState.cpp:292)
==69052== by 0x1A633C: GameState::initializeGameState() (GameState.cpp:59)
==69052== by 0x1A63EC: GameState::initialize() (GameState.cpp:81)
==69052== by 0x1D56A0: NAS2D::StateManager::setState(NAS2D::State*) (StateManager.cpp:59)