Skip to content

Commit 0f0801a

Browse files
WeiN76LQh0cyn
WeiN76LQh
authored and
0cyn
committed
[SharedCache] Make m_exportInfos map's values a shared_ptr
This avoids expensive copying when returning a value from the map in `SharedCache::GetExportListForHeader`. Additionally it ensures that the value stays alive and at the same location in memory if `m_exportInfos` is modified and requires its storage to be re-allocated. I was unable to use a `unique_ptr` instead of a `shared_ptr` because of copy semantics with `m_exportInfos` in `ViewStateCacheStore`. I don't see things being any worse using `shared_ptr` instead of `unique_ptr` anyway and it means less code changes.
1 parent 876a5e2 commit 0f0801a

File tree

2 files changed

+75
-65
lines changed

2 files changed

+75
-65
lines changed

‎view/sharedcache/core/SharedCache.cpp‎

Lines changed: 72 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ int count_trailing_zeros(uint64_t value) {
5757

5858
struct SharedCache::State
5959
{
60-
std::unordered_map<uint64_t, std::unordered_map<uint64_t, Ref<Symbol>>>
60+
std::unordered_map<uint64_t, std::shared_ptr<std::unordered_map<uint64_t, Ref<Symbol>>>>
6161
exportInfos;
6262
std::unordered_map<uint64_t, std::vector<std::pair<uint64_t, std::pair<BNSymbolType, std::string>>>>
6363
symbolInfos;
@@ -2668,44 +2668,47 @@ void SharedCache::InitializeHeader(
26682668
auto symbols = GetExportListForHeader(header, [&]() {
26692669
return vm->MappingAtAddress(header.linkeditSegment.vmaddr).first.fileAccessor->lock();
26702670
});
2671-
for (const auto& symPair : symbols)
2671+
if (symbols)
26722672
{
2673-
if (typeLib)
2673+
for (const auto& [symbolAddress, symbol] : *symbols)
26742674
{
2675-
auto type = m_dscView->ImportTypeLibraryObject(typeLib, symPair.second->GetRawName());
2676-
2677-
if (type)
2675+
if (typeLib)
26782676
{
2679-
view->DefineAutoSymbolAndVariableOrFunction(view->GetDefaultPlatform(), symPair.second, type);
2680-
}
2681-
else
2682-
view->DefineAutoSymbol(symPair.second);
2677+
auto type = m_dscView->ImportTypeLibraryObject(typeLib, symbol->GetRawName());
26832678

2684-
if (view->GetAnalysisFunction(view->GetDefaultPlatform(), symPair.first))
2685-
{
2686-
auto func = view->GetAnalysisFunction(view->GetDefaultPlatform(), symPair.first);
2687-
auto name = symPair.second->GetFullName();
2688-
if (name == "_objc_msgSend")
2679+
if (type)
26892680
{
2690-
func->SetHasVariableArguments(false);
2681+
view->DefineAutoSymbolAndVariableOrFunction(view->GetDefaultPlatform(), symbol, type);
26912682
}
2692-
else if (name.find("_objc_retain_x") != std::string::npos || name.find("_objc_release_x") != std::string::npos)
2683+
else
2684+
view->DefineAutoSymbol(symbol);
2685+
2686+
if (view->GetAnalysisFunction(view->GetDefaultPlatform(), symbolAddress))
26932687
{
2694-
auto x = name.rfind("x");
2695-
auto num = name.substr(x + 1);
2688+
auto func = view->GetAnalysisFunction(view->GetDefaultPlatform(), symbolAddress);
2689+
auto name = symbol->GetFullName();
2690+
if (name == "_objc_msgSend")
2691+
{
2692+
func->SetHasVariableArguments(false);
2693+
}
2694+
else if (name.find("_objc_retain_x") != std::string::npos || name.find("_objc_release_x") != std::string::npos)
2695+
{
2696+
auto x = name.rfind("x");
2697+
auto num = name.substr(x + 1);
26962698

2697-
std::vector<BinaryNinja::FunctionParameter> callTypeParams;
2698-
auto cc = m_dscView->GetDefaultArchitecture()->GetCallingConventionByName("apple-arm64-objc-fast-arc-" + num);
2699+
std::vector<BinaryNinja::FunctionParameter> callTypeParams;
2700+
auto cc = m_dscView->GetDefaultArchitecture()->GetCallingConventionByName("apple-arm64-objc-fast-arc-" + num);
26992701

2700-
callTypeParams.push_back({"obj", m_dscView->GetTypeByName({ "id" }), true, BinaryNinja::Variable()});
2702+
callTypeParams.push_back({"obj", m_dscView->GetTypeByName({ "id" }), true, BinaryNinja::Variable()});
27012703

2702-
auto funcType = BinaryNinja::Type::FunctionType(m_dscView->GetTypeByName({ "id" }), cc, callTypeParams);
2703-
func->SetUserType(funcType);
2704+
auto funcType = BinaryNinja::Type::FunctionType(m_dscView->GetTypeByName({ "id" }), cc, callTypeParams);
2705+
func->SetUserType(funcType);
2706+
}
27042707
}
27052708
}
2709+
else
2710+
view->DefineAutoSymbol(symbol);
27062711
}
2707-
else
2708-
view->DefineAutoSymbol(symPair.second);
27092712
}
27102713
}
27112714
view->EndBulkModifySymbols();
@@ -2808,7 +2811,7 @@ std::vector<Ref<Symbol>> SharedCache::ParseExportTrie(std::shared_ptr<MMappedFil
28082811
}
28092812

28102813

2811-
std::unordered_map<uint64_t, Ref<Symbol>> SharedCache::GetExportListForHeader(SharedCacheMachOHeader header, std::function<std::shared_ptr<MMappedFileAccessor>()> provideLinkeditFile, bool* didModifyExportList)
2814+
std::shared_ptr<std::unordered_map<uint64_t, Ref<Symbol>>> SharedCache::GetExportListForHeader(SharedCacheMachOHeader header, std::function<std::shared_ptr<MMappedFileAccessor>()> provideLinkeditFile, bool* didModifyExportList)
28122815
{
28132816
if (auto it = m_state->exportInfos.find(header.textBase); it != m_state->exportInfos.end())
28142817
{
@@ -2824,19 +2827,19 @@ std::unordered_map<uint64_t, Ref<Symbol>> SharedCache::GetExportListForHeader(Sh
28242827
{
28252828
if (didModifyExportList)
28262829
*didModifyExportList = false;
2827-
return std::unordered_map<uint64_t, Ref<Symbol>>();
2830+
return nullptr;
28282831
}
28292832

28302833
auto exportList = SharedCache::ParseExportTrie(linkeditFile, header);
2831-
std::unordered_map<uint64_t, Ref<Symbol>> exportMapping(exportList.size());
2834+
auto exportMapping = std::make_shared<std::unordered_map<uint64_t, Ref<Symbol>>>(exportList.size());
28322835
for (const auto& sym : exportList)
28332836
{
2834-
exportMapping[sym->GetAddress()] = sym;
2837+
exportMapping->insert_or_assign(sym->GetAddress(), sym);
28352838
}
2836-
m_state->exportInfos[header.textBase] = exportMapping;
2839+
MutableState().exportInfos.emplace(header.textBase, exportMapping);
28372840
if (didModifyExportList)
28382841
*didModifyExportList = true;
2839-
return exportMapping;
2842+
return m_state->exportInfos[header.textBase];
28402843
}
28412844
}
28422845

@@ -2874,9 +2877,11 @@ std::vector<std::pair<std::string, Ref<Symbol>>> SharedCache::LoadAllSymbolsAndW
28742877
return std::shared_ptr<MMappedFileAccessor>(nullptr);
28752878
}
28762879
}, &doSave);
2877-
for (const auto& symPair : exportList)
2880+
if (!exportList)
2881+
continue;
2882+
for (const auto& [_, symbol] : *exportList)
28782883
{
2879-
symbols.push_back({img.installName, symPair.second});
2884+
symbols.push_back({img.installName, symbol});
28802885
}
28812886
}
28822887

@@ -2974,39 +2979,42 @@ void SharedCache::FindSymbolAtAddrAndApplyToAddr(
29742979
}
29752980
});
29762981

2977-
if (auto it = exportList.find(symbolLocation); it != exportList.end())
2982+
if (exportList)
29782983
{
2979-
auto typeLib = TypeLibraryForImage(header->installName);
2980-
id = m_dscView->BeginUndoActions();
2981-
m_dscView->BeginBulkModifySymbols();
2982-
2983-
auto func = m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation);
2984-
if (func)
2985-
{
2986-
m_dscView->DefineUserSymbol(
2987-
new Symbol(FunctionSymbol, prefix + it->second->GetFullName(), targetLocation));
2988-
2989-
if (typeLib)
2990-
if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
2991-
func->SetUserType(type);
2992-
}
2993-
else
2984+
if (auto it = exportList->find(symbolLocation); it != exportList->end())
29942985
{
2995-
m_dscView->DefineUserSymbol(
2996-
new Symbol(it->second->GetType(), prefix + it->second->GetFullName(), targetLocation));
2986+
auto typeLib = TypeLibraryForImage(header->installName);
2987+
id = m_dscView->BeginUndoActions();
2988+
m_dscView->BeginBulkModifySymbols();
29972989

2998-
if (typeLib)
2999-
if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
3000-
m_dscView->DefineUserDataVariable(targetLocation, type);
3001-
}
3002-
if (triggerReanalysis)
3003-
{
2990+
auto func = m_dscView->GetAnalysisFunction(m_dscView->GetDefaultPlatform(), targetLocation);
30042991
if (func)
3005-
func->Reanalyze();
3006-
}
2992+
{
2993+
m_dscView->DefineUserSymbol(
2994+
new Symbol(FunctionSymbol, prefix + it->second->GetFullName(), targetLocation));
2995+
2996+
if (typeLib)
2997+
if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
2998+
func->SetUserType(type);
2999+
}
3000+
else
3001+
{
3002+
m_dscView->DefineUserSymbol(
3003+
new Symbol(it->second->GetType(), prefix + it->second->GetFullName(), targetLocation));
30073004

3008-
m_dscView->EndBulkModifySymbols();
3009-
m_dscView->ForgetUndoActions(id);
3005+
if (typeLib)
3006+
if (auto type = m_dscView->ImportTypeLibraryObject(typeLib, {it->second->GetFullName()}))
3007+
m_dscView->DefineUserDataVariable(targetLocation, type);
3008+
}
3009+
if (triggerReanalysis)
3010+
{
3011+
if (func)
3012+
func->Reanalyze();
3013+
}
3014+
3015+
m_dscView->EndBulkModifySymbols();
3016+
m_dscView->ForgetUndoActions(id);
3017+
}
30103018
}
30113019
}
30123020
}
@@ -3467,7 +3475,7 @@ void SharedCache::Store(SerializationContext& context) const
34673475
Serialize(context, "key", pair1.first);
34683476
Serialize(context, "value");
34693477
context.writer.StartArray();
3470-
for (const auto& pair2 : pair1.second)
3478+
for (const auto& pair2 : *pair1.second)
34713479
{
34723480
context.writer.StartObject();
34733481
Serialize(context, "key", pair2.first);
@@ -3552,7 +3560,7 @@ void SharedCache::Load(DeserializationContext& context)
35523560
raw.c_str(), raw.c_str(), addr, NoBinding, nullptr, 0));
35533561
}
35543562

3555-
MutableState().exportInfos[obj1["key"].GetUint64()] = std::move(innerVec);
3563+
MutableState().exportInfos[obj1["key"].GetUint64()] = std::make_shared<std::unordered_map<uint64_t, Ref<Symbol>>>(innerVec);
35563564
}
35573565

35583566
for (auto& symbolInfo : context.doc["symbolInfos"].GetArray())

‎view/sharedcache/core/SharedCache.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,9 @@ namespace SharedCacheCore {
642642
const uint8_t *end, const uint8_t* current, uint64_t textBase, const std::string& currentText);
643643
std::vector<Ref<Symbol>> ParseExportTrie(
644644
std::shared_ptr<MMappedFileAccessor> linkeditFile, const SharedCacheMachOHeader& header);
645-
std::unordered_map<uint64_t, Ref<Symbol>> GetExportListForHeader(SharedCacheMachOHeader header, std::function<std::shared_ptr<MMappedFileAccessor>()> provideLinkeditFile, bool* didModifyExportList = nullptr);
645+
std::shared_ptr<std::unordered_map<uint64_t, Ref<Symbol>>> GetExportListForHeader(SharedCacheMachOHeader header,
646+
std::function<std::shared_ptr<MMappedFileAccessor>()> provideLinkeditFile, bool* didModifyExportList = nullptr);
647+
646648

647649
Ref<TypeLibrary> TypeLibraryForImage(const std::string& installName);
648650

0 commit comments

Comments
 (0)