Followers 0

# Memory overwrite after allocating 4k ints

## 16 posts in this topic

After debugging some really weird behaviors, like vectors going from size 0 to negative values out of nowhere, I realized this was happening after allocating 4000 ints with new. I dont understand, 4k ints (~15KB) is not that huge, and even if it was, why its messing with my mem?

Video:

struct TileMap{

int * pTileMap; // indexes on m_vTiles
int mapW, mapH;
float hSpacing, vSpacing;

sprite::InstancedSprites m_tileInstSprites;
sprite::InstancesVertexBuffer m_IVB;

TileMap():pTileMap(nullptr){}
~TileMap(){ if(pTileMap) delete [] pTileMap; }
};
std::vector<TileMap> m_vTileMaps;


This is the structure you see in the vid.

Both InstancedSprites and  InstancesVertexBuffer hold vectors on its internals, those are getting screwed..that doesnt make any sense, theres a memory constrain Idont know about? Dx

Im hopping is something really stupid, but the vid pretty much shows Im not overwriting stuff myself right?

0

##### Share on other sites

Tsc, nevermind the allocation, after commeting it out, the same thing occurs, just two or three steps later... >_<

Any tip on how can I detect wheres the problem?

Edited by Icebone1000
0

##### Share on other sites

You are storing your Tilemaps in a std::vector. As soon as the memory initially reserved in that vector runs out, it will reallocate.

It does this by creating new objects and destroying the old ones, causing your destructor to clean up the memory pointed to by pTileMap.

You can fix this in a number of ways depending on your situation:

- Have your tile map not own the pointer, and make sure it gets cleaned up in another manner

- Use a shared pointer instead of a raw one

- Change the array of ints to a copyable structure, e.g. std::vector<int>.

1

##### Share on other sites

The problem is not the pointer, damnit, I new I was confident with that pointer u_u*

Its more ninja than that..

check this out:

InstancedSprites ctor:

sprite::InstancedSprites::InstancedSprites()
:
m_drawCall(6),
m_drawCall_warpException(6)
{
m_drawable.SetDrawCall( &m_drawCall );

m_drawable_warpException.SetDrawCall( &m_drawCall_warpException );
}


Now, what annoys me, is that everything inside m_drawable IS a std::vector...I cant quite tell exactly what happens that invalidates my shit.

Here how I "solved"

void sprite::InstancedSprites::Initialize( dx::BindPSShaderResourceView & pTextureBinder_p, dx::BindOMBlendState & pBlendBinder, dx::BindPSSampler & pSampleState, InstancesVertexBuffer & pIVB_p )
{
// DBG
m_drawable.Clear();
m_drawable.SetDrawCall( &m_drawCall );
m_drawable_warpException.Clear();
m_drawable_warpException.SetDrawCall( &m_drawCall_warpException );

m_pipeState.Reset();
// missing, IA and camera binds

m_pIVBref = &pIVB_p;
}


See? I already had that method, but it didnt have that same code as the ctor (the upper part), this solved the issue, basicly, my drawables where getting invalid.

Oh yeah, deleting the pointer in the dctor was stupid, for sure ¦D, the thing is, Im only allocating a single map for now, its not that Im forcing reallocation on already build vector elements..at max Im forcing on the first/only one, but then, the ctor should be doing its job? I just dont get it @.@

Edited by Icebone1000
0

##### Share on other sites

Ok, the problem was the copy ctor, that didnt exist, so I basically create a cpy ctor that doesnt copy anything (act just like a normal ctor).

This worked.

The rule of tree is quite annoying, having to implement all those methods every time a new class is born? not knowing if you will or will not ever use it...whats the policy for this? do it fanatically no matter what?

I wish I could spot this kind of issue ahead of time and do it just when necessary like in this case.

Hmmm.. I think will create a macro that create private empty "rule of tree" methods and put that on every new class, so things like that will popup sooner, and then I can implement on demand..

-1

##### Share on other sites

Creating empty copy constructor/assignment operators (you need both) does not fix your problem. It hides your problems because undefined behavior sometimes appears to work in your favour.

Apart from the obvious (putting the data you're putting in pTileMap instead into a std::vector<int>) DaBono also offered other possibilities.

As a general rule, if you feel 'every of your classes' needs special work to stick to the Rule of Three (or Five), you are doing something very wrong with your design. In all normal cases the automatically generated copy constructor/assignment operator does the exactly right thing. You usually don't manage memory in classes, you use an std::container or some sort of smart pointer (boost::shared_ptr, boost::shared_array, boost::intrusive_ptr, std::shared_ptr, std::unique_ptr, ...).

If the standard language constructs aren't enough or you need something extremely specialized then you write a class (or template class) which implements that functionality. That class needs to obey the Rule of Three. Every class using it does so automatically.

0

##### Share on other sites

The rule of tree is quite annoying, having to implement all those methods every time a new class is born? not knowing if you will or will not ever use it...whats the policy for this? do it fanatically no matter what?
I wish I could spot this kind of issue ahead of time and do it just when necessary like in this case.

Hmmm.. I think will create a macro that create private empty "rule of tree" methods and put that on every new class, so things like that will popup sooner, and then I can implement on demand..

That's exactly what BitMaster said; either implement sane copy semantics or make it uncopyable. Unless I know for sure that my class will be trivially copyable from the very beginning, I almost always begin my class by inheriting from boost::noncopyable to ensure that my class doesn't get value semantics until I actually need it and go about to implement it.

0

##### Share on other sites
You could write a macro to delete the default copy and assignment operator, if that suits your favor. But it doesn't do any good to put in empty methods.

The rule of three doesn't say that every class needs those three functions. It says that if it needs one of the three, then it probably needs the other two.
0

##### Share on other sites

Its not empty, its a copy  ctor that works like the already implemented ctor.

Im basically forcing the class to work inside vectors. (that Im assuming to require a copy ctor)

I will check on the implementation of boost::noncopyable.

0

##### Share on other sites

You could write a macro to delete the default copy and assignment operator, if that suits your favor. But it doesn't do any good to put in empty methods.

The rule of three doesn't say that every class needs those three functions. It says that if it needs one of the three, then it probably needs the other two.

Yeah, but the issue is knowing when you going to need it, thus the private ones would show you instantly.

0

##### Share on other sites

Thinking deeper on it, that class should be never copied..

See, the thing is, I merely wanted to allocate n tilemaps at beginning and then never mess with it again.

TileMap is a private struct, so its use is really scoped, its merely an aux.

There would never be the need of copying stuff around, if it where not by std::vector.

Copying InstancedSprites doesnt make any sense and should not be done. And it doesnt need to be done in my code.

Basically, the usage of std::vector was the problem in the first place. The fact that it have a friendly interface but with implicity complexities pisses me off.

Now, I dont want to have a cpy ctor on InstancedSprites, but I do want an array of tile maps, and I need to choose the size at initialization..What should I do?

Having used another raw  array (for TileMap) in the first place would have never caused me problems, isnt it ironic?

0

##### Share on other sites

What should I do?

I'd probably start with a std::vector of std::unique_ptr<TileMap>, and also make TileMap noncopyable to prevent mistakes.

Having used another raw array (for TileMap) in the first place would have never caused me problems, isnt it ironic?

Well, given that a raw array is totally different from a dynamic container, this isn't exactly a fair comparison. You could create a std::vector, reserve() sufficient memory and emplace_back() whatever instances you need.

In any case, you could still have a similar problem with a raw array if you accidentally assigned to or copied from the array.
0

##### Share on other sites

Basically, the usage of std::vector was the problem in the first place. The fact that it have a friendly interface but with implicity complexities pisses me off.

You would prefer an unfriendly interface? I'd glad you didn't write the STL.
0

##### Share on other sites

Its not empty, its a copy  ctor that works like the already implemented ctor.

Im basically forcing the class to work inside vectors. (that Im assuming to require a copy ctor)

A copy constructor is defaultly generated for you, so there's no point in writing it out yourself if there's no difference from the default behavior. Which is enough if your class doesn't manage a resource.

You could write a macro to delete the default copy and assignment operator, if that suits your favor. But it doesn't do any good to put in empty methods.

The rule of three doesn't say that every class needs those three functions. It says that if it needs one of the three, then it probably needs the other two.

Yeah, but the issue is knowing when you going to need it, thus the private ones would show you instantly.

You need it when your class manages a resource. For example, when you dynamically allocate an array. You should use the most narrow class you can to manage each resource, with at most one resource per class. Dynamic arrays are almost never necessary because you can use std::vector.

Thinking deeper on it, that class should be never copied..

See, the thing is, I merely wanted to allocate n tilemaps at beginning and then never mess with it again.

TileMap is a private struct, so its use is really scoped, its merely an aux.

There would never be the need of copying stuff around, if it where not by std::vector.

Copying InstancedSprites doesnt make any sense and should not be done. And it doesnt need to be done in my code.

Basically, the usage of std::vector was the problem in the first place. The fact that it have a friendly interface but with implicity complexities pisses me off.

Now, I dont want to have a cpy ctor on InstancedSprites, but I do want an array of tile maps, and I need to choose the size at initialization..What should I do?

Having used another raw  array (for TileMap) in the first place would have never caused me problems, isnt it ironic?

You should have an std::array<TileMap> each tile having a std::vector<int>. You should also avoid raw pointers that signify ownership. Prefer std::vector or smart pointers like std::unique_ptr.

Edited by King Mir
0

## Create an account

Register a new account