Skip to content

Commit

Permalink
WinAdapter: Make IUnknown and IMalloc pure-virtual classes
Browse files Browse the repository at this point in the history
`IUnknown` in Windows' `unknwn.h` and `IMalloc` in `ObjIdl.h` are marked
as pure virtual, and are best marked as such in `WinAdapter` for
non-Windows platforms too [1].  Only the shim for `IMalloc` was relying
on the default refcounting implementation, all other subclasses either
contain pure-virtual methods themselves or provide an implementation for
`AddRef`/`Release` as required.  Likewise the default implementation for
`IMalloc` was only instantiated once by `CoGetMalloc`, and has been
moved into a local class implementing the `IMalloc` interface instead.

[1]: microsoft#3793 (comment)
  • Loading branch information
MarijnS95 authored and pow2clk committed Jan 24, 2022
1 parent 418170f commit e8fb0ce
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 50 deletions.
16 changes: 6 additions & 10 deletions include/dxc/Support/WinAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -615,27 +615,23 @@ template <typename T> 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 ULONG AddRef() = 0;
virtual ULONG Release() = 0;
template <class Q> HRESULT QueryInterface(Q **pp) {
return QueryInterface(__uuidof(Q), (void **)pp);
}

private:
std::atomic<unsigned long> m_count;
};

CROSS_PLATFORM_UUIDOF(INoMarshal, "ECC8691B-C1DB-4DC0-855E-65F6C551AF49")
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;
};

CROSS_PLATFORM_UUIDOF(ISequentialStream, "0C733A30-2A1C-11CE-ADE5-00AA0044773D")
Expand Down
2 changes: 1 addition & 1 deletion include/dxc/Test/CompilationResult.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ inline HRESULT GetFirstChildFromCursor(IDxcCursor *cursor,
return hr;
}

class TrivialDxcUnsavedFile : IDxcUnsavedFile
class TrivialDxcUnsavedFile final : IDxcUnsavedFile
{
private:
volatile std::atomic<llvm::sys::cas_flag> m_dwRef;
Expand Down
16 changes: 7 additions & 9 deletions lib/DxcSupport/FileIOHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<IMalloc>(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] */
Expand All @@ -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)
{
return HeapSize(GetProcessHeap(), 0, pv);
}

virtual int STDMETHODCALLTYPE DidAlloc(
int STDMETHODCALLTYPE DidAlloc(
/* [annotation][in] */
_In_opt_ void *pv)
{
return -1; // don't know
}


virtual void STDMETHODCALLTYPE HeapMinimize(void) {}
void STDMETHODCALLTYPE HeapMinimize(void) override {}
};

static HeapMalloc g_HeapMalloc;
Expand Down
23 changes: 0 additions & 23 deletions lib/DxcSupport/WinAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +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;
}
//===--------------------------- 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() {
Expand Down
20 changes: 19 additions & 1 deletion lib/DxcSupport/WinFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <unistd.h>

#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) {
Expand Down Expand Up @@ -154,8 +155,25 @@ 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); }

private:
DXC_MICROCOM_REF_FIELD(m_dwRef)
};

HRESULT CoGetMalloc(DWORD dwMemContext, IMalloc **ppMalloc) {
*ppMalloc = new IMalloc;
*ppMalloc = new CoMalloc;
(*ppMalloc)->AddRef();
return S_OK;
}
Expand Down
7 changes: 4 additions & 3 deletions tools/clang/tools/dxclib/dxc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ int DxcContext::VerifyRootSignature() {
}
}

class DxcIncludeHandlerForInjectedSources : public IDxcIncludeHandler {
class DxcIncludeHandlerForInjectedSources final : public IDxcIncludeHandler {
private:
DXC_MICROCOM_REF_FIELD(m_dwRef)

Expand Down Expand Up @@ -707,15 +707,16 @@ void DxcContext::Recompile(IDxcBlob *pSource, IDxcLibrary *pLibrary,
IFT(pPdbUtils->GetEntryPoint(&pEntryPoint));

CComPtr<IDxcBlobEncoding> pCompileSource;
CComPtr<DxcIncludeHandlerForInjectedSources> pIncludeHandler = new DxcIncludeHandlerForInjectedSources();
DxcIncludeHandlerForInjectedSources *pIncludeHandlerForInjectedSources = new DxcIncludeHandlerForInjectedSources();
CComPtr<IDxcIncludeHandler> pIncludeHandler = pIncludeHandlerForInjectedSources;
UINT32 uSourceCount = 0;
IFT(pPdbUtils->GetSourceCount(&uSourceCount));
for (UINT32 i = 0; i < uSourceCount; i++) {
CComPtr<IDxcBlobEncoding> pSourceFile;
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);
}
Expand Down
2 changes: 1 addition & 1 deletion tools/clang/tools/libclang/dxcisenseimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions tools/clang/unittests/HLSL/ExtensionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ class IntrinsicTable {
}
};

class TestIntrinsicTable : public IDxcIntrinsicTable {
class TestIntrinsicTable final : public IDxcIntrinsicTable {
private:
DXC_MICROCOM_REF_FIELD(m_dwRef)
std::vector<IntrinsicTable> m_tables;
Expand Down Expand Up @@ -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<std::string> m_errorDefines;
Expand Down

0 comments on commit e8fb0ce

Please sign in to comment.