From 3d6533f153f3e37c9fb981a60cb0cdec1ce62016 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Tue, 16 May 2023 09:45:31 -0700 Subject: [PATCH 1/5] Fix DAC syncblk API --- src/coreclr/debug/daccess/request.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 29a0266f47a82a..2de7cfb92c16df 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -3676,7 +3676,7 @@ ClrDataAccess::GetSyncBlockData(unsigned int SBNumber, struct DacpSyncBlockData if (ste->m_SyncBlock != NULL) { - SyncBlock *pBlock = PTR_SyncBlock(ste->m_SyncBlock); + PTR_SyncBlock pBlock = ste->m_SyncBlock; pSyncBlockData->SyncBlockPointer = HOST_CDADDR(pBlock); #ifdef FEATURE_COMINTEROP if (pBlock->m_pInteropInfo) From 4cc7fffc076474cc506cc9ab4887066e8d4fdd5f Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Tue, 16 May 2023 14:00:52 -0700 Subject: [PATCH 2/5] Actually fix the problem: only fill in the syncblk data if there are blocks --- src/coreclr/debug/daccess/request.cpp | 65 ++++++++++++++------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 2de7cfb92c16df..a2bc424f700365 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -3667,46 +3667,49 @@ ClrDataAccess::GetSyncBlockData(unsigned int SBNumber, struct DacpSyncBlockData ZeroMemory(pSyncBlockData,sizeof(DacpSyncBlockData)); pSyncBlockData->SyncBlockCount = (SyncBlockCache::s_pSyncBlockCache->m_FreeSyncTableIndex) - 1; - PTR_SyncTableEntry ste = PTR_SyncTableEntry(dac_cast(g_pSyncTable)+(sizeof(SyncTableEntry) * SBNumber)); - pSyncBlockData->bFree = ((dac_cast(ste->m_Object.Load())) & 1); - - if (pSyncBlockData->bFree == FALSE) + if (pSyncBlockData->SyncBlockCount > 0) { - pSyncBlockData->Object = (CLRDATA_ADDRESS)dac_cast(ste->m_Object.Load()); + PTR_SyncTableEntry ste = PTR_SyncTableEntry(dac_cast(g_pSyncTable)+(sizeof(SyncTableEntry) * SBNumber)); + pSyncBlockData->bFree = ((dac_cast(ste->m_Object.Load())) & 1); - if (ste->m_SyncBlock != NULL) + if (pSyncBlockData->bFree == FALSE) { - PTR_SyncBlock pBlock = ste->m_SyncBlock; - pSyncBlockData->SyncBlockPointer = HOST_CDADDR(pBlock); -#ifdef FEATURE_COMINTEROP - if (pBlock->m_pInteropInfo) + pSyncBlockData->Object = (CLRDATA_ADDRESS)dac_cast(ste->m_Object.Load()); + + if (ste->m_SyncBlock != NULL) { - pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->DacGetRawRCW() != 0) ? SYNCBLOCKDATA_COMFLAGS_RCW : 0; - pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->GetCCW() != NULL) ? SYNCBLOCKDATA_COMFLAGS_CCW : 0; -#ifdef FEATURE_COMINTEROP_UNMANAGED_ACTIVATION - pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->GetComClassFactory() != NULL) ? SYNCBLOCKDATA_COMFLAGS_CF : 0; -#endif // FEATURE_COMINTEROP_UNMANAGED_ACTIVATION - } -#endif // FEATURE_COMINTEROP + PTR_SyncBlock pBlock = ste->m_SyncBlock; + pSyncBlockData->SyncBlockPointer = HOST_CDADDR(pBlock); +#ifd ef FEATURE_COMINTEROP + if (pBlock->m_pInteropInfo) + { + pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->DacGetRawRCW() != 0) ? SYNCBLOCKDATA_COMFLAGS_RCW : 0; + pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->GetCCW() != NULL) ? SYNCBLOCKDATA_COMFLAGS_CCW : 0; +#ifd ef FEATURE_COMINTEROP_UNMANAGED_ACTIVATION + pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->GetComClassFactory() != NULL) ? SYNCBLOCKDATA_COMFLAGS_CF : 0; +#end if // FEATURE_COMINTEROP_UNMANAGED_ACTIVATION + } +#end if // FEATURE_COMINTEROP - pSyncBlockData->MonitorHeld = pBlock->m_Monitor.GetMonitorHeldStateVolatile(); - pSyncBlockData->Recursion = pBlock->m_Monitor.GetRecursionLevel(); - pSyncBlockData->HoldingThread = HOST_CDADDR(pBlock->m_Monitor.GetHoldingThread()); - pSyncBlockData->appDomainPtr = PTR_HOST_TO_TADDR(AppDomain::GetCurrentDomain()); + pSyncBlockData->MonitorHeld = pBlock->m_Monitor.GetMonitorHeldStateVolatile(); + pSyncBlockData->Recursion = pBlock->m_Monitor.GetRecursionLevel(); + pSyncBlockData->HoldingThread = HOST_CDADDR(pBlock->m_Monitor.GetHoldingThread()); + pSyncBlockData->appDomainPtr = PTR_HOST_TO_TADDR(AppDomain::GetCurrentDomain()); - // TODO: Microsoft, implement the wait list - pSyncBlockData->AdditionalThreadCount = 0; + // TODO: Microsoft, implement the wait list + pSyncBlockData->AdditionalThreadCount = 0; - if (pBlock->m_Link.m_pNext != NULL) - { - PTR_SLink pLink = pBlock->m_Link.m_pNext; - do + if (pBlock->m_Link.m_pNext != NULL) { - pSyncBlockData->AdditionalThreadCount++; - pLink = pBlock->m_Link.m_pNext; + PTR_SLink pLink = pBlock->m_Link.m_pNext; + do + { + pSyncBlockData->AdditionalThreadCount++; + pLink = pBlock->m_Link.m_pNext; + } + while ((pLink != NULL) && + (pSyncBlockData->AdditionalThreadCount < 1000)); } - while ((pLink != NULL) && - (pSyncBlockData->AdditionalThreadCount < 1000)); } } } From 061ed1133014b16b8a93f450db318e3e55249cb0 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Tue, 16 May 2023 16:10:04 -0700 Subject: [PATCH 3/5] Fix build problems --- src/coreclr/debug/daccess/request.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index a2bc424f700365..a1475b8877755c 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -3680,16 +3680,16 @@ ClrDataAccess::GetSyncBlockData(unsigned int SBNumber, struct DacpSyncBlockData { PTR_SyncBlock pBlock = ste->m_SyncBlock; pSyncBlockData->SyncBlockPointer = HOST_CDADDR(pBlock); -#ifd ef FEATURE_COMINTEROP +#ifdef FEATURE_COMINTEROP if (pBlock->m_pInteropInfo) { pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->DacGetRawRCW() != 0) ? SYNCBLOCKDATA_COMFLAGS_RCW : 0; pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->GetCCW() != NULL) ? SYNCBLOCKDATA_COMFLAGS_CCW : 0; -#ifd ef FEATURE_COMINTEROP_UNMANAGED_ACTIVATION +#ifdef FEATURE_COMINTEROP_UNMANAGED_ACTIVATION pSyncBlockData->COMFlags |= (pBlock->m_pInteropInfo->GetComClassFactory() != NULL) ? SYNCBLOCKDATA_COMFLAGS_CF : 0; -#end if // FEATURE_COMINTEROP_UNMANAGED_ACTIVATION +#endif // FEATURE_COMINTEROP_UNMANAGED_ACTIVATION } -#end if // FEATURE_COMINTEROP +#endif // FEATURE_COMINTEROP pSyncBlockData->MonitorHeld = pBlock->m_Monitor.GetMonitorHeldStateVolatile(); pSyncBlockData->Recursion = pBlock->m_Monitor.GetRecursionLevel(); From c4ea2053acc3e08da3c0a43ce1b6b7e93ba2a7d6 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Tue, 16 May 2023 16:46:15 -0700 Subject: [PATCH 4/5] Add SBNumber index validation to API --- src/coreclr/debug/daccess/request.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index a1475b8877755c..5bc476b55384d8 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -3667,7 +3667,8 @@ ClrDataAccess::GetSyncBlockData(unsigned int SBNumber, struct DacpSyncBlockData ZeroMemory(pSyncBlockData,sizeof(DacpSyncBlockData)); pSyncBlockData->SyncBlockCount = (SyncBlockCache::s_pSyncBlockCache->m_FreeSyncTableIndex) - 1; - if (pSyncBlockData->SyncBlockCount > 0) + pSyncBlockData->bFree = TRUE; + if (pSyncBlockData->SyncBlockCount > 0 && SBNumber <= pSyncBlockData->SyncBlockCount) { PTR_SyncTableEntry ste = PTR_SyncTableEntry(dac_cast(g_pSyncTable)+(sizeof(SyncTableEntry) * SBNumber)); pSyncBlockData->bFree = ((dac_cast(ste->m_Object.Load())) & 1); From b471da09da4f399d0e56f3a9144354d325daf41a Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Tue, 16 May 2023 21:08:39 -0700 Subject: [PATCH 5/5] Revert change --- src/coreclr/debug/daccess/request.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 5bc476b55384d8..868593fae4651e 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -3668,6 +3668,7 @@ ClrDataAccess::GetSyncBlockData(unsigned int SBNumber, struct DacpSyncBlockData ZeroMemory(pSyncBlockData,sizeof(DacpSyncBlockData)); pSyncBlockData->SyncBlockCount = (SyncBlockCache::s_pSyncBlockCache->m_FreeSyncTableIndex) - 1; pSyncBlockData->bFree = TRUE; + if (pSyncBlockData->SyncBlockCount > 0 && SBNumber <= pSyncBlockData->SyncBlockCount) { PTR_SyncTableEntry ste = PTR_SyncTableEntry(dac_cast(g_pSyncTable)+(sizeof(SyncTableEntry) * SBNumber)); @@ -3679,7 +3680,7 @@ ClrDataAccess::GetSyncBlockData(unsigned int SBNumber, struct DacpSyncBlockData if (ste->m_SyncBlock != NULL) { - PTR_SyncBlock pBlock = ste->m_SyncBlock; + SyncBlock *pBlock = PTR_SyncBlock(ste->m_SyncBlock); pSyncBlockData->SyncBlockPointer = HOST_CDADDR(pBlock); #ifdef FEATURE_COMINTEROP if (pBlock->m_pInteropInfo)