[SharedCache] Fix detection of the class name of categories

The logic for determining the class name of a category did not correctly
handle classes defined in other images in the shared cache. There were
two problems:

1. If the class is defined in another image that is already loaded,
   `ObjCProcessor` has already renamed the symbol from `_OBJC_CLASS_$_`
   to `cls_`. Both forms of symbol name are now handled.

2. If the class is defined in an image that is not yet loaded, no symbol
   name is available. The category's class is now looked up in the
   shared cache symbol table, and the symbol's name is parsed as if it
   were an import symbol.

This fixes almost all cases of "Failed to determine base classname for
category" that I have come across.

Mason Reed: Fixed up to make objective-c processor always consult GetSymbol

4 files changed, 42 insertions, 4 deletions

diff --git a/objectivec/objc.cpp b/objectivec/objc.cpp
index f650500a..2fa6fb0d 100644
--- a/objectivec/objc.cpp
+++ b/objectivec/objc.cpp
@@ -581,13 +581,19 @@ void ObjCProcessor::LoadCategories(ObjCReader* reader, Ref<Section> classPtrSect
 			categoryBaseClassName = it->second.name;
 			category.associatedName = it->second.associatedName;
 		}
-		else if (auto symbol = m_data->GetSymbolByAddress(catLocation + m_data->GetAddressSize()))
+		else if (const auto symbol = GetSymbol(cat.cls))
 		{
-			if (symbol->GetType() == ImportedDataSymbol || symbol->GetType() == ImportAddressSymbol)
+			if (symbol->GetType() == ImportedDataSymbol || symbol->GetType() == ImportAddressSymbol || symbol->GetType() == DataSymbol)
 			{
-				const auto& symbolName = symbol->GetFullName();
+				// Symbols named `_OBJC_CLASS_$_` are references to external classes.
+				// Symbols named `cls_` are classes defined in a loaded image other than
+				// the image currently being analyzed. Classes from the current image
+				// are found via `m_classes`.
+				const std::string_view symbolName = symbol->GetFullNameRef();
 				if (symbolName.size() > 14 && symbolName.rfind("_OBJC_CLASS_$_", 0) == 0)
-					categoryBaseClassName = symbolName.substr(14, symbolName.size() - 14);
+					categoryBaseClassName = symbolName.substr(14);
+				else if (symbolName.size() > 4 && symbolName.rfind("cls_", 0) == 0)
+					categoryBaseClassName = symbolName.substr(4);
 			}
 		}
 		if (categoryBaseClassName.empty())
@@ -1247,6 +1253,11 @@ uint64_t ObjCProcessor::GetObjCRelativeMethodBaseAddress(ObjCReader* reader)
 	return 0;
 }
 
+Ref<Symbol> ObjCProcessor::GetSymbol(uint64_t address)
+{
+	return m_data->GetSymbolByAddress(address);
+}
+
 void ObjCProcessor::ProcessObjCData(std::optional<std::string> imageName)
 {
 	m_symbolQueue = new SymbolQueue();
diff --git a/objectivec/objc.h b/objectivec/objc.h
index 754d8d98..935773c1 100644
--- a/objectivec/objc.h
+++ b/objectivec/objc.h
@@ -324,6 +324,9 @@ namespace BinaryNinja {
 		virtual uint64_t GetObjCRelativeMethodBaseAddress(ObjCReader* reader);
 		virtual void GetRelativeMethod(ObjCReader* reader, method_t& meth);
 		virtual std::shared_ptr<ObjCReader> GetReader() = 0;
+		// Because an objective-c processor might have access to other non-view symbols that we want to retrieve.
+		// By default, this will just get symbol at the address in the view.
+		virtual Ref<Symbol> GetSymbol(uint64_t address);
 
 	public:
 		virtual ~ObjCProcessor() = default;
diff --git a/view/sharedcache/core/ObjC.cpp b/view/sharedcache/core/ObjC.cpp
index bb483542..9fee4628 100644
--- a/view/sharedcache/core/ObjC.cpp
+++ b/view/sharedcache/core/ObjC.cpp
@@ -148,6 +148,28 @@ uint64_t SharedCacheObjCProcessor::GetObjCRelativeMethodBaseAddress(ObjCReader*
 	return m_customRelativeMethodSelectorBase.value_or(0);
 }
 
+Ref<Symbol> SharedCacheObjCProcessor::GetSymbol(uint64_t address)
+{
+	if (const auto symbol = m_data->GetSymbolByAddress(address))
+		return symbol;
+
+	const auto controller = DSC::SharedCacheController::FromView(*m_data);
+	if (!controller)
+		return nullptr;
+
+	// No existing symbol located, try and search through the symbols of the cache.
+	auto cacheSymbol = controller->GetCache().GetSymbolAt(address);
+	if (!cacheSymbol.has_value())
+		return nullptr;
+
+	// Define the new symbol!
+	// While the method is "getting a symbol" and not applying it to the view, currently this is the more effective
+	// approach than monitoring every usage of this function to make sure they also define the symbol.
+	Ref<Symbol> symbol(new Symbol(cacheSymbol->type, cacheSymbol->name, address));
+	m_data->DefineAutoSymbol(symbol);
+	return symbol;
+}
+
 SharedCacheObjCProcessor::SharedCacheObjCProcessor(BinaryView* data, bool isBackedByDatabase) :
 	ObjCProcessor(data, "SharedCache.ObjC", isBackedByDatabase, true)
 {}
diff --git a/view/sharedcache/core/ObjC.h b/view/sharedcache/core/ObjC.h
index 6576564b..81e98f54 100644
--- a/view/sharedcache/core/ObjC.h
+++ b/view/sharedcache/core/ObjC.h
@@ -63,6 +63,8 @@ namespace DSCObjC {
 
 		void GetRelativeMethod(BinaryNinja::ObjCReader* reader, BinaryNinja::method_t& meth) override;
 
+		BinaryNinja::Ref<BinaryNinja::Symbol> GetSymbol(uint64_t address) override;
+
 	public:
 		SharedCacheObjCProcessor(BinaryNinja::BinaryView* data, bool isBackedByDatabase);