[SharedCache] Make `m_exportInfos` map's values a `shared_ptr`

This avoids expensive copying when returning a value from the map in `SharedCache::GetExportListForHeader`. Additionally it ensures that the value stays alive and at the same location in memory if `m_exportInfos` is modified and requires its storage to be re-allocated.

I was unable to use a `unique_ptr` instead of a `shared_ptr` because of copy semantics with `m_exportInfos` in `ViewStateCacheStore`. I don't see things being any worse using `shared_ptr` instead of `unique_ptr` anyway and it means less code changes.

1 files changed, 72 insertions, 64 deletions

diff --git a/view/sharedcache/core/SharedCache.cpp b/view/sharedcache/core/SharedCache.cpp
index c7591c3a..6eebc409 100644
--- a/view/sharedcache/core/SharedCache.cpp
+++ b/view/sharedcache/core/SharedCache.cpp
@@ -57,7 +57,7 @@ int count_trailing_zeros(uint64_t value) {
 
 struct SharedCache::State
 {
-	std::unordered_map<uint64_t, std::unordered_map<uint64_t, Ref<Symbol>>>
+	std::unordered_map<uint64_t, std::shared_ptr<std::unordered_map<uint64_t, Ref<Symbol>>>>
 		exportInfos;
 	std::unordered_map<uint64_t, std::vector<std::pair<uint64_t, std::pair<BNSymbolType, std::string>>>>
 		symbolInfos;
@@ -2668,44 +2668,47 @@ void SharedCache::InitializeHeader(
 		auto symbols = GetExportListForHeader(header, [&]() {
 			return vm->MappingAtAddress(header.linkeditSegment.vmaddr).first.fileAccessor->lock();
 		});
-		for (const auto& symPair : symbols)
+		if (symbols)
 		{
-			if (typeLib)
+			for (const auto& [symbolAddress, symbol] : *symbols)
 			{
-				auto type = m_dscView->ImportTypeLibraryObject(typeLib, symPair.second->GetRawName());
-
-				if (type)
+				if (typeLib)
 				{
-					view->DefineAutoSymbolAndVariableOrFunction(view->GetDefaultPlatform(), symPair.second, type);
-				}
-				else
-					view->DefineAutoSymbol(symPair.second);
+					auto type = m_dscView->ImportTypeLibraryObject(typeLib, symbol->GetRawName());
 
-				if (view->GetAnalysisFunction(view->GetDefaultPlatform(), symPair.first))
-				{
-					auto func = view->GetAnalysisFunction(view->GetDefaultPlatform(), symPair.first);
-					auto name = symPair.second->GetFullName();
-					if (name == "_objc_msgSend")
+					if (type)
 					{
-						func->SetHasVariableArguments(false);
+						view->DefineAutoSymbolAndVariableOrFunction(view->GetDefaultPlatform(), symbol, type);
 					}
-					else if (name.find("_objc_retain_x") != std::string::npos || name.find("_objc_release_x") != std::string::npos)
+					else
+						view->DefineAutoSymbol(symbol);
+
+					if (view->GetAnalysisFunction(view->GetDefaultPlatform(), symbolAddress))
 					{
-						auto x = name.rfind("x");
-						auto num = name.substr(x + 1);
+						auto func = view->GetAnalysisFunction(view->GetDefaultPlatform(), symbolAddress);
+						auto name = symbol->GetFullName();
+						if (name == "_objc_msgSend")
+						{
+							func->SetHasVariableArguments(false);
+						}
+						else if (name.find("_objc_retain_x") != std::string::npos || name.find("_objc_release_x") != std::string::npos)
+						{
+							auto x = name.rfind("x");
+							auto num = name.substr(x + 1);
 
-						std::vector<BinaryNinja::FunctionParameter> callTypeParams;
-						auto cc = m_dscView->GetDefaultArchitecture()->GetCallingConventionByName("apple-arm64-objc-fast-arc-" + num);
+							std::vector<BinaryNinja::FunctionParameter> callTypeParams;
+							auto cc = m_dscView->GetDefaultArchitecture()->GetCallingConventionByName("apple-arm64-objc-fast-arc-" + num);
 
-						callTypeParams.push_back({"obj", m_dscView->GetTypeByName({ "id" }), true, BinaryNinja::Variable()});
+							callTypeParams.push_back({"obj", m_dscView->GetTypeByName({ "id" }), true, BinaryNinja::Variable()});
 
-						auto funcType = BinaryNinja::Type::FunctionType(m_dscView->GetTypeByName({ "id" }), cc, callTypeParams);
-						func->SetUserType(funcType);
+							auto funcType = BinaryNinja::Type::FunctionType(m_dscView->GetTypeByName({ "id" }), cc, callTypeParams);
+							func->SetUserType(funcType);
+						}
 					}
 				}
+				else
+					view->DefineAutoSymbol(symbol);
 			}
-			else
-				view->DefineAutoSymbol(symPair.second);
 		}
 	}
 	view->EndBulkModifySymbols();
@@ -2808,7 +2811,7 @@ std::vector<Ref<Symbol>> SharedCache::ParseExportTrie(std::shared_ptr<MMappedFil
 }
 
 
-std::unordered_map<uint64_t, Ref<Symbol>> SharedCache::GetExportListForHeader(SharedCacheMachOHeader header, std::function<std::shared_ptr<MMappedFileAccessor>()> provideLinkeditFile, bool* didModifyExportList)
+std::shared_ptr<std::unordered_map<uint64_t, Ref<Symbol>>> SharedCache::GetExportListForHeader(SharedCacheMachOHeader header, std::function<std::shared_ptr<MMappedFileAccessor>()> provideLinkeditFile, bool* didModifyExportList)
 {
 	if (auto it = m_state->exportInfos.find(header.textBase); it != m_state->exportInfos.end())
 	{
@@ -2824,19 +2827,19 @@ std::unordered_map<uint64_t, Ref<Symbol>> SharedCache::GetExportListForHeader(Sh
 		{
 			if (didModifyExportList)
 				*didModifyExportList = false;
-			return std::unordered_map<uint64_t, Ref<Symbol>>();
+			return nullptr;
 		}
 
 		auto exportList = SharedCache::ParseExportTrie(linkeditFile, header);
-		std::unordered_map<uint64_t, Ref<Symbol>> exportMapping(exportList.size());
+		auto exportMapping = std::make_shared<std::unordered_map<uint64_t, Ref<Symbol>>>(exportList.size());
 		for (const auto& sym : exportList)
 		{
-			exportMapping[sym->GetAddress()] = sym;
+			exportMapping->insert_or_assign(sym->GetAddress(), sym);
 		}
-		m_state->exportInfos[header.textBase] = exportMapping;
+		MutableState().exportInfos.emplace(header.textBase, exportMapping);
 		if (didModifyExportList)
 			*didModifyExportList = true;
-		return exportMapping;
+		return m_state->exportInfos[header.textBase];
 	}
 }
 
@@ -2874,9 +2877,11 @@ std::vector<std::pair<std::string, Ref<Symbol>>> SharedCache::LoadAllSymbolsAndW
 				return std::shared_ptr<MMappedFileAccessor>(nullptr);
 			}
 		}, &doSave);
-		for (const auto& symPair : exportList)
+		if (!exportList)
+			continue;
+		for (const auto& [_, symbol] : *exportList)
 		{
-			symbols.push_back({img.installName, symPair.second});
+			symbols.push_back({img.installName, symbol});
 		}
 	}
 
@@ -2974,39 +2979,42 @@ void SharedCache::FindSymbolAtAddrAndApplyToAddr(
 			}
 		});
 
-		if (auto it = exportList.find(symbolLocation); it != exportList.end())
+		if (exportList)
 		{
-			auto typeLib = TypeLibraryForImage(header->installName);
-			id = m_dscView->BeginUndoActions();
-			m_dscView->BeginBulkModifySymbols();
-			
-			auto func = m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation);
-			if (func)
-			{
-				m_dscView->DefineUserSymbol(
-					new Symbol(FunctionSymbol, prefix + it->second->GetFullName(), targetLocation));
-
-				if (typeLib)
-					if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
-						func->SetUserType(type);
-			}
-			else
+			if (auto it = exportList->find(symbolLocation); it != exportList->end())
 			{
-				m_dscView->DefineUserSymbol(
-					new Symbol(it->second->GetType(), prefix + it->second->GetFullName(), targetLocation));
+				auto typeLib = TypeLibraryForImage(header->installName);
+				id = m_dscView->BeginUndoActions();
+				m_dscView->BeginBulkModifySymbols();
 
-				if (typeLib)
-					if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
-						m_dscView->DefineUserDataVariable(targetLocation, type);
-			}
-			if (triggerReanalysis)
-			{
+				auto func = m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation);
 				if (func)
-					func->Reanalyze();
-			}
+				{
+					m_dscView->DefineUserSymbol(
+						new Symbol(FunctionSymbol, prefix + it->second->GetFullName(), targetLocation));
+
+					if (typeLib)
+						if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
+							func->SetUserType(type);
+				}
+				else
+				{
+					m_dscView->DefineUserSymbol(
+						new Symbol(it->second->GetType(), prefix + it->second->GetFullName(), targetLocation));
 
-			m_dscView->EndBulkModifySymbols();
-			m_dscView->ForgetUndoActions(id);
+					if (typeLib)
+						if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
+							m_dscView->DefineUserDataVariable(targetLocation, type);
+				}
+				if (triggerReanalysis)
+				{
+					if (func)
+						func->Reanalyze();
+				}
+
+				m_dscView->EndBulkModifySymbols();
+				m_dscView->ForgetUndoActions(id);
+			}
 		}
 	}
 }
@@ -3467,7 +3475,7 @@ void SharedCache::Store(SerializationContext& context) const
 		Serialize(context, "key", pair1.first);
 		Serialize(context, "value");
 		context.writer.StartArray();
-		for (const auto& pair2 : pair1.second)
+		for (const auto& pair2 : *pair1.second)
 		{
 			context.writer.StartObject();
 			Serialize(context, "key", pair2.first);
@@ -3552,7 +3560,7 @@ void SharedCache::Load(DeserializationContext& context)
 				raw.c_str(), raw.c_str(), addr, NoBinding, nullptr, 0));
 		}
 
-		MutableState().exportInfos[obj1["key"].GetUint64()] = std::move(innerVec);
+		MutableState().exportInfos[obj1["key"].GetUint64()] = std::make_shared<std::unordered_map<uint64_t, Ref<Symbol>>>(innerVec);
 	}
 
 	for (auto& symbolInfo : context.doc["symbolInfos"].GetArray())