From d47977d385db9231c384f2c7722c01a0f92b302b Mon Sep 17 00:00:00 2001 From: Sepalani Date: Sun, 24 Nov 2024 12:26:21 +0400 Subject: [PATCH 1/3] PPCSymbolDB: Use ranges in SaveSymbolMap --- Source/Core/Core/PowerPC/PPCSymbolDB.cpp | 38 +++++++++++------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index 95102fa17967..77decee7ba90 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -6,11 +6,11 @@ #include #include #include +#include #include #include #include #include -#include #include @@ -466,42 +466,38 @@ bool PPCSymbolDB::SaveSymbolMap(const std::string& filename) const if (!f) return false; - std::vector function_symbols; - std::vector data_symbols; - - for (const auto& function : m_functions) - { - const Common::Symbol& symbol = function.second; - if (symbol.type == Common::Symbol::Type::Function) - function_symbols.push_back(&symbol); - else - data_symbols.push_back(&symbol); - } - // Write .text section + auto function_symbols = + m_functions | + std::views::filter([](auto f) { return f.second.type == Common::Symbol::Type::Function; }) | + std::views::transform([](auto f) { return f.second; }); f.WriteString(".text section layout\n"); for (const auto& symbol : function_symbols) { // Write symbol address, size, virtual address, alignment, name - std::string line = fmt::format("{0:08x} {1:06x} {2:08x} {3} {4}", symbol->address, symbol->size, - symbol->address, 0, symbol->name); + std::string line = fmt::format("{:08x} {:06x} {:08x} {} {}", symbol.address, symbol.size, + symbol.address, 0, symbol.name); // Also write the object name if it exists - if (!symbol->object_name.empty()) - line += fmt::format(" \t{0}", symbol->object_name); + if (!symbol.object_name.empty()) + line += fmt::format(" \t{0}", symbol.object_name); line += "\n"; f.WriteString(line); } // Write .data section + auto data_symbols = + m_functions | + std::views::filter([](auto f) { return f.second.type == Common::Symbol::Type::Data; }) | + std::views::transform([](auto f) { return f.second; }); f.WriteString("\n.data section layout\n"); for (const auto& symbol : data_symbols) { // Write symbol address, size, virtual address, alignment, name - std::string line = fmt::format("{0:08x} {1:06x} {2:08x} {3} {4}", symbol->address, symbol->size, - symbol->address, 0, symbol->name); + std::string line = fmt::format("{:08x} {:06x} {:08x} {} {}", symbol.address, symbol.size, + symbol.address, 0, symbol.name); // Also write the object name if it exists - if (!symbol->object_name.empty()) - line += fmt::format(" \t{0}", symbol->object_name); + if (!symbol.object_name.empty()) + line += fmt::format(" \t{0}", symbol.object_name); line += "\n"; f.WriteString(line); } From 024fac84edce8519cebbf1623ae8afecaaf66abc Mon Sep 17 00:00:00 2001 From: Sepalani Date: Sun, 24 Nov 2024 13:03:22 +0400 Subject: [PATCH 2/3] PPCSymbolDB: Deduplicate parsing of the 'entry of' string --- Source/Core/Core/PowerPC/PPCSymbolDB.cpp | 51 +++++++++++------------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index 77decee7ba90..39f8259be65d 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -328,7 +328,26 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& } u32 address, vaddress, size, offset, alignment; - char name[512], container[512]; + char name[512]; + static constexpr char ENTRY_OF_STRING[] = "(entry of "; + static constexpr std::string_view ENTRY_OF_VIEW(ENTRY_OF_STRING); + auto parse_entry_of = [](const char* line, char* name) { + const char* s = strstr(line, ENTRY_OF_STRING); + if (s) + { + char container[512]; + sscanf(s + ENTRY_OF_VIEW.size(), "%511s", container); + char* s2 = strchr(container, ')'); + // Skip sections, those start with a dot, e.g. (entry of .text) + if (s2 && container[0] != '.') + { + s2[0] = '\0'; + strcat(container, "::"); + strcat(container, name); + strcpy(name, container); + } + } + }; if (column_count == 4) { // sometimes there is no alignment value, and sometimes it is because it is an entry of @@ -337,19 +356,7 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& { alignment = 0; sscanf(line, "%08x %08x %08x %08x %511s", &address, &size, &vaddress, &offset, name); - char* s = strstr(line, "(entry of "); - if (s) - { - sscanf(s + 10, "%511s", container); - char* s2 = (strchr(container, ')')); - if (s2 && container[0] != '.') - { - s2[0] = '\0'; - strcat(container, "::"); - strcat(container, name); - strcpy(name, container); - } - } + parse_entry_of(line, name); } else { @@ -362,23 +369,11 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& // some entries in the table have a function name followed by " (entry of " followed by a // container name, followed by ")" // instead of a space followed by a number followed by a space followed by a name - if (length > 27 && line[27] != ' ' && strstr(line, "(entry of ")) + if (length > 27 && line[27] != ' ' && strstr(line, ENTRY_OF_STRING)) { alignment = 0; sscanf(line, "%08x %08x %08x %511s", &address, &size, &vaddress, name); - char* s = strstr(line, "(entry of "); - if (s) - { - sscanf(s + 10, "%511s", container); - char* s2 = (strchr(container, ')')); - if (s2 && container[0] != '.') - { - s2[0] = '\0'; - strcat(container, "::"); - strcat(container, name); - strcpy(name, container); - } - } + parse_entry_of(line, name); } else { From 30693773a394ec5802a3a96c4ca4b9afd61d97bb Mon Sep 17 00:00:00 2001 From: Sepalani Date: Sun, 24 Nov 2024 21:13:20 +0400 Subject: [PATCH 3/3] PPCSymbolDB: Add alignment detection heuristic Update parse_entry_of in accordance to the sscanf change --- Source/Core/Core/PowerPC/PPCSymbolDB.cpp | 100 +++++++++++------------ 1 file changed, 49 insertions(+), 51 deletions(-) diff --git a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp index 39f8259be65d..d7ebd5433ddb 100644 --- a/Source/Core/Core/PowerPC/PPCSymbolDB.cpp +++ b/Source/Core/Core/PowerPC/PPCSymbolDB.cpp @@ -18,6 +18,7 @@ #include "Common/IOFile.h" #include "Common/Logging/Log.h" #include "Common/StringUtil.h" +#include "Common/Unreachable.h" #include "Core/Core.h" #include "Core/Debugger/DebugInterface.h" #include "Core/PowerPC/MMU.h" @@ -310,7 +311,7 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& continue; column_count = 2; - // Three columns format: + // Three columns format (with optional alignment): // Starting Virtual // address Size address // ----------------------- @@ -319,7 +320,7 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& else iss.str(""); - // Four columns format: + // Four columns format (with optional alignment): // Starting Virtual File // address Size address offset // --------------------------------- @@ -327,75 +328,72 @@ bool PPCSymbolDB::LoadMap(const Core::CPUThreadGuard& guard, const std::string& column_count = 4; } - u32 address, vaddress, size, offset, alignment; - char name[512]; - static constexpr char ENTRY_OF_STRING[] = "(entry of "; + u32 address; + u32 vaddress; + u32 size = 0; + u32 offset = 0; + u32 alignment = 0; + char name[512]{}; + static constexpr char ENTRY_OF_STRING[] = " (entry of "; static constexpr std::string_view ENTRY_OF_VIEW(ENTRY_OF_STRING); - auto parse_entry_of = [](const char* line, char* name) { - const char* s = strstr(line, ENTRY_OF_STRING); - if (s) + auto parse_entry_of = [](char* name) { + if (char* s1 = strstr(name, ENTRY_OF_STRING); s1 != nullptr) { char container[512]; - sscanf(s + ENTRY_OF_VIEW.size(), "%511s", container); - char* s2 = strchr(container, ')'); + char* ptr = s1 + ENTRY_OF_VIEW.size(); + sscanf(ptr, "%511s", container); // Skip sections, those start with a dot, e.g. (entry of .text) - if (s2 && container[0] != '.') + if (char* s2 = strchr(container, ')'); s2 != nullptr && *container != '.') { - s2[0] = '\0'; + ptr += strlen(container); + // Preserve data after the entry part, usually it contains object names + strcpy(s1, ptr); + *s2 = '\0'; strcat(container, "::"); strcat(container, name); strcpy(name, container); } } }; - if (column_count == 4) + auto was_alignment = [](const char* name) { + return *name == ' ' || (*name >= '0' && *name <= '9'); + }; + auto parse_alignment = [](char* name, u32* alignment) { + const std::string buffer(StripWhitespace(name)); + return sscanf(buffer.c_str(), "%i %511[^\r\n]", alignment, name); + }; + switch (column_count) { + case 4: // sometimes there is no alignment value, and sometimes it is because it is an entry of // something else - if (length > 37 && line[37] == ' ') - { - alignment = 0; - sscanf(line, "%08x %08x %08x %08x %511s", &address, &size, &vaddress, &offset, name); - parse_entry_of(line, name); - } - else - { - sscanf(line, "%08x %08x %08x %08x %i %511s", &address, &size, &vaddress, &offset, - &alignment, name); - } - } - else if (column_count == 3) - { + sscanf(line, "%08x %08x %08x %08x %511[^\r\n]", &address, &size, &vaddress, &offset, name); + if (was_alignment(name)) + parse_alignment(name, &alignment); + // The `else` statement was omitted to handle symbol already saved in Dolphin symbol map + // since it doesn't omit the alignment on save for such case. + parse_entry_of(name); + break; + case 3: // some entries in the table have a function name followed by " (entry of " followed by a // container name, followed by ")" // instead of a space followed by a number followed by a space followed by a name - if (length > 27 && line[27] != ' ' && strstr(line, ENTRY_OF_STRING)) - { - alignment = 0; - sscanf(line, "%08x %08x %08x %511s", &address, &size, &vaddress, name); - parse_entry_of(line, name); - } - else - { - sscanf(line, "%08x %08x %08x %i %511s", &address, &size, &vaddress, &alignment, name); - } - } - else if (column_count == 2) - { - sscanf(line, "%08x %511s", &address, name); + sscanf(line, "%08x %08x %08x %511[^\r\n]", &address, &size, &vaddress, name); + if (was_alignment(name)) + parse_alignment(name, &alignment); + // The `else` statement was omitted to handle symbol already saved in Dolphin symbol map + // since it doesn't omit the alignment on save for such case. + parse_entry_of(name); + break; + case 2: + sscanf(line, "%08x %511[^\r\n]", &address, name); vaddress = address; - size = 0; - } - else - { + break; + default: + // Should never happen + Common::Unreachable(); break; } - const char* namepos = strstr(line, name); - if (namepos != nullptr) // would be odd if not :P - strcpy(name, namepos); - name[strlen(name) - 1] = 0; - if (name[strlen(name) - 1] == '\r') - name[strlen(name) - 1] = 0; // Split the current name string into separate parts, and get the object name // if it exists.