malloc() resulting in a SegFault?!

Hi,

i got a strange SegFault when trying to allocate memory (malloc).
First i setup the object like this:

Code:

//global
Accelerator_KDTree *g_KdTree;          //KD-Tree Objectpointer
...

int main(void) {
...
g_KdTree = new Accelerator_KDTree(SIMPLE);  //create the object
int res = g_KdTree->init(&Geometry, 1); // call init method
...
}

all goes well until i try to allocate memory iside the buildTree method. i got an ugly Segmentation Fault.

Code:

void Accelerator_KDTree::buildTree(POG* pog, int count)
{
	printf("constructing KD-Tree...\n");
	pc= new PrimitiveContainer(pog,count);
	KDTreeNode::pc=pc;
	KDTreeNode::maxDepth=512;
	KDTreeNode::cutoff=8;

	// WTF?
	//float* blah = (float*) malloc(10*sizeof(float));      // SEGFAULT!!!
	
	std::list<int> plist;
	AABoundingBox aabb;
	for(int i=0;i<pc->primitiveCount;i++)
	{
		//fill primitive list
		plist.push_back(i);                                  // SEGFAULT (when template calls malloc)

		//construct axis aligned bounding box
		aabb.pmax.x=MAX2(aabb.pmax.x,pc->getMaxAxisValue(i,0));
		aabb.pmax.y=MAX2(aabb.pmax.y,pc->getMaxAxisValue(i,1));
		aabb.pmax.z=MAX2(aabb.pmax.z,pc->getMaxAxisValue(i,2));

		aabb.pmin.x=MIN2(aabb.pmin.x,pc->getMinAxisValue(i,0));
		aabb.pmin.y=MIN2(aabb.pmin.y,pc->getMinAxisValue(i,1));
		aabb.pmin.z=MIN2(aabb.pmin.z,pc->getMinAxisValue(i,2));
	}
	root= KDTreeNode::buildTree(plist,aabb,4);
	printf("complete. %i nodes, max depth: %i,min depth: %i, avg depth: %i, %i",KDTreeNode::nodecount,KDTreeNode::treedepth,KDTreeNode::treemindepth,KDTreeNode::avgdepth,KDTreeNode::primcount);
}

I get the SegFault when i try to call push_back on my STL list. After debugging a bit i knew that the SegFault happens when push_back calls malloc(). So i tried to call a malloc() directly and guess what... SEGFAULT

I have no idea what goes wrong here so i would be grateful for any hint what possibly could cause this. Maybe i just miss something stupid.

Greets
cipher

Most likely, some previous code has overwritten some data needed by malloc - malloc uses a data structure immediately before the memory you get to store some "private" stuff. If free is called, a free will change some of the content in that block of memory, and some errant function may overwrite this block from an existing allocation that has the memory immediately before the freed memory. Next time you call malloc, it happily uses the data stored by free, and kaboom!

--
Mats

Thanks matsp for your advise!
After reviewing all the previsous calls of malloc() i saw that one allocated not enough memory for what i was writing to that memory location. Suprisingly the programm survived a few more mallocs util one SegFaults at last

Thanks for your fast help again

btw: i heard a few times now that i sould not typcast the returnvalue of malloc. maybe you know the answer why?

Code:

vertexPNCT* vertPNCT  = (vertexPNCT*) malloc ((FloatCount/3) * sizeof(vertexPNCT));        // why is the typecast bad style?

As you're using C++, shouldn't you be using new? I thought malloc was made essentially obsolete with new - but I could be wrong, thought I never use malloc anymore.

Originally Posted by samGwilliam

As you're using C++, shouldn't you be using new? I thought malloc was made essentially obsolete with new - but I could be wrong, thought I never use malloc anymore.

That is correct, new "replaces" malloc in C++. However, new is quite often implemented through malloc (or sometimes the other way around), so the actual problem wouldn't really change. Since C++ isn't meaning to break old applications, malloc() is not deprecated, but it's recommended that new code uses "new".

--
Mats

How could new call malloc? New is a keyword, not a function (like sizeof), surely? At least it is on my compiler, though it could be different on others.

Originally Posted by samGwilliam

How could new call malloc? New is a keyword, not a function (like sizeof), surely? At least it is on my compiler, though it could be different on others.

new is also an operator, but I was more thinking of what happens inside the code after the compiler has done it's job.

For example, we can imagine a compiler that compiles this:

Code:

class A 
{
   ... 
};

...
   A *a = new A;
...

could conceivably be generated as (written in C - it would naturally be machine code when the compiler is done with it):

Code:

    a = malloc(sizeof(A));
    a->A();  // call constructor.

--
Mats

Originally Posted by cipher82

btw: i heard a few times now that i sould not typcast the returnvalue of malloc. maybe you know the answer why?

Code:

vertexPNCT* vertPNCT  = (vertexPNCT*) malloc ((FloatCount/3) * sizeof(vertexPNCT));        // why is the typecast bad style?

FAQ on casting the return of malloc

Originally Posted by matsp

new is also an operator, but I was more thinking of what happens inside the code after the compiler has done it's job.

For example, we can imagine a compiler that compiles this:

Code:

class A 
{
   ... 
};

...
   A *a = new A;
...

could conceivably be generated as (written in C - it would naturally be machine code when the compiler is done with it):

Code:

    a = malloc(sizeof(A));
    a->A();  // call constructor.

--
Mats

Operator, yes. That's what I meant to say.

Originally Posted by matsp

new is also an operator, but I was more thinking of what happens inside the code after the compiler has done it's job.

For example, we can imagine a compiler that compiles this:

Code:

class A 
{
   ... 
};

...
   A *a = new A;
...

could conceivably be generated as (written in C - it would naturally be machine code when the compiler is done with it):

Code:

    a = malloc(sizeof(A));
    a->A();  // call constructor.

--
Mats

Quite true, matsp. Though I would not advise anyone make codes that take this piece of trivia as a universal rule.

Example:

Code:

int *x = new int;
free(x);

Example:

Code:

int *x = reinterpet_cast<int *>(malloc(sizeof(*x)));
int *y = new y;

free(x);
delete y;

Note: That is for the benefit of the OP and other people with similar misconceptions, not matsp who is well aware of these facts. It is important that people know by design you should not mix and match your C and C++ dynamic memory allocations.

FAQ on casting the return of malloc

Note that that FAQ pertains to C. This is C++, so casting the return value of malloc() is not bad style, but rather is required.

Originally Posted by laserlight

Note that that FAQ pertains to C. This is C++, so casting the return value of malloc() is not bad style, but rather is required.

Right, and you will notice that in my C++ example I did explicitly cast malloc() since C++ is a little more picky about implicit casting. I suppose you could make an inline template to handle this problem...

Example:

Code:

inline template<typename _T>
_T *malloc_ptr(size_t x)
{
  return reinterpret_cast<_T *>(malloc(size_t * sizeof(_T)));
}

inline template<typename _T>
_T &malloc_ref(size_t x)
{
  _T *ptr = malloc_ptr(x);

  if(!ptr)
  {
    throw std::bad_alloc("Null reference");
  }

  return *ptr;
}

The second function is kind of nasty... But you get the idea. And furthermore you see why one would typically just go ahead and opt to use new instead of some crazy homemade template functions.

The second function is kind of nasty... But you get the idea. And furthermore you see why one would typically just go ahead and opt to use new instead of some crazy homemade template functions.

Careful there, those crazy homemade function templates use some identifiers reserved to the implementation

EDIT:
hmm... or are they actually provided as an extension by your implementation, since I notice that __g__nothing is unused?

I changed the code to be a little less obtrusive by getting rid of the __g__nothing which was initially just a static pointer where another function could be used poll the return of the second function to determine if it is a null value or not (instead of just casting *(_T *)0). I think its a bit better just to throw an exception and give the programmer a fighting chance to know something went wrong instead of making changes to *(_T *)0.

[edit]I changed the code rather quickly so you are definitely a quick one, laserlight.[/edit]

I changed the code rather quickly so you are definitely a quick one, laserlight.

Nah, more like I happened to load the thread before you finished your edit, so I had all the time to read your original post even after you made your edit

By the way, _T is reserved to the (compiler and standard library) implementation for any use, since "each name that contains a double underscore (_ _) or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use" (ISO/IEC 14882:2003 Section 17.4.3.1.2).