diff --git a/src/coreclr/debug/di/module.cpp b/src/coreclr/debug/di/module.cpp index 558fccbb0a34f7..5dac6157145ab8 100644 --- a/src/coreclr/debug/di/module.cpp +++ b/src/coreclr/debug/di/module.cpp @@ -928,7 +928,7 @@ void CordbModule::InitPublicMetaData(TargetBuffer buffer) // copy it over from the remote process CoTaskMemHolder pMetaDataCopy; - CopyRemoteMetaData(buffer, pMetaDataCopy.GetAddr()); + CopyRemoteMetaData(buffer, &pMetaDataCopy); // @@ -955,7 +955,7 @@ void CordbModule::InitPublicMetaData(TargetBuffer buffer) reinterpret_cast( &m_pIMImport )); // MetaData has taken ownership -don't free the memory - pMetaDataCopy.SuppressRelease(); + pMetaDataCopy.Detach(); // Immediately restore the old setting. HRESULT hrRestore = pDisp->SetOption(MetaDataSetUpdate, &valueOld); @@ -1009,7 +1009,7 @@ void CordbModule::UpdatePublicMetaDataFromRemote(TargetBuffer bufferRemoteMetaDa // First copy it from the remote process CoTaskMemHolder pLocalMetaDataPtr; - CopyRemoteMetaData(bufferRemoteMetaData, pLocalMetaDataPtr.GetAddr()); + CopyRemoteMetaData(bufferRemoteMetaData, &pLocalMetaDataPtr); IMetaDataDispenserEx * pDisp = GetProcess()->GetDispenser(); _ASSERTE(pDisp != NULL); // throws on error. @@ -1037,7 +1037,7 @@ void CordbModule::UpdatePublicMetaDataFromRemote(TargetBuffer bufferRemoteMetaDa IfFailThrow(hr); // Success. MetaData now owns the metadata memory - pLocalMetaDataPtr.SuppressRelease(); + pLocalMetaDataPtr.Detach(); } //--------------------------------------------------------------------------------------- @@ -1058,7 +1058,7 @@ void CordbModule::UpdatePublicMetaDataFromRemote(TargetBuffer bufferRemoteMetaDa // Uses an allocator (CoTaskMemHolder) that lets us hand off the memory to the metadata. void CordbModule::CopyRemoteMetaData( TargetBuffer buffer, - CoTaskMemHolder * pLocalBuffer) + VOID** pLocalBuffer) { CONTRACTL { @@ -1071,20 +1071,16 @@ void CordbModule::CopyRemoteMetaData( // Allocate space for the local copy of the metadata // No need to zero out the memory since we'll fill it all here. - LPVOID pRawBuffer = CoTaskMemAlloc(buffer.cbSize); - if (pRawBuffer == NULL) + CoTaskMemHolder pBuffer{ (BYTE*)CoTaskMemAlloc(buffer.cbSize) }; + if (pBuffer == NULL) { ThrowOutOfMemory(); } - pLocalBuffer->Assign(pRawBuffer); - - - // Copy the metadata from the left side - GetProcess()->SafeReadBuffer(buffer, (BYTE *)pRawBuffer); + GetProcess()->SafeReadBuffer(buffer, pBuffer); - return; + *pLocalBuffer = pBuffer.Detach(); } HRESULT CordbModule::QueryInterface(REFIID id, void **pInterface) diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index 1bb37b99d72e85..49f3adb8d30a46 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -665,7 +665,7 @@ CordbProcess::CreateDacDbiInterface() // same directory as DBI if (m_hDacModule == NULL) { - m_hDacModule.Assign(ShimProcess::GetDacModule(m_cordb->GetDacModulePath())); + m_hDacModule = ShimProcess::GetDacModule(m_cordb->GetDacModulePath()); } // @@ -1541,7 +1541,7 @@ void CordbProcess::FreeDac() if (m_hDacModule != NULL) { LOG((LF_CORDB, LL_INFO1000, "Unloading DAC\n")); - m_hDacModule.Clear(); + m_hDacModule.Free(); } } diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index 6caf7a556bf06f..79db022e5b9cf8 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -4298,7 +4298,7 @@ class CordbModule : public CordbBase, BOOL IsFileMetaDataValid(); // Helper to copy metadata buffer from the Target to the host. - void CopyRemoteMetaData(TargetBuffer buffer, CoTaskMemHolder * pLocalBuffer); + void CopyRemoteMetaData(TargetBuffer buffer, VOID** pLocalBuffer); CordbAssembly * ResolveAssemblyInternal(mdToken tkAssemblyRef); @@ -6752,11 +6752,11 @@ typedef std::function ValueGette class CordbValueEnum : public CordbBase, public ICorDebugValueEnum { public: - CordbValueEnum(CordbProcess* pProcess, - UINT maxCount, + CordbValueEnum(CordbProcess* pProcess, + UINT maxCount, ValueGetter valueGetter, NeuterList* pNeuterList); - + virtual ~CordbValueEnum(); virtual void Neuter(); diff --git a/src/coreclr/inc/ex.h b/src/coreclr/inc/ex.h index c607250dc48a19..0c3f82b9c265b5 100644 --- a/src/coreclr/inc/ex.h +++ b/src/coreclr/inc/ex.h @@ -281,85 +281,18 @@ class Exception virtual Exception *DomainBoundCloneHelper() { return CloneHelper(); } }; -#if 1 - -inline void Exception__Delete(Exception* pvMemory) +struct ExceptionTraits final { - Exception::Delete(pvMemory); -} - -using ExceptionHolder = SpecializedWrapper; -#else - -//------------------------------------------------------------------------------ -// class ExceptionHolder -// -// This is a very lightweight holder class for use inside the EX_TRY family -// of macros. It is based on the standard Holder classes, but has been -// highly specialized for this one function, so that extra code can be -// removed, and the resulting code can be simple enough for all of the -// non-exceptional-case code to be inlined. -class ExceptionHolder -{ -private: - Exception *m_value; - BOOL m_acquired; - -public: - FORCEINLINE ExceptionHolder(Exception *pException = NULL, BOOL take = TRUE) - : m_value(pException) - { - m_acquired = pException && take; - } - - FORCEINLINE ~ExceptionHolder() - { - if (m_acquired) - { - Exception::Delete(m_value); - } - } - - Exception* operator->() { return m_value; } - - void operator=(Exception *p) + using Type = Exception*; + static constexpr Type Default() { return NULL; } + static void Free(Type value) { - Release(); - m_value = p; - Acquire(); + STATIC_CONTRACT_WRAPPER; + Exception::Delete(value); } - - BOOL IsNull() { return m_value == NULL; } - - operator Exception*() { return m_value; } - - Exception* GetValue() { return m_value; } - - void SuppressRelease() { m_acquired = FALSE; } - -private: - void Acquire() - { - _ASSERTE(!m_acquired); - - if (!IsNull()) - { - m_acquired = TRUE; - } - } - void Release() - { - if (m_acquired) - { - _ASSERTE(!IsNull()); - Exception::Delete(m_value); - m_acquired = FALSE; - } - } - }; -#endif +using ExceptionHolder = LifetimeHolder; // --------------------------------------------------------------------------- // HRException class. Implements exception API for exceptions generated from HRESULTs @@ -934,13 +867,13 @@ Exception *ExThrowWithInnerHelper(Exception *inner); #define EX_RETHROW \ { \ - __pException.SuppressRelease(); \ + __pException.Detach(); \ PAL_CPP_RETHROW; \ } \ // Define a copy of GET_EXCEPTION() that will not be redefined by clrex.h -#define GET_EXCEPTION() (__pException == NULL ? &__defaultException : __pException.GetValue()) -#define EXTRACT_EXCEPTION() (__pException.Extract()) +#define GET_EXCEPTION() (__pException == NULL ? &__defaultException : static_cast(__pException)) +#define EXTRACT_EXCEPTION() (__pException.Detach()) //============================================================================== diff --git a/src/coreclr/inc/holder.h b/src/coreclr/inc/holder.h index df1d7b365c05f5..82330d4c45590b 100644 --- a/src/coreclr/inc/holder.h +++ b/src/coreclr/inc/holder.h @@ -843,63 +843,6 @@ FORCEINLINE void DoTheRelease(TYPE *value) template using ReleaseHolder = SpecializedWrapper<_TYPE, DoTheRelease<_TYPE>>; -//----------------------------------------------------------------------------- -// StubHolder : holder for stubs -// -// Usage example: -// -// { -// StubHolder foo; -// foo = new Stub(); -// foo->AddRef(); -// // Note StubHolder doesn't call AddRef for you. -// } // foo->DecRef() on out of scope -// -//----------------------------------------------------------------------------- - -template -class ExecutableWriterHolderNoLog; - -class ExecutableAllocator; - -template -FORCEINLINE void StubRelease(TYPE* value) -{ - if (value) - { -#ifdef LOG_EXECUTABLE_ALLOCATOR_STATISTICS -#ifdef HOST_UNIX - LOGGER::LogUsage(__FILE__, __LINE__, __PRETTY_FUNCTION__); -#else - LOGGER::LogUsage(__FILE__, __LINE__, __FUNCTION__); -#endif -#endif // LOG_EXECUTABLE_ALLOCATOR_STATISTICS - ExecutableWriterHolderNoLog stubWriterHolder(value, sizeof(TYPE)); - stubWriterHolder.GetRW()->DecRef(); - } -} - -template -using StubHolder = SpecializedWrapper<_TYPE, StubRelease<_TYPE>>; - -//----------------------------------------------------------------------------- -// CoTaskMemHolder : CoTaskMemAlloc allocated memory holder -// -// { -// CoTaskMemHolder foo = (Foo*) CoTaskMemAlloc(sizeof(Foo)); -// } // delete foo on out of scope -//----------------------------------------------------------------------------- - -template -FORCEINLINE void DeleteCoTaskMem(TYPE *value) -{ - if (value) - CoTaskMemFree(value); -} - -template -using CoTaskMemHolder = SpecializedWrapper<_TYPE, DeleteCoTaskMem<_TYPE>>; - //----------------------------------------------------------------------------- // NewHolder : New'ed memory holder // @@ -999,46 +942,6 @@ class NewInterfaceArrayHolder : public NewArrayHolder }; -//----------------------------------------------------------------------------- -// ResetPointerHolder : pointer which needs to be set to NULL -// { -// ResetPointerHolder holder = &pFoo; -// } // "*pFoo=NULL" on out of scope -//----------------------------------------------------------------------------- -#ifdef __GNUC__ -// With -fvisibility-inlines-hidden, the Invoke methods below -// get hidden, which causes warnings when visible classes expose them. -#define VISIBLE __attribute__ ((visibility("default"))) -#else -#define VISIBLE -#endif // __GNUC__ - -namespace detail -{ - template - struct ZeroMem - { - static VISIBLE void Invoke(T * pVal) - { - ZeroMemory(pVal, sizeof(T)); - } - }; - - template - struct ZeroMem - { - static VISIBLE void Invoke(T ** pVal) - { - *pVal = NULL; - } - }; - -} -#undef VISIBLE - -template -using ResetPointerHolder = SpecializedWrapper<_TYPE, detail::ZeroMem<_TYPE>::Invoke>; - //----------------------------------------------------------------------------- // Wrap win32 functions using HANDLE //----------------------------------------------------------------------------- @@ -1054,10 +957,6 @@ typedef Wrapper, VoidCloseHandle, (UINT_PTR) -1> Handl // Misc holders //----------------------------------------------------------------------------- -// A holder for HMODULE. -FORCEINLINE void HolderFreeLibrary(HMODULE h) { FreeLibrary(h); } -typedef Wrapper, HolderFreeLibrary, 0> HModuleHolder; - template class LifetimeHolder final { @@ -1167,6 +1066,122 @@ struct LocalAllocTraits final template using LocalAllocHolder = LifetimeHolder>; +// A holder for HMODULE. +struct HModuleTraits final +{ + using Type = HMODULE; + static constexpr Type Default() { return NULL; } + static void Free(Type h) + { + STATIC_CONTRACT_WRAPPER; + if (h != NULL) + ::FreeLibrary(h); + } +}; + +using HModuleHolder = LifetimeHolder; + +//----------------------------------------------------------------------------- +// ResetPointerHolder : pointer which needs to be set to NULL (or zeroed) on +// scope exit. The holder stores the address of the slot to clear; on Free it +// nulls the pointer (for pointer T) or zeroes the storage (for non-pointer T). +// +// { +// ResetPointerHolder holder{ &pFoo }; +// } // "pFoo = NULL" on out of scope +//----------------------------------------------------------------------------- +template +struct ResetPointerTraits final +{ + using Type = T*; + static constexpr Type Default() { return NULL; } + static void Free(Type p) + { + STATIC_CONTRACT_WRAPPER; + if (p == NULL) + return; + + // If T is a pointer type, we set the pointer to NULL. If T is a non-pointer type, we zero the memory. + if constexpr (std::is_pointer::value) + *p = NULL; + else + ZeroMemory(p, sizeof(T)); + } +}; + +template +using ResetPointerHolder = LifetimeHolder>; + +//----------------------------------------------------------------------------- +// CoTaskMemHolder : holder for memory allocated by ::CoTaskMemAlloc. +// +// { +// CoTaskMemHolder foo{ (Foo*)::CoTaskMemAlloc(sizeof(Foo)) }; +// } // ::CoTaskMemFree(foo) on out of scope +//----------------------------------------------------------------------------- +template +struct CoTaskMemTraits final +{ + using Type = T*; + static constexpr Type Default() { return NULL; } + static void Free(Type value) + { + STATIC_CONTRACT_WRAPPER; + if (value != NULL) + ::CoTaskMemFree(value); + } +}; + +template +using CoTaskMemHolder = LifetimeHolder>; + +//----------------------------------------------------------------------------- +// StubHolder : holder for runtime-emitted Stub-like objects. +// On scope exit, calls DecRef through the executable-memory +// writer-holder so the refcount field can be written. +// +// Note: StubHolder does NOT call IncRef on assignment - the caller owns +// matching IncRef/DecRef pairing on the value it hands to the holder. +// +// Usage example: +// +// { +// StubHolder foo; +// foo = new Stub(); +// foo->AddRef(); +// } // foo->DecRef() on out of scope +//----------------------------------------------------------------------------- +template +class ExecutableWriterHolderNoLog; + +class ExecutableAllocator; + +template +struct StubTraits final +{ + using Type = T*; + static constexpr Type Default() { return nullptr; } + static void Free(Type value) + { + STATIC_CONTRACT_WRAPPER; + if (value != nullptr) + { +#ifdef LOG_EXECUTABLE_ALLOCATOR_STATISTICS +#ifdef HOST_UNIX + ExecutableAllocator::LogUsage(__FILE__, __LINE__, __PRETTY_FUNCTION__); +#else + ExecutableAllocator::LogUsage(__FILE__, __LINE__, __FUNCTION__); +#endif +#endif // LOG_EXECUTABLE_ALLOCATOR_STATISTICS + ExecutableWriterHolderNoLog stubWriterHolder(value, sizeof(T)); + stubWriterHolder.GetRW()->DecRef(); + } + } +}; + +template +using StubHolder = LifetimeHolder>; + // // We need the following methods to have volatile arguments, so that they can accept // raw pointers in addition to the results of the & operator on Volatile. diff --git a/src/coreclr/utilcode/util.cpp b/src/coreclr/utilcode/util.cpp index 4cb88c41fe08f2..8ff0ea8f0271a9 100644 --- a/src/coreclr/utilcode/util.cpp +++ b/src/coreclr/utilcode/util.cpp @@ -255,7 +255,7 @@ namespace _ASSERTE(wszDllPath != nullptr); // We've got the name of the DLL to load, so load it. - HModuleHolder hDll = WszLoadLibrary(wszDllPath, nullptr, GetLoadWithAlteredSearchPathFlag()); + HModuleHolder hDll{ WszLoadLibrary(wszDllPath, nullptr, GetLoadWithAlteredSearchPathFlag()) }; if (hDll == nullptr) return HRESULT_FROM_GetLastError(); @@ -267,10 +267,10 @@ namespace // Call the function to get a class object for the rclsid and riid passed in. IfFailRet(dllGetClassObject(rclsid, riid, ppv)); - hDll.SuppressRelease(); + HMODULE hLoadedDll = hDll.Detach(); if (phmodDll != nullptr) - *phmodDll = hDll.GetValue(); + *phmodDll = hLoadedDll; return hr; } @@ -334,11 +334,11 @@ HRESULT FakeCoCreateInstanceEx(REFCLSID rclsid, // necessary object. IfFailRet(classFactory->CreateInstance(NULL, riid, ppv)); - hDll.SuppressRelease(); + HMODULE hLoadedDll = hDll.Detach(); if (phmodDll != NULL) { - *phmodDll = hDll.GetValue(); + *phmodDll = hLoadedDll; } return hr; diff --git a/src/coreclr/vm/clrex.h b/src/coreclr/vm/clrex.h index 201077790a0a9f..b2f8e4e9d2bace 100644 --- a/src/coreclr/vm/clrex.h +++ b/src/coreclr/vm/clrex.h @@ -754,7 +754,7 @@ class EEFileLoadException : public EEException #if defined(_DEBUG) // Redefine GET_EXCEPTION to validate CLRLastThrownObjectException as much as possible. #undef GET_EXCEPTION - #define GET_EXCEPTION() (__pException == NULL ? __defaultException.Validate() : __pException.GetValue()) + #define GET_EXCEPTION() (__pException == NULL ? __defaultException.Validate() : static_cast(__pException)) #endif // _DEBUG LONG CLRNoCatchHandler(EXCEPTION_POINTERS* pExceptionInfo, PVOID pv); @@ -811,8 +811,8 @@ LONG CLRNoCatchHandler(EXCEPTION_POINTERS* pExceptionInfo, PVOID pv); /* a findstr /n will allow you to locate it in a pinch */ \ STRESS_LOG1(LF_EH, LL_INFO100, \ "EX_RETHROW " INDEBUG(__FILE__) " line %d\n", __LINE__); \ - __pException.SuppressRelease(); \ - if ((!__state.DidCatchCxx()) && (GetThreadNULLOk() != NULL)) \ + __pException.Detach(); \ + if ((!__state.DidCatchCxx()) && (GetThreadNULLOk() != NULL)) \ { \ if (GetThread()->PreemptiveGCDisabled()) \ { \ diff --git a/src/coreclr/vm/eetoprofinterfaceimpl.cpp b/src/coreclr/vm/eetoprofinterfaceimpl.cpp index c91dfbadf13fc8..5de81da403d444 100644 --- a/src/coreclr/vm/eetoprofinterfaceimpl.cpp +++ b/src/coreclr/vm/eetoprofinterfaceimpl.cpp @@ -683,8 +683,7 @@ HRESULT EEToProfInterfaceImpl::CreateProfiler( // belongs to this class, so NULL out locals without allowing them to release m_pCallback2 = pCallback2.Extract(); pCallback2 = NULL; - m_hmodProfilerDLL = hmodProfilerDLL.Extract(); - hmodProfilerDLL = NULL; + m_hmodProfilerDLL = hmodProfilerDLL.Detach(); // ATTENTION: Please update EEToProfInterfaceImpl::~EEToProfInterfaceImpl() after adding the next ICorProfilerCallback interface here !!! diff --git a/src/coreclr/vm/stubcache.cpp b/src/coreclr/vm/stubcache.cpp index 705c518a3e23cb..93e69014c15ea4 100644 --- a/src/coreclr/vm/stubcache.cpp +++ b/src/coreclr/vm/stubcache.cpp @@ -86,6 +86,7 @@ Stub *StubCacheBase::Canonicalize(const BYTE * pRawStub, const char *stubType) STUBHASHENTRY *phe = NULL; + StubHolder pstub; { CrstHolder ch(&m_crst); @@ -93,15 +94,13 @@ Stub *StubCacheBase::Canonicalize(const BYTE * pRawStub, const char *stubType) phe = (STUBHASHENTRY*)Find((LPVOID)pRawStub); if (phe) { - StubHolder pstub; pstub = phe->m_pStub; ExecutableWriterHolder stubWriterHolder(pstub, sizeof(Stub)); // IncRef as we're returning a reference to our caller. stubWriterHolder.GetRW()->IncRef(); - pstub.SuppressRelease(); - RETURN pstub; + RETURN pstub.Detach(); } } @@ -114,7 +113,6 @@ Stub *StubCacheBase::Canonicalize(const BYTE * pRawStub, const char *stubType) // and link up the stub. CodeLabel *plabel = psl->EmitNewCodeLabel(); psl->EmitBytes(pRawStub, Length(pRawStub)); - StubHolder pstub; pstub = psl->Link(m_heap, linkFlags, stubType); UINT32 offset = psl->GetLabelOffset(plabel); @@ -162,8 +160,7 @@ Stub *StubCacheBase::Canonicalize(const BYTE * pRawStub, const char *stubType) COMPlusThrowOM(); } - pstub.SuppressRelease(); - RETURN pstub; + RETURN pstub.Detach(); } diff --git a/src/coreclr/vm/stublink.cpp b/src/coreclr/vm/stublink.cpp index fe71d31f0d0118..972d5d487007ca 100644 --- a/src/coreclr/vm/stublink.cpp +++ b/src/coreclr/vm/stublink.cpp @@ -571,10 +571,10 @@ Stub *StubLinker::Link(LoaderHeap *pHeap, DWORD flags, const char *stubType) _ASSERTE(!pHeap || pHeap->IsExecutable()); - StubHolder pStub = Stub::NewStub( + StubHolder pStub{ Stub::NewStub( pHeap, size, - flags); + flags) }; ASSERT(pStub != NULL); EmitStub(pStub, globalsize, size, pHeap); @@ -583,7 +583,7 @@ Stub *StubLinker::Link(LoaderHeap *pHeap, DWORD flags, const char *stubType) PerfMap::LogStubs(__FUNCTION__, stubType, pStub->GetEntryPoint(), pStub->GetNumCodeBytes(), PerfMapStubType::Individual); #endif - return pStub.Extract(); + return pStub.Detach(); } int StubLinker::CalculateSize(int* pGlobalSize) diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index d2e2df0ed44c35..147775fa6f598b 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1620,8 +1620,7 @@ BOOL Thread::HasStarted() { if (__pException != NULL) { - __pException.SuppressRelease(); - m_pExceptionDuringStartup = __pException; + m_pExceptionDuringStartup = __pException.Detach(); } res = FALSE; }