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.

Quote:

---

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.

Quote:

---

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

Quote:

---

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

Quote:

---

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.

Quote:

---

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.

Quote:

---

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.

Quote:

---

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.

Quote:

---

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 :p

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]

Quote:

---

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 :D

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

Fair enough.

Example:

Code:

---

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

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

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

  return *ptr;
}

---

Alright, now everyone is happy. I have added a healthy level of sarcasm to a few lines of code (which always brightens my day). I still say you are just a quick one. Of course, your name is laserlight, I should expect you to be swift.

Quote:

---

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.

---

DOH! My bad! Of course, I agree that one should not be using malloc in a C++ environment any way :)

Placement new... But then again, who likes to explicitly call a destructor?

Example:

Code:

---

int *x = new(malloc(sizeof(*x))) int;

// At which point this line is perfectly acceptable
free(x);

---

Quote:

---

Originally Posted by master5001

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.

---

I must admit that my coding style is a hybrid of C and C++. I always use new and delete, but I've never bothered with templates or try/catch and I pass by reference the C way. I'm still in the habit of using the .h when including too.

It doesn't really matter what the contents is in a header. The extension is optional. You can use .h in C++, as well as C. I wouldn't say it's wrong or bad practice.
And especially in C++, I would avoid saying "passing by reference" if you aren't actually doing it (ie passing by pointer), because in C++ there are references and pointers.

But you should really try out templates, at the very least. Very powerful stuff right there.

Quote:

---

Originally Posted by Elysia

It doesn't really matter what the contents is in a header. The extension is optional. You can use .h in C++, as well as C. I wouldn't say it's wrong or bad practice.

---

I suspect that samGwilliam is referring to a habit of including the C standard headers instead of their C++ versions.

Oh. Maybe you're right, now that I do think of it...