modifying new[]'d objects

[Tim Conkling]

I'm having difficulty tracking down a bug in my program and I'm
wondering if it's due to some misuse on my part of new[]'d objects.
Here's the situation. If anyone can shed some light on this, I'd be
much obliged.

I have a simple class that looks something like this (this is a rather
abbreviated version):

class TileAttributes
{
... (a few other member variables of little consequence here)
std::string m_eventName;

TileAttributes() : m_eventName("") {}
TileAttributes(const TileAttributes& copy) : m_eventName(copy.m_eventName) {}
virtual ~TileAttributes() {}
TileAttributes& operator= (const TileAttributes& copy) {
m_eventName.assign(copy.m_eventName); }
};

I have two arrays of TileAttributes, created with the new[] operator,
and I do a lot of this sort of thing:

array1[x] = array2[y];

All the member variables of TileAttributes are integral types, except
for m_eventName. I just added the m_eventName member the other day, and
since then I've been getting crashes when modifying these arrays. I am
using gcc 4 and gdb 6.1 (via Apple Xcode) and I'm getting errors that
look like this:

malloc: *** error for object 0x166a0210: double free

These errors (usually a few of them) always precipitate a crash in the
operator=() function of TileAttributes, specifically the line where I
am doing:

m_eventName.assign(copy.m_eventName)

The callstack as it exists above my operator= function looks like this:

__cxa_throw
std::__throw_length_error
std::string::_Rep::_S_create
std::string::_Rep::_M_clone
std::string::assign

This problem only occurs when m_eventName.length() > 0. I didn't even
have a deep copy constructor or assignment operator before I added the
m_eventName member, but I added them to see if they might solve my
problem. They made it easier to pinpoint exactly where the error was
occuring, but I am still baffled. I can't see what I might be doing
wrong here.

Tim

 

[Victor Bazarov]

I'm having difficulty tracking down a bug in my program and I'm
wondering if it's due to some misuse on my part of new[]'d objects.
Here's the situation. If anyone can shed some light on this, I'd be much
obliged.

I have a simple class that looks something like this (this is a rather
abbreviated version):

class TileAttributes
{
... (a few other member variables of little consequence here)

"Of little consequence"? Are you sure? No dynamic objects?

std::string m_eventName;

TileAttributes() : m_eventName("") {}
TileAttributes(const TileAttributes& copy) :
m_eventName(copy.m_eventName) {}
virtual ~TileAttributes() {}
TileAttributes& operator= (const TileAttributes& copy) {
m_eventName.assign(copy.m_eventName); }
};

I have two arrays of TileAttributes, created with the new[] operator,
and I do a lot of this sort of thing:

array1[x] = array2[y];

All the member variables of TileAttributes are integral types, except
for m_eventName. I just added the m_eventName member the other day, and
since then I've been getting crashes when modifying these arrays. I am
using gcc 4 and gdb 6.1 (via Apple Xcode) and I'm getting errors that
look like this:

malloc: *** error for object 0x166a0210: double free

Sounds like bad dynamic memory management. Check this out:

class Bad {
char *str;
int a;
public:
Bad(const char* s) : str(new char[strlen(s)+1]) {
strcpy(str, s);
}
~Bad() { delete[] str; }
};

class HasBad {
Bad bad;
public:
HasBad() : bad("badbadbad") {}
HasBad(const HasBad& hb) : bad(hb.bad) {}
};

Now copy a 'HasBad' object or pass it into a function by value, and you
going to have all kinds of problems. Is "HasBad" the culprit? No. It's
the 'Bad'. So, in what you've shown 'TitleAttributes' is probably not
the class itself, but one of those things you think are "of little
consequence".

These errors (usually a few of them) always precipitate a crash in the
operator=() function of TileAttributes, specifically the line where I am
doing:

m_eventName.assign(copy.m_eventName)

The callstack as it exists above my operator= function looks like this:

__cxa_throw
std::__throw_length_error
std::string::_Rep::_S_create
std::string::_Rep::_M_clone
std::string::assign

This problem only occurs when m_eventName.length() > 0. I didn't even
have a deep copy constructor or assignment operator before I added the
m_eventName member, but I added them to see if they might solve my
problem. They made it easier to pinpoint exactly where the error was
occuring, but I am still baffled. I can't see what I might be doing
wrong here.

What you showed is OK. However, there is always that thing, you know,
that you didn't show. There is always doubt about it, you see...

Also, _sometimes_ (really rarely) your implementation of the standard
classes could be buggy. You could try switching to another library
implementation, but only when you exhaust all possibilities to fix your
own code.

V

 

[Tim Conkling]

I'm having difficulty tracking down a bug in my program and I'm
wondering if it's due to some misuse on my part of new[]'d objects.
Here's the situation. If anyone can shed some light on this, I'd be
much obliged.

I have a simple class that looks something like this (this is a rather
abbreviated version):

class TileAttributes
{
... (a few other member variables of little consequence here)

"Of little consequence"? Are you sure? No dynamic objects?

std::string m_eventName;

TileAttributes() : m_eventName("") {}
TileAttributes(const TileAttributes& copy) :
m_eventName(copy.m_eventName) {}
virtual ~TileAttributes() {}
TileAttributes& operator= (const TileAttributes& copy) {
m_eventName.assign(copy.m_eventName); }
};

I have two arrays of TileAttributes, created with the new[] operator,
and I do a lot of this sort of thing:

array1[x] = array2[y];

All the member variables of TileAttributes are integral types, except
for m_eventName. I just added the m_eventName member the other day, and
since then I've been getting crashes when modifying these arrays. I am
using gcc 4 and gdb 6.1 (via Apple Xcode) and I'm getting errors that
look like this:

malloc: *** error for object 0x166a0210: double free

Sounds like bad dynamic memory management. Check this out:

class Bad {
char *str;
int a;
public:
Bad(const char* s) : str(new char[strlen(s)+1]) {
strcpy(str, s);
}
~Bad() { delete[] str; }
};

class HasBad {
Bad bad;
public:
HasBad() : bad("badbadbad") {}
HasBad(const HasBad& hb) : bad(hb.bad) {}
};

Now copy a 'HasBad' object or pass it into a function by value, and you
going to have all kinds of problems. Is "HasBad" the culprit? No. It's
the 'Bad'. So, in what you've shown 'TitleAttributes' is probably not
the class itself, but one of those things you think are "of little
consequence".

These errors (usually a few of them) always precipitate a crash in the
operator=() function of TileAttributes, specifically the line where I
am doing:

m_eventName.assign(copy.m_eventName)

The callstack as it exists above my operator= function looks like this:

__cxa_throw
std::__throw_length_error
std::string::_Rep::_S_create
std::string::_Rep::_M_clone
std::string::assign

This problem only occurs when m_eventName.length() > 0. I didn't even
have a deep copy constructor or assignment operator before I added the
m_eventName member, but I added them to see if they might solve my
problem. They made it easier to pinpoint exactly where the error was
occuring, but I am still baffled. I can't see what I might be doing
wrong here.

What you showed is OK. However, there is always that thing, you know,
that you didn't show. There is always doubt about it, you see...

Also, _sometimes_ (really rarely) your implementation of the standard
classes could be buggy. You could try switching to another library
implementation, but only when you exhaust all possibilities to fix your
own code.

V

All the other member variables are basic built-in datatypes. There are
no complex objects and nothing is dynamically allocated. The class
looks exactly like this:

class TileAttributes
{
public:

enum CollisionType
{
kNoSides = 0,
kTopSide,
kAllSides
};

float friction;
CollisionType collisionType;
bool climbable;
unsigned short animationLength;
unsigned short animationPause;
short collisionClassLowerBound;
short collisionClassUpperBound;
std::string collisionEventName;
};

 

[Victor Bazarov]

[...]
All the other member variables are basic built-in datatypes. There are
no complex objects and nothing is dynamically allocated. The class looks
exactly like this:

No, it can't be. In the previous post there were two constructors,
a virtual destructor, and an assignment operator. Where did they go?
Also, there was a member called 'm_eventName'. Where is it?

So, you're not showing all the code again. What do you want us to do,
read your mind or the contents of your computer memory remotely?

class TileAttributes
{
public:

enum CollisionType
{
kNoSides = 0,
kTopSide,
kAllSides
};

float friction;
CollisionType collisionType;
bool climbable;
unsigned short animationLength;
unsigned short animationPause;
short collisionClassLowerBound;
short collisionClassUpperBound;
std::string collisionEventName;
};

OK, fine. The data part probably looks like that. Please, don't take my
suggestions too literally. I said that the problem was in the code you
didn't show. Was I wrong?

Here is how I arrived to the "not here" conclusion. You said there was
a problem. You showed some code. I looked at the code shown. There was
no problem with that code. Therefore, the problem _must_ be elsewhere.
Am I being unreasonable here?

You said the error you got was "double free". That's some bad memory
management (do you agree?). It could be due to some bad memory management
in one of subobjects of TitleAttributes. Or it could be due to some bad
memory management in some other place. It still could be the 'eventName'
(or whatever you decide to call it), although I strongly doubt that. You
should check the implementation of 'std::string' though. Try an older
version of the same compiler (with its older version of the standard
classes). Or try another compiler (if you can).

V

 

[tconkling]

I appreciate the help and I'm sorry for the confusion. What I meant to
say is that the _data_ portion of the class looks exactly like what I
copied-and-pasted in the previous message. I didn't include the member
functions because I had already shown what they generally looked like.

In my original message, I was attempting to save space, and I wrote
the class definition from memory. It didn't come out
character-for-character the way it appears in my header file. The point
is, there is only one member variable of note and its type is
std::string. All the other variables are built-in datatypes.

I seriously doubt the problem is with the std::string implementation,
so I'll look at other parts of the code.

 

[James Lothian]

[snip]
TileAttributes& operator= (const TileAttributes& copy) {
m_eventName.assign(copy.m_eventName); }
};

Presumably in reality this assignment operator returns something?

James

 

[Tim Conkling]

[snip]
TileAttributes& operator= (const TileAttributes& copy) {
m_eventName.assign(copy.m_eventName); }
};

Presumably in reality this assignment operator returns something?

James

Yes, it does... and I have learned my lesson about typing code from
memory. It's a bad thing, and causes all sorts of confusion when real
code is mixed in with code-off-the-top-of-my-head.

FWIW, the problem has been corrected. It was bad memory management on
my part. I was memcpy'ing an array of these objects to another array,
which worked fine back when all the objects data members were simple
data types, and obviously broke when I added the std::string member.
Stupid...

Sorry for the confusion, Victor and James.

Tail between my legs,
Tim