diff --git a/cmake/modules/HandleLLVMOptions.cmake b/cmake/modules/HandleLLVMOptions.cmake index cdf8babe46..2513311c83 100644 --- a/cmake/modules/HandleLLVMOptions.cmake +++ b/cmake/modules/HandleLLVMOptions.cmake @@ -412,19 +412,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 354442fecc..bb2a51cb54 100644 --- a/include/dxc/Support/WinAdapter.h +++ b/include/dxc/Support/WinAdapter.h @@ -614,17 +614,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") @@ -632,10 +628,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..5d80d252f5 100644 --- a/include/dxc/Support/microcom.h +++ b/include/dxc/Support/microcom.h @@ -76,6 +76,11 @@ class CComInterfaceArray { } }; +template +void DxcCallDestructor(T *obj) { + obj->T::~T(); +} + #define DXC_MICROCOM_REF_FIELD(m_dwRef) \ volatile std::atomic m_dwRef = {0}; #define DXC_MICROCOM_ADDREF_IMPL(m_dwRef) \ @@ -86,8 +91,10 @@ class CComInterfaceArray { DXC_MICROCOM_ADDREF_IMPL(m_dwRef) \ ULONG STDMETHODCALLTYPE Release() override { \ ULONG result = (ULONG)--m_dwRef; \ - if (result == 0) \ - delete this; \ + if (result == 0) { \ + DxcCallDestructor(this); \ + operator delete(this); \ + } \ return result; \ } @@ -99,11 +106,6 @@ inline T *CreateOnMalloc(IMalloc * pMalloc, Args&&... args) { return (T *)P; } -template -void DxcCallDestructor(T *obj) { - obj->~T(); -} - // The "TM" version keep an IMalloc field that, if not null, indicate // ownership of 'this' and of any allocations used during release. #define DXC_MICROCOM_TM_REF_FIELDS() \ diff --git a/include/dxc/Test/CompilationResult.h b/include/dxc/Test/CompilationResult.h index 50f5e7c6b3..fdc193c522 100644 --- a/include/dxc/Test/CompilationResult.h +++ b/include/dxc/Test/CompilationResult.h @@ -20,6 +20,7 @@ #include #include "dxc/Support/WinIncludes.h" +#include "dxc/Support/microcom.h" #include "dxc/dxcapi.h" #include "dxc/dxcisense.h" @@ -49,14 +50,15 @@ inline HRESULT GetFirstChildFromCursor(IDxcCursor *cursor, return hr; } -class TrivialDxcUnsavedFile : IDxcUnsavedFile +class TrivialDxcUnsavedFile : public IDxcUnsavedFile { private: - volatile std::atomic m_dwRef; + DXC_MICROCOM_REF_FIELD(m_dwRef) LPCSTR m_fileName; LPCSTR m_contents; unsigned m_length; public: + DXC_MICROCOM_ADDREF_RELEASE_IMPL(m_dwRef) TrivialDxcUnsavedFile(LPCSTR fileName, LPCSTR contents) : m_dwRef(0), m_fileName(fileName), m_contents(contents) { @@ -68,13 +70,8 @@ class TrivialDxcUnsavedFile : IDxcUnsavedFile CComPtr pNewValue = new TrivialDxcUnsavedFile(fileName, contents); return pNewValue.QueryInterface(pResult); } - ULONG STDMETHODCALLTYPE AddRef() { return (ULONG)++m_dwRef; } - ULONG STDMETHODCALLTYPE Release() { - ULONG result = (ULONG)--m_dwRef; - if (result == 0) delete this; - return result; - } - HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void** ppvObject) + + HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void** ppvObject) override { if (ppvObject == nullptr) return E_POINTER; if (IsEqualIID(iid, __uuidof(IUnknown)) || @@ -88,19 +85,19 @@ class TrivialDxcUnsavedFile : IDxcUnsavedFile return E_NOINTERFACE; } - HRESULT STDMETHODCALLTYPE GetFileName(LPSTR* pFileName) + HRESULT STDMETHODCALLTYPE GetFileName(LPSTR* pFileName) override { *pFileName = (LPSTR)CoTaskMemAlloc(1 + strlen(m_fileName)); strcpy(*pFileName, m_fileName); return S_OK; } - HRESULT STDMETHODCALLTYPE GetContents(LPSTR* pContents) + HRESULT STDMETHODCALLTYPE GetContents(LPSTR* pContents) override { *pContents = (LPSTR)CoTaskMemAlloc(m_length + 1); memcpy(*pContents, m_contents, m_length + 1); return S_OK; } - HRESULT STDMETHODCALLTYPE GetLength(unsigned* pLength) + HRESULT STDMETHODCALLTYPE GetLength(unsigned* pLength) override { *pLength = m_length; return S_OK; diff --git a/lib/DxcSupport/FileIOHelper.cpp b/lib/DxcSupport/FileIOHelper.cpp index 3644b90ca8..dee74841ed 100644 --- a/lib/DxcSupport/FileIOHelper.cpp +++ b/lib/DxcSupport/FileIOHelper.cpp @@ -44,13 +44,13 @@ struct HeapMalloc : public IMalloc { 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; @@ -321,7 +319,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; @@ -1138,7 +1136,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..9f55c86604 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 : 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/libclang/dxcisenseimpl.cpp b/tools/clang/tools/libclang/dxcisenseimpl.cpp index d2f1bcdbb9..6e02240956 100644 --- a/tools/clang/tools/libclang/dxcisenseimpl.cpp +++ b/tools/clang/tools/libclang/dxcisenseimpl.cpp @@ -581,7 +581,9 @@ HRESULT DxcBasicUnsavedFile::Create( HRESULT hr = newValue->Initialize(fileName, contents, contentLength); if (FAILED(hr)) { - delete newValue; + CComPtr pTmp(newValue->m_pMalloc); + newValue->DxcBasicUnsavedFile::~DxcBasicUnsavedFile(); + pTmp->Free(newValue); return hr; } newValue->AddRef(); diff --git a/tools/clang/unittests/HLSL/CompilerTest.cpp b/tools/clang/unittests/HLSL/CompilerTest.cpp index 3f8cedb983..2bc40b6cce 100644 --- a/tools/clang/unittests/HLSL/CompilerTest.cpp +++ b/tools/clang/unittests/HLSL/CompilerTest.cpp @@ -3104,17 +3104,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 @@ -3138,7 +3138,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) @@ -3149,7 +3149,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); @@ -3167,18 +3167,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;;