Warn about base classes with non-virtual destructors

NEW
Assigned to

Status

()

Core
Rewriting and Analysis
--
enhancement
8 years ago
7 years ago

People

(Reporter: Jesse Ruderman, Assigned: sneha)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

8 years ago
Does GCC warn when a base class has a non-virtual destructor?  If not, it would be nice to have a static analysis for it.

http://shawnwilsher.com/archives/255 is an example of a Firefox bug caused by this pattern.
these cppcheck warnings may be somewhat relevant to virtual stuff (single example from *old* log):

[./content/base/src/nsStubMutationObserver.h:61]: (error) Class nsStubMutationObserver which is inherited by class nsNodeIterator does not have a virtual destructor
One possible way to do this.

Check for any call to a destructor where:

1. The dtor isn't virtual
2. The class has subclasses
3. The caller isn't a subclass dtor

This isn't 100% correct since it could have been the base class that was instantiated. Or the subclass dtor might be empty (as is the case with nsAutoTArray).

However it's possible that this wouldn't give any false positives in the our current codebase, or that they are few enough to annotate (e.g. annotate nsAutoTArray as an ignorable subclass).
(Assignee)

Comment 3

8 years ago
I am a  student from SJCE, Mysore India and would like to take up this bug.
(Reporter)

Comment 4

8 years ago
Thanks, Sneha.  I suggest reading https://developer.mozilla.org/en/Dehydra and
asking any questions you have in irc.mozilla.org #static.  I look forward to seeing what you come up with!
Assignee: nobody → snehab09
(Assignee)

Comment 5

7 years ago
Hello,
 This is the script to warn for base classes with non-virtual destructors.

 function process_type(t)
{
  if(t.bases)
  {
   for each (let base in t.bases)
   {
     for (var i in base.type.members)
         
       if (base.type.members[i].name.match("~") && !base.type.members[i].name.isVirtual)
        warning( base.type.members[i].name + "This base class destructor must be made virtual")          
   }       
  }
} 

I would ask to take a look over it and suggest improvements.
You need to log in before you can comment on or make changes to this bug.