[SharedCache] Fix performance when loading images from iOS 14 shared caches

A logic error in `FindSymbolAtAddrAndApplyToAddr` meant that a code path
that would attempt to re-use existing symbol definitions in the view
would still fall back to searching the exported symbol list.

`FindSymbolAtAddrAndApplyToAddr` is updated to return early if a symbol
already exists at the target location and the target location is the
same as the symbol location. Additionally, in the event a symbol was found
in the view at the symbol location, it is applied to the target location
and then the function returns.

Additionally, `WillMutateState` is moved after these initial checks so
it is only called in the codepaths that attempt to locate the symbol
amongst the exported symbols.

After these changes it is now possible to load QuartzCore from the iOS
14 shared cache in around 70 seconds. Previously it would run for hours
without the load completing.

1 files changed, 64 insertions, 64 deletions

diff --git a/view/sharedcache/core/SharedCache.cpp b/view/sharedcache/core/SharedCache.cpp
index 61d0fd4e..e61e23f6 100644
--- a/view/sharedcache/core/SharedCache.cpp
+++ b/view/sharedcache/core/SharedCache.cpp
@@ -2925,85 +2925,85 @@ Ref<TypeLibrary> SharedCache::TypeLibraryForImage(const std::string& installName
 void SharedCache::FindSymbolAtAddrAndApplyToAddr(
 	uint64_t symbolLocation, uint64_t targetLocation, bool triggerReanalysis)
 {
-	WillMutateState();
-
 	std::string prefix = "";
 	if (symbolLocation != targetLocation)
 		prefix = "j_";
-	if (auto preexistingSymbol = m_dscView->GetSymbolByAddress(targetLocation))
-	{
-		if (preexistingSymbol->GetFullName().find("j_") != std::string::npos)
+
+	if (auto targetSymbol = m_dscView->GetSymbolByAddress(targetLocation)) {
+		// A symbol already exists at the target location. If the source and target address are the same,
+		// there's nothing more to do. If they're different but the symbol has the `j_` prefix that is added
+		// to stubs, there's also nothing more to do.
+		if (symbolLocation == targetLocation || targetSymbol->GetFullName().find("j_") != std::string::npos) {
 			return;
+		}
 	}
-	auto id = m_dscView->BeginUndoActions();
-	if (auto loadedSymbol = m_dscView->GetSymbolByAddress(symbolLocation))
-	{
-		if (m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation))
-			m_dscView->DefineUserSymbol(new Symbol(FunctionSymbol, prefix + loadedSymbol->GetFullName(), targetLocation));
-		else
-			m_dscView->DefineUserSymbol(new Symbol(loadedSymbol->GetType(), prefix + loadedSymbol->GetFullName(), targetLocation));
-	}
-	else if (auto sym = m_dscView->GetSymbolByAddress(symbolLocation))
-	{
-		if (m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation))
-			m_dscView->DefineUserSymbol(new Symbol(FunctionSymbol, prefix + sym->GetFullName(), targetLocation));
-		else
-			m_dscView->DefineUserSymbol(new Symbol(sym->GetType(), prefix + sym->GetFullName(), targetLocation));
+
+	if (symbolLocation != targetLocation) {
+		if (auto symbol = m_dscView->GetSymbolByAddress(symbolLocation)) {
+			// A symbol already exists at the source location. Add a stub symbol at `targetLocation` based on the existing symbol.
+			auto id = m_dscView->BeginUndoActions();
+			if (m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation))
+				m_dscView->DefineUserSymbol(new Symbol(FunctionSymbol, prefix + symbol->GetFullName(), targetLocation));
+			else
+				m_dscView->DefineUserSymbol(new Symbol(symbol->GetType(), prefix + symbol->GetFullName(), targetLocation));
+			m_dscView->ForgetUndoActions(id);
+			return;
+		}
 	}
-	m_dscView->ForgetUndoActions(id);
+
+	// No existing symbol was found at `symbolLocation` or `targetLocation`. Search the export list
+	// for the image containing `symbolLocation` to find a symbol corresponding to that address.
+
 	auto header = HeaderForAddress(symbolLocation);
-	if (header)
-	{
+	if (!header) {
+		return;
+	}
 
-		auto exportList = GetExportListForHeader(*header, [&]() {
-			try {
-				return MapFile(header->exportTriePath);
-			}
-			catch (...)
-			{
-				m_logger->LogWarn("Serious Error: Failed to open export trie %s for %s", header->exportTriePath.c_str(), header->installName.c_str());
-				return std::shared_ptr<MMappedFileAccessor>(nullptr);
-			}
-		});
+	WillMutateState();
+	auto exportList = GetExportListForHeader(*header, [&]() {
+		try {
+			return MapFile(header->exportTriePath);
+		} catch (...) {
+			m_logger->LogWarn("Serious Error: Failed to open export trie %s for %s", header->exportTriePath.c_str(), header->installName.c_str());
+			return std::shared_ptr<MMappedFileAccessor>(nullptr);
+		}
+	});
 
-		if (exportList)
-		{
-			if (auto it = exportList->find(symbolLocation); it != exportList->end())
-			{
-				auto typeLib = TypeLibraryForImage(header->installName);
-				id = m_dscView->BeginUndoActions();
-				m_dscView->BeginBulkModifySymbols();
+	if (!exportList) {
+		return;
+	}
 
-				auto func = m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation);
-				if (func)
-				{
-					m_dscView->DefineUserSymbol(
-						new Symbol(FunctionSymbol, prefix + it->second->GetFullName(), targetLocation));
+	auto it = exportList->find(symbolLocation);
+	if (it == exportList->end()) {
+		return;
+	}
 
-					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));
+	const auto& symbol = it->second;
+	auto id = m_dscView->BeginUndoActions();
+	auto typeLib = TypeLibraryForImage(header->installName);
+	auto type = typeLib ? m_dscView->ImportTypeLibraryObject(typeLib, {symbol->GetFullName()}) : nullptr;
 
-					if (typeLib)
-						if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
-							m_dscView->DefineUserDataVariable(targetLocation, type);
-				}
-				if (triggerReanalysis)
-				{
-					if (func)
-						func->Reanalyze();
-				}
+	if (auto func = m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation)) {
+		m_dscView->DefineUserSymbol(
+			new Symbol(FunctionSymbol, prefix + symbol->GetFullName(), targetLocation));
 
-				m_dscView->EndBulkModifySymbols();
-				m_dscView->ForgetUndoActions(id);
-			}
+		if (type) {
+			func->SetUserType(type);
+		}
+
+		if (triggerReanalysis) {
+			func->Reanalyze();
+		}
+	} else {
+		m_dscView->DefineUserSymbol(
+			new Symbol(symbol->GetType(), prefix + symbol->GetFullName(), targetLocation));
+
+		if (type) {
+			m_dscView->DefineUserDataVariable(targetLocation, type);
 		}
 	}
+
+	m_dscView->ForgetUndoActions(id);
 }