Patchwork add -Wdelete-non-virtual-dtor

login
register
mail settings
Submitter Jonathan Wakely
Date June 4, 2011, 12:45 p.m.
Message ID <BANLkTimvq59+E8N2YZRdF0jR3CaSVr=4RQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/98723/
State New
Headers show

Comments

Jonathan Wakely - June 4, 2011, 12:45 p.m.
New patch using CLASSTYPE_PURE_VIRTUALS, thanks for that.

Bootstrapped and tested again on x86_64-linux, no regressions.

ChangeLogs as before, OK for trunk?
Jason Merrill - June 4, 2011, 3:49 p.m.
On 06/04/2011 08:45 AM, Jonathan Wakely wrote:
> +		  if (CLASSTYPE_PURE_VIRTUALS (type))
> +		    warning(OPT_Wdelete_non_virtual_dtor,
> +			    "deleting object of abstract class type %qT"
> +			    " which has non-virtual destructor"
> +			    " will cause undefined behaviour", type);
> +		  else
> +		    warning(OPT_Wdelete_non_virtual_dtor,
> +			    "deleting object of polymorphic class type %qT"
> +			    " which has non-virtual destructor"
> +			    " may cause undefined behaviour", type);

Space before the (.  And let's use "might" instead of "may".  OK with 
those changes.

Jason
Jonathan Wakely - June 4, 2011, 4:19 p.m.
On 4 June 2011 16:49, Jason Merrill wrote:
> On 06/04/2011 08:45 AM, Jonathan Wakely wrote:
>>
>> +                 if (CLASSTYPE_PURE_VIRTUALS (type))
>> +                   warning(OPT_Wdelete_non_virtual_dtor,
>> +                           "deleting object of abstract class type %qT"
>> +                           " which has non-virtual destructor"
>> +                           " will cause undefined behaviour", type);
>> +                 else
>> +                   warning(OPT_Wdelete_non_virtual_dtor,
>> +                           "deleting object of polymorphic class type
>> %qT"
>> +                           " which has non-virtual destructor"
>> +                           " may cause undefined behaviour", type);
>
> Space before the (.  And let's use "might" instead of "may".  OK with those
> changes.

Fixed and committed, thanks.

Patch

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 174624)
+++ c-family/c.opt	(working copy)
@@ -331,6 +331,10 @@  Wdeclaration-after-statement
 C ObjC Var(warn_declaration_after_statement) Warning
 Warn when a declaration is found after a statement
 
+Wdelete-non-virtual-dtor
+C++ ObjC++ Var(warn_delnonvdtor) Warning
+Warn about deleting polymorphic objects with non-virtual destructors
+
 Wdeprecated
 C C++ ObjC ObjC++ Var(warn_deprecated) Init(1) Warning
 Warn if a deprecated compiler feature, class, method, or field is used
Index: c-family/c-opts.c
===================================================================
--- c-family/c-opts.c	(revision 174624)
+++ c-family/c-opts.c	(working copy)
@@ -405,6 +405,7 @@  c_common_handle_option (size_t scode, co
           warn_sign_compare = value;
 	  warn_reorder = value;
           warn_cxx0x_compat = value;
+          warn_delnonvdtor = value;
 	}
 
       cpp_opts->warn_trigraphs = value;
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 174624)
+++ cp/init.c	(working copy)
@@ -3421,6 +3421,25 @@  build_delete (tree type, tree addr, spec
 		}
 	      complete_p = false;
 	    }
+	  else if (warn_delnonvdtor && MAYBE_CLASS_TYPE_P (type)
+		   && !CLASSTYPE_FINAL (type) && TYPE_POLYMORPHIC_P (type))
+	    {
+	      tree dtor;
+	      dtor = CLASSTYPE_DESTRUCTORS (type);
+	      if (!dtor || !DECL_VINDEX (dtor))
+	        {
+		  if (CLASSTYPE_PURE_VIRTUALS (type))
+		    warning(OPT_Wdelete_non_virtual_dtor,
+			    "deleting object of abstract class type %qT"
+			    " which has non-virtual destructor"
+			    " will cause undefined behaviour", type);
+		  else
+		    warning(OPT_Wdelete_non_virtual_dtor,
+			    "deleting object of polymorphic class type %qT"
+			    " which has non-virtual destructor"
+			    " may cause undefined behaviour", type);
+		}
+	    }
 	}
       if (VOID_TYPE_P (type) || !complete_p || !MAYBE_CLASS_TYPE_P (type))
 	/* Call the builtin operator delete.  */
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 174624)
+++ doc/invoke.texi	(working copy)
@@ -2331,6 +2331,15 @@  Warn when a class seems unusable because
 destructors in that class are private, and it has neither friends nor
 public static member functions.
 
+@item -Wdelete-non-virtual-dtor @r{(C++ and Objective-C++ only)}
+@opindex Wdelete-non-virtual-dtor
+@opindex Wno-delete-non-virtual-dtor
+Warn when @samp{delete} is used to destroy an instance of a class which
+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 @option{-Wall}.
+
 @item -Wnoexcept @r{(C++ and Objective-C++ only)}
 @opindex Wnoexcept
 @opindex Wno-noexcept
Index: testsuite/g++.dg/warn/delete-non-virtual-dtor.C
===================================================================
--- testsuite/g++.dg/warn/delete-non-virtual-dtor.C	(revision 0)
+++ testsuite/g++.dg/warn/delete-non-virtual-dtor.C	(revision 0)
@@ -0,0 +1,44 @@ 
+// { dg-options "-std=gnu++0x -Wdelete-non-virtual-dtor" }
+// { dg-do compile }
+
+struct polyBase { virtual void f(); };
+
+void f(polyBase* p, polyBase* arr)
+{
+  delete p;      // { dg-warning "non-virtual destructor may" }
+  delete [] arr;
+}
+
+struct polyDerived : polyBase { };
+
+void f(polyDerived* p, polyDerived* arr)
+{
+  delete p;      // { dg-warning "non-virtual destructor may" }
+  delete [] arr;
+}
+
+struct absDerived : polyBase { virtual void g() = 0; };
+
+void f(absDerived* p, absDerived* arr)
+{
+  delete p;      // { dg-warning "non-virtual destructor will" }
+  delete [] arr;
+}
+
+struct finalDerived final : polyBase { };
+
+void f(finalDerived* p, finalDerived* arr)
+{
+  delete p;      // no error for final classes
+  delete [] arr;
+}
+
+struct safeBase { virtual ~safeBase(); };
+struct safeDerived : safeBase { virtual void f(); };
+
+void f(safeDerived* p, safeDerived* arr)
+{
+  delete p;      // no error because base has virtual dtor
+  delete [] arr;
+}
+