Vectors in copy constructor causes heap corruption.

Started by
21 comments, last by KazenoZ 12 years, 8 months ago
Hello,

I'm feeling that this is a very stupid problem on my part, but I just could not for the life of me figure it out in over a week now =\

I have a class called Event, and that has a member vector<string> commandList. Now, when I call the copy constructor, I want the commandList from the input to be copied to the new class.

So far I tried various ways:
Event::Event(const Event& it){
this->commandList = it.commandList;
}

Event::Event(const Event& it){
//Both with and without this->commandList.reserve(it.commandList.size());
for(unsigned int i = 0; i < it.commandList; ++i){
this->commandList.push_back(it.commandList);
}
}

Event::Event(const Event& it){
this->commandList.resize(it.commandList.size());
for(unsigned int i = 0; i < it.commandList; ++i){
this->commandList = it.commandList;
}
}


I tried using substrings, inserts, assigns and so many other ways, I even tried making a new string vector inside the ctor, just to check if the problem is with the source or target, but that works, the problem seems to be with the target vector, to weird thing is that it will start copying the members, but it'll give the error somewhere midway along the loop, if I had 39 objects, it will always crash at 28, for example, even though the capacity is already set to 39...

Whatever I tried, it will ALWAYS end up in the heap corruption error message
[size="1"]HEAP[GameEngine.exe]: HEAP: Free Heap block 3f9350 modified at 3f9d64 after it was freed

Windows has triggered a breakpoint in GameEngine.exe.

This may be due to a corruption of the heap, which indicates a bug in GameEngine.exe or any of the DLLs it has loaded.

This may also be due to the user pressing F12 while GameEngine.exe has focus.

The output window may have more diagnostic information.

HEAP[GameEngine.exe]: HEAP: Free Heap block 3f9d68 modified at 3fb118 after it was freed

Windows has triggered a breakpoint in GameEngine.exe.

This may be due to a corruption of the heap, which indicates a bug in GameEngine.exe or any of the DLLs it has loaded.

This may also be due to the user pressing F12 while GameEngine.exe has focus.

The output window may have more diagnostic information.

[/quote]

I'm just lost for ideas by this point, and I hope that someone can help me shed light on this =\
Advertisement
Maybe the problem is not in the class Event, but in the usage you make of it (you try to use an invalid event).

Without more code, we can't tell. You should show the full Event class, and also a minimalist usage that reproduces the problem.
There's nothing much else in the Event class to view though, it's all just a bunch of integer and boolean members, and they all get copied fine in the constructor, it's just the vector that causes problems.

I couldn't manage to reproduce the problem in a minimal program, as I don't even know where to start trying to =\

But in the program this goes in, this is usually the last frame in the stack before the ctor is called:

//...more value setting comes before this point
evC.y = it.y;
evC.name = "exEvent";
evC.solid = 2;

evList.push_back(evC);
++EVENT_COUNT;

it.cmdCount = 0;
++it.evCount;


evC and it are both Events, evList is a vector of Events.
The stack will point to ++it.evCount, saying that it is uninitiallized, though looking at the evC that drew values from it, and the copy constructor in the next stack frame, the source values are fine, and the crash still seems to come from the string vector... Again, I'm not really sure about what happens here...

Your thread title states that the copy construction causes heap corruption, but I don't think that's true. More likely, it reveals previously existing heap corruption as it makes a new allocation. The code you posted for the copy constructor is fine as far as I can tell (though it is unnecessary; the default copy constructor is probably fine).
Right... so... where do I look now, actually? What kind of operations do you reckon could cause this? Debugging seems to suggest the push_back method, but I just don't know, the values are valid before it and then corrupt after use, I don't really know where to look now...

Edit:
I tried using the console to make sure about the corruption around the push_back method, and it does seem to crash inside it, the call stack shows:
GameEngine.exe!std::_Construct<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::basic_string<char,std::char_traits<char>,std::allocator<char> > >(std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Ptr="", const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & _Val="Number: (zIndex2` add` set` 1)0") Line 52 + 0x36 bytes C++
GameEngine.exe!std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >::construct(std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Ptr="", const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & _Val="Number: (zIndex2` add` set` 1)0") Line 155 + 0xd bytes C++
GameEngine.exe!std::_Uninit_copy<std::basic_string<char,std::char_traits<char>,std::allocator<char> > *,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> > * _First="Number: (zIndex2` add` set` 1)0", std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Last=<Bad Ptr>, std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Dest="", std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > & _Al={...}, std::_Nonscalar_ptr_iterator_tag __formal={...}, std::_Nonscalar_ptr_iterator_tag __formal={...}) Line 131 + 0x10 bytes C++
GameEngine.exe!stdext::unchecked_uninitialized_copy<std::basic_string<char,std::char_traits<char>,std::allocator<char> > *,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> > * _First="Number: (zIndex2` add` set` 1)0", std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Last=<Bad Ptr>, std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Dest="", std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > & _Al={...}) Line 822 + 0x55 bytes C++
GameEngine.exe!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> > > >::_Ucopy<std::basic_string<char,std::char_traits<char>,std::allocator<char> > *>(std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _First="Number: (zIndex2` add` set` 1)0", std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Last=<Bad Ptr>, std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Ptr="") Line 1141 + 0x18 bytes C++
GameEngine.exe!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> > > >::operator=(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> > > > & _Right=[3]("Number: (zIndex2` add` set` 1)0","CreateEv: (zIndex2` ev` 7` Zombee` static` 2` 2` 16` 16` 8` Zombee)0","Wait: (set` 10000)0")) Line 595 + 0x1d bytes C++
> GameEngine.exe!Event::Event(const Event & it={...}) Line 647 C++
GameEngine.exe!std::_Construct<Event,Event>(Event * _Ptr=0x00ca73f0, const Event & _Val={...}) Line 52 + 0x36 bytes C++
GameEngine.exe!std::allocator<Event>::construct(Event * _Ptr=0x00ca73f0, const Event & _Val={...}) Line 155 + 0xd bytes C++
GameEngine.exe!std::_Uninit_copy<Event *,Event *,std::allocator<Event> >(Event * _First=0x003faae0, Event * _Last=0x003fb0bc, Event * _Dest=0x00ca73f0, std::allocator<Event> & _Al={...}, std::_Nonscalar_ptr_iterator_tag __formal={...}, std::_Nonscalar_ptr_iterator_tag __formal={...}) Line 131 + 0x10 bytes C++
GameEngine.exe!stdext::unchecked_uninitialized_copy<Event *,Event *,std::allocator<Event> >(Event * _First=0x003f9f28, Event * _Last=0x003fb0bc, Event * _Dest=0x00ca6838, std::allocator<Event> & _Al={...}) Line 822 + 0x55 bytes C++
GameEngine.exe!std::_Uninit_move<Event *,Event *,std::allocator<Event>,std::_Undefined_move_tag>(Event * _First=0x003f9f28, Event * _Last=0x003fb0bc, Event * _Dest=0x00ca6838, std::allocator<Event> & _Al={...}, std::_Undefined_move_tag __formal={...}, std::_Undefined_move_tag __formal={...}) Line 207 + 0x15 bytes C++
GameEngine.exe!stdext::_Unchecked_uninitialized_move<Event *,Event *,std::allocator<Event> >(Event * _First=0x003f9f28, Event * _Last=0x003fb0bc, Event * _Dest=0x00ca6838, std::allocator<Event> & _Al={...}) Line 864 + 0x51 bytes C++
GameEngine.exe!std::vector<Event,std::allocator<Event> >::_Umove<Event *>(Event * _First=0x003f9f28, Event * _Last=0x003fb0bc, Event * _Ptr=0x00ca6838) Line 1148 + 0x18 bytes C++
GameEngine.exe!std::vector<Event,std::allocator<Event> >::_Insert_n(std::_Vector_const_iterator<Event,std::allocator<Event> > _Where={index=-1414812757 name=<Bad Ptr> pic=0x003fc950 ...}, unsigned int _Count=1, const Event & _Val={...}) Line 1182 C++
GameEngine.exe!std::vector<Event,std::allocator<Event> >::insert(std::_Vector_const_iterator<Event,std::allocator<Event> > _Where={index=-1414812757 name=<Bad Ptr> pic=0x003fc950 ...}, const Event & _Val={...}) Line 878 C++
GameEngine.exe!std::vector<Event,std::allocator<Event> >::push_back(const Event & _Val={...}) Line 824 C++
GameEngine.exe!eventResolver(std::vector<Event,std::allocator<Event> > & evList=[15]({index=1 name="" pic=0xbaadf00d ...},{index=6 name="" pic=0xbaadf00d ...},{index=5 name="" pic=0xbaadf00d ...},{index=6001 name="" pic=0x01ec4ae0 ...},{index=5001 name="" pic=0x027b0008 ...},{index=3 name="" pic=0xbaadf00d ...},{index=2 name="" pic=0xbaadf00d ...},{index=3001 name="" pic=0x028f4008 ...},{index=1001 name="" pic=0x01ec4bf0 ...},{index=1001 name="" pic=0x023a0008 ...},{index= na,...), std::vector<Event,std::allocator<Event> > & gevList=[3]({index=-858993460 name="-1" pic=0xcccccccc ...},{index=-858993460 name="Zombee" pic=0xcccccccc ...},{index=-858993460 name="Damage" pic=0xcccccccc ...}), Event & it={...}, Chara & ch={...}, SDL_Surface * screen=0x003b6328, Project & p={...}, std::vector<vPoint,std::allocator<vPoint> > & pList=[1]({x=512 y=736 h=-858993460 ...}), std::vector<vNum,std::allocator<vNum> > & numList=[6]({x=1001.0000 name="zIndex" },{x=2001.0000 name="zIndex2" },{x=3001.0000 name="zIndex3" },{x=4001.0000 name="zIndex4" },{x=5001.0000 name="zIndex5" },{x=6001.0000 name="zIndex6" }), std::vector<vText,std::allocator<vText> > & strList=[0](), std::vector<vSwitch,std::allocator<vSwitch> > & sList=[2]({x=false name="Attacking" },{x=false name="Hit" }), std::vector<vPic,std::allocator<vPic> > & picList=[1]({x=0x003bff78 basePic=0x003bff78 name="Zombee" ...}), std::vector<vSound,std::allocator<vSound> > & sndList=[0](), int scale=2) Line 9527 C++
GameEngine.exe!Map::Play(Project project={...}, Chara ch={...}, std::basic_string<char,std::char_traits<char>,std::allocator<char> > data="~Properties{Name: [Zombees]Path: [D:\Mor'sC++\Phantasya Projects\Copy of Zombees\Zombees\Zombees.gpj]}~Settings{Font: [D:\Mor'sC++\Phantasya Projects\Copy of Zombees\\Zombees\font.ttf]TBox: [D:\Mor'sC++\Phantasya Projects\Copy of Zombees\\Zombees\messageBox.png]Hero: [D:\Mor'sC++\Phantasya Projects\Copy of Zombees\\Zombees\Kyou Sword.PNG]Width: [24]Height: [32]Frames: [4]ScreenW: [800]ScreenH:" ) Line 1027 + 0x76 bytes C++
GameEngine.exe!SDL_main(int argc=2, char * * argv=0x003f37d8) Line 124 C++
GameEngine.exe!_main() + 0xf5 bytes C
GameEngine.exe!__tmainCRTStartup() Line 586 + 0x17 bytes C


Lots of frames regarding the vectors and their operations, and the 2 frames that matter are the Event's copy constructor which points to the command list, like I stated above, and the EventResolver that points to the push_back method .


Edit 2:
Digging a little into the stack brought me this:
The frame above the eventResolver is push_back's code:
else
insert(end(), _Val);
}

Which calls the frame above it, insert, with the value to be put at the end of the vector, so far so good
This is the insert code from the stack:
iterator insert(const_iterator _Where, const _Ty& _Val)
{ // insert _Val at _Where
size_type _Off = size() == 0 ? 0 : _Where - begin();
_Insert_n(_Where, (size_type)1, _Val);
return (begin() + _Off);
}

Now, _Val still has the value that I need to push back, but the _Where iterator apparently points to an uninitialized Event, so that's probably where it starts going wrong. But why is that?
Default copy constructor will be fine for a class containing a std::vector of std::strings, no need to implement your own. What happens if you omit your own copy constructor completely?

Please show the entire event class - this is probably a violation of the rule of three somewhere.
Basically, I need an explicit copy constructor because I need to condition some of the members.
I.E
if(!it.noImg){
this->pic = it.pic;
this->basePic = it.basePic;
}

And not for the vector, but as to my understanding, don't I need to explicitly assign all members in case I do use a cctor?

Also, doesn't the Rule of Three state that if I have an explicit ctor, dtor or cctor, I need to make ones for the other 2 as well? In which case, I need to have a basic constructor to intialize my values correctly, so that would mean I need both a copy constructor and a destructor defined as well.
Either way, I have all 3 in the class =\

As par your question about removing the copy constructor, that doesn't do much good, the only difference it does is that now it doesn't show me the stack when it crashes =\
Do you mean to shallow copy that picture? How is it's lifecycle managed? If you delete it in the destructor you'd be in for a world of hurt.
That class looks like a blob ! Maybe fixing this will lead you to finding the problem (or to making it disappear). What is an Event by the way ? it seems to combine the attributes of an actual event, of an entity, and more ...

There is still the possibility that you are *using* the class incorrectly (ie: the problem is outside the class, like using a deleted Event). How about showing eventResolver ?

Also, doesn't the Rule of Three state that if I have an explicit ctor, dtor or cctor, I need to make ones for the other 2 as well? In which case, I need to have a basic constructor to intialize my values correctly, so that would mean I need both a copy constructor and a destructor defined as well.


No. Rule of three is that if you need any one of the following - copy constructor, destructor or assignment operator - you probably need all three. Constructors themselves are not really relevant to rule of three.

Could you show the body of your destructor for event? I agree with Slavik81 - shallow copy of the SDL_Image is the first thing I'd question.

This topic is closed to new replies.

Advertisement