diff --git a/cmake/modules/HandleLLVMOptions.cmake b/cmake/modules/HandleLLVMOptions.cmake index 8520fed06b..ebebacb216 100644 --- a/cmake/modules/HandleLLVMOptions.cmake +++ b/cmake/modules/HandleLLVMOptions.cmake @@ -399,19 +399,31 @@ elseif( LLVM_COMPILER_IS_GCC_COMPATIBLE ) append_if(USE_NO_UNINITIALIZED "-Wno-uninitialized" CMAKE_CXX_FLAGS) append_if(USE_NO_MAYBE_UNINITIALIZED "-Wno-maybe-uninitialized" CMAKE_CXX_FLAGS) - # Check if -Wnon-virtual-dtor warns even though the class is marked final. - # If it does, don't add it. So it won't be added on clang 3.4 and older. - # This also catches cases when -Wnon-virtual-dtor isn't supported by - # the compiler at all. - set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) - set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++11 -Werror=non-virtual-dtor") - CHECK_CXX_SOURCE_COMPILES("class base {public: virtual void anchor();protected: ~base();}; - class derived final : public base { public: ~derived();}; - int main() { return 0; }" - CXX_WONT_WARN_ON_FINAL_NONVIRTUALDTOR) - set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS}) - append_if(CXX_WONT_WARN_ON_FINAL_NONVIRTUALDTOR - "-Wnon-virtual-dtor" CMAKE_CXX_FLAGS) + # HLSL Change Starts + + # Windows' and by extension WinAdapter's non-Windows implementation for IUnknown + # use virtual methods without virtual destructor, as that would add two extra + # function-pointers to the vtable in turn offsetting those for every subclass, + # resulting in ABI mismatches: + # https://github.com/microsoft/DirectXShaderCompiler/issues/3783. + # The -Wnon-virtual-dtor warning is disabled to allow this, conforming + # with MSVC behaviour. + + # # Check if -Wnon-virtual-dtor warns even though the class is marked final. + # # If it does, don't add it. So it won't be added on clang 3.4 and older. + # # This also catches cases when -Wnon-virtual-dtor isn't supported by + # # the compiler at all. + # set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) + # set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++11 -Werror=non-virtual-dtor") + # CHECK_CXX_SOURCE_COMPILES("class base {public: virtual void anchor();protected: ~base();}; + # class derived final : public base { public: ~derived();}; + # int main() { return 0; }" + # CXX_WONT_WARN_ON_FINAL_NONVIRTUALDTOR) + # set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS}) + # append_if(CXX_WONT_WARN_ON_FINAL_NONVIRTUALDTOR + # "-Wnon-virtual-dtor" CMAKE_CXX_FLAGS) + + # HLSL Change Ends # Check if -Wcomment is OK with an // comment ending with '\' if the next # line is also a // comment. diff --git a/include/dxc/Support/WinAdapter.h b/include/dxc/Support/WinAdapter.h index 39007a4326..f3138d48e1 100644 --- a/include/dxc/Support/WinAdapter.h +++ b/include/dxc/Support/WinAdapter.h @@ -615,17 +615,13 @@ template inline void **IID_PPV_ARGS_Helper(T **pp) { CROSS_PLATFORM_UUIDOF(IUnknown, "00000000-0000-0000-C000-000000000046") struct IUnknown { - IUnknown() : m_count(0) {}; + IUnknown() {}; virtual HRESULT QueryInterface(REFIID riid, void **ppvObject) = 0; - virtual ULONG AddRef(); - virtual ULONG Release(); - virtual ~IUnknown(); + virtual ULONG AddRef() = 0; + virtual ULONG Release() = 0; template HRESULT QueryInterface(Q **pp) { return QueryInterface(__uuidof(Q), (void **)pp); } - -private: - std::atomic m_count; }; CROSS_PLATFORM_UUIDOF(INoMarshal, "ECC8691B-C1DB-4DC0-855E-65F6C551AF49") @@ -633,10 +629,12 @@ struct INoMarshal : public IUnknown {}; CROSS_PLATFORM_UUIDOF(IMalloc, "00000002-0000-0000-C000-000000000046") struct IMalloc : public IUnknown { - virtual void *Alloc(size_t size); - virtual void *Realloc(void *ptr, size_t size); - virtual void Free(void *ptr); - virtual HRESULT QueryInterface(REFIID riid, void **ppvObject); + virtual void *Alloc(size_t size) = 0; + virtual void *Realloc(void *ptr, size_t size) = 0; + virtual void Free(void *ptr) = 0; + virtual size_t GetSize(void *pv) = 0; + virtual int DidAlloc(void *pv) = 0; + virtual void HeapMinimize(void) = 0; }; CROSS_PLATFORM_UUIDOF(ISequentialStream, "0C733A30-2A1C-11CE-ADE5-00AA0044773D") diff --git a/include/dxc/Support/microcom.h b/include/dxc/Support/microcom.h index 4cc3cc9103..a3c6865d80 100644 --- a/include/dxc/Support/microcom.h +++ b/include/dxc/Support/microcom.h @@ -101,7 +101,7 @@ inline T *CreateOnMalloc(IMalloc * pMalloc, Args&&... args) { template void DxcCallDestructor(T *obj) { - obj->~T(); + obj->T::~T(); } // The "TM" version keep an IMalloc field that, if not null, indicate diff --git a/include/dxc/Test/CompilationResult.h b/include/dxc/Test/CompilationResult.h index 50f5e7c6b3..630e46d603 100644 --- a/include/dxc/Test/CompilationResult.h +++ b/include/dxc/Test/CompilationResult.h @@ -49,7 +49,7 @@ inline HRESULT GetFirstChildFromCursor(IDxcCursor *cursor, return hr; } -class TrivialDxcUnsavedFile : IDxcUnsavedFile +class TrivialDxcUnsavedFile final : IDxcUnsavedFile { private: volatile std::atomic m_dwRef; diff --git a/lib/DxcSupport/FileIOHelper.cpp b/lib/DxcSupport/FileIOHelper.cpp index e8f83c64f8..e66b8b8758 100644 --- a/lib/DxcSupport/FileIOHelper.cpp +++ b/lib/DxcSupport/FileIOHelper.cpp @@ -37,20 +37,20 @@ // Alias for CP_UTF16LE, which is the only one we actually handle. #define CP_UTF16 CP_UTF16LE -struct HeapMalloc : public IMalloc { +struct HeapMalloc final : public IMalloc { public: ULONG STDMETHODCALLTYPE AddRef() override { return 1; } ULONG STDMETHODCALLTYPE Release() override { return 1; } STDMETHODIMP QueryInterface(REFIID iid, void** ppvObject) override { return DoBasicQueryInterface(this, iid, ppvObject); } - virtual void *STDMETHODCALLTYPE Alloc ( + void *STDMETHODCALLTYPE Alloc ( /* [annotation][in] */ _In_ SIZE_T cb) override { return HeapAlloc(GetProcessHeap(), 0, cb); } - virtual void *STDMETHODCALLTYPE Realloc ( + void *STDMETHODCALLTYPE Realloc ( /* [annotation][in] */ _In_opt_ void *pv, /* [annotation][in] */ @@ -59,30 +59,28 @@ struct HeapMalloc : public IMalloc { return HeapReAlloc(GetProcessHeap(), 0, pv, cb); } - virtual void STDMETHODCALLTYPE Free ( + void STDMETHODCALLTYPE Free ( /* [annotation][in] */ _In_opt_ void *pv) override { HeapFree(GetProcessHeap(), 0, pv); } - - virtual SIZE_T STDMETHODCALLTYPE GetSize( + SIZE_T STDMETHODCALLTYPE GetSize( /* [annotation][in] */ - _In_opt_ _Post_writable_byte_size_(return) void *pv) + _In_opt_ _Post_writable_byte_size_(return) void *pv) override { return HeapSize(GetProcessHeap(), 0, pv); } - virtual int STDMETHODCALLTYPE DidAlloc( + int STDMETHODCALLTYPE DidAlloc( /* [annotation][in] */ - _In_opt_ void *pv) + _In_opt_ void *pv) override { return -1; // don't know } - - virtual void STDMETHODCALLTYPE HeapMinimize(void) {} + void STDMETHODCALLTYPE HeapMinimize(void) override {} }; static HeapMalloc g_HeapMalloc; @@ -319,7 +317,7 @@ class InternalDxcBlobEncoding_Impl : public _T { ULONG result = (ULONG)--m_dwRef; if (result == 0) { CComPtr pTmp(m_pMalloc); - this->~InternalDxcBlobEncoding_Impl(); + this->InternalDxcBlobEncoding_Impl::~InternalDxcBlobEncoding_Impl(); pTmp->Free(this); } return result; @@ -1124,7 +1122,7 @@ class MemoryStream : public AbstractMemoryStream, public IDxcBlob { ULONG result = (ULONG)--m_dwRef; if (result == 0) { CComPtr pTmp(m_pMalloc); - this->~MemoryStream(); + this->MemoryStream::~MemoryStream(); pTmp->Free(this); } return result; diff --git a/lib/DxcSupport/WinAdapter.cpp b/lib/DxcSupport/WinAdapter.cpp index 27bd7c3c7e..81a27b5979 100644 --- a/lib/DxcSupport/WinAdapter.cpp +++ b/lib/DxcSupport/WinAdapter.cpp @@ -12,31 +12,6 @@ #include "dxc/Support/WinAdapter.h" #include "dxc/Support/WinFunctions.h" -//===--------------------------- IUnknown ---------------------------------===// - -ULONG IUnknown::AddRef() { - ++m_count; - return m_count; -} -ULONG IUnknown::Release() { - ULONG result = --m_count; - if (m_count == 0) { - delete this; - } - return result; -} -IUnknown::~IUnknown() {} - -//===--------------------------- IMalloc ----------------------------------===// - -void *IMalloc::Alloc(size_t size) { return malloc(size); } -void *IMalloc::Realloc(void *ptr, size_t size) { return realloc(ptr, size); } -void IMalloc::Free(void *ptr) { free(ptr); } -HRESULT IMalloc::QueryInterface(REFIID riid, void **ppvObject) { - assert(false && "QueryInterface not implemented for IMalloc."); - return E_NOINTERFACE; -} - //===--------------------------- CAllocator -------------------------------===// void *CAllocator::Reallocate(void *p, size_t nBytes) throw() { diff --git a/lib/DxcSupport/WinFunctions.cpp b/lib/DxcSupport/WinFunctions.cpp index bf2b25e4f0..299d7197ba 100644 --- a/lib/DxcSupport/WinFunctions.cpp +++ b/lib/DxcSupport/WinFunctions.cpp @@ -20,6 +20,7 @@ #include #include "dxc/Support/WinFunctions.h" +#include "dxc/Support/microcom.h" HRESULT StringCchCopyEx(LPSTR pszDest, size_t cchDest, LPCSTR pszSrc, LPSTR *ppszDestEnd, size_t *pcchRemaining, DWORD dwFlags) { @@ -154,8 +155,28 @@ unsigned char _BitScanForward(unsigned long * Index, unsigned long Mask) { return 1; } +struct CoMalloc final : public IMalloc { + CoMalloc() : m_dwRef(0) {}; + + DXC_MICROCOM_ADDREF_RELEASE_IMPL(m_dwRef) + STDMETHODIMP QueryInterface(REFIID riid, void **ppvObject) override { + assert(false && "QueryInterface not implemented for CoMalloc."); + return E_NOINTERFACE; + } + + void *STDMETHODCALLTYPE Alloc(size_t size) override { return malloc(size); } + void *STDMETHODCALLTYPE Realloc(void *ptr, size_t size) override { return realloc(ptr, size); } + void STDMETHODCALLTYPE Free(void *ptr) override { free(ptr); } + size_t STDMETHODCALLTYPE GetSize(void *pv) override { return -1; } + int STDMETHODCALLTYPE DidAlloc(void *pv) override { return -1; } + void STDMETHODCALLTYPE HeapMinimize(void) override {} + +private: + DXC_MICROCOM_REF_FIELD(m_dwRef) +}; + HRESULT CoGetMalloc(DWORD dwMemContext, IMalloc **ppMalloc) { - *ppMalloc = new IMalloc; + *ppMalloc = new CoMalloc; (*ppMalloc)->AddRef(); return S_OK; } diff --git a/tools/clang/tools/dxclib/dxc.cpp b/tools/clang/tools/dxclib/dxc.cpp index 8bd05a6234..51ed963280 100644 --- a/tools/clang/tools/dxclib/dxc.cpp +++ b/tools/clang/tools/dxclib/dxc.cpp @@ -583,7 +583,7 @@ int DxcContext::VerifyRootSignature() { } } -class DxcIncludeHandlerForInjectedSources : public IDxcIncludeHandler { +class DxcIncludeHandlerForInjectedSources final : public IDxcIncludeHandler { private: DXC_MICROCOM_REF_FIELD(m_dwRef) @@ -686,7 +686,8 @@ void DxcContext::Recompile(IDxcBlob *pSource, IDxcLibrary *pLibrary, IFT(pPdbUtils->GetEntryPoint(&pEntryPoint)); CComPtr pCompileSource; - CComPtr pIncludeHandler = new DxcIncludeHandlerForInjectedSources(); + DxcIncludeHandlerForInjectedSources *pIncludeHandlerForInjectedSources = new DxcIncludeHandlerForInjectedSources(); + CComPtr pIncludeHandler = pIncludeHandlerForInjectedSources; UINT32 uSourceCount = 0; IFT(pPdbUtils->GetSourceCount(&uSourceCount)); for (UINT32 i = 0; i < uSourceCount; i++) { @@ -694,7 +695,7 @@ void DxcContext::Recompile(IDxcBlob *pSource, IDxcLibrary *pLibrary, CComBSTR pFileName; IFT(pPdbUtils->GetSource(i, &pSourceFile)); IFT(pPdbUtils->GetSourceName(i, &pFileName)); - IFT(pIncludeHandler->insertIncludeFile(pFileName, pSourceFile, 0)); + IFT(pIncludeHandlerForInjectedSources->insertIncludeFile(pFileName, pSourceFile, 0)); if (pMainFileName == pFileName) { pCompileSource.Attach(pSourceFile); } diff --git a/tools/clang/tools/libclang/dxcisenseimpl.cpp b/tools/clang/tools/libclang/dxcisenseimpl.cpp index d2f1bcdbb9..1b0bd000bb 100644 --- a/tools/clang/tools/libclang/dxcisenseimpl.cpp +++ b/tools/clang/tools/libclang/dxcisenseimpl.cpp @@ -51,7 +51,7 @@ HRESULT CreateDxcIntelliSense(_In_ REFIID riid, _Out_ LPVOID* ppv) throw() // This is exposed as a helper class, but the implementation works on // interfaces; we expect callers should be able to use their own. -class DxcBasicUnsavedFile : public IDxcUnsavedFile +class DxcBasicUnsavedFile final : public IDxcUnsavedFile { private: DXC_MICROCOM_TM_REF_FIELDS() diff --git a/tools/clang/unittests/HLSL/CompilerTest.cpp b/tools/clang/unittests/HLSL/CompilerTest.cpp index b3e420f232..5c767b40bf 100644 --- a/tools/clang/unittests/HLSL/CompilerTest.cpp +++ b/tools/clang/unittests/HLSL/CompilerTest.cpp @@ -2720,17 +2720,17 @@ struct InstrumentedHeapMalloc : public IMalloc { m_FailAlloc = index; } - ULONG STDMETHODCALLTYPE AddRef() { + ULONG STDMETHODCALLTYPE AddRef() override { return ++m_RefCount; } - ULONG STDMETHODCALLTYPE Release() { + ULONG STDMETHODCALLTYPE Release() override { if (m_RefCount == 0) VERIFY_FAIL(); return --m_RefCount; } - STDMETHODIMP QueryInterface(REFIID iid, void** ppvObject) { + STDMETHODIMP QueryInterface(REFIID iid, void** ppvObject) override { return DoBasicQueryInterface(this, iid, ppvObject); } - virtual void *STDMETHODCALLTYPE Alloc(_In_ SIZE_T cb) { + virtual void *STDMETHODCALLTYPE Alloc(_In_ SIZE_T cb) override { ++m_AllocCount; if (m_FailAlloc && m_AllocCount >= m_FailAlloc) { return nullptr; // breakpoint for i failure - m_FailAlloc == 1+VAL @@ -2752,7 +2752,7 @@ struct InstrumentedHeapMalloc : public IMalloc { return P + 1; } - virtual void *STDMETHODCALLTYPE Realloc(_In_opt_ void *pv, _In_ SIZE_T cb) { + virtual void *STDMETHODCALLTYPE Realloc(_In_opt_ void *pv, _In_ SIZE_T cb) override { SIZE_T priorSize = pv == nullptr ? (SIZE_T)0 : GetSize(pv); void *R = Alloc(cb); if (!R) @@ -2763,7 +2763,7 @@ struct InstrumentedHeapMalloc : public IMalloc { return R; } - virtual void STDMETHODCALLTYPE Free(_In_opt_ void *pv) { + virtual void STDMETHODCALLTYPE Free(_In_opt_ void *pv) override { if (!pv) return; PtrData *P = DataFromPtr(pv); @@ -2779,18 +2779,18 @@ struct InstrumentedHeapMalloc : public IMalloc { virtual SIZE_T STDMETHODCALLTYPE GetSize( /* [annotation][in] */ - _In_opt_ _Post_writable_byte_size_(return) void *pv) + _In_opt_ _Post_writable_byte_size_(return) void *pv) override { if (pv == nullptr) return 0; return DataFromPtr(pv)->Size; } virtual int STDMETHODCALLTYPE DidAlloc( - _In_opt_ void *pv) { + _In_opt_ void *pv) override { return -1; // don't know } - virtual void STDMETHODCALLTYPE HeapMinimize(void) {} + virtual void STDMETHODCALLTYPE HeapMinimize(void) override {} void DumpLeaks() { PtrData *ptr = (PtrData*)AllocList.Flink;; diff --git a/tools/clang/unittests/HLSL/ExtensionTest.cpp b/tools/clang/unittests/HLSL/ExtensionTest.cpp index e9a28fab70..d93dca96d7 100644 --- a/tools/clang/unittests/HLSL/ExtensionTest.cpp +++ b/tools/clang/unittests/HLSL/ExtensionTest.cpp @@ -291,7 +291,7 @@ class IntrinsicTable { } }; -class TestIntrinsicTable : public IDxcIntrinsicTable { +class TestIntrinsicTable final : public IDxcIntrinsicTable { private: DXC_MICROCOM_REF_FIELD(m_dwRef) std::vector m_tables; @@ -395,7 +395,7 @@ class TestIntrinsicTable : public IDxcIntrinsicTable { // and defines that should cause warnings. A more realistic validator // would look at the values and make sure (for example) they are // the correct type (integer, string, etc). -class TestSemanticDefineValidator : public IDxcSemanticDefineValidator { +class TestSemanticDefineValidator final : public IDxcSemanticDefineValidator { private: DXC_MICROCOM_REF_FIELD(m_dwRef) std::vector m_errorDefines;