diff --git a/Source/Core/Common/SymbolDB.cpp b/Source/Core/Common/SymbolDB.cpp index bfdd9f57db..88f281e497 100644 --- a/Source/Core/Common/SymbolDB.cpp +++ b/Source/Core/Common/SymbolDB.cpp @@ -44,15 +44,20 @@ void SymbolDB::List() bool SymbolDB::IsEmpty() const { - return m_functions.empty(); + return m_functions.empty() && m_notes.empty(); } -void SymbolDB::Clear(const char* prefix) +bool SymbolDB::Clear(const char* prefix) { // TODO: honor prefix + m_map_name.clear(); + if (IsEmpty()) + return false; + m_functions.clear(); m_notes.clear(); m_checksum_to_function.clear(); + return true; } void SymbolDB::Index() diff --git a/Source/Core/Common/SymbolDB.h b/Source/Core/Common/SymbolDB.h index 8930b95996..7763999453 100644 --- a/Source/Core/Common/SymbolDB.h +++ b/Source/Core/Common/SymbolDB.h @@ -100,7 +100,7 @@ public: const XNoteMap& Notes() const { return m_notes; } XFuncMap& AccessSymbols() { return m_functions; } bool IsEmpty() const; - void Clear(const char* prefix = ""); + bool Clear(const char* prefix = ""); void List(); void Index(); @@ -108,5 +108,6 @@ protected: XFuncMap m_functions; XNoteMap m_notes; XFuncPtrMap m_checksum_to_function; + std::string m_map_name; }; } // namespace Common diff --git a/Source/Core/Core/Boot/Boot.cpp b/Source/Core/Core/Boot/Boot.cpp index a3a984e35e..1b796952f8 100644 --- a/Source/Core/Core/Boot/Boot.cpp +++ b/Source/Core/Core/Boot/Boot.cpp @@ -354,39 +354,6 @@ bool CBoot::DVDReadDiscID(Core::System& system, const DiscIO::VolumeDisc& disc, return true; } -// Get map file paths for the active title. -bool CBoot::FindMapFile(std::string* existing_map_file, std::string* writable_map_file) -{ - const std::string& game_id = SConfig::GetInstance().m_debugger_game_id; - std::string path = File::GetUserPath(D_MAPS_IDX) + game_id + ".map"; - - if (writable_map_file) - *writable_map_file = path; - - if (File::Exists(path)) - { - if (existing_map_file) - *existing_map_file = std::move(path); - - return true; - } - - return false; -} - -bool CBoot::LoadMapFromFilename(const Core::CPUThreadGuard& guard, PPCSymbolDB& ppc_symbol_db) -{ - std::string strMapFilename; - bool found = FindMapFile(&strMapFilename, nullptr); - if (found && ppc_symbol_db.LoadMap(guard, strMapFilename)) - { - Host_PPCSymbolsChanged(); - return true; - } - - return false; -} - // If ipl.bin is not found, this function does *some* of what BS1 does: // loading IPL(BS2) and jumping to it. // It does not initialize the hardware or anything else like BS1 does. @@ -524,12 +491,6 @@ bool CBoot::BootUp(Core::System& system, const Core::CPUThreadGuard& guard, { SConfig& config = SConfig::GetInstance(); - if (auto& ppc_symbol_db = system.GetPPCSymbolDB(); !ppc_symbol_db.IsEmpty()) - { - ppc_symbol_db.Clear(); - Host_PPCSymbolsChanged(); - } - // PAL Wii uses NTSC framerate and linecount in 60Hz modes system.GetVideoInterface().Preset(DiscIO::IsNTSC(config.m_region) || (system.IsWii() && Config::Get(Config::SYSCONF_PAL60))); @@ -611,11 +572,18 @@ bool CBoot::BootUp(Core::System& system, const Core::CPUThreadGuard& guard, const std::string filename = PathToFileName(executable.path); - if (executable.reader->LoadSymbols(guard, system.GetPPCSymbolDB(), filename)) + auto& ppc_symbol_db = system.GetPPCSymbolDB(); + bool symbols_changed = ppc_symbol_db.Clear(); + + if (executable.reader->LoadSymbols(guard, ppc_symbol_db, filename)) { - Host_PPCSymbolsChanged(); + symbols_changed = true; HLE::PatchFunctions(system); } + + if (symbols_changed) + Host_PPCSymbolsChanged(); + return true; } diff --git a/Source/Core/Core/Boot/Boot.h b/Source/Core/Core/Boot/Boot.h index 100f1cfdfd..f892ee7c75 100644 --- a/Source/Core/Core/Boot/Boot.h +++ b/Source/Core/Core/Boot/Boot.h @@ -159,19 +159,6 @@ public: static bool BootUp(Core::System& system, const Core::CPUThreadGuard& guard, std::unique_ptr boot); - // Tries to find a map file for the current game by looking first in the - // local user directory, then in the shared user directory. - // - // If existing_map_file is not nullptr and a map file exists, it is set to the - // path to the existing map file. - // - // If writable_map_file is not nullptr, it is set to the path to where a map - // file should be saved. - // - // Returns true if a map file exists, false if none could be found. - static bool FindMapFile(std::string* existing_map_file, std::string* writable_map_file); - static bool LoadMapFromFilename(const Core::CPUThreadGuard& guard, PPCSymbolDB& ppc_symbol_db); - private: static bool DVDRead(Core::System& system, const DiscIO::VolumeDisc& disc, u64 dvd_offset, u32 output_address, u32 length, const DiscIO::Partition& partition); diff --git a/Source/Core/Core/ConfigManager.cpp b/Source/Core/Core/ConfigManager.cpp index e005e78341..4e0b7dd3ac 100644 --- a/Source/Core/Core/ConfigManager.cpp +++ b/Source/Core/Core/ConfigManager.cpp @@ -271,12 +271,9 @@ void SConfig::OnTitleDirectlyBooted(const Core::CPUThreadGuard& guard) return; auto& ppc_symbol_db = system.GetPPCSymbolDB(); - if (!ppc_symbol_db.IsEmpty()) - { - ppc_symbol_db.Clear(); + + if (ppc_symbol_db.LoadMapOnBoot(guard)) Host_PPCSymbolsChanged(); - } - CBoot::LoadMapFromFilename(guard, ppc_symbol_db); HLE::Reload(system); PatchEngine::Reload(system); diff --git a/Source/Core/Core/IOS/MIOS.cpp b/Source/Core/Core/IOS/MIOS.cpp index d8829ee5cc..d47400c9d2 100644 --- a/Source/Core/Core/IOS/MIOS.cpp +++ b/Source/Core/Core/IOS/MIOS.cpp @@ -71,18 +71,18 @@ bool Load(Core::System& system) auto& ppc_symbol_db = power_pc.GetSymbolDB(); // Load symbols for the IPL if they exist. - if (!ppc_symbol_db.IsEmpty()) - { - ppc_symbol_db.Clear(); - Host_PPCSymbolsChanged(); - } + bool symbols_changed = ppc_symbol_db.Clear(); + if (ppc_symbol_db.LoadMap(guard, File::GetUserPath(D_MAPS_IDX) + "mios-ipl.map")) { ::HLE::Clear(); ::HLE::PatchFunctions(system); - Host_PPCSymbolsChanged(); + symbols_changed = true; } + if (symbols_changed) + Host_PPCSymbolsChanged(); + const PowerPC::CoreMode core_mode = power_pc.GetMode(); power_pc.SetMode(PowerPC::CoreMode::Interpreter); diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index c837f1c041..e6077f6174 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -15,10 +16,12 @@ #include #include "Common/CommonTypes.h" +#include "Common/FileUtil.h" #include "Common/IOFile.h" #include "Common/Logging/Log.h" #include "Common/StringUtil.h" #include "Common/Unreachable.h" +#include "Core/ConfigManager.h" #include "Core/Core.h" #include "Core/Debugger/DebugInterface.h" #include "Core/PowerPC/MMU.h" @@ -133,6 +136,13 @@ 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()) + return nullptr; + auto it = m_functions.lower_bound(addr); if (it != m_functions.end()) @@ -154,7 +164,8 @@ Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr) Common::Note* PPCSymbolDB::GetNoteFromAddr(u32 addr) { - if (m_notes.empty()) + std::unique_lock lock(m_write_lock, std::try_to_lock); + if (!lock.owns_lock() || m_notes.empty()) return nullptr; auto itn = m_notes.lower_bound(addr); @@ -201,6 +212,10 @@ 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; + for (auto& p : m_functions) { p.second.callers.clear(); @@ -279,6 +294,49 @@ void PPCSymbolDB::LogFunctionCall(u32 addr) f.num_calls++; } +// Get map file paths for the active title. +bool PPCSymbolDB::FindMapFile(std::string* existing_map_file, std::string* writable_map_file) +{ + const std::string& game_id = SConfig::GetInstance().m_debugger_game_id; + std::string path = File::GetUserPath(D_MAPS_IDX) + game_id + ".map"; + + if (writable_map_file) + *writable_map_file = path; + + if (File::Exists(path)) + { + if (existing_map_file) + *existing_map_file = std::move(path); + + return true; + } + + return false; +} + +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; + + // Load map into cleared m_functions. + bool changed = Clear(); + + if (!LoadMap(guard, existing_map_file)) + return changed; + + return true; +} + // The use case for handling bad map files is when you have a game with a map file on the disc, // but you can't tell whether that map file is for the particular release version used in that game, // or when you know that the map file is not for that build, but perhaps half the functions in the @@ -532,6 +590,8 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& } } + m_map_name = filename; + Index(); DetermineNoteLayers(); NOTICE_LOG_FMT(SYMBOLS, "{} symbols loaded, {} symbols ignored.", good_count, bad_count); diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.h b/Source/Core/Core/PowerPC/PPCSymbolDB.h index b96ad2f42f..6e51392195 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.h +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.h @@ -3,6 +3,7 @@ #pragma once +#include #include #include @@ -37,6 +38,8 @@ public: std::string_view GetDescription(u32 addr); void FillInCallers(); + + bool LoadMapOnBoot(const Core::CPUThreadGuard& guard); bool LoadMap(const Core::CPUThreadGuard& guard, const std::string& filename, bool bad = false); bool SaveSymbolMap(const std::string& filename) const; bool SaveCodeMap(const Core::CPUThreadGuard& guard, const std::string& filename) const; @@ -44,4 +47,9 @@ public: void PrintCalls(u32 funcAddr) const; void PrintCallers(u32 funcAddr) const; void LogFunctionCall(u32 addr); + + static bool FindMapFile(std::string* existing_map_file, std::string* writable_map_file); + +private: + std::mutex m_write_lock; }; diff --git a/Source/Core/DolphinQt/MenuBar.cpp b/Source/Core/DolphinQt/MenuBar.cpp index ead86839cb..6f21030294 100644 --- a/Source/Core/DolphinQt/MenuBar.cpp +++ b/Source/Core/DolphinQt/MenuBar.cpp @@ -26,7 +26,6 @@ #include "Common/StringUtil.h" #include "Core/AchievementManager.h" -#include "Core/Boot/Boot.h" #include "Core/CommonTitles.h" #include "Core/Config/AchievementSettings.h" #include "Core/Config/MainSettings.h" @@ -1717,7 +1716,7 @@ void MenuBar::LoadSymbolMap() auto& ppc_symbol_db = system.GetPPCSymbolDB(); std::string existing_map_file, writable_map_file; - bool map_exists = CBoot::FindMapFile(&existing_map_file, &writable_map_file); + bool map_exists = PPCSymbolDB::FindMapFile(&existing_map_file, &writable_map_file); if (!map_exists) { @@ -1755,7 +1754,7 @@ void MenuBar::LoadSymbolMap() void MenuBar::SaveSymbolMap() { std::string existing_map_file, writable_map_file; - CBoot::FindMapFile(&existing_map_file, &writable_map_file); + PPCSymbolDB::FindMapFile(&existing_map_file, &writable_map_file); TrySaveSymbolMap(QString::fromStdString(writable_map_file)); } @@ -1811,7 +1810,7 @@ void MenuBar::SaveSymbolMapAs() void MenuBar::SaveCode() { std::string existing_map_file, writable_map_file; - CBoot::FindMapFile(&existing_map_file, &writable_map_file); + PPCSymbolDB::FindMapFile(&existing_map_file, &writable_map_file); const std::string path = writable_map_file.substr(0, writable_map_file.find_last_of('.')) + "_code.map";