NEW 155048
[CMake] Enable -Wnon-virtual-dtor and fix violations
https://bugs.webkit.org/show_bug.cgi?id=155048
Summary [CMake] Enable -Wnon-virtual-dtor and fix violations
Michael Catanzaro
Reported 2016-03-04 14:51:53 PST
We should enable -Wnon-virtual-dtor in WEBKIT_SET_EXTRA_COMPILER_FLAGS. It would have caught a crash we introduced on the GTK port recently. Currently we get -Wdelete-non-virtual-dtor via -Wall, but this warning is not as effective. But before doing so, we should make sure to fix whatever warnings pop up.
Attachments
Michael Catanzaro
Comment 1 2016-03-04 14:59:22 PST
For the record: -Wdelete-non-virtual-dtor (C++ and Objective-C++ only) Warn when "delete" is used to destroy an instance of a class that has virtual functions and non-virtual destructor. It is unsafe to delete an instance of a derived class through a pointer to a base class if the base class does not have a virtual destructor. This warning is enabled by -Wall. -Wnon-virtual-dtor (C++ and Objective-C++ only) Warn when a class has virtual functions and an accessible non- virtual destructor itself or in an accessible polymorphic base class, in which case it is possible but unsafe to delete an instance of a derived class through a pointer to the class itself or base class. This warning is automatically enabled if -Weffc++ is specified. Note that we currently use -Wall, but not spammy and nonstandard -Weffc++ (if you haven't heard of -Weffc++, it enables warnings about things Scott Meyers tells us not to do :)
Darin Adler
Comment 2 2016-03-05 12:05:32 PST
I believe we also want this warning turned on for Mac and iOS, so separately we should make sure it’s turned on in the .xcconfig files we currently use on those platforms, if it’s not already.
mitz
Comment 3 2016-03-05 12:09:27 PST
(In reply to comment #2) > I believe we also want this warning turned on for Mac and iOS, so separately > we should make sure it’s turned on in the .xcconfig files we currently use > on those platforms, if it’s not already. GCC_WARN_NON_VIRTUAL_DESTRUCTOR is already set to YES in every Base.xcconfig in Source except for ANGLE’s.
Michael Catanzaro
Comment 4 2016-03-05 14:08:13 PST
(In reply to comment #3) > GCC_WARN_NON_VIRTUAL_DESTRUCTOR is already set to YES in every Base.xcconfig > in Source except for ANGLE’s. Good. That explains why none of these bugs affect Apple ports: ../../Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:34:11: warning: 'WebCore::TextureMapperPlatformLayer::Client' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor] class Client { ^ ../../Source/WebCore/platform/graphics/texmap/TextureMapperLayer.h:44:11: warning: 'WebCore::TextureMapperLayer::ScrollingClient' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor] class ScrollingClient { ^ ../../Source/WebCore/platform/graphics/texmap/TextureMapperAnimation.h:33:11: warning: 'WebCore::TextureMapperAnimation::Client' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor] class Client { ^ ../../Source/WebCore/platform/geoclue/GeolocationProviderGeoclueClient.h:27:7: warning: 'WebCore::GeolocationProviderGeoclueClient' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor] class GeolocationProviderGeoclueClient { ^ ../../Source/WebCore/platform/network/soup/WebKitSoupRequestGenericClient.h:29:7: warning: 'WebCore::WebKitSoupRequestGenericClient' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor] class WebKitSoupRequestGenericClient { ^ ../../Source/WebKit2/NetworkProcess/CustomProtocols/soup/CustomProtocolManagerImpl.h:46:5: warning: 'WebKit::CustomProtocolManagerImpl' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor] ~CustomProtocolManagerImpl(); ^
Michael Catanzaro
Comment 5 2016-03-05 18:04:41 PST
(In reply to comment #4) > Good. That explains why none of these bugs affect Apple ports: I looked at these and none seem to be actual bugs, as these classes are never deleted polymorphically, but it's still a very bad idea to have public nonvirtual destructors.
Michael Catanzaro
Comment 6 2016-03-05 19:47:56 PST
Patch is slow in coming as I'm trying to figure out a nice way to avoid passing this flag when compiling C files, as that causes GCC to print a warning.
Note You need to log in before you can comment on or make changes to this bug.