Memory overwrite after allocating 4k ints

Started by
15 comments, last by King Mir 10 years, 1 month ago

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:

https://drive.google.com/file/d/0B_iN9pcbyoiKQV9lNllLWktfOGc/edit?usp=sharing


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?

Advertisement

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?

Your class violates the Rule of Three (depending on your needs Rule of Five in C++11). Either the class must be noncopyable or implement sane copy semantics.

If you have access to C++11 I would rather suggest to use std::vector instead of an int* pointer in the class and trust the compiler-generated copy/move constructors/assignment operators.

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>.

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.AddPipelineState( &m_pipeState );
	m_drawable.SetDrawCall( &m_drawCall );

	m_drawable_warpException.AddPipelineState( &m_pipeState );
	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.AddPipelineState( &m_pipeState );
	m_drawable.SetDrawCall( &m_drawCall );
	m_drawable_warpException.Clear();
	m_drawable_warpException.AddPipelineState( &m_pipeState );
	m_drawable_warpException.SetDrawCall( &m_drawCall_warpException );


	m_pipeState.Reset();
	m_pipeState.AddBinderCommand( &pTextureBinder_p );
	m_pipeState.AddBinderCommand( &pBlendBinder );
	m_pipeState.AddBinderCommand( &pSampleState );
	m_pipeState.AddBinderCommand( pIVB_p.GetBinder() );
	// 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 @.@

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..

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.

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.

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.

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.

This topic is closed to new replies.

Advertisement