12

We know that if there are virtual functions then the base class destructor should be marked as virtual as well, otherwise it is undefined behavior when explicitly deleted with base class pointer if we hope to delete derived object with base class pointer the base destructor should be marked as virtual, otherwise it is undefined behavior.

For example,

struct Base {
  virtual void greet() { std::cout << "base\n"; }
};

struct Derived : public Base {
  virtual void greet() override { std::cout << "derived\n"; }
};

call

Base *b = new Derived;
b->greet();
delete (b);

clang(gcc similarly) will emit such a warning when -Wdelete-non-virtual-dtor:

delete called on 'Base' that has virtual functions but non-virtual destructor

But neither of them report warnings for smart pointers:

std::unique_ptr<Base> sb = std::make_unique<Derived>();
//   std::unique_ptr<Base> sb = std::unique_ptr<Derived>(new Derived);
sb->greet();

I guess this still leads to undefined behavior, right?

M.M
  • 138,810
  • 21
  • 208
  • 365
Hongxu Chen
  • 5,240
  • 2
  • 45
  • 85
  • 5
    Well, your first sentence lumps up several unrelated things. The rule is that deleting derived class through base class pointer is UB if the destructor is non-virtual. That's all there is to it. Whether the class has any other virtual functions is completely irrelevant. Presence of other virtual functions is just a loose rule-of-the-thumb criterion that suggests that this class is *relatively likely* to be deleted through a base class pointer. – AnT stands with Russia Oct 12 '14 at 03:02
  • 1
    @AndreyT thanks for pointing out that, i updated the original question to avoid misunderstanding. But I just discover that gcc/clang won't emit warnings if no `virtual functions` called, why? – Hongxu Chen Oct 12 '14 at 03:19

3 Answers3

13

Yes, it's still undefined behavior. The problem is that the delete call happens inside std::default_delete, which is inside a system header. By default, the compiler doesn't generate warnings for code in system headers.

If you pass -Wsystem-headers, you'll see the warning. Unfortunately, it's buried inside a pile of other warnings.

T.C.
  • 133,968
  • 17
  • 288
  • 421
  • too many diagnostics that i guess regular programmers will never use `-Wsystem-headers`; in this sense we have to be more careful ourselves:-( – Hongxu Chen Oct 12 '14 at 03:05
  • 1
    @HongxuChen Yeah, I am *really* disappointed to discover this; it astounds me that `unique_ptr` is in any way *less* safe to use than raw pointers. Fortunately, both GCC and Clang do have `-Wnon-virtual-dtor`, which warns about non-virtual destructors even if you don't try to `delete` a pointer. – Kyle Strand Aug 21 '15 at 02:48
5

Not mentioned yet by the other answers:

This problem only exists for unique_ptr, not for the shared_ptr.

Both of these smart pointers can have custom deleters; however unique_ptr defaults to deleting the base pointer, and shared_ptr defaults to deleting the derived pointer (if you used make_shared<Derived> or equivalent).

Another way to solve the problem is to supply your own custom deleter for unique_ptr that deletes the derived pointer. This might be a good solution for cases where you want to avoid the overhead of introducing a vtable.

Further reading: unique_ptr deleter, shared_ptr deleter

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365
0

As of clang r312167 (which will be in clang 6.0 when it's released in about half a year), clang's -Wdelete-non-virtual-dtor warning will warn about this.

thakis
  • 5,405
  • 1
  • 33
  • 33