Sign in to follow this  
Modena360

STL std::vector problems

Recommended Posts

Hi, and thanks for reading... My question is related to the std::vector container. I've been using them in commercial applications for several years without any issues, all coding during this period was done with VS2003. Recently however (on a new machine and using VS2008) I'm finding that the debug versions of my programs will crash often, and it's always on a push_back/insert line related to one of my vectors. Here is the exact error I'm getting: > Sim.exe!_crt_debugger_hook(int _Reserved=1243120) Line 65 C Sim.exe!_invalid_parameter(const wchar_t * pszExpression=0x00000000, const wchar_t * pszFunction=0x00000000, const wchar_t * pszFile=0x00000000, unsigned int nLine=0, unsigned int pReserved=0) Line 112 + 0x7 bytes C++ Sim.exe!_invalid_parameter_noinfo() Line 125 + 0xc bytes C++ Sim.exe!std::_Vector_const_iterator<CARGCommentaryLine,std::allocator<CARGCommentaryLine> >::operator+=(int _Off=0) Line 160 + 0x14 bytes C++ Sim.exe!std::_Vector_iterator<CARGCommentaryLine,std::allocator<CARGCommentaryLine> >::operator+=(int _Off=0) Line 376 C++ Sim.exe!std::_Vector_iterator<CARGCommentaryLine,std::allocator<CARGCommentaryLine> >::operator+(int _Off=0) Line 382 + 0xc bytes C++ Sim.exe!std::vector<CARGCommentaryLine,std::allocator<CARGCommentaryLine> >::insert(std::_Vector_const_iterator<CARGCommentaryLine,std::allocator<CARGCommentaryLine> > _Where={mi_quarter=??? mi_secs=??? mdw_commentator=??? ...}, const CARGCommentaryLine & _Val={...}) Line 878 + 0x1b bytes C++ Sim.exe!std::vector<CARGCommentaryLine,std::allocator<CARGCommentaryLine> >::push_back(const CARGCommentaryLine & _Val={...}) Line 824 C++ Sim.exe!CARMatch::addCommentary(int type=100, unsigned long colour=4278190280, int timeDelay=0) Line 50 C++ Sim.exe!CARMatch::setupMatch(int team1code=1005, int team2code=1002, std::basic_string<char,std::char_traits<char>,std::allocator<char> > groundName="Casey Fields") Line 216 C++ Sim.exe!Launch_Match(CSeason & season={...}, int fixture=2) Line 1849 C++ Sim.exe!CSimGame::ContinueSimulation() Line 1112 + 0x13 bytes C++ Sim.exe!CGame::Read_Input() Line 471 C++ Sim.exe!CGame::Play() Line 212 C++ Sim.exe!WinMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ * hPrevInstance=0x00000000, char * lpCmdLine=0x001733fc, int nCmdShow=1) Line 119 C++ Sim.exe!__tmainCRTStartup() Line 263 + 0x1b bytes C kernel32.dll!76c23833() [Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll] ntdll.dll!774ca9bd() The CARGCommentaryLine class is fairly simple, the only significant thing really is a pointer to a CARMatch class (the one that created the CARGCommentaryLine in the first place). To me it seems that the addCommentary function (which has the push_back command at the end of it) is the cause of the problem, and that's where I've been looking. However my gut instinct tells me that somehow the stl::vector memory is getting corrupted, however I don't see how this could happen as I've tried moving the push_back command to literally the next instruction after the container is created and it still crashes. Any help would be greatly appreciated, I'm not an expert on the STL, and this is the first time that it's causing me problems. I've played around with including the #define _HAS_ITERATOR_DEBUGGING 0 #define _SECURE_SCL 0 lines and when enabled these actually make it crash earlier, on the first std::vector it encounters. I've got my fingers crossed that this is straight forward and that I'm gonna be embarrassed for missing something obvious! Thanks in advance, Modena

Share this post


Link to post
Share on other sites
What does your CARGCommentaryLine class look like? Or more specifically, does it obey the rule of three, or would you be able to post the copy-constructor, assignment operator and destructor here, along with the member variables of the class?

Share this post


Link to post
Share on other sites
Here is the definition of the CARGCommentaryLine class. Thanks for your help.



// Commentary line
class CARGCommentaryLine
{
private:

// Timing and actor variables
int mi_quarter; // What quarter was this commentary line uttered in
int mi_secs; // How many seconds were left in the quarter
unsigned long mdw_commentator;
// [USED FOR COLOUR IN THIS VERSION] Available so we can have different commentators saying different lines (-1 = not specified)

// Which commentary line was it? (links to the commentary line database unqiue code)
int mi_commentaryLineFormat; // Which type of template do we want?
int mi_commentaryTemplateCodeChosen; // Which template has actually been chosen
CARMatch* mP_match; // Pointer to the match this commentary line is about

std::vector<int> objectLinks;

std::string mc_constructedLine; // The line in completed form
bool mb_constructed; // Has the line been constructed yet?

public:

// Accessors
std::string getCompletedLine(CCommentarySubSystem* subSystem);
void constructCommentaryLine(CCommentarySubSystem* subSystem);
int getObjectLink(int which);
int getObjectLink0();
int getObjectLink1();
int getObjectLink2();

CARMatch* getMatchPointer(); // Get a pointer to the match we are talking about
int getCommentaryLineFormat();
int getCommentaryLineTemplateChosen();
int getTimingQuarter();
int getTimingSecond();
unsigned long getCommentator(); // colour on this version

// Mutators
void setTimingDetails(int quarter, int secondsRemaining);
void setCommentator(unsigned long commentator); // [USED FOR COLOUR IN THIS VERSION] Don't need to use this as -1 is specified by default
bool addObjectLink(int object); // Must be added in the format of the commentary line
void setCommentaryLineFormat(int newformat, CCommentarySubSystem* subSystem);
void setMatch(CARMatch* matchPointer); // Set the match that we are talking about

// Constructor
CARGCommentaryLine() { objectLinks.clear(); mi_commentaryTemplateCodeChosen = -1; mi_quarter = -1; mi_secs = -1; }

// Destructor
~CARGCommentaryLine() { objectLinks.clear(); }
};

Share this post


Link to post
Share on other sites
Quote:
Original post by Modena360
Here is the definition of the CARGCommentaryLine class. Thanks for your help.

[snip]
You don't have a copy constructor or operator= for your class, which you'll need if you want to store the objects by value in a STL container. If you don't have them, the compiler will implement one for you which will just call operator= for each member variable, meaning pointers will just get copied without any thought. If the mP_match variable points at anything related to the class instance, it'll get screwed up by the default operator=.

Share this post


Link to post
Share on other sites
Thanks for pointing that out, I was just reading up on The Rule of Three for vectors and it looks like I've broken some basic rules.

I'll add in those extra functions and see if the outcome is different.

Share this post


Link to post
Share on other sites
To be honest, since CARGCommentaryLine doesn't appear to take any ownership of the only pointer member, I can't see that making a difference.

Aside notes - the calls to objectLinks.clear() in the constructor and destructor are redundant - objectLinks will automatically be created as an empty vector when the CARGCommentaryLine is created and the vector will automatically be released when the CARGCommentaryLine goes out of scope.

Also, you should consider setting mP_match to zero in the constructor otherwise it will be filled with a random value and there will be no way to later tell whether it points at a valid match or random data. If you wish to create a CARGCommentaryLine with no match assigned to later assign through your accessor (probably a bad idea, but hey) then zero the pointer in the constructor.

Looks almost to me from the errors that you're maybe possibly getting an exception the first time you try to access a member of the vector instance, although I guess this is doubtful if you are only getting these errors in debug - could we see the code where you create the actual vector and use it?

Share this post


Link to post
Share on other sites
The actual crash I get says this:

Unhandled exception at 0x004031ca in Sim.exe: 0xC0000005: Access violation reading location 0x000000c0.
(Which to me looks like it's trying to read a NULL/empty pointer?)



I create the CARMatch object like this:

CARMatch* newmatch = NULL;

// SETUP MATCH
newmatch = new CARMatch();

// then the CARGCommentaryLine objects are created inside CARMatch::addCommentary


void CARMatch::addCommentary(int type, unsigned long colour, int timeDelay)
{

...

CARGCommentaryLine tmpcomm;

...

mc_commentary.push_back(tmpcomm); (add this new line to the CARMatch::mc_commentary vector); <------- this is where it crashes

}


The CARMatch's are themselves in a std::vector<CARMatch*>.

It does appear to crash on the first attempt to access mc_commentary?

Share this post


Link to post
Share on other sites
Quote:
Original post by Modena360
The actual crash I get says this:

Unhandled exception at 0x004031ca in Sim.exe: 0xC0000005: Access violation reading location 0x000000c0.
(Which to me looks like it's trying to read a NULL/empty pointer?)

[snip]

The CARMatch's are themselves in a std::vector<CARMatch*>.

It does appear to crash on the first attempt to access mc_commentary?
Yes, that's almost certainly caused by accessing a null pointer (192 bytes from the start of the object). If it's crashing when you try to access mc_commentary, that implies that the class containing that variable is null. The Debugger will be able to tell you if that's the case, and will let you walk up the call stack to find out why it's null.

Share this post


Link to post
Share on other sites
If mc_commentary is a member of CARMatch, my guess would be that you are calling addCommentary on a null instance of a CARMatch i.e. carMatch->addCommentary() when carMatch=NULL (oops, as Steve said already [smile]).

You can actually call a method on a null instance like that without that itself causing a crash. Things probably won't blow up until you actually access a member of the instance in the method, as that would normally be the first point that the this pointer gets dereferenced.

I'd take a look at everywhere you call addCommentary() and see if anywhere it is possible that the CARMatch pointer you are calling it through could be NULL.


class A
{
public:
int x;

void f();
};

void A::f()
{
std::cout << "hello";
std::cout << x;
}

void g()
{
A *a=NULL;
a->f(); // hello will probably print, will probably explode when try to access x
}


[EDIT] Added some "probablys" in light of Zahlman's comments below [smile]

[Edited by - EasilyConfused on October 14, 2008 12:42:38 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by EasilyConfused
You can actually call a method on a null instance like that without that itself causing a crash. Things probably won't blow up until you actually access a member of the instance in the method, as that would normally be the first point that the this pointer gets dereferenced.


*wince*

While that's what *usually* happens (because it's easiest), this is all well into the realm of undefined behaviour.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Quote:
Original post by EasilyConfused
You can actually call a method on a null instance like that without that itself causing a crash. Things probably won't blow up until you actually access a member of the instance in the method, as that would normally be the first point that the this pointer gets dereferenced.


*wince*

While that's what *usually* happens (because it's easiest), this is all well into the realm of undefined behaviour.

That's actually a useful thing to know. Most people initially think a bad pointer will crash at the moment of the method call, because the syntax is similar to a member variable access:
object->myVar = 5;
object->Func(5);

But a crash on the method call line is usually not the case, which causes much confusion to people debugging the crash half-way through the method.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Quote:
Original post by EasilyConfused
You can actually call a method on a null instance like that without that itself causing a crash. Things probably won't blow up until you actually access a member of the instance in the method, as that would normally be the first point that the this pointer gets dereferenced.


*wince*

While that's what *usually* happens (because it's easiest), this is all well into the realm of undefined behaviour.


I didn't mean that it was okay to do this, just that you don't normally realise you've done it until the first member access.

Obviously it's undefined from the moment you dereference the pointer using -> and when it explodes is entirely down to the discretion of the compiler.

Share this post


Link to post
Share on other sites
Quote:
Original post by EasilyConfused
Aside notes - the calls to objectLinks.clear() in the constructor and destructor are redundant - objectLinks will automatically be created as an empty vector when the CARGCommentaryLine is created and the vector will automatically be released when the CARGCommentaryLine goes out of scope.
I agree entirely. By not implementing a destructor, copy-constructor, or assignment operator, your class is also following the rule of three. I would omit them if mP_match is a pointer to an object that this class does not own. Typical case would be a pointer to the classes parent for example.

If mP_match should own the object it points to, then you need all three.

Share this post


Link to post
Share on other sites
Quote:
Original post by Modena360

I've played around with including the

#define _HAS_ITERATOR_DEBUGGING 0
#define _SECURE_SCL 0

lines and when enabled these actually make it crash earlier, on the first std::vector it encounters.


My best guess would be that it's still one of these definitions (probably _SECURE_SCL). How complex is your project? If you are linking multiple libs or DLLs and one of them is compiled with a different definition of any of these from the others you'll get issues like this.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this