From d62e21e7b6f47c4ae3fdf97ac0b111c9a584dd32 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 29 Jun 2025 23:24:08 +0200 Subject: [PATCH 1/5] PPCSymbolDB: Wait for locking to succeed 9395238 added a mutex to PPCSymbolDB, and made functions return with an "empty" result if called while the mutex is locked. This new behavior has the potential to affect not only less important call sites like the symbol printing mentioned in a comment, but also the JIT deciding if it should HLE a function. A later commit in this pull request decreases the amount of lock contention, reducing the performance impact of this commit. --- Source/Core/Core/PowerPC/PPCSymbolDB.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index e6077f6174..dcef06ed29 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -136,11 +136,8 @@ void PPCSymbolDB::DetermineNoteLayers() Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) { - // 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::unique_lock lock(m_write_lock); + if (m_functions.empty()) return nullptr; auto it = m_functions.lower_bound(addr); @@ -164,8 +161,8 @@ Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) Common::Note* PPCSymbolDB::GetNoteFromAddr(u32 addr) { - std::unique_lock lock(m_write_lock, std::try_to_lock); - if (!lock.owns_lock() || m_notes.empty()) + std::unique_lock lock(m_write_lock); + if (m_notes.empty()) return nullptr; auto itn = m_notes.lower_bound(addr); @@ -212,9 +209,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::unique_lock lock(m_write_lock); for (auto& p : m_functions) { @@ -314,10 +309,9 @@ 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; From 59d126d21582d2fd2df05dc33d5eeebaf097204a Mon Sep 17 00:00:00 2001 From: JosJuice Date: Tue, 1 Jul 2025 20:16:36 +0200 Subject: [PATCH 2/5] PPCSymbolDB: Rename m_write_lock to m_mutex This mutex needs to be locked both when reading and writing, not just when writing. --- Source/Core/Core/PowerPC/PPCSymbolDB.cpp | 8 ++++---- Source/Core/Core/PowerPC/PPCSymbolDB.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index dcef06ed29..e7bf9bd923 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -136,7 +136,7 @@ void PPCSymbolDB::DetermineNoteLayers() Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) { - std::unique_lock lock(m_write_lock); + std::lock_guard lock(m_mutex); if (m_functions.empty()) return nullptr; @@ -161,7 +161,7 @@ Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) Common::Note* PPCSymbolDB::GetNoteFromAddr(u32 addr) { - std::unique_lock lock(m_write_lock); + std::lock_guard lock(m_mutex); if (m_notes.empty()) return nullptr; @@ -209,7 +209,7 @@ std::string_view PPCSymbolDB::GetDescription(u32 addr) void PPCSymbolDB::FillInCallers() { - std::unique_lock lock(m_write_lock); + std::lock_guard lock(m_mutex); for (auto& p : m_functions) { @@ -312,7 +312,7 @@ bool PPCSymbolDB::FindMapFile(std::string* existing_map_file, std::string* writa // Returns true if m_functions was changed. bool PPCSymbolDB::LoadMapOnBoot(const Core::CPUThreadGuard& guard) { - std::lock_guard lock(m_write_lock); + std::lock_guard lock(m_mutex); std::string existing_map_file; if (!PPCSymbolDB::FindMapFile(&existing_map_file, nullptr)) diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.h b/Source/Core/Core/PowerPC/PPCSymbolDB.h index 6e51392195..b182023a54 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.h +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.h @@ -51,5 +51,5 @@ public: static bool FindMapFile(std::string* existing_map_file, std::string* writable_map_file); private: - std::mutex m_write_lock; + std::mutex m_mutex; }; From 803e6b017bf8bb82ee65b48fa244eb1e4c89034d Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 30 Jun 2025 19:12:00 +0200 Subject: [PATCH 3/5] PPCSymbolDB: Reduce lock contention in LoadMap/LoadMapOnBoot By building the map in a local variable and then swapping it with the member variable, we avoid the need to hold a lock while building the map. --- Source/Core/Common/SymbolDB.cpp | 10 ++- Source/Core/Common/SymbolDB.h | 4 + Source/Core/Core/PowerPC/PPCSymbolDB.cpp | 98 +++++++++++++++--------- Source/Core/Core/PowerPC/PPCSymbolDB.h | 11 ++- 4 files changed, 84 insertions(+), 39 deletions(-) diff --git a/Source/Core/Common/SymbolDB.cpp b/Source/Core/Common/SymbolDB.cpp index 88f281e497..ad438b8277 100644 --- a/Source/Core/Common/SymbolDB.cpp +++ b/Source/Core/Common/SymbolDB.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -49,6 +50,8 @@ bool SymbolDB::IsEmpty() const bool SymbolDB::Clear(const char* prefix) { + std::lock_guard lock(m_mutex); + // TODO: honor prefix m_map_name.clear(); if (IsEmpty()) @@ -61,9 +64,14 @@ bool SymbolDB::Clear(const char* prefix) } void SymbolDB::Index() +{ + Index(&m_functions); +} + +void SymbolDB::Index(XFuncMap* functions) { int i = 0; - for (auto& func : m_functions) + for (auto& func : *functions) { func.second.index = i++; } diff --git a/Source/Core/Common/SymbolDB.h b/Source/Core/Common/SymbolDB.h index 7763999453..69515532b1 100644 --- a/Source/Core/Common/SymbolDB.h +++ b/Source/Core/Common/SymbolDB.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include #include @@ -105,9 +106,12 @@ public: void Index(); protected: + static void Index(XFuncMap* functions); + XFuncMap m_functions; XNoteMap m_notes; XFuncPtrMap m_checksum_to_function; std::string m_map_name; + std::mutex m_mutex; }; } // namespace Common diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index e7bf9bd923..51fb3eaa62 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -56,8 +56,17 @@ 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()) + 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 +79,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 +94,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 +105,14 @@ 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); + 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,22 +126,27 @@ 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()) + 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; @@ -312,21 +331,19 @@ bool PPCSymbolDB::FindMapFile(std::string* existing_map_file, std::string* writa // Returns true if m_functions was changed. bool PPCSymbolDB::LoadMapOnBoot(const Core::CPUThreadGuard& guard) { - std::lock_guard lock(m_mutex); - 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; } @@ -338,14 +355,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. @@ -356,12 +370,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 @@ -573,9 +591,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 { @@ -584,10 +607,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; } diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.h b/Source/Core/Core/PowerPC/PPCSymbolDB.h index b182023a54..079328147d 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.h +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.h @@ -3,7 +3,6 @@ #pragma once -#include #include #include @@ -40,7 +39,7 @@ public: 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_mutex; + 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); }; From fef77a5f20d03f22315b2d77f7970c3fed895b13 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 30 Jun 2025 19:26:24 +0200 Subject: [PATCH 4/5] PPCSymbolDB: Add missing locking 9395238 added locking in some PPCSymbolDB functions that access member variables, but far from all. To ensure thread safety, this commit adds the missing locking. --- Source/Core/Common/SymbolDB.cpp | 11 +++++++++++ Source/Core/Common/SymbolDB.h | 2 +- Source/Core/Core/PowerPC/PPCSymbolDB.cpp | 19 ++++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Source/Core/Common/SymbolDB.cpp b/Source/Core/Common/SymbolDB.cpp index ad438b8277..0784256f0b 100644 --- a/Source/Core/Common/SymbolDB.cpp +++ b/Source/Core/Common/SymbolDB.cpp @@ -35,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, @@ -45,6 +46,7 @@ void SymbolDB::List() bool SymbolDB::IsEmpty() const { + std::lock_guard lock(m_mutex); return m_functions.empty() && m_notes.empty(); } @@ -65,6 +67,7 @@ bool SymbolDB::Clear(const char* prefix) void SymbolDB::Index() { + std::lock_guard lock(m_mutex); Index(&m_functions); } @@ -79,6 +82,8 @@ void SymbolDB::Index(XFuncMap* functions) Symbol* SymbolDB::GetSymbolFromName(std::string_view name) { + std::lock_guard lock(m_mutex); + for (auto& func : m_functions) { if (func.second.function_name == name) @@ -90,6 +95,7 @@ Symbol* SymbolDB::GetSymbolFromName(std::string_view name) std::vector SymbolDB::GetSymbolsFromName(std::string_view name) { + std::lock_guard lock(m_mutex); std::vector symbols; for (auto& func : m_functions) @@ -103,6 +109,8 @@ std::vector SymbolDB::GetSymbolsFromName(std::string_view name) Symbol* SymbolDB::GetSymbolFromHash(u32 hash) { + std::lock_guard lock(m_mutex); + auto iter = m_checksum_to_function.find(hash); if (iter == m_checksum_to_function.end()) return nullptr; @@ -112,6 +120,8 @@ Symbol* SymbolDB::GetSymbolFromHash(u32 hash) std::vector SymbolDB::GetSymbolsFromHash(u32 hash) { + std::lock_guard lock(m_mutex); + const auto iter = m_checksum_to_function.find(hash); if (iter == m_checksum_to_function.cend()) @@ -122,6 +132,7 @@ std::vector SymbolDB::GetSymbolsFromHash(u32 hash) void SymbolDB::AddCompleteSymbol(const Symbol& symbol) { + std::lock_guard lock(m_mutex); m_functions.emplace(symbol.address, symbol); } } // namespace Common diff --git a/Source/Core/Common/SymbolDB.h b/Source/Core/Common/SymbolDB.h index 69515532b1..b035b20c53 100644 --- a/Source/Core/Common/SymbolDB.h +++ b/Source/Core/Common/SymbolDB.h @@ -112,6 +112,6 @@ protected: XNoteMap m_notes; XFuncPtrMap m_checksum_to_function; std::string m_map_name; - std::mutex m_mutex; + std::recursive_mutex m_mutex; }; } // namespace Common diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index 51fb3eaa62..362d0ad6ee 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -37,6 +37,8 @@ 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) { + std::lock_guard lock(m_mutex); + // It's already in the list if (m_functions.contains(start_addr)) return nullptr; @@ -56,6 +58,7 @@ void PPCSymbolDB::AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAdd const std::string& name, const std::string& object_name, Common::Symbol::Type type) { + std::lock_guard lock(m_mutex); AddKnownSymbol(guard, startAddr, size, name, object_name, type, &m_functions, &m_checksum_to_function); } @@ -105,6 +108,7 @@ void PPCSymbolDB::AddKnownSymbol(const Core::CPUThreadGuard& guard, u32 startAdd void PPCSymbolDB::AddKnownNote(u32 start_addr, u32 size, const std::string& name) { + std::lock_guard lock(m_mutex); AddKnownNote(start_addr, size, name, &m_notes); } @@ -132,6 +136,7 @@ void PPCSymbolDB::AddKnownNote(u32 start_addr, u32 size, const std::string& name void PPCSymbolDB::DetermineNoteLayers() { + std::lock_guard lock(m_mutex); DetermineNoteLayers(&m_notes); } @@ -211,11 +216,13 @@ 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); } @@ -261,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()) { @@ -282,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; @@ -300,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; @@ -627,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 | @@ -677,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 @@ -692,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; From 9f32562e3633b62526918cbf0cfb920b96a263da Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 30 Jun 2025 21:31:06 +0200 Subject: [PATCH 5/5] PPCSymbolDB: Don't return non-const pointers For thread safety, we shouldn't return any pointers or references that can be used to mutate the state of the PPCSymbolDB. This should be the final part of making PPCSymbolDB thread safe unless I've missed something. --- Source/Core/Common/SymbolDB.cpp | 39 ++++++++-- Source/Core/Common/SymbolDB.h | 62 ++++++++++++--- Source/Core/Core/Debugger/DebugInterface.h | 3 +- .../Core/Core/Debugger/Debugger_SymbolMap.cpp | 6 +- .../Core/Core/Debugger/PPCDebugInterface.cpp | 2 +- Source/Core/Core/Debugger/PPCDebugInterface.h | 2 +- Source/Core/Core/Debugger/RSO.cpp | 5 +- .../Core/Core/PowerPC/JitCommon/JitCache.cpp | 2 +- Source/Core/Core/PowerPC/PPCAnalyst.cpp | 76 +++++++++---------- Source/Core/Core/PowerPC/PPCSymbolDB.cpp | 8 +- Source/Core/Core/PowerPC/PPCSymbolDB.h | 8 +- .../PowerPC/SignatureDB/MEGASignatureDB.cpp | 8 +- .../Core/PowerPC/SignatureDB/SignatureDB.cpp | 19 +++-- .../DolphinQt/Debugger/CodeViewWidget.cpp | 16 ++-- Source/Core/DolphinQt/Debugger/CodeWidget.cpp | 24 +++--- 15 files changed, 172 insertions(+), 108 deletions(-) diff --git a/Source/Core/Common/SymbolDB.cpp b/Source/Core/Common/SymbolDB.cpp index 0784256f0b..2fa514ba86 100644 --- a/Source/Core/Common/SymbolDB.cpp +++ b/Source/Core/Common/SymbolDB.cpp @@ -80,7 +80,7 @@ void SymbolDB::Index(XFuncMap* functions) } } -Symbol* SymbolDB::GetSymbolFromName(std::string_view name) +const Symbol* SymbolDB::GetSymbolFromName(std::string_view name) const { std::lock_guard lock(m_mutex); @@ -93,10 +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::lock_guard lock(m_mutex); - std::vector symbols; + std::vector symbols; for (auto& func : m_functions) { @@ -107,7 +107,7 @@ 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); @@ -118,7 +118,7 @@ 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); @@ -133,6 +133,33 @@ std::vector SymbolDB::GetSymbolsFromHash(u32 hash) void SymbolDB::AddCompleteSymbol(const Symbol& symbol) { std::lock_guard lock(m_mutex); - m_functions.emplace(symbol.address, symbol); + 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 b035b20c53..8a1767d696 100644 --- a/Source/Core/Common/SymbolDB.h +++ b/Source/Core/Common/SymbolDB.h @@ -14,6 +14,7 @@ #include #include +#include "Common/Assert.h" #include "Common/CommonTypes.h" namespace Core @@ -88,18 +89,59 @@ 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(); @@ -112,6 +154,6 @@ protected: XNoteMap m_notes; XFuncPtrMap m_checksum_to_function; std::string m_map_name; - std::recursive_mutex m_mutex; + 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 362d0ad6ee..02da35f94b 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -35,7 +35,7 @@ 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); @@ -158,7 +158,7 @@ void PPCSymbolDB::DetermineNoteLayers(XNoteMap* notes) } } -Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) +const Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) const { std::lock_guard lock(m_mutex); if (m_functions.empty()) @@ -183,7 +183,7 @@ Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) return nullptr; } -Common::Note* PPCSymbolDB::GetNoteFromAddr(u32 addr) +const Common::Note* PPCSymbolDB::GetNoteFromAddr(u32 addr) const { std::lock_guard lock(m_mutex); if (m_notes.empty()) @@ -226,7 +226,7 @@ void PPCSymbolDB::DeleteNote(u32 start_address) 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; diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.h b/Source/Core/Core/PowerPC/PPCSymbolDB.h index 079328147d..d106f68aa4 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.h +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.h @@ -21,20 +21,20 @@ 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(); 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(); }