Fix continuation ownership for overlapping entry wrappers

ps2xRecomp/src/lib/function_table_emitter.cpp

@@ -105,8 +105,52 @@ namespace ps2recomp
             addEntry(address, name);
         }
 
+        for (const auto &[address, name] : stubFunctions)
+        {
+            addEntry(address, name);
+        }
+        for (const auto &[address, name] : systemCallFunctions)
+        {
+            addEntry(address, name);
+        }
+        for (const auto &[address, name] : libraryFunctions)
+        {
+            addEntry(address, name);
+        }
+
+        // Explicit function starts, including configured runtime handlers,
+        // own their addresses even when another wrapper resumes there.
+        std::unordered_set<uint32_t> entryWrapperStarts;
+        for (const auto &function : functions)
+        {
+            if (function.name.rfind("entry_", 0) == 0)
+            {
+                entryWrapperStarts.insert(function.start);
+            }
+        }
+        std::vector<uint32_t> resumeOwners;
+        resumeOwners.reserve(cg.m_resumeEntryTargetsByOwner.size());
         for (const auto &[ownerStart, targets] : cg.m_resumeEntryTargetsByOwner)
         {
+            (void)targets;
+            resumeOwners.push_back(ownerStart);
+        }
+        std::sort(resumeOwners.begin(), resumeOwners.end(), [&](uint32_t left, uint32_t right)
+                  {
+                      const bool leftEntry = entryWrapperStarts.contains(left);
+                      const bool rightEntry = entryWrapperStarts.contains(right);
+                      if (leftEntry != rightEntry)
+                      {
+                          return !leftEntry;
+                      }
+                      // Prefer the most specific owner, as containing-owner
+                      // discovery does. Synthetic entries only fill gaps left
+                      // by normal owners, regardless of unordered-map layout.
+                      return left > right;
+                  });
+        for (uint32_t ownerStart : resumeOwners)
+        {
+            const auto &targets = cg.m_resumeEntryTargetsByOwner.at(ownerStart);
             const std::string ownerName = cg.getFunctionName(ownerStart);
             if (ownerName.empty())
             {
@@ -119,19 +163,6 @@ namespace ps2recomp
             }
         }
 
-        for (const auto &[address, name] : stubFunctions)
-        {
-            addEntry(address, name);
-        }
-        for (const auto &[address, name] : systemCallFunctions)
-        {
-            addEntry(address, name);
-        }
-        for (const auto &[address, name] : libraryFunctions)
-        {
-            addEntry(address, name);
-        }
-
         std::sort(entries.begin(), entries.end(), [](const auto &a, const auto &b)
                   { return a.first < b.first; });
 

ps2xRecomp/src/lib/ps2_recompiler.cpp

@@ -2002,9 +2002,9 @@ namespace ps2recomp
             return best;
         };
 
-        // Ghidra exports standalone code blocks (static initializers, thunks, ...) as entry_* labels. Only entry_*
-        // blocks nested inside a real function are synthetic wrappers; standalone ones need their own resume
-        // points, otherwise a thread switched out inside a call from them cannot be resumed.
+        // Keep standalone entry_* external branches discoverable, as in #271.
+        // Nested wrappers only need their own continuations; their normal owner
+        // already contributes cross-function targets from the same instructions.
         std::vector<std::pair<uint32_t, uint32_t>> ownerRanges;
         for (const auto &function : m_functions)
         {
@@ -2041,11 +2041,8 @@ namespace ps2recomp
                 continue;
             }
 
-            if (isEntryFunctionName(function.name) && nestedInRealFunction(function.start))
-            {
-                continue;
-            }
-
+            // Match FunctionEmitter's resume switch for every decoded wrapper,
+            // including overlapping entry_* wrappers as well as standalone ones.
             auto decodedIt = m_decodedFunctions.find(function.start);
             if (decodedIt == m_decodedFunctions.end())
             {
@@ -2064,6 +2061,13 @@ namespace ps2recomp
                                 analysisResult.indirectFallbackEntryPoints.begin(),
                                 analysisResult.indirectFallbackEntryPoints.end());
 
+            // Only nested wrappers suppress duplicate external promotion.
+            // A standalone entry can legitimately branch into another owner.
+            if (isEntryFunctionName(function.name) && nestedInRealFunction(function.start))
+            {
+                continue;
+            }
+
             for (uint32_t target : analysisResult.externalEntryPoints)
             {
                 const Function *owner = findContainingFunction(target);

ps2xTest/src/code_generator_tests.cpp

@@ -734,6 +734,97 @@ void register_code_generator_tests()
                  "multiple resume pcs should register to the same owner wrapper");
     });
 
+    tc.Run("explicit function handlers take precedence over resume aliases", [](TestCase &t) {
+        auto function = [](const std::string &name, uint32_t start) {
+            Function fn{};
+            fn.name = name;
+            fn.start = start;
+            fn.end = start + 0x40u;
+            fn.isRecompiled = true;
+            return fn;
+        };
+        Function stub = function("synthetic_stub", 0x700cu);
+        stub.isRecompiled = false;
+        stub.isStub = true;
+        Function syscall = function("SleepThread", 0x7010u);
+        syscall.isRecompiled = false;
+        syscall.isStub = true;
+        Function library = function("synthetic_library", 0x7014u);
+        library.isRecompiled = false;
+        library.isSkipped = true;
+        const std::vector<Section> sections;
+        CodeGenerator gen({}, sections);
+        gen.setRenamedFunctions({{0x7000u, "owner"}, {0x7008u, "normal"},
+                                 {stub.start, "stub"}, {syscall.start, "syscall"},
+                                 {library.start, "library"}});
+        gen.setResumeEntryTargets({{0x7000u, {0x7004u, 0x7008u, 0x700cu,
+                                               0x7010u, 0x7014u, 0x7004u}}});
+        t.IsTrue(PS2Recompiler::resolveStubTarget(syscall.name) == StubTarget::Syscall,
+                 "the syscall collision must exercise a real runtime binding");
+        const std::string registration = gen.generateFunctionRegistration(
+            {function("entry_owner", 0x7000u), function("normal", 0x7008u),
+             stub, syscall, library}, {});
+        for (const auto &[name, address] : std::vector<std::pair<std::string, std::string>>{
+                 {"owner", "7000"}, {"owner", "7004"}, {"normal", "7008"},
+                 {"stub", "700c"}, {"syscall", "7010"}, {"library", "7014"}})
+        {
+            t.IsTrue(registration.find(" = " + name + "; // 0x" + address + "\n") != std::string::npos,
+                     "an explicit function start must retain its own binding at " + address);
+        }
+        const std::regex assignment(R"(g_ps2RecompiledFunctionTable\[[0-9]+\] = )");
+        t.Equals(std::distance(std::sregex_iterator(registration.begin(), registration.end(), assignment),
+                               std::sregex_iterator()), static_cast<std::ptrdiff_t>(6),
+                 "repeated aliases must not duplicate table assignments");
+    });
+
+    tc.Run("overlapping resume owners are deterministic and prefer normal wrappers", [](TestCase &t) {
+        const std::vector<Section> sections;
+        const std::vector<std::pair<uint32_t, std::vector<uint32_t>>> aliases{
+            {0x8000u, {0x8018u, 0x8024u}}, {0x8008u, {0x8018u, 0x8028u}},
+            {0x800cu, {0x8018u, 0x8020u, 0x8024u}}, {0x8010u, {0x8018u, 0x8020u}}};
+        std::vector<Function> functions;
+        for (const auto &[address, name] : std::vector<std::pair<uint32_t, std::string>>{
+                 {0x8000u, "normal_wide"}, {0x8008u, "normal_narrow"},
+                 {0x800cu, "entry_overlay"}, {0x8010u, "entry_narrow"}})
+        {
+            Function fn{};
+            fn.name = name;
+            fn.start = address;
+            fn.end = address + 0x40u;
+            fn.isRecompiled = true;
+            functions.push_back(fn);
+        }
+        std::string firstRegistration;
+        for (bool reverse : {false, true})
+        {
+            CodeGenerator gen({}, sections);
+            gen.setRenamedFunctions({{0x8000u, "normal_wide"}, {0x8008u, "normal_narrow"},
+                                     {0x800cu, "entry_overlay"}, {0x8010u, "entry_narrow"}});
+            std::unordered_map<uint32_t, std::vector<uint32_t>> owners;
+            for (size_t i = 0; i < aliases.size(); ++i)
+            {
+                const auto &[address, targets] = aliases[reverse ? aliases.size() - i - 1u : i];
+                owners.emplace(address, targets);
+            }
+            gen.setResumeEntryTargets(owners);
+            const std::string registration = gen.generateFunctionRegistration(functions, {});
+            for (const auto &[name, address] : std::vector<std::pair<std::string, std::string>>{
+                     {"normal_wide", "8000"}, {"normal_narrow", "8008"},
+                     {"entry_overlay", "800c"}, {"entry_narrow", "8010"},
+                     {"normal_narrow", "8018"}, {"entry_narrow", "8020"},
+                     {"normal_wide", "8024"}, {"normal_narrow", "8028"}})
+            {
+                t.IsTrue(registration.find(" = " + name + "; // 0x" + address + "\n") != std::string::npos,
+                         "normal owners precede entries, then the most specific owner wins at " + address);
+            }
+            if (reverse)
+                t.Equals(registration, firstRegistration,
+                         "unordered-map insertion order must not change generated registration");
+            else
+                firstRegistration = registration;
+        }
+    });
+
     tc.Run("configured internal guest handlers register to their owner wrapper", [](TestCase &t) {
         Function owner;
         owner.name = "sdk_bootstrap_owner";

ps2xTest/src/ps2_recompiler_tests.cpp

@@ -12,6 +12,10 @@
 #include <filesystem>
 #include <fstream>
 #include <iterator>
+#include <map>
+#include <regex>
+#include <sstream>
+#include <tuple>
 #include <unordered_map>
 #include <vector>
 
@@ -329,6 +333,62 @@ static bool writeMinimalMipsElfWithStandaloneEntryBlock(const std::filesystem::p
     return writer.save(elfPath.string());
 }
 
+static bool writeMinimalMipsElfWithOverlappingResumeOwners(const std::filesystem::path &elfPath)
+{
+    ELFIO::elfio writer;
+    writer.create(ELFIO::ELFCLASS32, ELFIO::ELFDATA2LSB);
+    writer.set_type(ELFIO::ET_EXEC);
+    writer.set_machine(ELFIO::EM_MIPS);
+    writer.set_entry(0x00100000u);
+    auto *text = writer.sections.add(".text");
+    text->set_type(ELFIO::SHT_PROGBITS);
+    text->set_flags(ELFIO::SHF_ALLOC | ELFIO::SHF_EXECINSTR);
+    text->set_addr_align(4u);
+    text->set_address(0x00100000u);
+    std::array<uint32_t, 48> words{};
+    // The normal owner can resolve this configured jump table. Its overlapping
+    // entry_100014 slice cannot see the table setup and needs fallback entries.
+    words[0] = 0x3c080020u; // lui t0,0x20
+    words[1] = 0x010a4821u; // addu t1,t0,t2
+    words[2] = 0x8d390000u; // lw t9,0(t1)
+    words[3] = 0x0c040018u; // jal 100060, resume 100014
+    words[5] = 0x0320f809u; // jalr t9, resume 10001c
+    words[7] = 0x0000000cu; // syscall, resume 100020
+    words[9] = 0x03e00008u; // jr ra
+    words[24] = 0x03e00008u; // callee at 100060
+    words[28] = 0x03e00008u; // other at 100070
+    words[32] = 0x0804001du; // standalone entry: j 100074, an unrelated owner's interior
+    words[40] = 0x0c040018u; // standalone entry: jal 100060, resume 1000a8
+    words[42] = 0x0300f809u; // jalr t8, resume 1000b0
+    words[44] = 0x0000000cu; // syscall, resume 1000b4
+    words[45] = 0x03e00008u; // jr ra
+    text->set_data(reinterpret_cast<const char *>(words.data()), sizeof(words));
+    auto *strings = writer.sections.add(".strtab");
+    strings->set_type(ELFIO::SHT_STRTAB);
+    strings->set_addr_align(1u);
+    auto *symbols = writer.sections.add(".symtab");
+    symbols->set_type(ELFIO::SHT_SYMTAB);
+    symbols->set_info(1u);
+    symbols->set_link(strings->get_index());
+    symbols->set_addr_align(4u);
+    symbols->set_entry_size(writer.get_default_entry_size(ELFIO::SHT_SYMTAB));
+    ELFIO::symbol_section_accessor accessor(writer, symbols);
+    ELFIO::string_section_accessor names(strings);
+    accessor.add_symbol(names, "", 0, 0, ELFIO::STB_LOCAL, ELFIO::STT_NOTYPE, 0, ELFIO::SHN_UNDEF);
+    for (const auto &[name, start, size] : std::vector<std::tuple<std::string, uint32_t, uint32_t>>{
+             {"normal_owner", 0x100000u, 0x40u}, {"entry_00100014", 0x100014u, 0xcu},
+             {"callee", 0x100060u, 8u},
+             {"other", 0x100070u, 8u}, {"entry_00100080", 0x100080u, 8u},
+             {"entry_001000a0", 0x1000a0u, 0x1cu}})
+        accessor.add_symbol(names, name.c_str(), start, size, ELFIO::STB_GLOBAL, ELFIO::STT_FUNC, 0, text->get_index());
+    auto *segment = writer.segments.add();
+    segment->set_type(ELFIO::PT_LOAD);
+    segment->set_flags(ELFIO::PF_R | ELFIO::PF_X);
+    segment->set_align(0x1000u);
+    segment->add_section_index(text->get_index(), text->get_addr_align());
+    return writer.save(elfPath.string());
+}
+
 static bool writeMinimalMipsElfWithAddressTakenCallbacks(const std::filesystem::path &elfPath,
                                                          bool includePartialDwarf = false)
 {
@@ -1510,6 +1570,96 @@ void register_ps2_recompiler_tests()
             std::filesystem::remove_all(tempRoot, removeError);
         });
 
+        tc.Run("all entry wrappers retain resume targets and standalone external branches", [](TestCase &t) {
+            const std::string uniqueSuffix =
+                std::to_string(std::chrono::steady_clock::now().time_since_epoch().count());
+            const auto tempRoot = std::filesystem::temp_directory_path() /
+                ("ps2recomp-resume-owners-" + uniqueSuffix);
+            for (unsigned mode = 0u; mode < 3u; ++mode)
+            {
+                const auto root = tempRoot / std::to_string(mode);
+                const auto elfPath = root / "resume-owners.elf";
+                const auto configPath = root / "resume-owners.toml";
+                const auto outputPath = root / "output";
+                std::filesystem::create_directories(root);
+                const bool written = writeMinimalMipsElfWithOverlappingResumeOwners(elfPath) &&
+                    writeRecompilerTestConfig(configPath, elfPath, outputPath, {});
+                t.IsTrue(written, "synthetic overlapping-owner inputs should be generated");
+                if (!written)
+                    continue;
+                {
+                    std::ofstream config(configPath, std::ios::app);
+                    config << "single_file_output = " << (mode == 2u ? "true" : "false") << '\n'
+                           << "output_worker_threads = " << (mode == 1u ? 2u : 1u) << '\n'
+                           << "[jump_tables]\n[[jump_tables.table]]\naddress = \"0x200000\"\n"
+                           << "entries = [{ index = 0, target = \"0x100024\" }]\n";
+                }
+                PS2Recompiler recompiler(configPath.string());
+                const bool initialized = recompiler.initialize();
+                t.IsTrue(initialized, "synthetic overlapping-owner config should initialize");
+                if (!initialized)
+                    continue;
+                const bool recompiled = recompiler.recompile();
+                t.IsTrue(recompiled, "actual discovery should decode overlapping entry wrappers");
+                if (!recompiled)
+                    continue;
+                recompiler.generateOutput();
+                auto read = [](const std::filesystem::path &path) {
+                    std::ifstream file(path);
+                    return std::string(std::istreambuf_iterator<char>(file), {});
+                };
+                const std::string registration = read(outputPath / "register_functions.cpp");
+                std::map<uint32_t, std::string> bindings;
+                const std::regex assignment(R"(g_ps2RecompiledFunctionTable\[[0-9]+\] = ([a-zA-Z0-9_]+); // 0x([0-9a-f]+))");
+                for (std::sregex_iterator i(registration.begin(), registration.end(), assignment), end; i != end; ++i)
+                {
+                    const uint32_t address = static_cast<uint32_t>(std::stoul((*i)[2].str(), nullptr, 16));
+                    t.IsTrue(bindings.emplace(address, (*i)[1].str()).second,
+                             "each generated dispatch address must have exactly one assignment");
+                }
+                std::string generated;
+                for (const auto &file : std::filesystem::directory_iterator(outputPath))
+                    if (file.path().extension() == ".cpp" && file.path().filename() != "register_functions.cpp")
+                        generated += read(file.path());
+                const auto nested = bindings.find(0x100014u);
+                t.IsTrue(nested != bindings.end() && nested->second.rfind("entry_", 0u) == 0u,
+                         "the actual pipeline must retain the nested entry wrapper");
+                if (nested != bindings.end())
+                {
+                    const auto fallback = bindings.find(0x100018u);
+                    t.IsTrue(fallback != bindings.end() && fallback->second == nested->second,
+                             "nested indirect fallback must register to its own decoded wrapper");
+                    t.IsTrue(generated.find("case 0x100018u: goto label_100018;") != std::string::npos,
+                             "nested fallback registration must have a matching emitted resume switch");
+                }
+                const auto standalone = bindings.find(0x1000a0u);
+                t.IsTrue(standalone != bindings.end(), "standalone entry support from #271 must remain intact");
+                for (uint32_t address : {0x1000a8u, 0x1000b0u, 0x1000b4u})
+                {
+                    const auto resume = bindings.find(address);
+                    t.IsTrue(standalone != bindings.end() && resume != bindings.end() &&
+                                 resume->second == standalone->second,
+                             "standalone JAL, JALR and syscall continuations must register to their owner");
+                    std::ostringstream resumeCase;
+                    resumeCase << "case 0x" << std::hex << address << "u: goto label_"
+                               << address << ';';
+                    t.IsTrue(generated.find(resumeCase.str()) != std::string::npos,
+                             "each registered standalone continuation must match its emitted resume switch");
+                }
+                const auto externalOwner = bindings.find(0x100070u);
+                const auto externalTarget = bindings.find(0x100074u);
+                t.IsTrue(externalOwner != bindings.end() && externalTarget != bindings.end() &&
+                             externalTarget->second == externalOwner->second,
+                         "a standalone entry jump must register its real external target to the containing owner");
+                t.IsTrue(generated.find("case 0x100074u: goto label_100074;") != std::string::npos,
+                         "the external jump destination must have a matching owner resume switch");
+                t.IsFalse(bindings.contains(0x1000bcu),
+                          "entry wrappers must not register a continuation outside their decoded range");
+            }
+            std::error_code removeError;
+            std::filesystem::remove_all(tempRoot, removeError);
+        });
+
         tc.Run("elf parser ignores STT_FUNC symbols in non-executable sections", [](TestCase &t) {
             const auto uniqueSuffix = std::to_string(
                 static_cast<unsigned long long>(std::chrono::steady_clock::now().time_since_epoch().count()));