8

The very well known scenario:

#include <memory>

class A{};
class B : public A {};

int main()
{
    std::unique_ptr<A> a = std::make_unique<B>();
    // bam, when a gets deleted, we have undefined behavior.
    return 0; 
}

Moreover, not even Valgrind can catch such an error, provided that the size of A and B are the same.

Is there some tool that could catch such errors at least for a debug build, or some idiom which would detect such errors for a specified class?

Martin Drozdik
  • 12,742
  • 22
  • 81
  • 146
  • 1
    Do you want to have the functionality? Otherwise, you can just disable inheritance: http://stackoverflow.com/a/12319547/1287254 – Shaggi Mar 27 '16 at 15:14
  • Great! That solves the practical problem I am solving right now, but I would like to know the general case, just out of curiosity. – Martin Drozdik Mar 27 '16 at 15:16
  • 1
    Depends, but [std::has_virtual_destructor](http://en.cppreference.com/w/cpp/types/has_virtual_destructor) might be part of what you are looking for. – kec Mar 27 '16 at 15:24

2 Answers2

5

For gcc you can specify:

-Wdelete-non-virtual-dtor -Wsystem-headers

to see delete-non-virtual-dtor warning generated by std::default_delete, it will look as follows:

/usr/local/include/c++/5.3.0/bits/unique_ptr.h:76:2: warning: deleting object of polymorphic class type 'B' which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor] delete __ptr;

live

btw. your sample classes lack at least one virtual function in base class.

[edit]

to turn it into error use:

 -Werror=delete-non-virtual-dtor -Wsystem-headers
marcinj
  • 48,511
  • 9
  • 79
  • 100
  • I don't think you need a virtual destructor when you're using `unique_ptr`. – David G Mar 27 '16 at 15:33
  • 1
    @0x499602D2 you're thinking of shared_ptr, which type-erases the deleter. Unique_ptr needs a virtual destructor on the base class. – Richard Hodges Mar 27 '16 at 15:36
  • This only happens if you add a virtual function, but then you can catch the problem much earlier with -Weffc++. – n. m. could be an AI Mar 27 '16 at 15:50
  • adding virtual function should not be required, I agree. I tried this with `-Weffc++` but I dont see it catches non-virtual base destructor problem for this sample code, or I dont know how to use it: http://coliru.stacked-crooked.com/a/17c5c165298c7b5a – marcinj Mar 27 '16 at 16:04
  • -Weffc++ warns when you define a class with a virtual function but non-virtual dtor. – n. m. could be an AI Mar 27 '16 at 19:06
1

It is my strong opinion that one should strictly separate "value-based" classes and "OO-based" classes (for lack of better term).

Value-based classes should have no public bases, and should normally support copy semantics (unless specially designed to disable copying). Objects of such classes have no identities apart from their values. They are interchangeable with their copies.

OO-based classes should have virtual destructors, and should never have copying members publicly accessible. Objects of such classes should only be copied via a virtual clone method. Such objects have identities separate from their values.

There should be no class with a public base that has a non-virtual destructor or a public copy/move ctor or a public copy/move assignment operator (in the base).

If you maintain this separation, you will have no accidents with object slicing or deletion through a non-polymorphic base pointer.

Unfortunately there are no tools (that I know of) that help maintain this separation. So you need to exercise due diligence while inheriting. It's really simple. Does it have a virtual dtor and inaccessible/deleted copy ctor and copy assignment? You can inherit publicly. No? Avoid potential mess, use composition or private inheritance.

A good class designed for inheritance would have its copying members protected in order to facilitate cloning in descendant classes.

Unfortunately, with third-party classes you don't have a choice, as authors usually leave copying members public, so there's still risk of object slicing. But you will have no risk of improper deletion through a base pointer.

TL;DR no tools, only programmer's due diligence.

n. m. could be an AI
  • 112,515
  • 14
  • 128
  • 243