From aeffede4d4dfa7c6d6028d8931d0287cac08af29 Mon Sep 17 00:00:00 2001 From: Sean Gillespie Date: Mon, 24 Apr 2017 10:44:25 -0700 Subject: [PATCH] [Local GC] Ensure that handle creation returns null on failure instead of throwing (dotnet/coreclr#11092) * [Local GC] Ensure that handle creation returns null on failure instead of throwing * Fix some clang pedantry about jumping past variable initialization * Throw OOM if initialization of handle store fails * Perform the handle OOM check in each handle helper Commit migrated from https://github.com/dotnet/coreclr/commit/69d43a0f8cfe095336b286e7bb892fe49c702e30 --- src/coreclr/src/gc/gchandletable.cpp | 6 +- src/coreclr/src/gc/handletable.cpp | 12 +-- src/coreclr/src/gc/handletablecore.cpp | 35 ++++---- src/coreclr/src/gc/handletablescan.cpp | 2 +- src/coreclr/src/gc/objecthandle.cpp | 26 ++++-- src/coreclr/src/vm/appdomain.cpp | 5 ++ src/coreclr/src/vm/appdomain.hpp | 21 ++++- src/coreclr/src/vm/gchandleutilities.h | 152 ++++++++++++++++++++++++++++----- 8 files changed, 202 insertions(+), 57 deletions(-) diff --git a/src/coreclr/src/gc/gchandletable.cpp b/src/coreclr/src/gc/gchandletable.cpp index 75646e8..52fede6 100644 --- a/src/coreclr/src/gc/gchandletable.cpp +++ b/src/coreclr/src/gc/gchandletable.cpp @@ -47,8 +47,12 @@ OBJECTHANDLE GCHandleStore::CreateDependentHandle(Object* primary, Object* secon { HHANDLETABLE handletable = _underlyingBucket.pTable[GetCurrentThreadHomeHeapNumber()]; OBJECTHANDLE handle = ::HndCreateHandle(handletable, HNDTYPE_DEPENDENT, ObjectToOBJECTREF(primary)); - ::SetDependentHandleSecondary(handle, ObjectToOBJECTREF(secondary)); + if (!handle) + { + return nullptr; + } + ::SetDependentHandleSecondary(handle, ObjectToOBJECTREF(secondary)); return handle; } diff --git a/src/coreclr/src/gc/handletable.cpp b/src/coreclr/src/gc/handletable.cpp index eee1819..05137e4 100644 --- a/src/coreclr/src/gc/handletable.cpp +++ b/src/coreclr/src/gc/handletable.cpp @@ -285,12 +285,7 @@ OBJECTHANDLE HndCreateHandle(HHANDLETABLE hTable, uint32_t uType, OBJECTREF obje { CONTRACTL { -#ifdef FEATURE_REDHAWK - // Redhawk returns NULL on failure. NOTHROW; -#else - THROWS; -#endif GC_NOTRIGGER; if (object != NULL) { @@ -308,8 +303,7 @@ OBJECTHANDLE HndCreateHandle(HHANDLETABLE hTable, uint32_t uType, OBJECTREF obje if (g_pConfig->ShouldInjectFault(INJECTFAULT_HANDLETABLE)) { FAULT_NOT_FATAL(); - char *a = new char; - delete a; + return NULL; } #endif // _DEBUG && !FEATURE_REDHAWK @@ -331,11 +325,7 @@ OBJECTHANDLE HndCreateHandle(HHANDLETABLE hTable, uint32_t uType, OBJECTREF obje // did the allocation succeed? if (!handle) { -#ifdef FEATURE_REDHAWK return NULL; -#else - ThrowOutOfMemory(); -#endif } #ifdef DEBUG_DestroyedHandleValue diff --git a/src/coreclr/src/gc/handletablecore.cpp b/src/coreclr/src/gc/handletablecore.cpp index edd0d94..00ab6a24 100644 --- a/src/coreclr/src/gc/handletablecore.cpp +++ b/src/coreclr/src/gc/handletablecore.cpp @@ -961,12 +961,12 @@ BOOL SegmentHandleAsyncPinHandles (TableSegment *pSegment) } // Replace an async pin handle with one from default domain -void SegmentRelocateAsyncPinHandles (TableSegment *pSegment, HandleTable *pTargetTable) +bool SegmentRelocateAsyncPinHandles (TableSegment *pSegment, HandleTable *pTargetTable) { CONTRACTL { GC_NOTRIGGER; - THROWS; + NOTHROW; MODE_COOPERATIVE; } CONTRACTL_END; @@ -975,7 +975,7 @@ void SegmentRelocateAsyncPinHandles (TableSegment *pSegment, HandleTable *pTarge if (uBlock == BLOCK_INVALID) { // There is no pinning handles. - return; + return true; } for (uBlock = 0; uBlock < pSegment->bEmptyLine; uBlock ++) { @@ -1005,11 +1005,19 @@ void SegmentRelocateAsyncPinHandles (TableSegment *pSegment, HandleTable *pTarge BashMTForPinnedObject(ObjectToOBJECTREF(value)); overlapped->m_pinSelf = HndCreateHandle((HHANDLETABLE)pTargetTable, HNDTYPE_ASYNCPINNED, ObjectToOBJECTREF(value)); + if (!overlapped->m_pinSelf) + { + // failed to allocate a new handle - callers have to handle this. + return false; + } + *pValue = NULL; } pValue ++; } while (pValue != pLast); } + + return true; } // Mark all non-pending AsyncPinHandle ready for cleanup. @@ -1068,6 +1076,7 @@ void TableRelocateAsyncPinHandles(HandleTable *pTable, HandleTable *pTargetTable BOOL fGotException = FALSE; TableSegment *pSegment = pTable->pSegmentList; + bool wasSuccessful = true; #ifdef _DEBUG // on debug builds, execute the OOM path 10% of the time. @@ -1076,21 +1085,18 @@ void TableRelocateAsyncPinHandles(HandleTable *pTable, HandleTable *pTargetTable #endif // Step 1: replace pinning handles with ones from default domain - EX_TRY + while (pSegment) { - while (pSegment) + wasSuccessful = wasSuccessful && SegmentRelocateAsyncPinHandles (pSegment, pTargetTable); + if (!wasSuccessful) { - SegmentRelocateAsyncPinHandles (pSegment, pTargetTable); - pSegment = pSegment->pNextSegment; + break; } + + pSegment = pSegment->pNextSegment; } - EX_CATCH - { - fGotException = TRUE; - } - EX_END_CATCH(SwallowAllExceptions); - if (!fGotException) + if (wasSuccessful) { return; } @@ -2720,9 +2726,8 @@ void TableFreeBulkUnpreparedHandles(HandleTable *pTable, uint32_t uType, const O { CONTRACTL { - THROWS; + NOTHROW; WRAPPER(GC_TRIGGERS); - INJECT_FAULT(COMPlusThrowOM()); } CONTRACTL_END; diff --git a/src/coreclr/src/gc/handletablescan.cpp b/src/coreclr/src/gc/handletablescan.cpp index 065ba0d..967aca5 100644 --- a/src/coreclr/src/gc/handletablescan.cpp +++ b/src/coreclr/src/gc/handletablescan.cpp @@ -1423,7 +1423,7 @@ PTR_TableSegment CALLBACK StandardSegmentIterator(PTR_HandleTable pTable, PTR_Ta { CONTRACTL { - WRAPPER(THROWS); + WRAPPER(NOTHROW); WRAPPER(GC_TRIGGERS); FORBID_FAULT; SUPPORTS_DAC; diff --git a/src/coreclr/src/gc/objecthandle.cpp b/src/coreclr/src/gc/objecthandle.cpp index cd64ae2..dd43ec2 100644 --- a/src/coreclr/src/gc/objecthandle.cpp +++ b/src/coreclr/src/gc/objecthandle.cpp @@ -740,9 +740,9 @@ bool Ref_InitializeHandleTableBucket(HandleTableBucket* bucket, void* context) { CONTRACTL { - THROWS; + NOTHROW; WRAPPER(GC_TRIGGERS); - INJECT_FAULT(COMPlusThrowOM()); + INJECT_FAULT(return false); } CONTRACTL_END; @@ -759,13 +759,18 @@ bool Ref_InitializeHandleTableBucket(HandleTableBucket* bucket, void* context) HandleTableBucketHolder bucketHolder(result, n_slots); - result->pTable = new HHANDLETABLE[n_slots]; + result->pTable = new (nothrow) HHANDLETABLE[n_slots]; + if (!result->pTable) + { + return false; + } + ZeroMemory(result->pTable, n_slots * sizeof(HHANDLETABLE)); for (int uCPUindex=0; uCPUindex < n_slots; uCPUindex++) { result->pTable[uCPUindex] = HndCreateHandleTable(s_rgTypeFlags, _countof(s_rgTypeFlags), ADIndex((DWORD)(uintptr_t)context)); if (!result->pTable[uCPUindex]) - COMPlusThrowOM(); + return false; } for (;;) { @@ -792,9 +797,18 @@ bool Ref_InitializeHandleTableBucket(HandleTableBucket* bucket, void* context) // No free slot. // Let's create a new node NewHolder newMap; - newMap = new HandleTableMap; + newMap = new (nothrow) HandleTableMap; + if (!newMap) + { + return false; + } + + newMap->pBuckets = new (nothrow) HandleTableBucket * [ INITIAL_HANDLE_TABLE_ARRAY_SIZE ]; + if (!newMap->pBuckets) + { + return false; + } - newMap->pBuckets = new HandleTableBucket * [ INITIAL_HANDLE_TABLE_ARRAY_SIZE ]; newMap.SuppressRelease(); newMap->dwMaxIndex = last->dwMaxIndex + INITIAL_HANDLE_TABLE_ARRAY_SIZE; diff --git a/src/coreclr/src/vm/appdomain.cpp b/src/coreclr/src/vm/appdomain.cpp index 4fd35b0..bd05991 100644 --- a/src/coreclr/src/vm/appdomain.cpp +++ b/src/coreclr/src/vm/appdomain.cpp @@ -4280,6 +4280,11 @@ void AppDomain::Init() m_handleStore = GCHandleUtilities::GetGCHandleManager()->CreateHandleStore((void*)(uintptr_t)m_dwIndex.m_dwIndex); } + if (!m_handleStore) + { + COMPlusThrowOM(); + } + #endif // CROSSGEN_COMPILE #ifdef FEATURE_TYPEEQUIVALENCE diff --git a/src/coreclr/src/vm/appdomain.hpp b/src/coreclr/src/vm/appdomain.hpp index 101524f..a5bd00d 100644 --- a/src/coreclr/src/vm/appdomain.hpp +++ b/src/coreclr/src/vm/appdomain.hpp @@ -1244,7 +1244,14 @@ public: OBJECTHANDLE CreateTypedHandle(OBJECTREF object, int type) { WRAPPER_NO_CONTRACT; - return m_handleStore->CreateHandleOfType(OBJECTREFToObject(object), type); + + OBJECTHANDLE hnd = m_handleStore->CreateHandleOfType(OBJECTREFToObject(object), type); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } OBJECTHANDLE CreateHandle(OBJECTREF object) @@ -1317,7 +1324,7 @@ public: { CONTRACTL { - THROWS; + NOTHROW; GC_NOTRIGGER; MODE_COOPERATIVE; } @@ -1337,13 +1344,19 @@ public: { CONTRACTL { - THROWS; + NOTHROW; GC_NOTRIGGER; MODE_COOPERATIVE; } CONTRACTL_END; - return m_handleStore->CreateDependentHandle(OBJECTREFToObject(primary), OBJECTREFToObject(secondary)); + OBJECTHANDLE hnd = m_handleStore->CreateDependentHandle(OBJECTREFToObject(primary), OBJECTREFToObject(secondary)); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } #endif // DACCESS_COMPILE && !CROSSGEN_COMPILE diff --git a/src/coreclr/src/vm/gchandleutilities.h b/src/coreclr/src/vm/gchandleutilities.h index 72c38a6..665c1da 100644 --- a/src/coreclr/src/vm/gchandleutilities.h +++ b/src/coreclr/src/vm/gchandleutilities.h @@ -70,52 +70,112 @@ inline BOOL ObjectHandleIsNull(OBJECTHANDLE handle) inline OBJECTHANDLE CreateHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_DEFAULT); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_DEFAULT); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateWeakHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_DEFAULT); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_DEFAULT); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateShortWeakHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_SHORT); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_SHORT); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateLongWeakHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_LONG); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_LONG); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateStrongHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_STRONG); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_STRONG); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreatePinningHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_PINNED); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_PINNED); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateAsyncPinningHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_ASYNCPINNED); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_ASYNCPINNED); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateRefcountedHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_REFCOUNTED); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_REFCOUNTED); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateSizedRefHandle(IGCHandleStore* store, OBJECTREF object) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_SIZEDREF); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_SIZEDREF); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateSizedRefHandle(IGCHandleStore* store, OBJECTREF object, int heapToAffinitizeTo) { - return store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_SIZEDREF, heapToAffinitizeTo); + OBJECTHANDLE hnd = store->CreateHandleOfType(OBJECTREFToObject(object), HNDTYPE_SIZEDREF, heapToAffinitizeTo); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } // Global handle creation convenience functions @@ -123,39 +183,81 @@ inline OBJECTHANDLE CreateSizedRefHandle(IGCHandleStore* store, OBJECTREF object inline OBJECTHANDLE CreateGlobalHandle(OBJECTREF object) { CONDITIONAL_CONTRACT_VIOLATION(ModeViolation, object == NULL); - return GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_DEFAULT); + OBJECTHANDLE hnd = GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_DEFAULT); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateGlobalWeakHandle(OBJECTREF object) { - return GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_DEFAULT); + OBJECTHANDLE hnd = GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_DEFAULT); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateGlobalShortWeakHandle(OBJECTREF object) { CONDITIONAL_CONTRACT_VIOLATION(ModeViolation, object == NULL); - return GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_SHORT); + OBJECTHANDLE hnd = GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_SHORT); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateGlobalLongWeakHandle(OBJECTREF object) { - return GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_LONG); + OBJECTHANDLE hnd = GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_WEAK_LONG); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateGlobalStrongHandle(OBJECTREF object) { CONDITIONAL_CONTRACT_VIOLATION(ModeViolation, object == NULL); - return GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_STRONG); + OBJECTHANDLE hnd = GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_STRONG); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateGlobalPinningHandle(OBJECTREF object) { - return GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_PINNED); + OBJECTHANDLE hnd = GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_PINNED); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } inline OBJECTHANDLE CreateGlobalRefcountedHandle(OBJECTREF object) { - return GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_REFCOUNTED); + OBJECTHANDLE hnd = GCHandleUtilities::GetGCHandleManager()->CreateGlobalHandleOfType(OBJECTREFToObject(object), HNDTYPE_REFCOUNTED); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } // Special handle creation convenience functions @@ -163,14 +265,26 @@ inline OBJECTHANDLE CreateGlobalRefcountedHandle(OBJECTREF object) #ifdef FEATURE_COMINTEROP inline OBJECTHANDLE CreateWinRTWeakHandle(IGCHandleStore* store, OBJECTREF object, IWeakReference* pWinRTWeakReference) { - return store->CreateHandleWithExtraInfo(OBJECTREFToObject(object), HNDTYPE_WEAK_WINRT, (void*)pWinRTWeakReference); + OBJECTHANDLE hnd = store->CreateHandleWithExtraInfo(OBJECTREFToObject(object), HNDTYPE_WEAK_WINRT, (void*)pWinRTWeakReference); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } #endif // FEATURE_COMINTEROP // Creates a variable-strength handle inline OBJECTHANDLE CreateVariableHandle(IGCHandleStore* store, OBJECTREF object, uint32_t type) { - return store->CreateHandleWithExtraInfo(OBJECTREFToObject(object), HNDTYPE_VARIABLE, (void*)((uintptr_t)type)); + OBJECTHANDLE hnd = store->CreateHandleWithExtraInfo(OBJECTREFToObject(object), HNDTYPE_VARIABLE, (void*)((uintptr_t)type)); + if (!hnd) + { + COMPlusThrowOM(); + } + + return hnd; } // Handle object manipulation convenience functions -- 2.7.4