[SharedCache] Fix some bugs

16 files changed, 157 insertions, 75 deletions

diff --git a/objectivec/objc.cpp b/objectivec/objc.cpp
index 68956a2c..9a12e0fc 100644
--- a/objectivec/objc.cpp
+++ b/objectivec/objc.cpp
@@ -592,7 +592,7 @@ void ObjCProcessor::LoadCategories(ObjCReader* reader, Ref<Section> classPtrSect
 		}
 		if (categoryBaseClassName.empty())
 		{
-			m_logger->LogError(
+			m_logger->LogWarn(
 				"Failed to determine base classname for category at 0x%llx. Using base address as stand-in classname",
 				catLocation);
 			categoryBaseClassName = std::to_string(catLocation);
@@ -604,7 +604,7 @@ void ObjCProcessor::LoadCategories(ObjCReader* reader, Ref<Section> classPtrSect
 		}
 		catch (...)
 		{
-			m_logger->LogError(
+			m_logger->LogWarn(
 				"Failed to read category name for category at 0x%llx. Using base address as stand-in category name",
 				catLocation);
 			categoryAdditionsName = std::to_string(catLocation);
diff --git a/view/sharedcache/api/sharedcache.cpp b/view/sharedcache/api/sharedcache.cpp
index 6d552f0d..8c9b2234 100644
--- a/view/sharedcache/api/sharedcache.cpp
+++ b/view/sharedcache/api/sharedcache.cpp
@@ -9,21 +9,21 @@ using namespace SharedCacheAPI;
 
 BNSharedCacheImage ImageToApi(CacheImage image)
 {
-	BNSharedCacheImage apiImage;
+	BNSharedCacheImage apiImage {};
 	apiImage.name = BNAllocString(image.name.c_str());
 	apiImage.headerAddress = image.headerAddress;
 	apiImage.regionStartCount = image.regionStarts.size();
-	// TODO: If we alloc then core cannot delete
 	uint64_t *regionStarts = new uint64_t[image.regionStarts.size()];
 	for (size_t i = 0; i < image.regionStarts.size(); i++)
 		regionStarts[i] = image.regionStarts[i];
-	apiImage.regionStarts = regionStarts;
+	apiImage.regionStarts = BNSharedCacheAllocRegionList(regionStarts, image.regionStarts.size());
+	delete[] regionStarts;
 	return apiImage;
 }
 
 CacheImage ImageFromApi(BNSharedCacheImage image)
 {
-	CacheImage apiImage;
+	CacheImage apiImage {};
 	apiImage.name = image.name;
 	apiImage.headerAddress = image.headerAddress;
 	apiImage.regionStarts.reserve(image.regionStartCount);
@@ -34,7 +34,7 @@ CacheImage ImageFromApi(BNSharedCacheImage image)
 
 BNSharedCacheRegion RegionToApi(const CacheRegion &region)
 {
-	BNSharedCacheRegion apiRegion;
+	BNSharedCacheRegion apiRegion {};
 	apiRegion.vmAddress = region.start;
 	apiRegion.name = BNAllocString(region.name.c_str());
 	apiRegion.size = region.size;
@@ -47,7 +47,7 @@ BNSharedCacheRegion RegionToApi(const CacheRegion &region)
 
 CacheRegion RegionFromApi(BNSharedCacheRegion apiRegion)
 {
-	CacheRegion region;
+	CacheRegion region {};
 	region.start = apiRegion.vmAddress;
 	region.name = apiRegion.name;
 	region.size = apiRegion.size;
@@ -58,7 +58,7 @@ CacheRegion RegionFromApi(BNSharedCacheRegion apiRegion)
 
 BNSharedCacheMappingInfo MappingToApi(const CacheMappingInfo &mapping)
 {
-	BNSharedCacheMappingInfo apiMapping;
+	BNSharedCacheMappingInfo apiMapping {};
 	apiMapping.vmAddress = mapping.vmAddress;
 	apiMapping.size = mapping.size;
 	apiMapping.fileOffset = mapping.fileOffset;
@@ -67,7 +67,7 @@ BNSharedCacheMappingInfo MappingToApi(const CacheMappingInfo &mapping)
 
 CacheMappingInfo MappingFromApi(BNSharedCacheMappingInfo apiMapping)
 {
-	CacheMappingInfo mapping;
+	CacheMappingInfo mapping {};
 	mapping.vmAddress = apiMapping.vmAddress;
 	mapping.size = apiMapping.size;
 	mapping.fileOffset = apiMapping.fileOffset;
@@ -76,7 +76,7 @@ CacheMappingInfo MappingFromApi(BNSharedCacheMappingInfo apiMapping)
 
 BNSharedCacheEntry EntryToApi(const CacheEntry &entry)
 {
-	BNSharedCacheEntry apiEntry;
+	BNSharedCacheEntry apiEntry {};
 	apiEntry.path = BNAllocString(entry.path.c_str());
 	apiEntry.entryType = entry.entryType;
 	const auto &mappings = entry.mappings;
@@ -90,7 +90,7 @@ BNSharedCacheEntry EntryToApi(const CacheEntry &entry)
 
 CacheEntry EntryFromApi(BNSharedCacheEntry apiEntry)
 {
-	CacheEntry entry;
+	CacheEntry entry {};
 	entry.path = apiEntry.path;
 	entry.entryType = apiEntry.entryType;
 	entry.mappings.reserve(apiEntry.mappingCount);
@@ -125,6 +125,21 @@ std::string SharedCacheAPI::GetRegionTypeAsString(const BNSharedCacheRegionType
 	}
 }
 
+std::string SharedCacheAPI::GetSymbolTypeAsString(const BNSymbolType &type)
+{
+	// NOTE: We currently only use the function and data symbol for cache symbols.
+	// update this if that changes.
+	switch (type)
+	{
+	case FunctionSymbol:
+		return "Function";
+	case DataSymbol:
+		return "Data";
+	default:
+		return "Unknown";
+	}
+}
+
 SharedCacheController::SharedCacheController(BNSharedCacheController *controller)
 {
 	m_object = controller;
diff --git a/view/sharedcache/api/sharedcacheapi.h b/view/sharedcache/api/sharedcacheapi.h
index e12582f2..ee49140e 100644
--- a/view/sharedcache/api/sharedcacheapi.h
+++ b/view/sharedcache/api/sharedcacheapi.h
@@ -289,6 +289,8 @@ namespace SharedCacheAPI {
 		std::string name;
 	};
 
+	std::string GetSymbolTypeAsString(const BNSymbolType& type);
+
 	class SharedCacheController : public DSCCoreRefCountObject<BNSharedCacheController, BNNewSharedCacheControllerReference, BNFreeSharedCacheControllerReference> {
 	public:
 		explicit SharedCacheController(BNSharedCacheController* controller);
diff --git a/view/sharedcache/core/MachOProcessor.cpp b/view/sharedcache/core/MachOProcessor.cpp
index be2e9007..893aa8fe 100644
--- a/view/sharedcache/core/MachOProcessor.cpp
+++ b/view/sharedcache/core/MachOProcessor.cpp
@@ -35,11 +35,7 @@ void SharedCacheMachOProcessor::ApplyHeader(SharedCacheMachOHeader& header)
 			auto targetPlatform = m_view->GetDefaultPlatform();
 			auto functions = header.ReadFunctionTable(*m_vm);
 			for (const auto& func : functions)
-			{
-				// TODO: Check to make sure the func exists in a loaded region?
-				// TODO: ^ this check existed prior so we should add it back.
-				m_view->AddFunctionForAnalysis(targetPlatform, func);
-			}
+				m_view->AddFunctionForAnalysis(targetPlatform, func, true);
 		}
 
 		auto typeLib = m_view->GetTypeLibrary(header.installName);
@@ -52,10 +48,9 @@ void SharedCacheMachOProcessor::ApplyHeader(SharedCacheMachOHeader& header)
 			// Mach-O View symtab processing with
 			// a ton of stuff cut out so it can work
 			// NOTE: This table is read relative to the link edit segment file base.
-			// TODO: Remove this and use the m_symbols in the cache?
 			const auto symbols = header.ReadSymbolTable(*m_view, *m_vm);
 			for (const auto& sym : symbols)
-				ApplySymbol(m_view, typeLib, sym.ToBNSymbol());
+				ApplySymbol(m_view, typeLib, sym.ToBNSymbol(*m_view));
 		}
 
 		// Apply symbols from export trie.
@@ -65,7 +60,7 @@ void SharedCacheMachOProcessor::ApplyHeader(SharedCacheMachOHeader& header)
 			// TODO: Remove this and use the m_symbols in the cache?
 			const auto exportSymbols = header.ReadExportSymbolTrie(*m_vm);
 			for (const auto& sym : exportSymbols)
-				ApplySymbol(m_view, typeLib, sym.ToBNSymbol());
+				ApplySymbol(m_view, typeLib, sym.ToBNSymbol(*m_view));
 		}
 		m_view->EndBulkModifySymbols();
 	}
diff --git a/view/sharedcache/core/SharedCache.cpp b/view/sharedcache/core/SharedCache.cpp
index 56d2040d..d0c3d25b 100644
--- a/view/sharedcache/core/SharedCache.cpp
+++ b/view/sharedcache/core/SharedCache.cpp
@@ -11,11 +11,12 @@ using namespace BinaryNinja;
 // The next id to use when calling Cache::AddEntry
 static CacheEntryId nextId = 1;
 
-Ref<Symbol> CacheSymbol::ToBNSymbol() const
+Ref<Symbol> CacheSymbol::ToBNSymbol(BinaryView& view) const
 {
 	QualifiedName qname;
+	Ref<Type> outType;
 	std::string shortName = name;
-	if (DemangleLLVM(name, qname, true))
+	if (DemangleGeneric(view.GetDefaultArchitecture(), name, outType, qname, &view, true))
 		shortName = qname.GetString();
 	return new Symbol(type, shortName, shortName, name, address, nullptr);
 }
@@ -458,7 +459,7 @@ std::optional<CacheImage> SharedCache::GetImageContaining(const uint64_t address
 {
 	// TODO: What if we are using this on a shared region? Return a list of images?
 	auto region = GetRegionContaining(address);
-	if (region.has_value())
+	if (region.has_value() && region->imageStart.has_value())
 		return GetImageAt(*region->imageStart);
 	return std::nullopt;
 }
diff --git a/view/sharedcache/core/SharedCache.h b/view/sharedcache/core/SharedCache.h
index 54ec9dfd..674e00c3 100644
--- a/view/sharedcache/core/SharedCache.h
+++ b/view/sharedcache/core/SharedCache.h
@@ -32,7 +32,7 @@ struct CacheSymbol
 	CacheSymbol& operator=(CacheSymbol&& other) noexcept = default;
 
 	// NOTE: you should really only call this when adding the symbol to the view.
-	BinaryNinja::Ref<BinaryNinja::Symbol> ToBNSymbol() const;
+	BinaryNinja::Ref<BinaryNinja::Symbol> ToBNSymbol(BinaryNinja::BinaryView& view) const;
 };
 
 enum class CacheRegionType
diff --git a/view/sharedcache/core/SharedCacheController.cpp b/view/sharedcache/core/SharedCacheController.cpp
index 29c1192a..72d5d950 100644
--- a/view/sharedcache/core/SharedCacheController.cpp
+++ b/view/sharedcache/core/SharedCacheController.cpp
@@ -60,6 +60,9 @@ SharedCacheController::SharedCacheController(SharedCache cache, Ref<Logger> logg
 	m_logger = std::move(logger);
 	m_loadedRegions = {};
 	m_loadedImages = {};
+	m_processObjC = true;
+	m_processCFStrings = true;
+	m_regionFilter = std::regex(".*LINKEDIT.*");
 }
 
 DSCRef<SharedCacheController> SharedCacheController::Initialize(BinaryView& view, SharedCache cache)
@@ -111,16 +114,16 @@ bool SharedCacheController::ApplyRegionAtAddress(BinaryView& view, const uint64_
 
 bool SharedCacheController::ApplyRegion(BinaryView& view, const CacheRegion& region)
 {
-	// TODO: Check m_loadedRegions? This seems redundant...
+	std::unique_lock<std::shared_mutex> lock(m_loadMutex);
 	// Loads the given region into the BinaryView and marks it as loaded.
-	// First check to make sure there isn't already a segment at the address in the view.
-	if (view.GetSegmentAt(region.start))
+	// First check to make sure we haven't already loaded the region.
+	if (m_loadedRegions.find(region.start) != m_loadedRegions.end())
 		return false;
 
 	// Skip filtered regions, this defaults to just LINKEDIT regions.
 	if (std::regex_match(region.name, m_regionFilter))
 	{
-		LogDebug("Skipping filtered region at %llx", region.start);
+		m_logger->LogDebug("Skipping filtered region at %llx", region.start);
 		return false;
 	}
 
@@ -161,8 +164,9 @@ bool SharedCacheController::ApplyRegion(BinaryView& view, const CacheRegion& reg
 	return true;
 }
 
-bool SharedCacheController::IsRegionLoaded(const CacheRegion& region) const
+bool SharedCacheController::IsRegionLoaded(const CacheRegion& region)
 {
+	std::shared_lock<std::shared_mutex> lock(m_loadMutex);
 	return std::any_of(m_loadedRegions.begin(), m_loadedRegions.end(), [&](const auto& loadedRegion) {
 		return loadedRegion == region.start;
 	});
@@ -170,15 +174,19 @@ bool SharedCacheController::IsRegionLoaded(const CacheRegion& region) const
 
 bool SharedCacheController::ApplyImage(BinaryView& view, const CacheImage& image)
 {
-	// TODO: Check m_loadedImages? This seems redundant...
 	// Load all regions of an image and mark the image as loaded.
+	// NOTE: The regions lock m_loadMutex themselves, so we do not hold it up here.
 	bool loadedRegion = false;
 	for (const auto& regionStart : image.regionStarts)
 		if (ApplyRegionAtAddress(view, regionStart))
 			loadedRegion = true;
 
+	// The ApplyRegionAtAddress no longer holds the lock, we can take it now.
+	std::unique_lock<std::shared_mutex> lock(m_loadMutex);
 	// If there was no loaded regions than we just want to forgo loading the image.
-	if (!loadedRegion)
+	// We also skip if we already loaded the image itself. We do this after loading regions
+	// as we regions have their own check.
+	if (!loadedRegion || m_loadedImages.find(image.headerAddress) != m_loadedImages.end())
 		return false;
 
 	if (image.header)
@@ -187,17 +195,16 @@ bool SharedCacheController::ApplyImage(BinaryView& view, const CacheImage& image
 		auto machoProcessor = SharedCacheMachOProcessor(&view, m_cache.GetVirtualMemory());
 		machoProcessor.ApplyHeader(*image.header);
 
-		// TODO: Make this optional.
-		// Load objective-c information.
-		auto objcProcessor = DSCObjC::SharedCacheObjCProcessor(&view, false);
 		// TODO: Passing in an image name here is weird considering this is shared with the MACHO view.
 		// TODO: We should abstract out the "image" into an objc image type that represents what is required, which ig is the name?
+		// Load objective-c information.
+		auto objcProcessor = DSCObjC::SharedCacheObjCProcessor(&view, false);
 		try
 		{
 			if (m_processObjC)
 				objcProcessor.ProcessObjCData(image.GetName());
-			if (m_processObjC)
-			objcProcessor.ProcessCFStrings(image.GetName());
+			if (m_processCFStrings)
+				objcProcessor.ProcessCFStrings(image.GetName());
 		}
 		catch (std::exception& e)
 		{
@@ -216,8 +223,9 @@ bool SharedCacheController::ApplyImage(BinaryView& view, const CacheImage& image
 	return true;
 }
 
-bool SharedCacheController::IsImageLoaded(const CacheImage& image) const
+bool SharedCacheController::IsImageLoaded(const CacheImage& image)
 {
+	std::shared_lock<std::shared_mutex> lock(m_loadMutex);
 	return std::any_of(m_loadedImages.begin(), m_loadedImages.end(), [&](const auto& loadedImage) {
 		return loadedImage == image.headerAddress;
 	});
diff --git a/view/sharedcache/core/SharedCacheController.h b/view/sharedcache/core/SharedCacheController.h
index 354762eb..3b92f198 100644
--- a/view/sharedcache/core/SharedCacheController.h
+++ b/view/sharedcache/core/SharedCacheController.h
@@ -18,8 +18,11 @@ namespace BinaryNinja::DSC {
 	{
 		IMPLEMENT_DSC_API_OBJECT(BNSharedCacheController);
 		Ref<Logger> m_logger;
-
 		SharedCache m_cache;
+
+		// Locks on load attempts (region or image).
+		std::shared_mutex m_loadMutex;
+
 		// Store the open images.
 		// Things other than the cache here will be serialized.
 		std::unordered_set<uint64_t> m_loadedRegions;
@@ -49,13 +52,13 @@ namespace BinaryNinja::DSC {
 
 		bool ApplyRegion(BinaryView& view, const CacheRegion& region);
 
-		bool IsRegionLoaded(const CacheRegion& region) const;
+		bool IsRegionLoaded(const CacheRegion& region);
 
 		// Loads the relevant image info into the view. This does not update analysis so if you
 		// call this make sure at some point you update analysis and likely with linear sweep.
 		bool ApplyImage(BinaryView& view, const CacheImage& image);
 
-		bool IsImageLoaded(const CacheImage& image) const;
+		bool IsImageLoaded(const CacheImage& image);
 
 		// Get the metadata for saving the state of the shared cache.
 		Ref<Metadata> GetMetadata() const;
diff --git a/view/sharedcache/core/SharedCacheView.cpp b/view/sharedcache/core/SharedCacheView.cpp
index bf80c6eb..f27f7c14 100644
--- a/view/sharedcache/core/SharedCacheView.cpp
+++ b/view/sharedcache/core/SharedCacheView.cpp
@@ -82,7 +82,7 @@ Ref<Settings> SharedCacheViewType::GetLoadSettingsForData(BinaryView* data)
 	R"({
 		"title" : "Region Regex Filter",
 		"type" : "string",
-		"default" : "LINKEDIT",
+		"default" : ".*LINKEDIT.*",
 		"description" : "Regex filter for region names to skip loading, by default this filters out the link edit region which is not necessary to be applied to the view."
 		})");
 
@@ -784,19 +784,22 @@ bool SharedCacheView::Init()
 		// After this we should have all the mappings available as well.
 		CacheProcessor cacheProcessor = CacheProcessor(this);
 		auto startTime = std::chrono::high_resolution_clock::now();
-		if (!cacheProcessor.ProcessCache(sharedCache))
+		bool result = cacheProcessor.ProcessCache(sharedCache);
+		auto endTime = std::chrono::high_resolution_clock::now();
+		std::chrono::duration<double> elapsed = endTime - startTime;
+		logger->LogInfo("Processing %zu entries took %.3f seconds", sharedCache.GetEntries().size(), elapsed.count());
+
+		if (!result)
 		{
-			// Oh no, we failed to process the cache, this likely means the primary on-disk file was not able to be
-			// found.
 			// TODO: Prompt the user to select the primary cache file? We can still recover from here.
+			// Oh no, we failed to process the cache, this likely means the primary on-disk file was not able to be found.
 			logger->LogError("Failed to process cache, likely missing cache files.");
 			// NOTE: An interaction handler headlessly could select yes to this, however for the purposes of this we will just let them continue by defualt.
 			if (IsUIEnabled() && ShowMessageBox("Continue opening", "This shared cache file was unable to be processed, would you like to open without shared cache information?", YesNoButtonSet, QuestionIcon) != YesButton)
 				return false;
+			// Skip all the other shared cache stuff.
+			return true;
 		}
-		auto endTime = std::chrono::high_resolution_clock::now();
-		std::chrono::duration<double> elapsed = endTime - startTime;
-		logger->LogInfo("Processing %zu entries took %.3f seconds", sharedCache.GetEntries().size(), elapsed.count());
 	}
 
 	{
diff --git a/view/sharedcache/core/Utility.cpp b/view/sharedcache/core/Utility.cpp
index 91729578..c4891930 100644
--- a/view/sharedcache/core/Utility.cpp
+++ b/view/sharedcache/core/Utility.cpp
@@ -74,7 +74,7 @@ void ApplySymbol(Ref<BinaryView> view, Ref<TypeLibrary> typeLib, Ref<Symbol> sym
 	if (symbol->GetType() == FunctionSymbol)
 	{
 		Ref<Platform> targetPlatform = view->GetDefaultPlatform();
-		func = view->AddFunctionForAnalysis(targetPlatform, symbolAddress);
+		func = view->AddFunctionForAnalysis(targetPlatform, symbolAddress, true);
 	}
 
 	if (typeLib)
diff --git a/view/sharedcache/core/Utility.h b/view/sharedcache/core/Utility.h
index 86715557..e6f272e7 100644
--- a/view/sharedcache/core/Utility.h
+++ b/view/sharedcache/core/Utility.h
@@ -36,9 +36,6 @@ uint64_t readValidULEB128(const uint8_t*& current, const uint8_t* end);
 void ApplySymbol(BinaryNinja::Ref<BinaryNinja::BinaryView> view, BinaryNinja::Ref<BinaryNinja::TypeLibrary> typeLib,
 	BinaryNinja::Ref<BinaryNinja::Symbol> symbol);
 
-// Returns the on disk file for a given path, this is required to support project files.
-std::string ResolveFilePath(BinaryNinja::Ref<BinaryNinja::BinaryView> dscView, const std::string& path);
-
 // Returns the "image name" for a given path.
 // /blah/foo/bar/libObjCThing.dylib -> libObjCThing.dylib
 std::string BaseFileName(const std::string& path);
diff --git a/view/sharedcache/core/VirtualMemory.h b/view/sharedcache/core/VirtualMemory.h
index fb00ebaf..43a9acb4 100644
--- a/view/sharedcache/core/VirtualMemory.h
+++ b/view/sharedcache/core/VirtualMemory.h
@@ -55,6 +55,7 @@ public:
 
 	bool IsAddressMapped(uint64_t address);
 
+	// TODO: Bulk pointer writes here would alleviate a lot of the time spent in the slide info processor.
 	// Write a pointer at a given address. This pointer will be persisted
 	// for a given `VirtualMemoryRegion` region, unlike using the MappedFileAccessor directly.
 	// The persistence is provided through the WeakFileAccessor itself and thus is unique to the construction.
diff --git a/view/sharedcache/ui/dscpicker.cpp b/view/sharedcache/ui/dscpicker.cpp
index c6448ff4..340b8aa8 100644
--- a/view/sharedcache/ui/dscpicker.cpp
+++ b/view/sharedcache/ui/dscpicker.cpp
@@ -11,7 +11,7 @@ using namespace SharedCacheAPI;
 
 void DisplayDSCPicker(UIContext* ctx, Ref<BinaryView> dscView)
 {
-	static auto getImageNames = [dscView](QVariant var) {
+	auto getImageNames = [dscView](QVariant var) {
 		auto controller = SharedCacheController::GetController(*dscView);
 		if (!controller)
 			return QStringList();
@@ -22,7 +22,7 @@ void DisplayDSCPicker(UIContext* ctx, Ref<BinaryView> dscView)
 		return entries;
 	};
 
-	static auto getChosenImage = [ctx](QVariant var) {
+	auto getChosenImage = [ctx](QVariant var) {
 		QStringList entries = var.toStringList();
 
 		auto choiceDialog = new MetadataChoiceDialog(ctx ? ctx->mainWindow() : nullptr, "Pick Image", "Select", entries);
@@ -36,7 +36,7 @@ void DisplayDSCPicker(UIContext* ctx, Ref<BinaryView> dscView)
 		return QVariant(QString::fromStdString(entries.at((qsizetype)choiceDialog->GetChosenEntry().value().idx).toStdString()));
 	};
 
-	static auto loadSelectedImage = [dscView](QVariant var) {
+	auto loadSelectedImage = [dscView](QVariant var) {
 		auto selectedImageName = var.toString().toStdString();
 		if (selectedImageName.empty())
 			return;
@@ -54,8 +54,8 @@ void DisplayDSCPicker(UIContext* ctx, Ref<BinaryView> dscView)
 
 
 	BackgroundThread::create(ctx ? ctx->mainWindow() : nullptr)
-		->thenBackground([](QVariant var){ return getImageNames(var); })
-		->thenMainThread([](QVariant var){ return getChosenImage(var); })
-		->thenBackground([](QVariant var){ return loadSelectedImage(var); })
+		->thenBackground([getImageNames](QVariant var){ return getImageNames(var); })
+		->thenMainThread([getChosenImage](QVariant var){ return getChosenImage(var); })
+		->thenBackground([loadSelectedImage](QVariant var){ return loadSelectedImage(var); })
 		->start();
 }
diff --git a/view/sharedcache/ui/dsctriage.cpp b/view/sharedcache/ui/dsctriage.cpp
index aab71aac..297612d5 100644
--- a/view/sharedcache/ui/dsctriage.cpp
+++ b/view/sharedcache/ui/dsctriage.cpp
@@ -54,7 +54,7 @@ DSCTriageView::DSCTriageView(QWidget* parent, BinaryViewRef data) : QWidget(pare
 
 	QWidget* defaultWidget = nullptr;
 
-	static auto loadImagesWithAddr = [this](const std::vector<uint64_t>& addresses) {
+	auto loadImagesWithAddr = [this](const std::vector<uint64_t>& addresses) {
 		auto controller = SharedCacheController::GetController(*this->m_data);
 		if (!controller)
 			return;
@@ -102,10 +102,13 @@ DSCTriageView::DSCTriageView(QWidget* parent, BinaryViewRef data) : QWidget(pare
 			m_imageModel->setHorizontalHeaderLabels({"Address", "Loaded", "Name"});
 		} // loadImageModel
 
+		// Show the icon in the loaded column.
+		loadImageTable->setItemDelegateForColumn(1, new LoadedDelegate());
+
 		auto loadImageButton = new CustomStyleFlatPushButton();
 		{
 			connect(loadImageButton, &QPushButton::clicked,
-				[loadImageTable](bool) {
+				[loadImageTable, loadImagesWithAddr](bool) {
 					auto selected = loadImageTable->selectionModel()->selectedRows();
 					std::vector<uint64_t> addresses;
 					for (const auto& row : selected)
@@ -188,7 +191,7 @@ DSCTriageView::DSCTriageView(QWidget* parent, BinaryViewRef data) : QWidget(pare
 		auto loadSymbolImageButton = new CustomStyleFlatPushButton();
 		{
 			connect(loadSymbolImageButton, &QPushButton::clicked,
-				[this](bool) {
+				[this, loadImagesWithAddr](bool) {
 					auto selected = m_symbolTable->selectionModel()->selectedRows();
 					std::vector<uint64_t> addresses;
 					for (const auto& row : selected)
@@ -236,12 +239,6 @@ DSCTriageView::DSCTriageView(QWidget* parent, BinaryViewRef data) : QWidget(pare
 		auto symbolWidget = new QWidget;
 		symbolWidget->setLayout(symbolLayout);
 
-		m_symbolTable->horizontalHeader()->setSectionResizeMode(0, QHeaderView::ResizeToContents); // Address
-		m_symbolTable->horizontalHeader()->setSectionResizeMode(1, QHeaderView::Stretch);          // Name
-
-		m_symbolTable->setSelectionBehavior(QAbstractItemView::SelectRows);
-		m_symbolTable->setSelectionMode(QAbstractItemView::SingleSelection);
-
 		connect(m_symbolTable, &SymbolTableView::activated, this, [=](const QModelIndex& index){
 				auto symbol = m_symbolTable->getSymbolAtRow(index.row());
 				auto dialog = new QMessageBox(this);
@@ -444,9 +441,6 @@ void DSCTriageView::RefreshData()
 
 void DSCTriageView::setImageLoaded(const uint64_t imageHeaderAddr)
 {
-	// TODO: Better icon... probably something like ✅
-	static QIcon loadedIcon(":/icons/images/plus.png");
-
 	// Go through the m_loadImageModel and find the image associated with the address
 	// then set the image as loaded.
 	for (int i = 0; i < m_imageModel->rowCount(); i++)
@@ -456,8 +450,9 @@ void DSCTriageView::setImageLoaded(const uint64_t imageHeaderAddr)
 		if (addr == imageHeaderAddr)
 		{
 			auto statusCol = m_imageModel->index(i, 1);
-			m_imageModel->setData(statusCol, QString("•"), Qt::DisplayRole);
+			// See the `LoadedDelegate` class, we set 1 to indicate that this image is loaded.
+			m_imageModel->setData(statusCol, "1", Qt::DisplayRole);
 			break;
 		}
 	}
-}
+}
\ No newline at end of file
diff --git a/view/sharedcache/ui/dsctriage.h b/view/sharedcache/ui/dsctriage.h
index 8c4ceb3f..5aa36646 100644
--- a/view/sharedcache/ui/dsctriage.h
+++ b/view/sharedcache/ui/dsctriage.h
@@ -3,6 +3,7 @@
 //
 
 #include <binaryninjaapi.h>
+#include <QItemDelegate>
 #include <QStyledItemDelegate>
 
 #include "uitypes.h"
@@ -137,5 +138,58 @@ public:
 	static void Register();
 };
 
+class LoadedDelegate : public QItemDelegate
+{
+	Q_OBJECT
+public:
+	void paint(QPainter *painter, const QStyleOptionViewItem &option,
+			   const QModelIndex &index) const override
+	{
+		if (!index.isValid())
+			return;
+
+		painter->save();
+
+		// Highlight if the item is selected
+		if (option.state & QStyle::State_Selected)
+			painter->fillRect(option.rect, option.palette.highlight());
+
+		// "1" is the indicator that its loaded.
+		if (index.data(Qt::DisplayRole).toString() == "1")
+		{
+			QPixmap loadedIcon;
+			pixmapForBWMaskIcon(":/icons/images/download.png", &loadedIcon, SidebarHeaderTextColor);
+			if (!loadedIcon.isNull())
+			{
+				QSize pixmapSize(20, 20);
+				QPixmap scaledPixmap = loadedIcon.scaled(pixmapSize, Qt::KeepAspectRatio, Qt::SmoothTransformation);
+
+				// Calculate the rectangle for centering the pixmap
+				int x = option.rect.x() + (option.rect.width() - scaledPixmap.width()) / 2; // Center horizontally
+				int y = option.rect.y() + (option.rect.height() - scaledPixmap.height()) / 2; // Center vertically
+				QRect iconRect(x, y, scaledPixmap.width(), scaledPixmap.height());
+
+				// Draw the pixmap
+				painter->drawPixmap(iconRect, scaledPixmap);
+			}
+		}
+
+		painter->restore();
+	}
+
+	QSize sizeHint(const QStyleOptionViewItem &option,
+				   const QModelIndex &index) const override
+	{
+		Q_UNUSED(option);
+		Q_UNUSED(index);
+		return QSize(50, 24);
+	}
+
+	void setEditorData(QWidget *editor, const QModelIndex &index) const override
+	{
+		Q_UNUSED(editor);
+		Q_UNUSED(index);
+	}
+};
 
 #endif	// BINARYNINJA_DSCTRIAGE_H
diff --git a/view/sharedcache/ui/symboltable.cpp b/view/sharedcache/ui/symboltable.cpp
index c5e2d7bf..c7393d59 100644
--- a/view/sharedcache/ui/symboltable.cpp
+++ b/view/sharedcache/ui/symboltable.cpp
@@ -19,8 +19,8 @@ int SymbolTableModel::rowCount(const QModelIndex& parent) const {
 
 int SymbolTableModel::columnCount(const QModelIndex& parent) const {
 	Q_UNUSED(parent);
-	// We have 2 columns: Address, Name
-	return 2;
+	// We have 3 columns: Address, Type, Name
+	return 3;
 }
 
 QVariant SymbolTableModel::data(const QModelIndex& index, int role) const {
@@ -29,11 +29,14 @@ QVariant SymbolTableModel::data(const QModelIndex& index, int role) const {
 	}
 
 	const CacheSymbol& symbol = m_symbols.at(index.row());
+	auto symbolType = GetSymbolTypeAsString(symbol.type);
 
 	switch (index.column()) {
 	case 0: // Address column
 		return QString("0x%1").arg(symbol.address, 0, 16); // Display address as hexadecimal
-	case 1: // Name column
+	case 1: // Type column
+		return QString::fromUtf8(symbolType.c_str(), symbolType.size());
+	case 2: // Name column
 		return QString::fromUtf8(symbol.name.c_str(), symbol.name.size());
 	default:
 		return QVariant();
@@ -49,6 +52,8 @@ QVariant SymbolTableModel::headerData(int section, Qt::Orientation orientation,
 	case 0:
 		return QString("Address");
 	case 1:
+		return QString("Type");
+	case 2:
 		return QString("Name");
 	default:
 		return QVariant();
@@ -100,7 +105,9 @@ SymbolTableView::SymbolTableView(QWidget* parent)
 	setModel(m_model);
 
 	// Configure view settings
-	horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch);
+	horizontalHeader()->setSectionResizeMode(0, QHeaderView::ResizeToContents);
+	horizontalHeader()->setSectionResizeMode(1, QHeaderView::ResizeToContents);
+	horizontalHeader()->setSectionResizeMode(2, QHeaderView::Stretch);
 	setSelectionBehavior(QAbstractItemView::SelectRows);
 	setSelectionMode(QAbstractItemView::SingleSelection);
 
@@ -116,6 +123,7 @@ void SymbolTableView::populateSymbols(BinaryView &view)
 	if (auto controller = SharedCacheController::GetController(view)) {
 		BackgroundThread::create(this)
 			->thenBackground([this, controller]() {
+				// TODO: There is a crash related to `this` being destructed. https://github.com/Vector35/binaryninja-api/issues/6300
 				std::vector<CacheSymbol> newSymbols = controller->GetSymbols();
 				m_symbols.swap(newSymbols);
 			})