Merge pull request #13247 from sepalani/debug-map-ranges

PPCSymbolDB: Refactor SymbolMap Save/Load

Source/Core/Core/PowerPC/PPCSymbolDB.cpp

@@ -6,18 +6,19 @@
 #include <algorithm>
 #include <cstring>
 #include <map>
+#include <ranges>
 #include <sstream>
 #include <string>
 #include <string_view>
 #include <utility>
-#include <vector>
 
 #include <fmt/format.h>
 
 #include "Common/CommonTypes.h"
 #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,93 +320,85 @@ 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
       //  ---------------------------------
       if (iss && iss >> word && word.length() == 8 && is_hex_str(word))
         column_count = 4;
     }
 
-    u32 address, vaddress, size, offset, alignment;
-    char name[512], container[512];
-    if (column_count == 4)
-    {
-      // sometimes there is no alignment value, and sometimes it is because it is an entry of
-      // something else
-      if (length > 37 && line[37] == ' ')
+    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 = [](char* name) {
+      if (char* s1 = strstr(name, ENTRY_OF_STRING); s1 != nullptr)
       {
-        alignment = 0;
-        sscanf(line, "%08x %08x %08x %08x %511s", &address, &size, &vaddress, &offset, name);
-        char* s = strstr(line, "(entry of ");
-        if (s)
+        char container[512];
+        char* ptr = s1 + ENTRY_OF_VIEW.size();
+        sscanf(ptr, "%511s", container);
+        // Skip sections, those start with a dot, e.g. (entry of .text)
+        if (char* s2 = strchr(container, ')'); s2 != nullptr && *container != '.')
         {
-          sscanf(s + 10, "%511s", container);
-          char* s2 = (strchr(container, ')'));
-          if (s2 && container[0] != '.')
-          {
-            s2[0] = '\0';
-            strcat(container, "::");
-            strcat(container, name);
-            strcpy(name, container);
-          }
+          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);
         }
       }
-      else
-      {
-        sscanf(line, "%08x %08x %08x %08x %i %511s", &address, &size, &vaddress, &offset,
-               &alignment, name);
-      }
-    }
-    else if (column_count == 3)
+    };
+    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
+      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 "))
-      {
-        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);
-          }
-        }
-      }
-      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.
     const std::vector<std::string> parts = SplitString(name, '\t');
-    const std::string name_string(StripWhitespace(parts[0]));
+    const std::string name_string(StripWhitespace(parts.size() > 0 ? parts[0] : name));
     const std::string object_filename_string =
         parts.size() > 1 ? std::string(StripWhitespace(parts[1])) : "";
 
@@ -466,42 +459,38 @@ bool PPCSymbolDB::SaveSymbolMap(const std::string& filename) const
   if (!f)
     return false;
 
-  std::vector<const Common::Symbol*> function_symbols;
-  std::vector<const Common::Symbol*> 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);
   }