Do not delete polymorphic objects without a virtual destructor (#23705)

SEI CERT C++ Coding Standard says:

> Do not delete an object of derived class type through a pointer to its
> base class type that has a non-virtual destructor. Instead, the base
> class should be defined with a virtual destructor. Deleting an object
> through a pointer to a type without a virtual destructor results in
> undefined behavior.

See https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP52-CPP.+Do+not+delete+a+polymorphic+object+without+a+virtual+destructor

Clang generally warns about this, but we disabled the warning via
-Wno-delete-non-virtual-dtor. This commit re-enables the warning and
fixes up all the code that hits the warning.

diff --git a/src/ToolBox/SOS/lldbplugin/CMakeLists.txt b/src/ToolBox/SOS/lldbplugin/CMakeLists.txt
index fadb374..6a279ed 100644 (file)
--- a/src/ToolBox/SOS/lldbplugin/CMakeLists.txt
+++ b/src/ToolBox/SOS/lldbplugin/CMakeLists.txt
@@ -124,8 +124,6 @@ endif()
 
 message(STATUS "LLDB_H: ${LLDB_H}")
 
-add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-delete-non-virtual-dtor>)
-
 include_directories(inc)
 include_directories("${LLDB_H}")
 include_directories(${CLR_DIR}/src/debug/inc)

diff --git a/src/ToolBox/SOS/lldbplugin/inc/lldbservices.h b/src/ToolBox/SOS/lldbplugin/inc/lldbservices.h
index ffb2224..7bb9b1a 100644 (file)
--- a/src/ToolBox/SOS/lldbplugin/inc/lldbservices.h
+++ b/src/ToolBox/SOS/lldbplugin/inc/lldbservices.h
@@ -237,6 +237,7 @@ MIDL_INTERFACE("2E6C569A-9E14-4DA4-9DFC-CDB73A532566")
 ILLDBServices : public IUnknown
 {
 public:
+
     //----------------------------------------------------------------------------
     // ILLDBServices
     //----------------------------------------------------------------------------

diff --git a/src/ToolBox/SOS/lldbplugin/services.h b/src/ToolBox/SOS/lldbplugin/services.h
index 6509040..a8fb32f 100644 (file)
--- a/src/ToolBox/SOS/lldbplugin/services.h
+++ b/src/ToolBox/SOS/lldbplugin/services.h
@@ -26,7 +26,7 @@ private:
 
 public:
     LLDBServices(lldb::SBDebugger &debugger, lldb::SBCommandReturnObject &returnObject, lldb::SBProcess *process = nullptr, lldb::SBThread *thread = nullptr);
-    ~LLDBServices();
+    virtual ~LLDBServices();
  
     //----------------------------------------------------------------------------
     // IUnknown

diff --git a/src/dlls/mscorpe/CMakeLists.txt b/src/dlls/mscorpe/CMakeLists.txt
index 6a89193..eb8489c 100644 (file)
--- a/src/dlls/mscorpe/CMakeLists.txt
+++ b/src/dlls/mscorpe/CMakeLists.txt
@@ -9,10 +9,6 @@ set(MSCORPE_SOURCES
   ceefilegenwritertokens.cpp
 )
 
-if(NOT WIN32)
-  add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-delete-non-virtual-dtor>)
-endif()
-
 add_library_clr(mscorpe STATIC
   ${MSCORPE_SOURCES}
 )

diff --git a/src/ilasm/CMakeLists.txt b/src/ilasm/CMakeLists.txt
index f780dca..6117030 100644 (file)
--- a/src/ilasm/CMakeLists.txt
+++ b/src/ilasm/CMakeLists.txt
@@ -51,7 +51,7 @@ if(CLR_CMAKE_PLATFORM_UNIX)
   # Need generate a right form of asmparse.cpp to avoid the following options.
   # Clang also produces a bad-codegen on this prebuilt file with optimization.
   # https://github.com/dotnet/coreclr/issues/2305
-  add_compile_options("$<$<COMPILE_LANGUAGE:CXX>:-Wno-delete-non-virtual-dtor;-Wno-register>")
+  add_compile_options("$<$<COMPILE_LANGUAGE:CXX>:-Wno-register>")
   add_compile_options(-Wno-array-bounds)
   add_compile_options(-Wno-unused-label)
   set_source_files_properties( prebuilt/asmparse.cpp PROPERTIES COMPILE_FLAGS "-O0" )

diff --git a/src/ilasm/asmparse.h b/src/ilasm/asmparse.h
index 3c18f35..9d0c841 100644 (file)
--- a/src/ilasm/asmparse.h
+++ b/src/ilasm/asmparse.h
@@ -19,6 +19,8 @@
 class ReadStream {
 public:
 
+    virtual ~ReadStream() = default;
+
     virtual unsigned getAll(__out char** ppch) = 0;
 
     // read at most 'buffLen' bytes into 'buff', Return the
@@ -276,7 +278,7 @@ class AsmParse : public ErrorReporter
 {
 public:
     AsmParse(ReadStream* stream, Assembler *aAssem);
-    ~AsmParse();
+    virtual ~AsmParse();
     void CreateEnvironment(ReadStream* stream);
        void ParseFile(ReadStream* stream); 
         // The parser knows how to put line numbers on things and report the error 

diff --git a/src/inc/iceefilegen.h b/src/inc/iceefilegen.h
index 7fa31d2..65289f4 100644 (file)
--- a/src/inc/iceefilegen.h
+++ b/src/inc/iceefilegen.h
@@ -76,6 +76,8 @@ typedef HRESULT (__stdcall * PFN_DestroyICeeFileGen)(ICeeFileGen ** ceeFileGen);
 
 class ICeeFileGen {
   public:
+    virtual ~ICeeFileGen() = default;
+
     virtual HRESULT CreateCeeFile(HCEEFILE *ceeFile); // call this to instantiate a file handle
 
     // <TODO>@FUTURE: remove this function. We no longer support mdScope.</TODO>