792634 - nsDeque.cpp:100:10: warning: deleting object of abstract class type ‘nsDequeFunctor’ which has non-virtual destructor will cause undefined behaviour [-Wdelete-non-virtual-dtor]

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Daniel Holbert [:dholbert]]

/xpcom/glue/nsDeque.cpp triggers this build warning right now (in GCC 4.7):
{
nsDeque.cpp: In member function ‘void nsDeque::SetDeallocator(nsDequeFunctor*)’:
nsDeque.cpp:100:10: warning: deleting object of abstract class type ‘nsDequeFunctor’ which has non-virtual destructor will cause undefined behaviour [-Wdelete-non-virtual-dtor]
}

The class in question, nsDequeFunctor, is indeed abstract (it has a pure virtual method) and it does have subclasses -- so this warning is legit.  Hypothetically, if its subclasses had explicit destructors, they wouldn't be getting invoked here, and that's bad.

From a cursory glance at a few of the subclasses, it looks like they *don't* have destructors of their own, so I think we're OK at the moment.  But we shouldn't assume that'll always be true. (unless we had some sort of static-analysis assurance of that)

So -- I think we should give nsDequeFunctor a virtual destructor.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 2

[Benjamin Smedberg]

Comment on attachment 662753 [details] [diff] [review]
fix

Sure. If it already has a vtable the virtual destructor doesn't hurt.

Comment 3

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8309d954e2c

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a8309d954e2c