What is the reason for a segmentation fault with this C++ code using lists?

Question

I have some complicated C++ code but the problem narrows down to doing a push_back on a list of structures:

list<cache_page> cachedPages;

void f()
{
    cache_page cpage(a,b);
    cachedPages.push_back(cpage);
}

I have commented all the data members of the struct cache_page and still the error persists. If I comment the push_back line, there is no error.

What could be the reason?

I have tried using GDB and the error occurs in _List_Node_base::hook() function.

template < class T >
class A
{
    T x;
    public:
        void func()
        {
          x->f();
        }

};

class B : public A < B* >
{
    list<cache_page> cachedPages;
    public:
        void f()
        {
            cache_page cpage;
            cachedPages.push_back(cpage);
        }
};

I have a do nothing copy constructor. I have no data members in cache_page.

Answer 1

I guess list does a copy of the cpage object, have you checked that the copy constructor if cpage doesn't segfault on that situation?

Answer 2

If I recall correctly, some if not all STL containers require a copy constructor and assignment operator. If you've relied on the default of those two, or are doing a shallow copy when you should be doing a deep copy, that could be the cause of your segfault.

Answer 3

Sounds like cachedPages doesn't really exist. Could it already have been deleted?

Alternatively, is f() a member function? Are you sure that its (this) object still exists? I've been puzzled by many weird looking problems inside member functions, only to print *this in gdb and realise that I've de-referenced a bad pointer in the next stack frame up.

Answer 4

You need to specify the assignment (=) operator so that sort routines can assign a new order to the members of the list. After that I think you'd be fine.

Answer 5

You may be double deleting. Does the destructor of cpage do some cleanup? If that is the case, and cpage does not have a copy constructor which increases a refcount or makes a deep copy, then the cleanup will occur twice.

Answer 6

Found the bug: Its a really subtle one. In the code, x is a pointer and it is not being initialized in the base class. Calling x->f() accesses the vtable to invoke the correct function in the derived class B. But, since the pointer value is uninitialized, the "this" parameter is wrong. Trying to access the list is then an invalid operation and the code crashed.

To correct this error, pass an argument of the com in the constrof type T, which will be initialized to this by the constructor of the derived class.

class A { public: A(T p): x(p) { }

};

class B { public: B() : A(this) { } };

Answer 7

You're crossing the streams. Haven't you seen Ghostbusters? Don't cross the streams.

You're crossing the streams here:

class B : public A < B *>

I don't understand the point of this. What are you trying to do? CRTP? This is not the way it's done.

The problem is not in the push back, the problem is, "this" being invalid.

When you have

  void f()
  {
    cache_page cpage;
  }

It's compiled to a NOP. this is not acceded everything is fine.

  void f()
  {
    cache_page cpage;
    // oops this access
    this->cachedPages.push_back(cpage);
  }

Except it's called in the context of A. What is the value of this ? It's not initialized anywhere. So this is equal to whatever is in memory, where a happy uninitialized list is waiting.

The fix?

template < class T >
class A
{
  T * _x;
public:
 explicit A(T * x) : _x(x) {}

 void func()
 {
   _x->f();
 }

};


class B : public A < B >
{
  list<cache_page> cachedPages;
public:
  B(void) : A<B>(this) {}

  void f()
  {
    cache_page cpage;
    cachedPages.push_back(cpage);
  }
};

This should work better. But what about...

template < class T >
class A
{
public:
 void func()
 {
   static_cast<T>(this)->f();
 }

};


class B : public A<B>
{
  list<cache_page> cachedPages;
public:
  void f()
  {
    cache_page cpage;
    cachedPages.push_back(cpage);
  }

};

That's the way CRTP is done.