Segfault when calling virtual function of derived

Question

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

Answer 1

Useless has provided the correct explanation. Here is the requirement in the Standard (section 3.8):

The lifetime of an object is a runtime property of the object. An object is said to have non-trivial initialization if it is of a class or aggregate type and it or one of its members is initialized by a constructor other than a trivial default constructor. [ Note: initialization by a trivial copy/move constructor is non-trivial initialization. — end note ]

The lifetime of an object of type T begins when:

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

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

The lifetime of an object of type T ends when:

• if T is a class type with a non-trivial destructor, the destructor call starts, or

• the storage which the object occupies is reused or released.

Answer 2

Use new, not malloc. You allocated memmory, but did not created the object. Using new, with the respective constructor the virtual functions dispatch table will be created, and only then you can use it. For example:

if((camera = new (nothrow) Camera()) == NULL){
  cout << "Could not allocate memory for camera" << endl;
}

if(!strcmp(nextString,"camera")){
  camera->parse(infile); //segfault here
}

Alternatively you can use simple

camera = new Camera;

And somewhere catch the posible exception bad_alloc.

Answer 3

malloc doesn't call constructors.

You have to change (The C++ way of creating objects)

camera = (Camera*)malloc(sizeof(Camera)))

to

camera = new Camera; // Now you camera object will be created and constructed.

Answer 4

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