diff --git a/Source/Core/Common/SymbolDB.cpp b/Source/Core/Common/SymbolDB.cpp index 88f281e497..2fa514ba86 100644 --- a/Source/Core/Common/SymbolDB.cpp +++ b/Source/Core/Common/SymbolDB.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -34,6 +35,7 @@ void Symbol::Rename(const std::string& symbol_name) void SymbolDB::List() { + std::lock_guard lock(m_mutex); for (const auto& func : m_functions) { DEBUG_LOG_FMT(OSHLE, "{} @ {:08x}: {} bytes (hash {:08x}) : {} calls", func.second.name, @@ -44,11 +46,14 @@ void SymbolDB::List() bool SymbolDB::IsEmpty() const { + std::lock_guard lock(m_mutex); return m_functions.empty() && m_notes.empty(); } bool SymbolDB::Clear(const char* prefix) { + std::lock_guard lock(m_mutex); + // TODO: honor prefix m_map_name.clear(); if (IsEmpty()) @@ -61,16 +66,24 @@ bool SymbolDB::Clear(const char* prefix) } void SymbolDB::Index() +{ + std::lock_guard lock(m_mutex); + Index(&m_functions); +} + +void SymbolDB::Index(XFuncMap* functions) { int i = 0; - for (auto& func : m_functions) + for (auto& func : *functions) { func.second.index = i++; } } -Symbol* SymbolDB::GetSymbolFromName(std::string_view name) +const Symbol* SymbolDB::GetSymbolFromName(std::string_view name) const { + std::lock_guard lock(m_mutex); + for (auto& func : m_functions) { if (func.second.function_name == name) @@ -80,9 +93,10 @@ Symbol* SymbolDB::GetSymbolFromName(std::string_view name) return nullptr; } -std::vector SymbolDB::GetSymbolsFromName(std::string_view name) +std::vector SymbolDB::GetSymbolsFromName(std::string_view name) const { - std::vector symbols; + std::lock_guard lock(m_mutex); + std::vector symbols; for (auto& func : m_functions) { @@ -93,8 +107,10 @@ std::vector SymbolDB::GetSymbolsFromName(std::string_view name) return symbols; } -Symbol* SymbolDB::GetSymbolFromHash(u32 hash) +const Symbol* SymbolDB::GetSymbolFromHash(u32 hash) const { + std::lock_guard lock(m_mutex); + auto iter = m_checksum_to_function.find(hash); if (iter == m_checksum_to_function.end()) return nullptr; @@ -102,8 +118,10 @@ Symbol* SymbolDB::GetSymbolFromHash(u32 hash) return *iter->second.begin(); } -std::vector SymbolDB::GetSymbolsFromHash(u32 hash) +std::vector SymbolDB::GetSymbolsFromHash(u32 hash) const { + std::lock_guard lock(m_mutex); + const auto iter = m_checksum_to_function.find(hash); if (iter == m_checksum_to_function.cend()) @@ -114,6 +132,34 @@ std::vector SymbolDB::GetSymbolsFromHash(u32 hash) void SymbolDB::AddCompleteSymbol(const Symbol& symbol) { - m_functions.emplace(symbol.address, symbol); + std::lock_guard lock(m_mutex); + m_functions[symbol.address] = symbol; } + +bool SymbolDB::RenameSymbol(const Symbol& symbol, const std::string& symbol_name) +{ + std::lock_guard lock(m_mutex); + + auto it = m_functions.find(symbol.address); + if (it == m_functions.end()) + return false; + + it->second.Rename(symbol_name); + return true; +} + +bool SymbolDB::RenameSymbol(const Symbol& symbol, const std::string& symbol_name, + const std::string& object_name) +{ + std::lock_guard lock(m_mutex); + + auto it = m_functions.find(symbol.address); + if (it == m_functions.end()) + return false; + + it->second.Rename(symbol_name); + it->second.object_name = object_name; + return true; +} + } // namespace Common diff --git a/Source/Core/Common/SymbolDB.h b/Source/Core/Common/SymbolDB.h index 7763999453..8a1767d696 100644 --- a/Source/Core/Common/SymbolDB.h +++ b/Source/Core/Common/SymbolDB.h @@ -7,12 +7,14 @@ #pragma once #include +#include #include #include #include #include #include +#include "Common/Assert.h" #include "Common/CommonTypes.h" namespace Core @@ -87,27 +89,71 @@ public: SymbolDB(); virtual ~SymbolDB(); - virtual Symbol* GetSymbolFromAddr(u32 addr) { return nullptr; } - virtual Symbol* AddFunction(const Core::CPUThreadGuard& guard, u32 start_addr) { return nullptr; } + virtual const Symbol* GetSymbolFromAddr(u32 addr) const { return nullptr; } + virtual const Symbol* AddFunction(const Core::CPUThreadGuard& guard, u32 start_addr) + { + return nullptr; + } void AddCompleteSymbol(const Symbol& symbol); + bool RenameSymbol(const Symbol& symbol, const std::string& symbol_name); + bool RenameSymbol(const Symbol& symbol, const std::string& symbol_name, + const std::string& object_name); - Symbol* GetSymbolFromName(std::string_view name); - std::vector GetSymbolsFromName(std::string_view name); - Symbol* GetSymbolFromHash(u32 hash); - std::vector GetSymbolsFromHash(u32 hash); + const Symbol* GetSymbolFromName(std::string_view name) const; + std::vector GetSymbolsFromName(std::string_view name) const; + const Symbol* GetSymbolFromHash(u32 hash) const; + std::vector GetSymbolsFromHash(u32 hash) const; + + template + void ForEachSymbol(F f) const + { + std::lock_guard lock(m_mutex); + for (const auto& [addr, symbol] : m_functions) + f(symbol); + } + + template + void ForEachSymbolWithMutation(F f) + { + std::lock_guard lock(m_mutex); + for (auto& [addr, symbol] : m_functions) + { + f(symbol); + ASSERT_MSG(COMMON, addr == symbol.address, "Symbol address was unexpectedly changed"); + } + } + + template + void ForEachNote(F f) const + { + std::lock_guard lock(m_mutex); + for (const auto& [addr, note] : m_notes) + f(note); + } + + template + void ForEachNoteWithMutation(F f) + { + std::lock_guard lock(m_mutex); + for (auto& [addr, note] : m_notes) + { + f(note); + ASSERT_MSG(COMMON, addr == note.address, "Note address was unexpectedly changed"); + } + } - const XFuncMap& Symbols() const { return m_functions; } - const XNoteMap& Notes() const { return m_notes; } - XFuncMap& AccessSymbols() { return m_functions; } bool IsEmpty() const; bool Clear(const char* prefix = ""); void List(); void Index(); protected: + static void Index(XFuncMap* functions); + XFuncMap m_functions; XNoteMap m_notes; XFuncPtrMap m_checksum_to_function; std::string m_map_name; + mutable std::recursive_mutex m_mutex; }; } // namespace Common diff --git a/Source/Core/Core/Debugger/DebugInterface.h b/Source/Core/Core/Debugger/DebugInterface.h index cc07d06d3b..607cbe8bba 100644 --- a/Source/Core/Core/Debugger/DebugInterface.h +++ b/Source/Core/Core/Debugger/DebugInterface.h @@ -6,7 +6,6 @@ #include #include #include -#include #include #include "Common/CommonTypes.h" @@ -104,7 +103,7 @@ public: { return 0xFFFFFFFF; } - virtual std::string_view GetDescription(u32 /*address*/) const = 0; + virtual std::string GetDescription(u32 /*address*/) const = 0; virtual void Clear(const CPUThreadGuard& guard) = 0; }; } // namespace Core diff --git a/Source/Core/Core/Debugger/Debugger_SymbolMap.cpp b/Source/Core/Core/Debugger/Debugger_SymbolMap.cpp index d0abe78be0..7ee5868fce 100644 --- a/Source/Core/Core/Debugger/Debugger_SymbolMap.cpp +++ b/Source/Core/Core/Debugger/Debugger_SymbolMap.cpp @@ -80,7 +80,7 @@ bool GetCallstack(const Core::CPUThreadGuard& guard, std::vector }); WalkTheStack(guard, [&output, &ppc_symbol_db](u32 func_addr) { - std::string_view func_desc = ppc_symbol_db.GetDescription(func_addr); + std::string func_desc = ppc_symbol_db.GetDescription(func_addr); if (func_desc.empty() || func_desc == "Invalid") func_desc = "(unknown)"; @@ -107,14 +107,14 @@ void PrintCallstack(const Core::CPUThreadGuard& guard, Common::Log::LogType type GENERIC_LOG_FMT(type, level, " LR = 0 - this is bad"); } - if (const std::string_view lr_desc = ppc_symbol_db.GetDescription(LR(ppc_state)); + if (const std::string lr_desc = ppc_symbol_db.GetDescription(LR(ppc_state)); lr_desc != ppc_symbol_db.GetDescription(ppc_state.pc)) { GENERIC_LOG_FMT(type, level, " * {} [ LR = {:08x} ]", lr_desc, LR(ppc_state)); } WalkTheStack(guard, [type, level, &ppc_symbol_db](u32 func_addr) { - std::string_view func_desc = ppc_symbol_db.GetDescription(func_addr); + std::string func_desc = ppc_symbol_db.GetDescription(func_addr); if (func_desc.empty() || func_desc == "Invalid") func_desc = "(unknown)"; GENERIC_LOG_FMT(type, level, " * {} [ addr = {:08x} ]", func_desc, func_addr); diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.cpp b/Source/Core/Core/Debugger/PPCDebugInterface.cpp index 2c840d0ae5..dea3aa25a9 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.cpp +++ b/Source/Core/Core/Debugger/PPCDebugInterface.cpp @@ -439,7 +439,7 @@ u32 PPCDebugInterface::GetColor(const Core::CPUThreadGuard* guard, u32 address) } // ============= -std::string_view PPCDebugInterface::GetDescription(u32 address) const +std::string PPCDebugInterface::GetDescription(u32 address) const { return m_ppc_symbol_db.GetDescription(address); } diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.h b/Source/Core/Core/Debugger/PPCDebugInterface.h index 797f2d5254..456b987ab9 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.h +++ b/Source/Core/Core/Debugger/PPCDebugInterface.h @@ -102,7 +102,7 @@ public: void Step() override {} void RunTo(u32 address) override; u32 GetColor(const Core::CPUThreadGuard* guard, u32 address) const override; - std::string_view GetDescription(u32 address) const override; + std::string GetDescription(u32 address) const override; std::shared_ptr NetworkLogger(); diff --git a/Source/Core/Core/Debugger/RSO.cpp b/Source/Core/Core/Debugger/RSO.cpp index f793096967..7ee9481df3 100644 --- a/Source/Core/Core/Debugger/RSO.cpp +++ b/Source/Core/Core/Debugger/RSO.cpp @@ -381,7 +381,7 @@ void RSOView::Apply(const Core::CPUThreadGuard& guard, PPCSymbolDB* symbol_db) c u32 address = GetExportAddress(rso_export); if (address != 0) { - Common::Symbol* symbol = symbol_db->AddFunction(guard, address); + const Common::Symbol* symbol = symbol_db->AddFunction(guard, address); if (!symbol) symbol = symbol_db->GetSymbolFromAddr(address); @@ -389,8 +389,7 @@ void RSOView::Apply(const Core::CPUThreadGuard& guard, PPCSymbolDB* symbol_db) c if (symbol) { // Function symbol - symbol->Rename(export_name); - symbol->object_name = rso_name; + symbol_db->RenameSymbol(*symbol, export_name, rso_name); } else { diff --git a/Source/Core/Core/PowerPC/JitCommon/JitCache.cpp b/Source/Core/Core/PowerPC/JitCommon/JitCache.cpp index 06cbcbb9c4..c9050be730 100644 --- a/Source/Core/Core/PowerPC/JitCommon/JitCache.cpp +++ b/Source/Core/Core/PowerPC/JitCommon/JitCache.cpp @@ -187,7 +187,7 @@ void JitBaseBlockCache::FinalizeBlock(JitBlock& block, bool block_link, LinkBlock(block); } - Common::Symbol* symbol = nullptr; + const Common::Symbol* symbol = nullptr; if (Common::JitRegister::IsEnabled() && (symbol = m_jit.m_ppc_symbol_db.GetSymbolFromAddr(block.effectiveAddress)) != nullptr) { diff --git a/Source/Core/Core/PowerPC/PPCAnalyst.cpp b/Source/Core/Core/PowerPC/PPCAnalyst.cpp index c8ab26f884..49338af10b 100644 --- a/Source/Core/Core/PowerPC/PPCAnalyst.cpp +++ b/Source/Core/Core/PowerPC/PPCAnalyst.cpp @@ -346,10 +346,10 @@ static void FindFunctionsFromHandlers(const Core::CPUThreadGuard& guard, PPCSymb if (read_result.valid && PPCTables::IsValidInstruction(read_result.hex, entry.first)) { // Check if this function is already mapped - Common::Symbol* f = func_db->AddFunction(guard, entry.first); + const Common::Symbol* f = func_db->AddFunction(guard, entry.first); if (!f) continue; - f->Rename(entry.second); + func_db->RenameSymbol(*f, entry.second); } } } @@ -359,8 +359,8 @@ static void FindFunctionsAfterReturnInstruction(const Core::CPUThreadGuard& guar { std::vector funcAddrs; - for (const auto& func : func_db->Symbols()) - funcAddrs.push_back(func.second.address + func.second.size); + func_db->ForEachSymbol( + [&](const Common::Symbol& symbol) { funcAddrs.push_back(symbol.address + symbol.size); }); auto& mmu = guard.GetSystem().GetMMU(); for (u32& location : funcAddrs) @@ -380,7 +380,7 @@ static void FindFunctionsAfterReturnInstruction(const Core::CPUThreadGuard& guar if (read_result.valid && PPCTables::IsValidInstruction(read_result.hex, location)) { // check if this function is already mapped - Common::Symbol* f = func_db->AddFunction(guard, location); + const Common::Symbol* f = func_db->AddFunction(guard, location); if (!f) break; else @@ -407,45 +407,45 @@ void FindFunctions(const Core::CPUThreadGuard& guard, u32 startAddr, u32 endAddr int numLeafs = 0, numNice = 0, numUnNice = 0; int numTimer = 0, numRFI = 0, numStraightLeaf = 0; int leafSize = 0, niceSize = 0, unniceSize = 0; - for (auto& func : func_db->AccessSymbols()) - { - if (func.second.address == 4) + func_db->ForEachSymbolWithMutation([&](Common::Symbol& f) { + if (f.address == 4) { WARN_LOG_FMT(SYMBOLS, "Weird function"); - continue; - } - AnalyzeFunction2(func_db, &(func.second)); - Common::Symbol& f = func.second; - if (f.name.substr(0, 3) == "zzz") - { - if (f.flags & Common::FFLAG_LEAF) - f.Rename(f.name + "_leaf"); - if (f.flags & Common::FFLAG_STRAIGHT) - f.Rename(f.name + "_straight"); - } - if (f.flags & Common::FFLAG_LEAF) - { - numLeafs++; - leafSize += f.size; - } - else if (f.flags & Common::FFLAG_ONLYCALLSNICELEAFS) - { - numNice++; - niceSize += f.size; } else { - numUnNice++; - unniceSize += f.size; - } + AnalyzeFunction2(func_db, &f); + if (f.name.substr(0, 3) == "zzz") + { + if (f.flags & Common::FFLAG_LEAF) + f.Rename(f.name + "_leaf"); + if (f.flags & Common::FFLAG_STRAIGHT) + f.Rename(f.name + "_straight"); + } + if (f.flags & Common::FFLAG_LEAF) + { + numLeafs++; + leafSize += f.size; + } + else if (f.flags & Common::FFLAG_ONLYCALLSNICELEAFS) + { + numNice++; + niceSize += f.size; + } + else + { + numUnNice++; + unniceSize += f.size; + } - if (f.flags & Common::FFLAG_TIMERINSTRUCTIONS) - numTimer++; - if (f.flags & Common::FFLAG_RFI) - numRFI++; - if ((f.flags & Common::FFLAG_STRAIGHT) && (f.flags & Common::FFLAG_LEAF)) - numStraightLeaf++; - } + if (f.flags & Common::FFLAG_TIMERINSTRUCTIONS) + numTimer++; + if (f.flags & Common::FFLAG_RFI) + numRFI++; + if ((f.flags & Common::FFLAG_STRAIGHT) && (f.flags & Common::FFLAG_LEAF)) + numStraightLeaf++; + } + }); if (numLeafs == 0) leafSize = 0; else diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index e6077f6174..02da35f94b 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -35,8 +35,10 @@ PPCSymbolDB::PPCSymbolDB() = default; PPCSymbolDB::~PPCSymbolDB() = default; // Adds the function to the list, unless it's already there -Common::Symbol* PPCSymbolDB::AddFunction(const Core::CPUThreadGuard& guard, u32 start_addr) +const Common::Symbol* PPCSymbolDB::AddFunction(const Core::CPUThreadGuard& guard, u32 start_addr) { + std::lock_guard lock(m_mutex); + // It's already in the list if (m_functions.contains(start_addr)) return nullptr; @@ -56,8 +58,18 @@ void PPCSymbolDB::AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAdd const std::string& name, const std::string& object_name, Common::Symbol::Type type) { - auto iter = m_functions.find(startAddr); - if (iter != m_functions.end()) + std::lock_guard lock(m_mutex); + AddKnownSymbol(guard, startAddr, size, name, object_name, type, &m_functions, + &m_checksum_to_function); +} + +void PPCSymbolDB::AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAddr, u32 size, + const std::string& name, const std::string& object_name, + Common::Symbol::Type type, XFuncMap* functions, + XFuncPtrMap* checksum_to_function) +{ + auto iter = functions->find(startAddr); + if (iter != functions->end()) { // already got it, let's just update name, checksum & size to be sure. Common::Symbol* tempfunc = &iter->second; @@ -70,7 +82,7 @@ void PPCSymbolDB::AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAdd else { // new symbol. run analyze. - auto& new_symbol = m_functions.emplace(startAddr, name).first->second; + auto& new_symbol = functions->emplace(startAddr, name).first->second; new_symbol.object_name = object_name; new_symbol.type = type; new_symbol.address = startAddr; @@ -85,7 +97,7 @@ void PPCSymbolDB::AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAdd name, size, new_symbol.size); new_symbol.size = size; } - m_checksum_to_function[new_symbol.hash].insert(&new_symbol); + (*checksum_to_function)[new_symbol.hash].insert(&new_symbol); } else { @@ -96,9 +108,15 @@ void PPCSymbolDB::AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAdd void PPCSymbolDB::AddKnownNote(u32 start_addr, u32 size, const std::string& name) { - auto iter = m_notes.find(start_addr); + std::lock_guard lock(m_mutex); + AddKnownNote(start_addr, size, name, &m_notes); +} - if (iter != m_notes.end()) +void PPCSymbolDB::AddKnownNote(u32 start_addr, u32 size, const std::string& name, XNoteMap* notes) +{ + auto iter = notes->find(start_addr); + + if (iter != notes->end()) { // Already got it, just update the name and size. Common::Note* tempfunc = &iter->second; @@ -112,35 +130,38 @@ void PPCSymbolDB::AddKnownNote(u32 start_addr, u32 size, const std::string& name tf.address = start_addr; tf.size = size; - m_notes[start_addr] = tf; + (*notes)[start_addr] = tf; } } void PPCSymbolDB::DetermineNoteLayers() { - if (m_notes.empty()) + std::lock_guard lock(m_mutex); + DetermineNoteLayers(&m_notes); +} + +void PPCSymbolDB::DetermineNoteLayers(XNoteMap* notes) +{ + if (notes->empty()) return; - for (auto& note : m_notes) + for (auto& note : *notes) note.second.layer = 0; - for (auto iter = m_notes.begin(); iter != m_notes.end(); ++iter) + for (auto iter = notes->begin(); iter != notes->end(); ++iter) { const u32 range = iter->second.address + iter->second.size; - auto search = m_notes.lower_bound(range); + auto search = notes->lower_bound(range); while (--search != iter) search->second.layer += 1; } } -Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) +const Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) const { - // If m_functions is changing, there should be a PPCSymbolsChanged signal afterward. The signal - // will re-update persistent symbol displays by calling this function. Only one-off calls to this - // function, such as printing the symbol to console, should be affected by leaving early. - std::unique_lock lock(m_write_lock, std::try_to_lock); - if (!lock.owns_lock() || m_functions.empty()) + std::lock_guard lock(m_mutex); + if (m_functions.empty()) return nullptr; auto it = m_functions.lower_bound(addr); @@ -162,10 +183,10 @@ Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) return nullptr; } -Common::Note* PPCSymbolDB::GetNoteFromAddr(u32 addr) +const Common::Note* PPCSymbolDB::GetNoteFromAddr(u32 addr) const { - std::unique_lock lock(m_write_lock, std::try_to_lock); - if (!lock.owns_lock() || m_notes.empty()) + std::lock_guard lock(m_mutex); + if (m_notes.empty()) return nullptr; auto itn = m_notes.lower_bound(addr); @@ -195,15 +216,17 @@ Common::Note* PPCSymbolDB::GetNoteFromAddr(u32 addr) void PPCSymbolDB::DeleteFunction(u32 start_address) { + std::lock_guard lock(m_mutex); m_functions.erase(start_address); } void PPCSymbolDB::DeleteNote(u32 start_address) { + std::lock_guard lock(m_mutex); m_notes.erase(start_address); } -std::string_view PPCSymbolDB::GetDescription(u32 addr) +std::string PPCSymbolDB::GetDescription(u32 addr) const { if (const Common::Symbol* const symbol = GetSymbolFromAddr(addr)) return symbol->name; @@ -212,9 +235,7 @@ std::string_view PPCSymbolDB::GetDescription(u32 addr) void PPCSymbolDB::FillInCallers() { - std::unique_lock lock(m_write_lock, std::try_to_lock); - if (!lock.owns_lock()) - return; + std::lock_guard lock(m_mutex); for (auto& p : m_functions) { @@ -247,6 +268,8 @@ void PPCSymbolDB::FillInCallers() void PPCSymbolDB::PrintCalls(u32 funcAddr) const { + std::lock_guard lock(m_mutex); + const auto iter = m_functions.find(funcAddr); if (iter == m_functions.end()) { @@ -268,6 +291,8 @@ void PPCSymbolDB::PrintCalls(u32 funcAddr) const void PPCSymbolDB::PrintCallers(u32 funcAddr) const { + std::lock_guard lock(m_mutex); + const auto iter = m_functions.find(funcAddr); if (iter == m_functions.end()) return; @@ -286,6 +311,8 @@ void PPCSymbolDB::PrintCallers(u32 funcAddr) const void PPCSymbolDB::LogFunctionCall(u32 addr) { + std::lock_guard lock(m_mutex); + auto iter = m_functions.find(addr); if (iter == m_functions.end()) return; @@ -314,25 +341,22 @@ bool PPCSymbolDB::FindMapFile(std::string* existing_map_file, std::string* writa return false; } +// Returns true if m_functions was changed. bool PPCSymbolDB::LoadMapOnBoot(const Core::CPUThreadGuard& guard) { - // Loads from emuthread and can crash with main thread accessing the map. Any other loads will be - // done on the main thread and should be safe. Returns true if m_functions was changed. - std::lock_guard lock(m_write_lock); - std::string existing_map_file; if (!PPCSymbolDB::FindMapFile(&existing_map_file, nullptr)) return Clear(); - // If the map is already loaded (such as restarting the same game), skip reloading. - if (!IsEmpty() && existing_map_file == m_map_name) - return false; + { + std::lock_guard lock(m_mutex); + // If the map is already loaded (such as restarting the same game), skip reloading. + if (!IsEmpty() && existing_map_file == m_map_name) + return false; + } - // Load map into cleared m_functions. - bool changed = Clear(); - - if (!LoadMap(guard, existing_map_file)) - return changed; + if (!LoadMap(guard, std::move(existing_map_file))) + return Clear(); return true; } @@ -344,14 +368,11 @@ bool PPCSymbolDB::LoadMapOnBoot(const Core::CPUThreadGuard& guard) // function names and addresses that have a BLR before the start and at the end, but ignore any that // don't, and then tell you how many were good and how many it ignored. That way you either find out // it is all good and use it, find out it is partly good and use the good part, or find out that -// only -// a handful of functions lined up by coincidence and then you can clear the symbols. In the future -// I -// want to make it smarter, so it checks that there are no BLRs in the middle of the function -// (by checking the code length), and also make it cope with added functions in the middle or work -// based on the order of the functions and their approximate length. Currently that process has to -// be -// done manually and is very tedious. +// only a handful of functions lined up by coincidence and then you can clear the symbols. In the +// future I want to make it smarter, so it checks that there are no BLRs in the middle of the +// function (by checking the code length), and also make it cope with added functions in the middle +// or work based on the order of the functions and their approximate length. Currently that process +// has to be done manually and is very tedious. // The use case for separate handling of map files that aren't bad is that you usually want to also // load names that aren't functions(if included in the map file) without them being rejected as // invalid. @@ -362,12 +383,16 @@ bool PPCSymbolDB::LoadMapOnBoot(const Core::CPUThreadGuard& guard) // This one can load both leftover map files on game discs (like Zelda), and mapfiles // produced by SaveSymbolMap below. // bad=true means carefully load map files that might not be from exactly the right version -bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& filename, bool bad) +bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, std::string filename, bool bad) { File::IOFile f(filename, "r"); if (!f) return false; + XFuncMap new_functions; + XNoteMap new_notes; + XFuncPtrMap checksum_to_function; + // Two columns are used by Super Smash Bros. Brawl Korean map file // Three columns are commonly used // Four columns are used in American Mensa Academy map files and perhaps other games @@ -579,9 +604,14 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& ++good_count; if (section_name == ".note") - AddKnownNote(vaddress, size, name); + { + AddKnownNote(vaddress, size, name, &new_notes); + } else - AddKnownSymbol(guard, vaddress, size, name_string, object_filename_string, type); + { + AddKnownSymbol(guard, vaddress, size, name_string, object_filename_string, type, + &new_functions, &checksum_to_function); + } } else { @@ -590,10 +620,15 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& } } - m_map_name = filename; + Index(&new_functions); + DetermineNoteLayers(&new_notes); + + std::lock_guard lock(m_mutex); + std::swap(m_functions, new_functions); + std::swap(m_notes, new_notes); + std::swap(m_checksum_to_function, checksum_to_function); + std::swap(m_map_name, filename); - Index(); - DetermineNoteLayers(); NOTICE_LOG_FMT(SYMBOLS, "{} symbols loaded, {} symbols ignored.", good_count, bad_count); return true; } @@ -605,6 +640,8 @@ bool PPCSymbolDB::SaveSymbolMap(const std::string& filename) const if (!file) return false; + std::lock_guard lock(m_mutex); + // Write .text section auto function_symbols = m_functions | @@ -655,7 +692,7 @@ bool PPCSymbolDB::SaveSymbolMap(const std::string& filename) const return true; } -// Save code map (won't work if Core is running) +// Save code map // // Notes: // - Dolphin doesn't load back code maps @@ -670,6 +707,8 @@ bool PPCSymbolDB::SaveCodeMap(const Core::CPUThreadGuard& guard, const std::stri // Write ".text" at the top f.WriteString(".text\n"); + std::lock_guard lock(m_mutex); + const auto& ppc_debug_interface = guard.GetSystem().GetPowerPC().GetDebugInterface(); u32 next_address = 0; diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.h b/Source/Core/Core/PowerPC/PPCSymbolDB.h index 6e51392195..d106f68aa4 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.h +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.h @@ -3,7 +3,6 @@ #pragma once -#include #include #include @@ -22,25 +21,25 @@ public: PPCSymbolDB(); ~PPCSymbolDB() override; - Common::Symbol* AddFunction(const Core::CPUThreadGuard& guard, u32 start_addr) override; + const Common::Symbol* AddFunction(const Core::CPUThreadGuard& guard, u32 start_addr) override; void AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAddr, u32 size, const std::string& name, const std::string& object_name, Common::Symbol::Type type = Common::Symbol::Type::Function); void AddKnownNote(u32 start_addr, u32 size, const std::string& name); - Common::Symbol* GetSymbolFromAddr(u32 addr) override; + const Common::Symbol* GetSymbolFromAddr(u32 addr) const override; bool NoteExists() const { return !m_notes.empty(); } - Common::Note* GetNoteFromAddr(u32 addr); + const Common::Note* GetNoteFromAddr(u32 addr) const; void DetermineNoteLayers(); void DeleteFunction(u32 start_address); void DeleteNote(u32 start_address); - std::string_view GetDescription(u32 addr); + std::string GetDescription(u32 addr) const; void FillInCallers(); bool LoadMapOnBoot(const Core::CPUThreadGuard& guard); - bool LoadMap(const Core::CPUThreadGuard& guard, const std::string& filename, bool bad = false); + bool LoadMap(const Core::CPUThreadGuard& guard, std::string filename, bool bad = false); bool SaveSymbolMap(const std::string& filename) const; bool SaveCodeMap(const Core::CPUThreadGuard& guard, const std::string& filename) const; @@ -51,5 +50,11 @@ public: static bool FindMapFile(std::string* existing_map_file, std::string* writable_map_file); private: - std::mutex m_write_lock; + static void AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAddr, u32 size, + const std::string& name, const std::string& object_name, + Common::Symbol::Type type, XFuncMap* functions, + XFuncPtrMap* checksum_to_function); + static void AddKnownNote(u32 start_addr, u32 size, const std::string& name, XNoteMap* notes); + + static void DetermineNoteLayers(XNoteMap* notes); }; diff --git a/Source/Core/Core/PowerPC/SignatureDB/MEGASignatureDB.cpp b/Source/Core/Core/PowerPC/SignatureDB/MEGASignatureDB.cpp index a6aefc088b..6b0f573b27 100644 --- a/Source/Core/Core/PowerPC/SignatureDB/MEGASignatureDB.cpp +++ b/Source/Core/Core/PowerPC/SignatureDB/MEGASignatureDB.cpp @@ -160,20 +160,18 @@ bool MEGASignatureDB::Save(const std::string& file_path) const void MEGASignatureDB::Apply(const Core::CPUThreadGuard& guard, PPCSymbolDB* symbol_db) const { - for (auto& it : symbol_db->AccessSymbols()) - { - auto& symbol = it.second; + symbol_db->ForEachSymbol([&](const Common::Symbol& symbol) { for (const auto& sig : m_signatures) { if (Compare(guard, symbol.address, symbol.size, sig)) { - symbol.name = sig.name; + symbol_db->RenameSymbol(symbol, sig.name); INFO_LOG_FMT(SYMBOLS, "Found {} at {:08x} (size: {:08x})!", sig.name, symbol.address, symbol.size); break; } } - } + }); symbol_db->Index(); } diff --git a/Source/Core/Core/PowerPC/SignatureDB/SignatureDB.cpp b/Source/Core/Core/PowerPC/SignatureDB/SignatureDB.cpp index b9cec4a2be..c67000b9bb 100644 --- a/Source/Core/Core/PowerPC/SignatureDB/SignatureDB.cpp +++ b/Source/Core/Core/PowerPC/SignatureDB/SignatureDB.cpp @@ -128,7 +128,7 @@ void HashSignatureDB::Apply(const Core::CPUThreadGuard& guard, PPCSymbolDB* symb for (const auto& function : symbol_db->GetSymbolsFromHash(entry.first)) { // Found the function. Let's rename it according to the symbol file. - function->Rename(entry.second.name); + symbol_db->RenameSymbol(*function, entry.second.name); if (entry.second.size == static_cast(function->size)) { INFO_LOG_FMT(SYMBOLS, "Found {} at {:08x} (size: {:08x})!", entry.second.name, @@ -146,18 +146,17 @@ void HashSignatureDB::Apply(const Core::CPUThreadGuard& guard, PPCSymbolDB* symb void HashSignatureDB::Populate(const PPCSymbolDB* symbol_db, const std::string& filter) { - for (const auto& symbol : symbol_db->Symbols()) - { - if ((filter.empty() && (!symbol.second.name.empty()) && - symbol.second.name.substr(0, 3) != "zz_" && symbol.second.name.substr(0, 1) != ".") || - ((!filter.empty()) && symbol.second.name.substr(0, filter.size()) == filter)) + symbol_db->ForEachSymbol([&](const Common::Symbol& symbol) { + if ((filter.empty() && (!symbol.name.empty()) && symbol.name.substr(0, 3) != "zz_" && + symbol.name.substr(0, 1) != ".") || + ((!filter.empty()) && symbol.name.substr(0, filter.size()) == filter)) { DBFunc temp_dbfunc; - temp_dbfunc.name = symbol.second.name; - temp_dbfunc.size = symbol.second.size; - m_database[symbol.second.hash] = temp_dbfunc; + temp_dbfunc.name = symbol.name; + temp_dbfunc.size = symbol.size; + m_database[symbol.hash] = temp_dbfunc; } - } + }); } u32 HashSignatureDB::ComputeCodeChecksum(const Core::CPUThreadGuard& guard, u32 offsetStart, diff --git a/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp b/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp index a430be15a3..f94b640c41 100644 --- a/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp @@ -335,7 +335,7 @@ void CodeViewWidget::Update(const Core::CPUThreadGuard* guard) std::string ins = (split == std::string::npos ? disas : disas.substr(0, split)); std::string param = (split == std::string::npos ? "" : disas.substr(split + 1)); - const std::string_view desc = debug_interface.GetDescription(addr); + const std::string desc = debug_interface.GetDescription(addr); const Common::Note* note = m_ppc_symbol_db.GetNoteFromAddr(addr); std::string note_string; @@ -950,7 +950,7 @@ void CodeViewWidget::OnFollowBranch() void CodeViewWidget::OnEditSymbol() { const u32 addr = GetContextAddress(); - Common::Symbol* const symbol = m_ppc_symbol_db.GetSymbolFromAddr(addr); + const Common::Symbol* const symbol = m_ppc_symbol_db.GetSymbolFromAddr(addr); if (symbol == nullptr) { @@ -974,12 +974,14 @@ void CodeViewWidget::OnEditSymbol() } if (symbol->name != name) - symbol->Rename(name); + m_ppc_symbol_db.RenameSymbol(*symbol, name); if (symbol->size != size) { Core::CPUThreadGuard guard(m_system); - PPCAnalyst::ReanalyzeFunction(guard, symbol->address, *symbol, size); + Common::Symbol new_symbol = *symbol; + PPCAnalyst::ReanalyzeFunction(guard, symbol->address, new_symbol, size); + m_ppc_symbol_db.AddCompleteSymbol(new_symbol); } emit Host::GetInstance()->PPCSymbolsChanged(); @@ -988,7 +990,7 @@ void CodeViewWidget::OnEditSymbol() void CodeViewWidget::OnDeleteSymbol() { const u32 addr = GetContextAddress(); - Common::Symbol* const symbol = m_ppc_symbol_db.GetSymbolFromAddr(addr); + const Common::Symbol* const symbol = m_ppc_symbol_db.GetSymbolFromAddr(addr); if (symbol == nullptr) return; @@ -1039,7 +1041,7 @@ void CodeViewWidget::OnSelectionChanged() void CodeViewWidget::OnEditNote() { const u32 context_address = GetContextAddress(); - Common::Note* const note = m_ppc_symbol_db.GetNoteFromAddr(context_address); + const Common::Note* const note = m_ppc_symbol_db.GetNoteFromAddr(context_address); if (note == nullptr) return; @@ -1071,7 +1073,7 @@ void CodeViewWidget::OnEditNote() void CodeViewWidget::OnDeleteNote() { const u32 context_address = GetContextAddress(); - Common::Note* const note = m_ppc_symbol_db.GetNoteFromAddr(context_address); + const Common::Note* const note = m_ppc_symbol_db.GetNoteFromAddr(context_address); if (note == nullptr) return; diff --git a/Source/Core/DolphinQt/Debugger/CodeWidget.cpp b/Source/Core/DolphinQt/Debugger/CodeWidget.cpp index e5aaa1c2cd..3e4eedf49d 100644 --- a/Source/Core/DolphinQt/Debugger/CodeWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/CodeWidget.cpp @@ -423,14 +423,13 @@ void CodeWidget::UpdateSymbols() m_symbols_list->selectedItems()[0]->text(); m_symbols_list->clear(); - for (const auto& symbol : m_ppc_symbol_db.Symbols()) - { - QString name = QString::fromStdString(symbol.second.name); + m_ppc_symbol_db.ForEachSymbol([&](const Common::Symbol& symbol) { + QString name = QString::fromStdString(symbol.name); // If the symbol has an object name, add it to the entry name. - if (!symbol.second.object_name.empty()) + if (!symbol.object_name.empty()) { - name += QString::fromStdString(fmt::format(" ({})", symbol.second.object_name)); + name += QString::fromStdString(fmt::format(" ({})", symbol.object_name)); } auto* item = new QListWidgetItem(name); @@ -438,14 +437,14 @@ void CodeWidget::UpdateSymbols() item->setSelected(true); // Disable non-function symbols as you can't do anything with them. - if (symbol.second.type != Common::Symbol::Type::Function) + if (symbol.type != Common::Symbol::Type::Function) item->setFlags(Qt::NoItemFlags); - item->setData(Qt::UserRole, symbol.second.address); + item->setData(Qt::UserRole, symbol.address); if (name.contains(m_symbol_filter, Qt::CaseInsensitive)) m_symbols_list->addItem(item); - } + }); m_symbols_list->sortItems(); } @@ -457,19 +456,18 @@ void CodeWidget::UpdateNotes() m_note_list->selectedItems()[0]->text(); m_note_list->clear(); - for (const auto& note : m_ppc_symbol_db.Notes()) - { - const QString name = QString::fromStdString(note.second.name); + m_ppc_symbol_db.ForEachNote([&](const Common::Note& note) { + const QString name = QString::fromStdString(note.name); auto* item = new QListWidgetItem(name); if (name == selection) item->setSelected(true); - item->setData(Qt::UserRole, note.second.address); + item->setData(Qt::UserRole, note.address); if (name.toUpper().indexOf(m_symbol_filter.toUpper()) != -1) m_note_list->addItem(item); - } + }); m_note_list->sortItems(); }