[SharedCache] Add a named symbol map

Fixes #6561

- Also tightens the SharedCache class to move only, to prevent accidental copies.
- Also removes some extra copies in FFI when should pass by ref
- Also adds `get_symbol_with_name` to python API

The current named symbol map is populated in a worker thread spawned in the view init. This is because populating the map can take about 1 second.
If we are fine with another 1 second added to the view init time then we can add it serially but I don't think this way is _that_ bad, no analysis consults
this, however a user might add a workflow that would be racing this. So we need to add a mutex.

Author: emesare

view/sharedcache/api/python/sharedcache.py

@@ -214,6 +214,14 @@ def get_symbol_at(self, address: int) -> Optional[CacheSymbol]:
         sccore.BNSharedCacheFreeSymbol(api_symbol)
         return symbol
 
+    def get_symbol_with_name(self, name: str) -> Optional[CacheSymbol]:
+        api_symbol = sccore.BNSharedCacheSymbol()
+        if not sccore.BNSharedCacheControllerGetSymbolWithName(self.handle, name, api_symbol):
+            return None
+        symbol = symbol_from_api(api_symbol)
+        sccore.BNSharedCacheFreeSymbol(api_symbol)
+        return symbol
+
     @property
     def regions(self) -> [CacheRegion]:
         count = ctypes.c_ulonglong()

view/sharedcache/core/SharedCache.cpp

@@ -201,10 +201,10 @@ void SharedCache::AddSymbol(CacheSymbol symbol)
 	m_symbols.insert({symbol.address, std::move(symbol)});
 }
 
-void SharedCache::AddSymbols(std::vector<CacheSymbol> symbols)
+void SharedCache::AddSymbols(std::vector<CacheSymbol>&& symbols)
 {
-	for (auto& symbol : symbols)
-		m_symbols.insert({symbol.address, std::move(symbol)});
+	for (auto&& symbol : symbols)
+		m_symbols.emplace(symbol.address, std::move(symbol));
 }
 
 CacheEntryId SharedCache::AddEntry(CacheEntry entry)
@@ -403,6 +403,14 @@ void SharedCache::ProcessEntrySlideInfo(const CacheEntry& entry)
 	slideInfoProcessor.ProcessEntry(*m_vm, entry);
 }
 
+void SharedCache::ProcessSymbols()
+{
+	// Populate the named symbols from the regular symbols map.
+	m_namedSymbols.reserve(m_symbols.size());
+	for (const auto& [address, symbol] : m_symbols)
+		m_namedSymbols.emplace(symbol.name, address);
+}
+
 std::optional<CacheEntry> SharedCache::GetEntryContaining(const uint64_t address) const
 {
 	for (const auto& [_, entry] : m_entries)
@@ -482,10 +490,10 @@ std::optional<CacheSymbol> SharedCache::GetSymbolAt(uint64_t address) const
 
 std::optional<CacheSymbol> SharedCache::GetSymbolWithName(const std::string& name) const
 {
-	for (const auto& [address, symbol] : m_symbols)
-		if (symbol.name == name)
-			return symbol;
-	return std::nullopt;
+	const auto it = m_namedSymbols.find(name);
+	if (it == m_namedSymbols.end())
+		return std::nullopt;
+	return GetSymbolAt(it->second);
 }
 
 CacheProcessor::CacheProcessor(Ref<BinaryView> view)

view/sharedcache/core/SharedCache.h

@@ -199,8 +199,11 @@ class SharedCache
 	AddressRangeMap<CacheRegion> m_regions {};
 	// Describes the images of the cache.
 	std::unordered_map<uint64_t, CacheImage> m_images {};
-	// All the symbols for this cache. Both mapped and unmapped (not in the view).
+	// All the external symbols for this cache. Both mapped and unmapped (not in the view).
 	std::unordered_map<uint64_t, CacheSymbol> m_symbols {};
+	// Quickly lookup a symbol by name, populated by `FinalizeSymbols`.
+	// `m_namedSymbols` is modified in a worker thread spawned by view init so we must not get a symbol until its populated.
+	std::unordered_map<std::string, uint64_t> m_namedSymbols {};
 
 	bool ProcessEntryImage(const std::string& path, const dyld_cache_image_info& info);
 
@@ -211,12 +214,19 @@ class SharedCache
 public:
 	explicit SharedCache(uint64_t addressSize);
 
+	SharedCache(const SharedCache &) = delete;
+	SharedCache &operator=(const SharedCache &) = delete;
+
+	SharedCache(SharedCache &&) noexcept = default;
+	SharedCache &operator=(SharedCache &&) noexcept = default;
+
 	uint64_t GetBaseAddress() const { return m_baseAddress; }
 	std::shared_ptr<VirtualMemory> GetVirtualMemory() { return m_vm; }
 	const std::unordered_map<CacheEntryId, CacheEntry>& GetEntries() const { return m_entries; }
 	const AddressRangeMap<CacheRegion>& GetRegions() const { return m_regions; }
 	const std::unordered_map<uint64_t, CacheImage>& GetImages() const { return m_images; }
 	const std::unordered_map<uint64_t, CacheSymbol>& GetSymbols() const { return m_symbols; }
+	const std::unordered_map<std::string, uint64_t>& GetNamedSymbols() const { return m_namedSymbols; }
 
 	void AddImage(CacheImage image);
 
@@ -225,7 +235,7 @@ class SharedCache
 
 	void AddSymbol(CacheSymbol symbol);
 
-	void AddSymbols(std::vector<CacheSymbol> symbols);
+	void AddSymbols(std::vector<CacheSymbol>&& symbols);
 
 	// Adds the cache entry and populates the virtual memory using the mapping information.
 	// After being added the entry is read only, there is nothing that can modify it.
@@ -237,6 +247,9 @@ class SharedCache
 
 	void ProcessEntrySlideInfo(const CacheEntry& entry);
 
+	// Construct the named symbols lookup map for use with `GetSymbolWithName`.
+	void ProcessSymbols();
+
 	std::optional<CacheEntry> GetEntryContaining(uint64_t address) const;
 
 	std::optional<CacheEntry> GetEntryWithImage(const CacheImage& image) const;

view/sharedcache/core/SharedCacheController.cpp

@@ -54,7 +54,7 @@ void RegisterSharedCacheControllerDestructor()
 	BNRegisterObjectDestructionCallbacks(&callbacks);
 }
 
-SharedCacheController::SharedCacheController(SharedCache cache, Ref<Logger> logger) : m_cache(std::move(cache))
+SharedCacheController::SharedCacheController(SharedCache&& cache, Ref<Logger> logger) : m_cache(std::move(cache))
 {
 	INIT_DSC_API_OBJECT();
 	m_logger = std::move(logger);
@@ -65,7 +65,7 @@ SharedCacheController::SharedCacheController(SharedCache cache, Ref<Logger> logg
 	m_regionFilter = std::regex(".*LINKEDIT.*");
 }
 
-DSCRef<SharedCacheController> SharedCacheController::Initialize(BinaryView& view, SharedCache cache)
+DSCRef<SharedCacheController> SharedCacheController::Initialize(BinaryView& view, SharedCache&& cache)
 {
 	auto id = GetViewIdFromView(view);
 	std::unique_lock<std::shared_mutex> lock(GlobalControllersMutex);

view/sharedcache/core/SharedCacheController.h

@@ -35,11 +35,11 @@ namespace BinaryNinja::DSC {
 		bool m_processObjC;
 		bool m_processCFStrings;
 
-		explicit SharedCacheController(SharedCache cache, Ref<Logger> logger);
+		explicit SharedCacheController(SharedCache&& cache, Ref<Logger> logger);
 
 	public:
 		// Initialize the DSCacheView, this should be called from the view initialize function only!
-		static DSCRef<SharedCacheController> Initialize(BinaryView& view, SharedCache cache);
+		static DSCRef<SharedCacheController> Initialize(BinaryView& view, SharedCache&& cache);
 
 		// NOTE: This will not create one if it does not exist. To create one for the view call `Initialize`.
 		static DSCRef<SharedCacheController> FromView(BinaryView& view);

view/sharedcache/core/SharedCacheView.cpp

@@ -842,6 +842,19 @@ bool SharedCacheView::Init()
 
 	auto cacheController = SharedCacheController::Initialize(*this, std::move(sharedCache));
 
+	{
+		// Load up all the symbols into the named symbols lookup map.
+		// NOTE: We do this on a separate thread as image & region loading does not consult this.
+		WorkerPriorityEnqueue([logger, cacheController]() {
+			auto& sharedCache = cacheController->GetCache();
+			auto startTime = std::chrono::high_resolution_clock::now();
+			sharedCache.ProcessSymbols();
+			auto endTime = std::chrono::high_resolution_clock::now();
+			std::chrono::duration<double> elapsed = endTime - startTime;
+			logger->LogInfo("Processing %zu symbols took %.3f seconds (separate thread)", sharedCache.GetSymbols().size(), elapsed.count());
+		});
+	}
+
 	// Users can adjust which images are loaded by default using the `loader.dsc.autoLoadPattern` setting.
 	std::string autoLoadPattern = ".*libsystem_c.dylib";
 	if (settings && settings->Contains("loader.dsc.autoLoadPattern"))

view/sharedcache/core/ffi.cpp

@@ -195,7 +195,7 @@ extern "C"
 	bool BNSharedCacheControllerGetRegionAt(
 		BNSharedCacheController* controller, uint64_t address, BNSharedCacheRegion* outRegion)
 	{
-		auto region = controller->object->GetCache().GetRegionAt(address);
+		const auto region = controller->object->GetCache().GetRegionAt(address);
 		if (!region)
 			return false;
 		*outRegion = RegionToApi(*region);
@@ -205,7 +205,7 @@ extern "C"
 	bool BNSharedCacheControllerGetRegionContaining(
 		BNSharedCacheController* controller, uint64_t address, BNSharedCacheRegion* outRegion)
 	{
-		auto region = controller->object->GetCache().GetRegionContaining(address);
+		const auto region = controller->object->GetCache().GetRegionContaining(address);
 		if (!region)
 			return false;
 		*outRegion = RegionToApi(*region);
@@ -214,7 +214,7 @@ extern "C"
 
 	BNSharedCacheRegion* BNSharedCacheControllerGetRegions(BNSharedCacheController* controller, size_t* count)
 	{
-		auto regions = controller->object->GetCache().GetRegions();
+		const auto& regions = controller->object->GetCache().GetRegions();
 		*count = regions.size();
 		BNSharedCacheRegion* apiRegions = new BNSharedCacheRegion[*count];
 		int idx = 0;
@@ -225,7 +225,7 @@ extern "C"
 
 	BNSharedCacheRegion* BNSharedCacheControllerGetLoadedRegions(BNSharedCacheController* controller, size_t* count)
 	{
-		auto loadedRegionStarts = controller->object->GetLoadedRegions();
+		const auto& loadedRegionStarts = controller->object->GetLoadedRegions();
 
 		// TODO: This translation should likely exist in the core cache controller class?
 		std::vector<CacheRegion> loadedRegions;
@@ -271,7 +271,7 @@ extern "C"
 	bool BNSharedCacheControllerGetImageAt(
 		BNSharedCacheController* controller, uint64_t address, BNSharedCacheImage* outImage)
 	{
-		auto image = controller->object->GetCache().GetImageAt(address);
+		const auto image = controller->object->GetCache().GetImageAt(address);
 		if (!image)
 			return false;
 		*outImage = ImageToApi(*image);
@@ -281,7 +281,7 @@ extern "C"
 	bool BNSharedCacheControllerGetImageContaining(
 		BNSharedCacheController* controller, uint64_t address, BNSharedCacheImage* outImage)
 	{
-		auto image = controller->object->GetCache().GetImageContaining(address);
+		const auto image = controller->object->GetCache().GetImageContaining(address);
 		if (!image)
 			return false;
 		*outImage = ImageToApi(*image);
@@ -291,7 +291,7 @@ extern "C"
 	bool BNSharedCacheControllerGetImageWithName(
 		BNSharedCacheController* controller, const char* name, BNSharedCacheImage* outImage)
 	{
-		auto image = controller->object->GetCache().GetImageWithName(name);
+		const auto image = controller->object->GetCache().GetImageWithName(name);
 		if (!image)
 			return false;
 		*outImage = ImageToApi(*image);
@@ -317,7 +317,7 @@ extern "C"
 
 	BNSharedCacheImage* BNSharedCacheControllerGetImages(BNSharedCacheController* controller, size_t* count)
 	{
-		auto images = controller->object->GetCache().GetImages();
+		const auto& images = controller->object->GetCache().GetImages();
 		*count = images.size();
 		BNSharedCacheImage* apiImages = new BNSharedCacheImage[*count];
 		size_t idx = 0;
@@ -328,7 +328,7 @@ extern "C"
 
 	BNSharedCacheImage* BNSharedCacheControllerGetLoadedImages(BNSharedCacheController* controller, size_t* count)
 	{
-		auto loadedImageStarts = controller->object->GetLoadedImages();
+		const auto& loadedImageStarts = controller->object->GetLoadedImages();
 
 		// TODO: This translation should likely exist in the core cache controller class?
 		std::vector<CacheImage> loadedImages;
@@ -362,7 +362,7 @@ extern "C"
 	bool BNSharedCacheControllerGetSymbolAt(
 		BNSharedCacheController* controller, uint64_t address, BNSharedCacheSymbol* outSymbol)
 	{
-		auto symbol = controller->object->GetCache().GetSymbolAt(address);
+		const auto symbol = controller->object->GetCache().GetSymbolAt(address);
 		if (!symbol)
 			return false;
 		*outSymbol = SymbolToApi(*symbol);
@@ -372,7 +372,7 @@ extern "C"
 	bool BNSharedCacheControllerGetSymbolWithName(
 		BNSharedCacheController* controller, const char* name, BNSharedCacheSymbol* outSymbol)
 	{
-		auto symbol = controller->object->GetCache().GetSymbolWithName(name);
+		const auto symbol = controller->object->GetCache().GetSymbolWithName(name);
 		if (!symbol)
 			return false;
 		*outSymbol = SymbolToApi(*symbol);
@@ -381,7 +381,7 @@ extern "C"
 
 	BNSharedCacheSymbol* BNSharedCacheControllerGetSymbols(BNSharedCacheController* controller, size_t* count)
 	{
-		auto symbols = controller->object->GetCache().GetSymbols();
+		const auto& symbols = controller->object->GetCache().GetSymbols();
 		*count = symbols.size();
 		BNSharedCacheSymbol* apiSymbols = new BNSharedCacheSymbol[*count];
 		size_t idx = 0;
@@ -405,7 +405,7 @@ extern "C"
 
 	BNSharedCacheEntry* BNSharedCacheControllerGetEntries(BNSharedCacheController* controller, size_t* count)
 	{
-		auto entries = controller->object->GetCache().GetEntries();
+		const auto& entries = controller->object->GetCache().GetEntries();
 		*count = entries.size();
 		BNSharedCacheEntry* apiEntries = new BNSharedCacheEntry[*count];
 		size_t idx = 0;