I'm seeing a lot of base classes without proper virtual destructors. If the inherited class is destroyed via interface, the destructor of the inherited class is not called and e.g. memory released in destructor is not released. http://en.wikipedia.org/wiki/Virtual_method#Virtual_destructors It is quite a lot of work to check all cases, especially since there is so many of these situations, so easy fix is to add a virtual destructor (It is important that the destructor has also a body, empty {} or non-empty.) to each of the base classes that are inherited by others. IMHO if there is no good reason not to use virtual destructors in base classes, it should be used there to prevent problems that could be really hard to find. I am posting here a list of examples of such situations where virtual destructor should probably be added. Note that some base classes are inherited by multiple classes, so some base classes are listed multiple times in the list. The list was generated by cppcheck: http://cppcheck.wiki.sourceforge.net/ [../WebKit/JavaScriptCore/runtime/JSVariableObject.h:43]: Class JSVariableObject which is inherited by class JSStaticScopeObject does not have a virtual destructor [../WebKit/JavaScriptCore/bytecode/CodeBlock.h:221]: Class CodeBlock which is inherited by class ProgramCodeBlock does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class ArrayNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:123]: Class Node which is inherited by class PropertyListNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class ObjectLiteralNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class BracketAccessorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class BracketAccessorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class DotAccessorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class DotAccessorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:123]: Class Node which is inherited by class ArgumentListNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class NewExprNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class NewExprNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class EvalFunctionCallNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class EvalFunctionCallNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class FunctionCallValueNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class FunctionCallValueNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class FunctionCallResolveNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class FunctionCallResolveNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class FunctionCallBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:306]: Class ThrowableSubExpressionData which is inherited by class FunctionCallBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class FunctionCallDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:306]: Class ThrowableSubExpressionData which is inherited by class FunctionCallDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class PostfixBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:306]: Class ThrowableSubExpressionData which is inherited by class PostfixBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class PostfixDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:306]: Class ThrowableSubExpressionData which is inherited by class PostfixDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class PostfixErrorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:306]: Class ThrowableSubExpressionData which is inherited by class PostfixErrorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class DeleteBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class DeleteBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class DeleteDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class DeleteDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class DeleteValueNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class VoidNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class TypeOfValueNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class PrefixBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:336]: Class ThrowablePrefixedSubExpressionData which is inherited by class PrefixBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class PrefixDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:336]: Class ThrowablePrefixedSubExpressionData which is inherited by class PrefixDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class PrefixErrorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class PrefixErrorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class UnaryOpNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class BinaryOpNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class LogicalOpNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class ConditionalNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class ReadModifyResolveNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class ReadModifyResolveNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class AssignResolveNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class AssignResolveNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class ReadModifyBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:306]: Class ThrowableSubExpressionData which is inherited by class ReadModifyBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class AssignBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class AssignBracketNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class AssignDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class AssignDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class ReadModifyDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:306]: Class ThrowableSubExpressionData which is inherited by class ReadModifyDotNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class AssignErrorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class AssignErrorNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class CommaNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class ConstDeclNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class ConstStatementNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class BlockNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class VarStatementNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class IfNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class DoWhileNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class WhileNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class ForNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class ForInNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class ForInNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class ReturnNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class ReturnNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class WithNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class LabelNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class LabelNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class ThrowNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:269]: Class ThrowableExpressionData which is inherited by class ThrowNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class TryNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class ScopeNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:157]: Class ExpressionNode which is inherited by class FuncExprNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class FuncDeclNode does not have a virtual destructor [../WebKit/JavaScriptCore/parser/Nodes.h:186]: Class StatementNode which is inherited by class SwitchNode does not have a virtual destructor [../WebKit/JavaScriptCore/runtime/JSWrapperObject.h:31]: Class JSWrapperObject which is inherited by class DateInstance does not have a virtual destructor [../WebKit/JavaScriptCore/runtime/JSVariableObject.h:43]: Class JSVariableObject which is inherited by class JSActivation does not have a virtual destructor [../WebKit/JavaScriptCore/runtime/InternalFunction.h:34]: Class InternalFunction which is inherited by class JSFunction does not have a virtual destructor [../WebKit/JavaScriptCore/runtime/JSVariableObject.h:43]: Class JSVariableObject which is inherited by class JSGlobalObject does not have a virtual destructor
Hmm, I'm not sure this tool is saying much interesting – the bulk of the cases are in the parse tree, and I believe the destructors will all already be virtual by virtue of the ParserRefCounted destructor being virtual (I'd have to check the spec & do a quick audit of the code, but I'm pretty confident our behavior is all correct here). In the longer term it is likely that we will be moving in quite the opposite direction – that all parse nodes will be pool allocated, so no destructors will be called at all. Probably worth checking, but I don't think there is a problem here right now, and I doubt we want to be littering the code with more virtual destructors if it is unnecessary to do so. The CodeBlock case is a sufficiently trivial hierarchy that this is not a real concern. There are only three classes, and there is one place that each is referenced, always with a pointer of the specific type. That said, the code could be cleaned up a little to make it a little more obvious what is going on (the hierarchy is a little asymmetric at the minute; I think it'd be clearer if EvalCodeBlock didn't inherit from ProgramCodeBlock, and if we added a FunctionCodeBlock type, too). Or we could possibly move the marking up the the ProgramNode/EvalNode so we can remove m_globalObject, and we could unify the CodeBlock types. I can't comment on the JSVariableObject case, I'll have to look at this, but my guess would be like like CodeBlock this is an example of inheritance for reuse, rather than a true virtual inheritance hierarchy. Unless anyone else pipes up on this, I'll probably assign this bug to myself & look into checking / fixing a couple of these things when I get a chance.
Do we have any implication from this 2009 old bug or we can close this now?