From 1dc9179158d8af2555e9182c6e0425b86d378909 Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Sun, 24 Nov 2024 16:54:16 -0800 Subject: [SharedCache] Split view-specific state into a separate struct The existing view-specific state was stored in several global unordered maps. Many of these were accessed without locking, including `viewSpecificMutexes`, which is racy in the face of multiple threads. View-specific state is stored in a new heap-allocated `ViewSpecificState` struct that is reference counted via `std::shared_ptr`. A static map holds a `std::weak_ptr` to each view-specific state, keyed by session id. `SharedCache` retrieves its view-specific state during its constructor. Since `ViewSpecificState` is reference counted it will naturally be deallocated when the last `SharedCache` instance that references it goes away. Its corresponding entry will remain in the static map, though since it only holds a `std::weak_ptr` rather than any state it will not use much memory. The next time view-specific state is retrieved any expired entries will be removed from the map. --- view/sharedcache/core/SharedCache.cpp | 164 +++++++++++++++++++--------------- 1 file changed, 93 insertions(+), 71 deletions(-) (limited to 'view/sharedcache/core/SharedCache.cpp') diff --git a/view/sharedcache/core/SharedCache.cpp b/view/sharedcache/core/SharedCache.cpp index d45785e8..cafa6bd8 100644 --- a/view/sharedcache/core/SharedCache.cpp +++ b/view/sharedcache/core/SharedCache.cpp @@ -27,6 +27,8 @@ #include "SharedCache.h" #include "ObjC.h" #include +#include +#include #include #include #include @@ -78,19 +80,51 @@ struct SharedCache::State DSCViewState viewState = DSCViewStateUnloaded; }; -static std::recursive_mutex viewStateMutex; -static std::unordered_map> viewStateCache; +struct SharedCache::ViewSpecificState { + std::mutex typeLibraryMutex; + std::mutex viewOperationsThatInfluenceMetadataMutex; -std::mutex progressMutex; -std::unordered_map progressMap; + std::atomic progress; -struct ViewSpecificMutexes { - std::mutex viewOperationsThatInfluenceMetadataMutex; - std::mutex typeLibraryLookupAndApplicationMutex; + std::mutex stateMutex; + std::shared_ptr cachedState; }; -static std::unordered_map viewSpecificMutexes; +std::shared_ptr ViewSpecificStateForId(uint64_t viewIdentifier, bool insertIfNeeded = true) { + static std::mutex viewSpecificStateMutex; + static std::unordered_map> viewSpecificState; + + std::lock_guard lock(viewSpecificStateMutex); + + if (auto it = viewSpecificState.find(viewIdentifier); it != viewSpecificState.end()) { + if (auto statePtr = it->second.lock()) { + return statePtr; + } + } + + if (!insertIfNeeded) { + return nullptr; + } + + auto statePtr = std::make_shared(); + viewSpecificState[viewIdentifier] = statePtr; + + // Prune entries for any views that are no longer in use. + for (auto it = viewSpecificState.begin(); it != viewSpecificState.end(); ) { + if (it->second.expired()) { + it = viewSpecificState.erase(it); + } else { + ++it; + } + } + + return statePtr; +} + +std::shared_ptr ViewSpecificStateForView(Ref view) { + return ViewSpecificStateForId(view->GetFile()->GetSessionId()); +} std::string base_name(std::string const& path) { @@ -220,9 +254,7 @@ void SharedCache::PerformInitialLoad() auto path = m_dscView->GetFile()->GetOriginalFilename(); auto baseFile = MMappedFileAccessor::Open(m_dscView, m_dscView->GetFile()->GetSessionId(), path)->lock(); - progressMutex.lock(); - progressMap[m_dscView->GetFile()->GetSessionId()] = LoadProgressLoadingCaches; - progressMutex.unlock(); + m_viewSpecificState->progress = LoadProgressLoadingCaches; WillMutateState(); @@ -737,9 +769,8 @@ void SharedCache::PerformInitialLoad() } } baseFile.reset(); - progressMutex.lock(); - progressMap[m_dscView->GetFile()->GetSessionId()] = LoadProgressLoadingImages; - progressMutex.unlock(); + + m_viewSpecificState->progress = LoadProgressLoadingImages; // We have set up enough metadata to map VM now. @@ -952,9 +983,7 @@ void SharedCache::PerformInitialLoad() m_logger->LogDebug("Finished initial load of Shared Cache"); - progressMutex.lock(); - progressMap[m_dscView->GetFile()->GetSessionId()] = LoadProgressFinished; - progressMutex.unlock(); + m_viewSpecificState->progress = LoadProgressFinished; } std::shared_ptr SharedCache::GetVMMap(bool mapPages) @@ -983,10 +1012,10 @@ void SharedCache::DeserializeFromRawView() { if (m_dscView->QueryMetadata(SharedCacheMetadataTag)) { - std::unique_lock viewStateCacheLock(viewStateMutex); - if (auto it = viewStateCache.find(m_dscView->GetFile()->GetSessionId()); it != viewStateCache.end()) + std::lock_guard lock(m_viewSpecificState->stateMutex); + if (m_viewSpecificState->cachedState) { - m_state = it->second; + m_state = m_viewSpecificState->cachedState; m_stateIsShared = true; m_metadataValid = true; } @@ -1367,7 +1396,7 @@ void SharedCache::ParseAndApplySlideInfoForFile(std::shared_ptr dscView) : m_dscView(dscView) +SharedCache::SharedCache(BinaryNinja::Ref dscView) : m_dscView(dscView), m_viewSpecificState(ViewSpecificStateForView(dscView)) { if (dscView->GetTypeName() != VIEW_NAME) { @@ -1381,46 +1410,43 @@ SharedCache::SharedCache(BinaryNinja::Ref dscView) : m_ DeserializeFromRawView(); if (!m_metadataValid) return; - if (State().viewState == DSCViewStateUnloaded) + + if (State().viewState != DSCViewStateUnloaded) { + m_viewSpecificState->progress = LoadProgressFinished; + return; + } + + std::unique_lock lock(m_viewSpecificState->viewOperationsThatInfluenceMetadataMutex); + try { + PerformInitialLoad(); + } + catch (...) { - std::unique_lock lock(viewSpecificMutexes[m_dscView->GetFile()->GetSessionId()].viewOperationsThatInfluenceMetadataMutex); - try { - PerformInitialLoad(); - } - catch (...) - { - m_logger->LogError("Failed to perform initial load of Shared Cache"); - } + m_logger->LogError("Failed to perform initial load of Shared Cache"); + } - auto settings = m_dscView->GetLoadSettings(VIEW_NAME); - bool autoLoadLibsystem = true; - if (settings && settings->Contains("loader.dsc.autoLoadLibSystem")) - { - autoLoadLibsystem = settings->Get("loader.dsc.autoLoadLibSystem", m_dscView); - } - if (autoLoadLibsystem) + auto settings = m_dscView->GetLoadSettings(VIEW_NAME); + bool autoLoadLibsystem = true; + if (settings && settings->Contains("loader.dsc.autoLoadLibSystem")) + { + autoLoadLibsystem = settings->Get("loader.dsc.autoLoadLibSystem", m_dscView); + } + if (autoLoadLibsystem) + { + for (const auto& [_, header] : State().headers) { - for (const auto& [_, header] : State().headers) + if (header.installName.find("libsystem_c.dylib") != std::string::npos) { - if (header.installName.find("libsystem_c.dylib") != std::string::npos) - { - lock.unlock(); - m_logger->LogInfo("Loading core libsystem_c.dylib library"); - LoadImageWithInstallName(header.installName, false); - lock.lock(); - break; - } + lock.unlock(); + m_logger->LogInfo("Loading core libsystem_c.dylib library"); + LoadImageWithInstallName(header.installName, false); + break; } } - MutableState().viewState = DSCViewStateLoaded; - SaveToDSCView(); - } - else - { - progressMutex.lock(); - progressMap[m_dscView->GetFile()->GetSessionId()] = LoadProgressFinished; - progressMutex.unlock(); } + + MutableState().viewState = DSCViewStateLoaded; + SaveToDSCView(); } SharedCache::~SharedCache() { @@ -1536,7 +1562,7 @@ bool SharedCache::LoadImageContainingAddress(uint64_t address, bool skipObjC) bool SharedCache::LoadSectionAtAddress(uint64_t address) { - std::unique_lock lock(viewSpecificMutexes[m_dscView->GetFile()->GetSessionId()].viewOperationsThatInfluenceMetadataMutex); + std::unique_lock lock(m_viewSpecificState->viewOperationsThatInfluenceMetadataMutex); DeserializeFromRawView(); WillMutateState(); @@ -1806,7 +1832,7 @@ bool SharedCache::LoadImageWithInstallName(std::string installName, bool skipObj { auto settings = m_dscView->GetLoadSettings(VIEW_NAME); - std::unique_lock lock(viewSpecificMutexes[m_dscView->GetFile()->GetSessionId()].viewOperationsThatInfluenceMetadataMutex); + std::unique_lock lock(m_viewSpecificState->viewOperationsThatInfluenceMetadataMutex); DeserializeFromRawView(); WillMutateState(); @@ -1880,7 +1906,7 @@ bool SharedCache::LoadImageWithInstallName(std::string installName, bool skipObj return false; } - std::unique_lock typelibLock(viewSpecificMutexes[m_dscView->GetFile()->GetSessionId()].typeLibraryLookupAndApplicationMutex); + std::unique_lock typelibLock(m_viewSpecificState->typeLibraryMutex); auto typeLib = m_dscView->GetTypeLibrary(header.installName); if (!typeLib) @@ -2875,7 +2901,7 @@ std::vector>> SharedCache::LoadAllSymbolsAndW { WillMutateState(); - std::unique_lock initialLoadBlock(viewSpecificMutexes[m_dscView->GetFile()->GetSessionId()].viewOperationsThatInfluenceMetadataMutex); + std::lock_guard initialLoadBlock(m_viewSpecificState->viewOperationsThatInfluenceMetadataMutex); std::vector>> symbols; for (const auto& img : State().images) @@ -2973,7 +2999,7 @@ void SharedCache::FindSymbolAtAddrAndApplyToAddr( } auto exportList = SharedCache::ParseExportTrie(mapping, *header); std::vector>> exportMapping; - std::unique_lock lock(viewSpecificMutexes[m_dscView->GetFile()->GetSessionId()].typeLibraryLookupAndApplicationMutex); + std::unique_lock lock(m_viewSpecificState->typeLibraryMutex); auto typeLib = m_dscView->GetTypeLibrary(header->installName); if (!typeLib) { @@ -3020,7 +3046,7 @@ void SharedCache::FindSymbolAtAddrAndApplyToAddr( } } { - std::unique_lock _lock(viewSpecificMutexes[m_dscView->GetFile()->GetSessionId()].viewOperationsThatInfluenceMetadataMutex); + std::lock_guard lock(m_viewSpecificState->viewOperationsThatInfluenceMetadataMutex); MutableState().exportInfos[header->textBase] = std::move(exportMapping); } m_dscView->EndBulkModifySymbols(); @@ -3044,8 +3070,8 @@ bool SharedCache::SaveToDSCView() m_state = cachedState; m_stateIsShared = true; - std::unique_lock viewStateCacheLock(viewStateMutex); - viewStateCache[m_dscView->GetFile()->GetSessionId()] = std::move(cachedState); + std::lock_guard lock(m_viewSpecificState->stateMutex); + m_viewSpecificState->cachedState = std::move(cachedState); m_metadataValid = true; @@ -3055,7 +3081,7 @@ bool SharedCache::SaveToDSCView() } std::vector SharedCache::GetMappedRegions() const { - std::unique_lock lock(viewSpecificMutexes[m_dscView->GetFile()->GetSessionId()].viewOperationsThatInfluenceMetadataMutex); + std::lock_guard lock(m_viewSpecificState->viewOperationsThatInfluenceMetadataMutex); return State().regionsMappedIntoMemory; } @@ -3392,15 +3418,11 @@ extern "C" BNDSCViewLoadProgress BNDSCViewGetLoadProgress(uint64_t sessionID) { - progressMutex.lock(); - if (progressMap.find(sessionID) == progressMap.end()) - { - progressMutex.unlock(); - return LoadProgressNotStarted; + if (auto viewSpecificState = ViewSpecificStateForId(sessionID, false)) { + return viewSpecificState->progress; } - auto progress = progressMap[sessionID]; - progressMutex.unlock(); - return progress; + + return LoadProgressNotStarted; } uint64_t BNDSCViewFastGetBackingCacheCount(BNBinaryView* data) -- cgit v1.3.1