Search code examples
c++functioninheritancesegmentation-faultvirtual

Segfault when calling virtual function of derived class


I've been having a problem with segfaults when I call virtual function of a derived class. However, these segfaults do not occur if I change the name of the function to be different from the name of the virtual function in the base class. Here's some code:

//in main
//initialize scene objects
//camera
if((camera = (Camera*)malloc(sizeof(Camera))) == NULL){
  cout << "Could not allocate memory for camera" << endl;
}
//...code in middle
//inside file parsing...
//infile is an ifstream
//nextString is a char*
if(!strcmp(nextString,"camera")){
  camera->parse(infile); //segfault here
}

Here is the base class header (the .cpp only instantiates variables in the constructor):

class WorldObj{
public:
  WorldObj();
  ~WorldObj();
  virtual void parse(ifstream&) =0;
  vec3 loc; //location
};

And here is the code inside my Camera class I use to write the virtual function:

void Camera::parse(ifstream &infile){
  //do parsing stuff
}

parse() is declared in the header file as virtual void parse(ifstream&);

My problem here is that if I rename parse() inside Camera to something like CameraParse() and completely ignore the fact that there is a virtual function to be implemented, the code works completely fine!

Could you shed some light on why calling the virtual function causes a segfault? I've checked with Valgrind to see if there are any memory issues, and it tells me that there's an invalid read/write of 8 bytes. I understand this means that I haven't allocated memory properly for my objects, but I don't know where I'm going wrong with the allocation.

Any help would be appreciated :)


Solution

  • You can't (just) malloc a non-POD object, you have to new it.

    This is because malloc reserves the right amount of space, but doesn't construct the object, which is non-trivial for any class with virtual functions even if the constructor is defaulted.

    Now, the specific issue only arises here when you make a virtual function call, because this depends on the extra initialization carried out by new, but it's still wrong to use an un-constructed instance of any non-POD type.


    Note that I'm using POD (Plain Old Data) as a lazy shorthand for anything with only trivial initialization. In general, a class (or struct) is trivially initializable if neither it, nor any of its members or base classes have a constructor that does something. For our purposes, every class with one or more virtual methods (even if they're inherited, or in a data member) requires non-trivial initialization.

    Specifically, the standard quote in Ben Voigt's answer describes two stages beginning the lifetime of an object (the time during which you can safely make method calls, especially virtual ones):

    • storage with the proper alignment and size for type T is obtained,

    which happens when you call malloc

    • if the object has non-trivial initialization, its initialization is complete

    which only happens for a non-trivially-initialized type when you use new.


    For reference, this is the normal use closest to your existing code:

    Camera *camera = new Camera;
    // don't need to check for NULL, this will throw std::bad_alloc if it fails
    camera->parse(file);
    // don't forget to:
    delete camera;
    

    this is better style, though:

    std::unique_ptr<Camera> camera(new Camera);
    camera->parse(file);
    // destruction handled for you
    

    and only if you really need to use malloc or some other specific allocator:

    Camera *camera = (Camera *)malloc(sizeof(*camera));
    new (camera) Camera; // turn your pointer into a real object
    camera->parse(file);
    // destruction becomes uglier though
    camera->~Camera();
    free(camera);