From 3d759f3c89bc12c1be18f8b565c9bb9ce4ca82ef Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Tue, 14 Apr 2026 14:18:57 -0700 Subject: [PATCH] [ObjC] Store Objective-C metadata in the database instead of regenerating it on load Processed Obj-C metadata was stored ephemerally on the view and not serialized to the .bndb. On reopen the Obj-C processor had to be re-run on every loaded image to regenerate the metadata, which caused many functions to be reanalyzed due to types and symbols being reapplied. Commit 1bb00127874e tried to suppress that from inside `DefineObjCSymbol` but ended up skipping method type application on fresh shared-cache loads. As a result, methods whose names were already in the symbol table (some methods in macOS shared caches) did not have argument types applied. We now save the Objective-C metadata to the .bndb and only process Objective-C data in the binary if the metadata is absent or out of date. Additionally, each `ProcessObjCData` call now merges its output into any existing metadata so multiple shared cache images accumulate metadata into a single unified object instead of clobbering each other. An "Objective-C Literals" metadata key is added with its own version, so future changes to Obj-C literal processing can force reprocessing on existing databases without touching the main Obj-C metadata version. The changes made to `ObjCProcessor` in 1bb00127874e are reverted as they were incorrect and are no longer needed with this new approach. Fixes https://github.com/Vector35/binaryninja-api/issues/8087. --- objectivec/objc.cpp | 220 ++++++++++++++++-- objectivec/objc.h | 33 +++ .../src/metadata/global_state.rs | 4 +- view/macho/machoview.cpp | 65 +++--- view/macho/machoview.h | 1 + .../core/SharedCacheController.cpp | 45 ++-- view/sharedcache/core/SharedCacheController.h | 5 +- view/sharedcache/core/SharedCacheView.cpp | 2 +- 8 files changed, 295 insertions(+), 80 deletions(-) diff --git a/objectivec/objc.cpp b/objectivec/objc.cpp index 35adfda88..3dcd76238 100644 --- a/objectivec/objc.cpp +++ b/objectivec/objc.cpp @@ -68,6 +68,150 @@ namespace { return Type::NamedType(builder.Finalize()); } + constexpr uint64_t kObjCMetadataVersion = 2; + constexpr const char* kObjCMetadataKey = "Objective-C"; + constexpr uint64_t kObjCLiteralsVersion = 1; + constexpr const char* kObjCLiteralsKey = "Objective-C Literals"; + + bool HasUpToDateVersion(BinaryView* view, const char* key, uint64_t currentVersion) + { + auto existing = view->QueryMetadata(key); + if (!existing) + return false; + auto kv = existing->GetKeyValueStore(); + auto it = kv.find("version"); + if (it == kv.end()) + return false; + return it->second->GetUnsignedInteger() >= currentVersion; + } + + // Classes, categories, and methods are arrays of records keyed on `loc`. Entry locations + // are unique across images in a shared cache so we can safely merge by concatenating. + Ref MergeObjCRecordsByLocation(Ref existing, Ref fresh) + { + std::vector> combined = existing->GetArray(); + if (fresh) + { + auto freshEntries = fresh->GetArray(); + combined.insert(combined.end(), freshEntries.begin(), freshEntries.end()); + } + return new Metadata(combined); + } + + // selImplementations and selRefImplementations are lists of [addr, impls] pairs. Selector + // live in shared regions of the dyld shared cache and legitimately appear under the same + // key in multiple images, so entries must be merged key-wise rather than concatenated. + // For each key, the union of impls from both inputs is written out. + Ref MergeSelectorImplLists(Ref existing, Ref fresh) + { + std::map> merged; + + auto append = [&merged](const Ref& array) { + if (!array) + return; + for (const auto& entry : array->GetArray()) + { + auto pair = entry->GetArray(); + if (pair.size() != 2) + continue; + uint64_t key = pair[0]->GetUnsignedInteger(); + auto& dest = merged[key]; + for (uint64_t impl : pair[1]->GetUnsignedIntegerList()) + { + if (std::find(dest.begin(), dest.end(), impl) == dest.end()) + dest.push_back(impl); + } + } + }; + + append(existing); + append(fresh); + + std::vector> result; + result.reserve(merged.size()); + for (const auto& [key, impls] : merged) + { + std::vector> pair = {new Metadata(key), new Metadata(impls)}; + result.push_back(new Metadata(pair)); + } + return new Metadata(result); + } + + // selRefToName is a list of [selref_addr, name] pairs. Selref addresses are per-image and + // should not collide, but in case an image is reprocessed, keep the first value observed. + Ref MergeSelRefNames(Ref existing, Ref fresh) + { + std::map merged; + + auto insert = [&merged](const Ref& array) { + if (!array) + return; + for (const auto& entry : array->GetArray()) + { + auto pair = entry->GetArray(); + if (pair.size() != 2) + continue; + uint64_t key = pair[0]->GetUnsignedInteger(); + merged.emplace(key, pair[1]->GetString()); + } + }; + + insert(existing); + insert(fresh); + + std::vector> result; + result.reserve(merged.size()); + for (const auto& [key, name] : merged) + { + std::vector> pair = {new Metadata(key), new Metadata(name)}; + result.push_back(new Metadata(pair)); + } + return new Metadata(result); + } + + Ref MergeObjCMetadata(Ref existing, Ref fresh) + { + if (!existing) + return fresh; + + auto existingKv = existing->GetKeyValueStore(); + + // Merging records from an older metadata version would duplicate entries, as they + // describe the same locations as the fresh records. Replace them instead. + auto existingVersion = existingKv.find("version"); + if (existingVersion == existingKv.end() + || existingVersion->second->GetUnsignedInteger() != kObjCMetadataVersion) + return fresh; + + auto freshKv = fresh->GetKeyValueStore(); + std::map> merged = freshKv; + + auto lookup = [](const std::map>& kv, const char* key) -> Ref { + auto it = kv.find(key); + return it != kv.end() ? it->second : nullptr; + }; + + for (const char* key : {"classes", "categories", "methods"}) + { + if (auto existingArray = lookup(existingKv, key)) + merged[key] = MergeObjCRecordsByLocation(existingArray, lookup(freshKv, key)); + } + + for (const char* key : {"selImplementations", "selRefImplementations"}) + { + if (existingKv.count(key) || freshKv.count(key)) + merged[key] = MergeSelectorImplLists(lookup(existingKv, key), lookup(freshKv, key)); + } + + if (existingKv.count("selRefToName") || freshKv.count("selRefToName")) + { + merged["selRefToName"] = MergeSelRefNames( + lookup(existingKv, "selRefToName"), lookup(freshKv, "selRefToName")); + } + + return new Metadata(merged); + } + } // namespace Ref ObjCProcessor::SerializeMethod(uint64_t loc, const Method& method) @@ -104,10 +248,59 @@ Ref ObjCProcessor::SerializeClass(uint64_t loc, const Class& cls) return new Metadata(clsMeta); } +bool ObjCProcessor::HasUpToDateMetadata(BinaryView* view) +{ + return HasUpToDateVersion(view, kObjCMetadataKey, kObjCMetadataVersion); +} + +bool ObjCProcessor::HasUpToDateLiterals(BinaryView* view) +{ + return HasUpToDateVersion(view, kObjCLiteralsKey, kObjCLiteralsVersion); +} + +ObjCProcessor::Tasks ObjCProcessor::NeededTasks(BinaryView* view, Tasks requested) +{ + Tasks needed = Tasks::None; + if (HasTask(requested, Tasks::Metadata) && !HasUpToDateMetadata(view)) + needed |= Tasks::Metadata; + if (HasTask(requested, Tasks::Literals) && !HasUpToDateLiterals(view)) + needed |= Tasks::Literals; + return needed; +} + +void ObjCProcessor::Process(Tasks tasks) +{ + if (HasTask(tasks, Tasks::Literals)) + { + try + { + ProcessObjCLiterals(); + } + catch (std::exception& ex) + { + m_logger->LogError("Failed to process Objective-C literals. Binary may be malformed"); + m_logger->LogErrorF("Error: {:?}", ex.what()); + } + } + + if (HasTask(tasks, Tasks::Metadata)) + { + try + { + ProcessObjCData(); + } + catch (std::exception& ex) + { + m_logger->LogError("Failed to process Objective-C metadata. Binary may be malformed"); + m_logger->LogErrorF("Error: {:?}", ex.what()); + } + } +} + Ref ObjCProcessor::SerializeMetadata() { std::map> viewMeta; - viewMeta["version"] = new Metadata((uint64_t)1); + viewMeta["version"] = new Metadata(kObjCMetadataVersion); std::vector> classes; classes.reserve(m_classes.size()); @@ -355,17 +548,10 @@ void ObjCProcessor::DefineObjCSymbol( Ref symbol = new Symbol(type, name, name, name, addr, LocalBinding, nameSpace); uint64_t symbolAddress = symbol->GetAddress(); - if (Ref existingSymbol = m_data->GetSymbolByAddress(symbolAddress)) - { - if (existingSymbol->IsAutoDefined() && existingSymbol->GetType() == symbol->GetType() - && existingSymbol->GetRawNameRef() == symbol->GetRawNameRef()) - return; - - m_data->UndefineAutoSymbol(existingSymbol); - } - // Armv7/Thumb: This will rewrite the symbol's address. // e.g. We pass in 0xc001, it will rewrite it to 0xc000 and create the function w/ the "thumb2" arch. + if (Ref existingSymbol = m_data->GetSymbolByAddress(symbolAddress)) + m_data->UndefineAutoSymbol(existingSymbol); Ref targetPlatform = m_data->GetDefaultPlatform()->GetAssociatedPlatformByAddress(symbolAddress); if (symbol->GetType() == FunctionSymbol) { @@ -1119,11 +1305,6 @@ void ObjCProcessor::GenerateClassTypes() { for (auto& [_, cls] : m_classes) { - QualifiedName classTypeName = cls.name; - std::string classTypeId = Type::GenerateAutoTypeId("objc", classTypeName); - if (m_data->GetTypeById(classTypeId)) - continue; - QualifiedName typeName; StructureBuilder classTypeBuilder; bool failedToDecodeType = false; @@ -1156,6 +1337,8 @@ void ObjCProcessor::GenerateClassTypes() if (failedToDecodeType) continue; auto classTypeStruct = classTypeBuilder.Finalize(); + QualifiedName classTypeName = cls.name; + std::string classTypeId = Type::GenerateAutoTypeId("objc", classTypeName); Ref classType = Type::StructureType(classTypeStruct); QualifiedName classQualName = m_data->DefineType(classTypeId, classTypeName, classType); cls.associatedName = classTypeName; @@ -1585,7 +1768,8 @@ void ObjCProcessor::ProcessObjCData() PostProcessObjCSections(reader.get()); auto meta = SerializeMetadata(); - m_data->StoreMetadata("Objective-C", meta, MetadataStoreEphemeral); + auto existing = m_data->QueryMetadata(kObjCMetadataKey); + m_data->StoreMetadata(kObjCMetadataKey, MergeObjCMetadata(existing, meta), MetadataStorePersistent); m_relocationPointerRewrites.clear(); } @@ -1598,6 +1782,10 @@ void ObjCProcessor::ProcessObjCLiterals() ProcessNSConstantIntegerNumbers(); ProcessNSConstantFloatingPointNumbers(); ProcessNSConstantDatas(); + + std::map> versionMeta; + versionMeta["version"] = new Metadata(kObjCLiteralsVersion); + m_data->StoreMetadata(kObjCLiteralsKey, new Metadata(versionMeta), MetadataStorePersistent); } void ObjCProcessor::ProcessCFStrings() diff --git a/objectivec/objc.h b/objectivec/objc.h index 2751fcf31..c95cb8f81 100644 --- a/objectivec/objc.h +++ b/objectivec/objc.h @@ -349,9 +349,42 @@ namespace BinaryNinja { public: virtual ~ObjCProcessor() = default; + enum class Tasks : uint8_t + { + None = 0, + Metadata = 1 << 0, + Literals = 1 << 1, + }; + ObjCProcessor(BinaryView* data, const char* loggerName, bool skipClassBaseProtocols = false); void ProcessObjCData(); void ProcessObjCLiterals(); void AddRelocatedPointer(uint64_t location, uint64_t rewrite); + + // Run the requested Obj-C processing tasks. Each sub-task's exception is caught and + // logged independently. Failure of one does not skip the others. + void Process(Tasks tasks); + + static constexpr bool HasTask(Tasks tasks, Tasks task) + { + return (static_cast(tasks) & static_cast(task)) != 0; + } + + // Returns the subset of `requested` tasks for which `view` does not already have + // up-to-date persisted metadata. Use this to decide what `Process` should run. + static Tasks NeededTasks(BinaryView* view, Tasks requested); + + private: + static bool HasUpToDateMetadata(BinaryView* view); + static bool HasUpToDateLiterals(BinaryView* view); }; + + constexpr ObjCProcessor::Tasks operator|(ObjCProcessor::Tasks a, ObjCProcessor::Tasks b) + { + return static_cast(static_cast(a) | static_cast(b)); + } + constexpr ObjCProcessor::Tasks& operator|=(ObjCProcessor::Tasks& a, ObjCProcessor::Tasks b) + { + return a = a | b; + } } diff --git a/plugins/workflow_objc/src/metadata/global_state.rs b/plugins/workflow_objc/src/metadata/global_state.rs index 1bad8425b..c04c523e2 100644 --- a/plugins/workflow_objc/src/metadata/global_state.rs +++ b/plugins/workflow_objc/src/metadata/global_state.rs @@ -157,9 +157,9 @@ impl AnalysisInfo { fn load_selector_impls(&self, bv: &BinaryView) -> Option { let meta = bv.get_metadata::>>("Objective-C")?; let version_meta = meta.get("version")?; - if version_meta.get_unsigned_integer()? != 1 { + if version_meta.get_unsigned_integer()? != 2 { tracing::error!( - "workflow_objc: Unexpected Objective-C metadata version. Expected 1, got {}.", + "workflow_objc: Unexpected Objective-C metadata version. Expected 2, got {}.", version_meta.get_unsigned_integer()? ); return None; diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index a63054d0d..a4971c589 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -3,6 +3,7 @@ #include "chained_fixups.h" #include "fatmachoview.h" #include "lowlevelilinstruction.h" +#include "objectivec/objc.h" #include "rapidjsonwrapper.h" #include "universaltransform.h" #include "universalview.h" @@ -32,6 +33,23 @@ constexpr std::string_view kPseudoLibraryMainExecutable = "
"; constexpr std::string_view kPseudoLibraryFlatLookup = ""; constexpr std::string_view kPseudoLibraryWeakLookup = ""; +ObjCProcessor::Tasks ObjCTasksFromLoadSettings(MachoView* view) +{ + Ref settings = view->GetLoadSettings(view->GetTypeName()); + auto settingEnabled = [&](const char* key) { + return !settings || !settings->Contains(key) || settings->Get(key, view); + }; + + ObjCProcessor::Tasks requested = ObjCProcessor::Tasks::None; + if (settingEnabled("loader.macho.processObjectiveC") && MachoObjCProcessor::ViewHasObjCMetadata(view)) + requested |= ObjCProcessor::Tasks::Metadata; + + if (settingEnabled("loader.macho.processCFStrings") && view->GetSectionByName("__cfstring")) + requested |= ObjCProcessor::Tasks::Literals; + + return ObjCProcessor::NeededTasks(view, requested); +} + string CommandToString(uint32_t lcCommand) { switch(lcCommand) @@ -2099,19 +2117,10 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_ analysisSettings->Set("analysis.workflows.functionWorkflow", "core.function.metaAnalysis", this); } - bool parseObjCStructs = true; - bool parseCFStrings = true; - if (settings && settings->Contains("loader.macho.processObjectiveC")) - parseObjCStructs = settings->Get("loader.macho.processObjectiveC", this); - if (settings && settings->Contains("loader.macho.processCFStrings")) - parseCFStrings = settings->Get("loader.macho.processCFStrings", this); - if (!MachoObjCProcessor::ViewHasObjCMetadata(this)) - parseObjCStructs = false; - if (!GetSectionByName("__cfstring")) - parseCFStrings = false; + ObjCProcessor::Tasks objcTasks = ObjCTasksFromLoadSettings(this); std::unique_ptr objcProcessor; - if (parseObjCStructs || parseCFStrings) + if (objcTasks != ObjCProcessor::Tasks::None) { objcProcessor = std::make_unique(this); } @@ -2651,32 +2660,20 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_ } } - if (parseCFStrings) - { - try { - objcProcessor->ProcessObjCLiterals(); - } - catch (std::exception& ex) - { - m_logger->LogError("Failed to process CFStrings. Binary may be malformed"); - m_logger->LogErrorF("Error: {:?}", ex.what()); - } - } + // Process Objective-C metadata when loading a new binary. When loading from a database, + // types and symbols have already been applied. Objective-C metadata may be re-processed + // in OnAfterSnapshotDataApplied if the database version is too old. + if (!m_backedByDatabase && objcProcessor) + objcProcessor->Process(objcTasks); - if (parseObjCStructs) - { - try { - objcProcessor->ProcessObjCData(); - } - catch (std::exception& ex) - { - m_logger->LogError("Failed to process Objective-C Metadata. Binary may be malformed"); - m_logger->LogErrorF("Error: {:?}", ex.what()); - } - } + return true; +} - return true; +void MachoView::OnAfterSnapshotDataApplied() +{ + if (ObjCProcessor::Tasks objcTasks = ObjCTasksFromLoadSettings(this); objcTasks != ObjCProcessor::Tasks::None) + MachoObjCProcessor(this).Process(objcTasks); } diff --git a/view/macho/machoview.h b/view/macho/machoview.h index 57b494d72..ee3b84d17 100644 --- a/view/macho/machoview.h +++ b/view/macho/machoview.h @@ -1533,6 +1533,7 @@ namespace BinaryNinja MachoView(const std::string& typeName, BinaryView* data, bool parseOnly = false); virtual bool Init() override; + void OnAfterSnapshotDataApplied() override; }; class MachoViewType: public BinaryViewType diff --git a/view/sharedcache/core/SharedCacheController.cpp b/view/sharedcache/core/SharedCacheController.cpp index e8bd52b7c..3b04517d9 100644 --- a/view/sharedcache/core/SharedCacheController.cpp +++ b/view/sharedcache/core/SharedCacheController.cpp @@ -220,20 +220,13 @@ bool SharedCacheController::ApplyImage(BinaryView& view, const CacheImage& image view.SetFunctionAnalysisUpdateDisabled(prevDisabledState); // Load objective-c information. - auto objcProcessor = DSCObjC::SharedCacheObjCProcessor(&view, image.headerAddress); - try - { - if (m_processObjC) - objcProcessor.ProcessObjCData(); - if (m_processCFStrings) - objcProcessor.ProcessObjCLiterals(); - } - catch (std::exception& e) - { - // Let the user know there was an error in processing the objc stuff but let the image load - // regardless, as its non-critical. - m_logger->LogErrorF("Failed to process ObjC information: {}", e.what()); - } + ObjCProcessor::Tasks tasks = ObjCProcessor::Tasks::None; + if (m_processObjC) + tasks |= ObjCProcessor::Tasks::Metadata; + if (m_processCFStrings) + tasks |= ObjCProcessor::Tasks::Literals; + if (tasks != ObjCProcessor::Tasks::None) + DSCObjC::SharedCacheObjCProcessor(&view, image.headerAddress).Process(tasks); } m_loadedImages.insert(image.headerAddress); @@ -294,9 +287,19 @@ void SharedCacheController::LoadMetadata(const Metadata& metadata) } -void SharedCacheController::ProcessObjCForLoadedImages(BinaryView& view) +void SharedCacheController::ProcessObjCForLoadedImagesIfNeeded(BinaryView& view) { - if (!m_processObjC || m_loadedImages.empty()) + if (m_loadedImages.empty()) + return; + + ObjCProcessor::Tasks requested = ObjCProcessor::Tasks::None; + if (m_processObjC) + requested |= ObjCProcessor::Tasks::Metadata; + if (m_processCFStrings) + requested |= ObjCProcessor::Tasks::Literals; + + ObjCProcessor::Tasks tasks = ObjCProcessor::NeededTasks(&view, requested); + if (tasks == ObjCProcessor::Tasks::None) return; for (const auto& headerAddress : m_loadedImages) @@ -305,15 +308,7 @@ void SharedCacheController::ProcessObjCForLoadedImages(BinaryView& view) if (!image) continue; - auto objcProcessor = DSCObjC::SharedCacheObjCProcessor(&view, image->headerAddress); - try - { - objcProcessor.ProcessObjCData(); - } - catch (std::exception& e) - { - m_logger->LogErrorForExceptionF(e, "Failed to restore ObjC metadata for image at {:#x}: {}", headerAddress, e.what()); - } + DSCObjC::SharedCacheObjCProcessor(&view, image->headerAddress).Process(tasks); } } diff --git a/view/sharedcache/core/SharedCacheController.h b/view/sharedcache/core/SharedCacheController.h index 07c5ce788..829a89314 100644 --- a/view/sharedcache/core/SharedCacheController.h +++ b/view/sharedcache/core/SharedCacheController.h @@ -66,8 +66,9 @@ namespace BinaryNinja::DSC { void LoadMetadata(const Metadata& metadata); - // Re-run the ObjC processor for loaded images to restore Objective-C metadata. - void ProcessObjCForLoadedImages(BinaryView& view); + // Run Obj-C processing for previously-loaded images iff their persisted metadata is + // missing or out-of-date (older databases, or databases from before a processor change). + void ProcessObjCForLoadedImagesIfNeeded(BinaryView& view); std::unique_ptr CreateStringScanner(); }; diff --git a/view/sharedcache/core/SharedCacheView.cpp b/view/sharedcache/core/SharedCacheView.cpp index f3d60e2ea..6827ac496 100644 --- a/view/sharedcache/core/SharedCacheView.cpp +++ b/view/sharedcache/core/SharedCacheView.cpp @@ -1026,7 +1026,7 @@ bool SharedCacheView::InitController() void SharedCacheView::OnAfterSnapshotDataApplied() { if (auto controller = SharedCacheController::FromView(*this)) - controller->ProcessObjCForLoadedImages(*this); + controller->ProcessObjCForLoadedImagesIfNeeded(*this); }