From 4334e7434214281ef3d70fc0fbc0af978512bf80 Mon Sep 17 00:00:00 2001 From: Ranieri Date: Wed, 18 Feb 2026 23:34:27 -0300 Subject: [PATCH] =?UTF-8?q?fix:=20fixed=20entry=20slicing=20logic=20to=20a?= =?UTF-8?q?void=20=E2=80=9Csame=20tail=20repeated=E2=80=9D=20generation=20?= =?UTF-8?q?(#67)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ps2xRecomp/include/ps2recomp/ps2_recompiler.h | 4 + ps2xRecomp/src/lib/ps2_recompiler.cpp | 527 +++++++++++------- ps2xTest/CMakeLists.txt | 1 + ps2xTest/src/main.cpp | 2 + ps2xTest/src/ps2_recompiler_tests.cpp | 155 ++++++ 5 files changed, 488 insertions(+), 201 deletions(-) create mode 100644 ps2xTest/src/ps2_recompiler_tests.cpp diff --git a/ps2xRecomp/include/ps2recomp/ps2_recompiler.h b/ps2xRecomp/include/ps2recomp/ps2_recompiler.h index 41a29fe..fc2aa3c 100644 --- a/ps2xRecomp/include/ps2recomp/ps2_recompiler.h +++ b/ps2xRecomp/include/ps2recomp/ps2_recompiler.h @@ -32,6 +32,10 @@ namespace ps2recomp void generateOutput(); static StubTarget resolveStubTarget(const std::string& name); + static size_t DiscoverAdditionalEntryPoints( + std::vector &functions, + std::unordered_map> &decodedFunctions, + const std::vector
§ions); private: ConfigManager m_configManager; diff --git a/ps2xRecomp/src/lib/ps2_recompiler.cpp b/ps2xRecomp/src/lib/ps2_recompiler.cpp index 3cd6d02..81523fe 100644 --- a/ps2xRecomp/src/lib/ps2_recompiler.cpp +++ b/ps2xRecomp/src/lib/ps2_recompiler.cpp @@ -14,6 +14,7 @@ #include #include #include +#include namespace fs = std::filesystem; @@ -233,6 +234,283 @@ namespace ps2recomp selector.name = trimmed; return selector; } + + struct EntryDiscoveryStats + { + size_t discoveredCount = 0; + size_t passCount = 0; + }; + + EntryDiscoveryStats discoverAdditionalEntryPointsImpl( + std::vector &functions, + std::unordered_map> &decodedFunctions, + const std::vector
§ions, + const std::function &decodeExternalFunction) + { + std::unordered_set existingStarts; + for (const auto &function : functions) + { + existingStarts.insert(function.start); + } + + auto isExecutableAddress = [&](uint32_t address) -> bool + { + for (const auto §ion : sections) + { + if (!section.isCode) + { + continue; + } + if (address >= section.address && address < (section.address + section.size)) + { + return true; + } + } + return false; + }; + + auto getStaticEntryTarget = [](const Instruction &inst) -> std::optional + { + if (inst.opcode == OPCODE_J || inst.opcode == OPCODE_JAL) + { + return decodeAbsoluteJumpTarget(inst.address, inst.target); + } + + if (inst.opcode == OPCODE_SPECIAL && + (inst.function == SPECIAL_JR || inst.function == SPECIAL_JALR)) + { + return std::nullopt; + } + + return std::nullopt; + }; + + auto findContainingFunction = [&](uint32_t address) -> const Function * + { + const Function *best = nullptr; + for (const auto &function : functions) + { + if (address < function.start || address >= function.end) + { + continue; + } + + if (!function.isRecompiled || function.isStub || function.isSkipped) + { + continue; + } + + auto decodedIt = decodedFunctions.find(function.start); + if (decodedIt == decodedFunctions.end()) + { + continue; + } + + const auto &decoded = decodedIt->second; + const bool hasAddress = std::any_of(decoded.begin(), decoded.end(), + [&](const Instruction &candidate) + { return candidate.address == address; }); + if (!hasAddress) + { + continue; + } + + if (!best || function.start > best->start) + { + best = &function; + } + } + return best; + }; + + EntryDiscoveryStats stats{}; + + while (true) + { + ++stats.passCount; + struct PendingEntry + { + uint32_t target = 0; + std::optional containingStart; + uint32_t containingEnd = 0; + }; + + std::vector pendingEntries; + std::vector newEntries; + std::unordered_set pendingStarts; + + for (const auto &function : functions) + { + if (!function.isRecompiled || function.isStub || function.isSkipped) + { + continue; + } + + auto decodedIt = decodedFunctions.find(function.start); + if (decodedIt == decodedFunctions.end()) + { + continue; + } + + const auto &instructions = decodedIt->second; + + for (const auto &inst : instructions) + { + auto targetOpt = getStaticEntryTarget(inst); + if (!targetOpt.has_value()) + { + continue; + } + + uint32_t target = targetOpt.value(); + + if ((target & 0x3) != 0 || !isExecutableAddress(target)) + { + continue; + } + + if (existingStarts.contains(target) || pendingStarts.contains(target)) + { + continue; + } + + const Function *containingFunction = findContainingFunction(target); + if (containingFunction && containingFunction->start == function.start) + { + // Internal branches within the same function are handled as labels/gotos and should not produce separate entry wrappers. + continue; + } + + PendingEntry pending{}; + pending.target = target; + if (containingFunction) + { + pending.containingStart = containingFunction->start; + pending.containingEnd = containingFunction->end; + } + + pendingEntries.push_back(pending); + pendingStarts.insert(target); + } + } + + if (pendingEntries.empty()) + { + break; + } + + std::sort(pendingEntries.begin(), pendingEntries.end(), + [](const PendingEntry &a, const PendingEntry &b) + { return a.target < b.target; }); + + std::vector boundaryStarts; + boundaryStarts.reserve(existingStarts.size() + pendingStarts.size()); + boundaryStarts.insert(boundaryStarts.end(), existingStarts.begin(), existingStarts.end()); + boundaryStarts.insert(boundaryStarts.end(), pendingStarts.begin(), pendingStarts.end()); + std::sort(boundaryStarts.begin(), boundaryStarts.end()); + boundaryStarts.erase(std::unique(boundaryStarts.begin(), boundaryStarts.end()), boundaryStarts.end()); + + auto findNextBoundaryStart = [&](uint32_t address) -> std::optional + { + auto it = std::upper_bound(boundaryStarts.begin(), boundaryStarts.end(), address); + if (it == boundaryStarts.end()) + { + return std::nullopt; + } + return *it; + }; + + std::unordered_set successfulStarts; + + for (const auto &pending : pendingEntries) + { + const uint32_t target = pending.target; + + Function entryFunction; + std::stringstream name; + name << "entry_" << std::hex << target; + entryFunction.name = name.str(); + entryFunction.start = target; + entryFunction.isStub = false; + entryFunction.isSkipped = false; + entryFunction.isRecompiled = true; + + if (pending.containingStart.has_value()) + { + auto containingDecodedIt = decodedFunctions.find(*pending.containingStart); + if (containingDecodedIt == decodedFunctions.end()) + { + continue; + } + + const auto &containingInstructions = containingDecodedIt->second; + auto sliceIt = std::find_if(containingInstructions.begin(), containingInstructions.end(), + [&](const Instruction &candidate) + { return candidate.address == target; }); + + if (sliceIt == containingInstructions.end()) + { + continue; + } + + uint32_t sliceEndAddress = pending.containingEnd; + auto nextStartOpt = findNextBoundaryStart(target); + if (nextStartOpt.has_value() && nextStartOpt.value() < sliceEndAddress) + { + sliceEndAddress = nextStartOpt.value(); + } + + if (sliceEndAddress <= target) + { + continue; + } + + auto sliceEndIt = std::find_if(sliceIt, containingInstructions.end(), + [&](const Instruction &candidate) + { return candidate.address >= sliceEndAddress; }); + if (sliceEndIt == sliceIt) + { + continue; + } + + std::vector slicedInstructions(sliceIt, sliceEndIt); + decodedFunctions[target] = std::move(slicedInstructions); + entryFunction.end = sliceEndAddress; + } + else + { + auto nextStartOpt = findNextBoundaryStart(target); + if (!nextStartOpt.has_value() || nextStartOpt.value() <= target) + { + continue; + } + + entryFunction.end = nextStartOpt.value(); + if (!decodeExternalFunction(entryFunction)) + { + continue; + } + } + + newEntries.push_back(entryFunction); + successfulStarts.insert(target); + } + + if (newEntries.empty()) + { + break; + } + + stats.discoveredCount += newEntries.size(); + functions.insert(functions.end(), newEntries.begin(), newEntries.end()); + existingStarts.insert(successfulStarts.begin(), successfulStarts.end()); + std::sort(functions.begin(), functions.end(), + [](const Function &a, const Function &b) + { return a.start < b.start; }); + } + + return stats; + } } PS2Recompiler::PS2Recompiler(const std::string &configPath) @@ -481,6 +759,19 @@ namespace ps2recomp { sanitized = "func"; } + + if (sanitized.rfind("entry_", 0) == 0) + { + std::stringstream expectedStartName; + expectedStartName << "entry_" << std::hex << function.start; + if (sanitized == expectedStartName.str()) + { + std::stringstream entryName; + entryName << sanitized << "_0x" << std::hex << function.end; + return entryName.str(); + } + } + std::stringstream ss; ss << sanitized << "_0x" << std::hex << function.start; return ss.str(); @@ -784,203 +1075,18 @@ namespace ps2recomp void PS2Recompiler::discoverAdditionalEntryPoints() { - std::unordered_set existingStarts; - for (const auto &function : m_functions) + const EntryDiscoveryStats stats = discoverAdditionalEntryPointsImpl( + m_functions, + m_decodedFunctions, + m_sections, + [&](Function &entryFunction) + { return decodeFunction(entryFunction); }); + + if (stats.discoveredCount > 0) { - existingStarts.insert(function.start); - } - - auto getStaticEntryTarget = [](const Instruction &inst) -> std::optional - { - if (inst.opcode == OPCODE_J || inst.opcode == OPCODE_JAL) - { - return decodeAbsoluteJumpTarget(inst.address, inst.target); - } - - if (inst.opcode == OPCODE_SPECIAL && - (inst.function == SPECIAL_JR || inst.function == SPECIAL_JALR)) - { - return std::nullopt; - } - - return std::nullopt; - }; - - auto findContainingFunction = [&](uint32_t address) -> const Function * - { - const Function *best = nullptr; - for (const auto &function : m_functions) - { - if (address < function.start || address >= function.end) - { - continue; - } - - if (!function.isRecompiled || function.isStub || function.isSkipped) - { - continue; - } - - auto decodedIt = m_decodedFunctions.find(function.start); - if (decodedIt == m_decodedFunctions.end()) - { - continue; - } - - const auto &decoded = decodedIt->second; - const bool hasAddress = std::any_of(decoded.begin(), decoded.end(), - [&](const Instruction &candidate) - { return candidate.address == address; }); - if (!hasAddress) - { - continue; - } - - if (!best || function.start > best->start) - { - best = &function; - } - } - return best; - }; - - auto findNextFunctionStart = [&](uint32_t address) -> std::optional - { - uint32_t nextStart = std::numeric_limits::max(); - for (const auto &function : m_functions) - { - if (function.start > address && function.start < nextStart) - { - nextStart = function.start; - } - } - - if (nextStart == std::numeric_limits::max()) - { - return std::nullopt; - } - - return nextStart; - }; - - size_t totalDiscovered = 0; - size_t passCount = 0; - - while (true) - { - ++passCount; - std::vector newEntries; - - for (const auto &function : m_functions) - { - if (!function.isRecompiled || function.isStub || function.isSkipped) - { - continue; - } - - auto decodedIt = m_decodedFunctions.find(function.start); - if (decodedIt == m_decodedFunctions.end()) - { - continue; - } - - const auto &instructions = decodedIt->second; - - for (const auto &inst : instructions) - { - auto targetOpt = getStaticEntryTarget(inst); - if (!targetOpt.has_value()) - { - continue; - } - - uint32_t target = targetOpt.value(); - - if ((target & 0x3) != 0 || !m_elfParser->isValidAddress(target)) - { - continue; - } - - if (existingStarts.contains(target)) - { - continue; - } - - const Function *containingFunction = findContainingFunction(target); - if (containingFunction && containingFunction->start == function.start) - { - // Internal branches within the same function are handled as labels/gotos and should not produce separate entry wrappers. - continue; - } - - Function entryFunction; - std::stringstream name; - name << "entry_" << std::hex << target; - entryFunction.name = name.str(); - entryFunction.start = target; - entryFunction.isStub = false; - entryFunction.isSkipped = false; - entryFunction.isRecompiled = true; - - if (containingFunction) - { - auto containingDecodedIt = m_decodedFunctions.find(containingFunction->start); - if (containingDecodedIt == m_decodedFunctions.end()) - { - continue; - } - - const auto &containingInstructions = containingDecodedIt->second; - auto sliceIt = std::find_if(containingInstructions.begin(), containingInstructions.end(), - [&](const Instruction &candidate) - { return candidate.address == target; }); - - if (sliceIt == containingInstructions.end()) - { - continue; - } - - std::vector slicedInstructions(sliceIt, containingInstructions.end()); - m_decodedFunctions[target] = slicedInstructions; - entryFunction.end = containingFunction->end; - } - else - { - auto nextStartOpt = findNextFunctionStart(target); - if (!nextStartOpt.has_value() || nextStartOpt.value() <= target) - { - continue; - } - - entryFunction.end = nextStartOpt.value(); - if (!decodeFunction(entryFunction)) - { - continue; - } - } - - newEntries.push_back(entryFunction); - existingStarts.insert(target); - } - } - - if (newEntries.empty()) - { - break; - } - - totalDiscovered += newEntries.size(); - m_functions.insert(m_functions.end(), newEntries.begin(), newEntries.end()); - std::sort(m_functions.begin(), m_functions.end(), - [](const Function &a, const Function &b) - { return a.start < b.start; }); - } - - if (totalDiscovered > 0) - { - std::cout << "Discovered " << totalDiscovered + std::cout << "Discovered " << stats.discoveredCount << " additional entry point(s) inside existing functions across " - << passCount << " pass(es)." << std::endl; + << stats.passCount << " pass(es)." << std::endl; } } @@ -1108,10 +1214,12 @@ namespace ps2recomp std::filesystem::path PS2Recompiler::getOutputPath(const Function &function) const { std::string safeName; + bool usedRenamedName = false; auto renameIt = m_functionRenames.find(function.start); if (renameIt != m_functionRenames.end() && !renameIt->second.empty()) { safeName = renameIt->second; + usedRenamedName = true; } else { @@ -1130,13 +1238,16 @@ namespace ps2recomp safeName = ss.str(); } - std::stringstream suffix; - suffix << "_0x" << std::hex << function.start; - const std::string suffixText = suffix.str(); - if (safeName.size() < suffixText.size() || - safeName.compare(safeName.size() - suffixText.size(), suffixText.size(), suffixText) != 0) + if (!usedRenamedName) { - safeName += suffixText; + std::stringstream suffix; + suffix << "_0x" << std::hex << function.start; + const std::string suffixText = suffix.str(); + if (safeName.size() < suffixText.size() || + safeName.compare(safeName.size() - suffixText.size(), suffixText.size(), suffixText) != 0) + { + safeName += suffixText; + } } std::filesystem::path outputPath = m_config.outputPath; @@ -1166,6 +1277,20 @@ namespace ps2recomp return sanitized; } + size_t PS2Recompiler::DiscoverAdditionalEntryPoints( + std::vector &functions, + std::unordered_map> &decodedFunctions, + const std::vector
§ions) + { + const EntryDiscoveryStats stats = discoverAdditionalEntryPointsImpl( + functions, + decodedFunctions, + sections, + [](Function &) + { return false; }); + return stats.discoveredCount; + } + StubTarget PS2Recompiler::resolveStubTarget(const std::string &name) { if (!ps2_runtime_calls::resolveSyscallName(name).empty()) diff --git a/ps2xTest/CMakeLists.txt b/ps2xTest/CMakeLists.txt index d91cadc..2cd08bb 100644 --- a/ps2xTest/CMakeLists.txt +++ b/ps2xTest/CMakeLists.txt @@ -11,6 +11,7 @@ add_executable(ps2x_tests src/r5900_decoder_tests.cpp src/elf_analyzer_tests.cpp src/ps2_runtime_io_tests.cpp + src/ps2_recompiler_tests.cpp ) option(PRINT_GENERATED_CODE "Print generated code in tests" OFF) diff --git a/ps2xTest/src/main.cpp b/ps2xTest/src/main.cpp index 940b0da..9c65451 100644 --- a/ps2xTest/src/main.cpp +++ b/ps2xTest/src/main.cpp @@ -4,6 +4,7 @@ void register_code_generator_tests(); void register_r5900_decoder_tests(); void register_elf_analyzer_tests(); void register_ps2_runtime_io_tests(); +void register_ps2_recompiler_tests(); int main() { @@ -11,5 +12,6 @@ int main() register_r5900_decoder_tests(); register_elf_analyzer_tests(); register_ps2_runtime_io_tests(); + register_ps2_recompiler_tests(); return MiniTest::Run(); } diff --git a/ps2xTest/src/ps2_recompiler_tests.cpp b/ps2xTest/src/ps2_recompiler_tests.cpp new file mode 100644 index 0000000..dd8149b --- /dev/null +++ b/ps2xTest/src/ps2_recompiler_tests.cpp @@ -0,0 +1,155 @@ +#include "MiniTest.h" +#include "ps2recomp/ps2_recompiler.h" +#include "ps2recomp/instructions.h" +#include "ps2recomp/types.h" +#include +#include +#include + +using namespace ps2recomp; + +static Instruction makeNopLike(uint32_t address) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = OPCODE_ADDIU; + inst.rt = 0; + inst.raw = 0; + return inst; +} + +static Instruction makeAbsJump(uint32_t address, uint32_t target, uint32_t opcode) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = opcode; + inst.target = (target >> 2) & 0x03FFFFFFu; + inst.hasDelaySlot = true; + inst.raw = (opcode << 26) | inst.target; + return inst; +} + +static Function makeFunction(const std::string &name, uint32_t start, uint32_t end) +{ + Function fn{}; + fn.name = name; + fn.start = start; + fn.end = end; + fn.isRecompiled = true; + fn.isStub = false; + fn.isSkipped = false; + return fn; +} + +void register_ps2_recompiler_tests() +{ + MiniTest::Case("PS2Recompiler", [](TestCase &tc) + { + tc.Run("additional entries split at nearest discovered boundary", [](TestCase &t) { + std::vector
sections = { + {".text", 0x1000u, 0x3000u, 0u, true, false, false, true, nullptr} + }; + + std::vector functions = { + makeFunction("container", 0x1000u, 0x1018u), + makeFunction("caller", 0x2000u, 0x2010u) + }; + + std::unordered_map> decodedFunctions; + decodedFunctions[0x1000u] = { + makeNopLike(0x1000u), + makeNopLike(0x1004u), + makeNopLike(0x1008u), + makeNopLike(0x100Cu), + makeNopLike(0x1010u), + makeNopLike(0x1014u) + }; + decodedFunctions[0x2000u] = { + makeAbsJump(0x2000u, 0x1008u, OPCODE_JAL), + makeNopLike(0x2004u), + makeAbsJump(0x2008u, 0x100Cu, OPCODE_J), + makeNopLike(0x200Cu) + }; + + size_t discovered = PS2Recompiler::DiscoverAdditionalEntryPoints( + functions, decodedFunctions, sections); + + t.Equals(discovered, static_cast(2), + "expected two additional entries to be discovered"); + + auto findByStart = [&](uint32_t start) -> const Function* { + auto it = std::find_if(functions.begin(), functions.end(), + [&](const Function &fn) { return fn.start == start; }); + if (it == functions.end()) + { + return nullptr; + } + return &(*it); + }; + + const Function *entry1008 = findByStart(0x1008u); + const Function *entry100C = findByStart(0x100Cu); + t.IsNotNull(entry1008, "entry at 0x1008 should exist"); + t.IsNotNull(entry100C, "entry at 0x100C should exist"); + if (entry1008 && entry100C) + { + t.Equals(entry1008->end, 0x100Cu, + "entry 0x1008 should end at nearest discovered start 0x100C"); + t.Equals(entry100C->end, 0x1018u, + "entry 0x100C should end at containing function end"); + } + + auto decoded1008It = decodedFunctions.find(0x1008u); + auto decoded100CIt = decodedFunctions.find(0x100Cu); + t.IsTrue(decoded1008It != decodedFunctions.end(), "decoded slice for 0x1008 should exist"); + t.IsTrue(decoded100CIt != decodedFunctions.end(), "decoded slice for 0x100C should exist"); + if (decoded1008It != decodedFunctions.end()) + { + t.Equals(decoded1008It->second.size(), static_cast(1), + "entry 0x1008 slice should stop before 0x100C"); + if (!decoded1008It->second.empty()) + { + t.Equals(decoded1008It->second.front().address, 0x1008u, + "entry 0x1008 slice should begin at 0x1008"); + } + } + if (decoded100CIt != decodedFunctions.end() && !decoded100CIt->second.empty()) + { + t.Equals(decoded100CIt->second.front().address, 0x100Cu, + "entry 0x100C slice should begin at 0x100C"); + } + }); + + tc.Run("non-executable section targets are ignored", [](TestCase &t) { + std::vector
sections = { + {".text", 0x1000u, 0x2000u, 0u, true, false, false, true, nullptr}, + {".data", 0x3000u, 0x1000u, 0u, false, true, false, false, nullptr} + }; + + std::vector functions = { + makeFunction("data_container", 0x3000u, 0x3010u), + makeFunction("caller", 0x1800u, 0x1810u) + }; + + std::unordered_map> decodedFunctions; + decodedFunctions[0x3000u] = { + makeNopLike(0x3000u), + makeNopLike(0x3004u), + makeNopLike(0x3008u), + makeNopLike(0x300Cu) + }; + decodedFunctions[0x1800u] = { + makeAbsJump(0x1800u, 0x3004u, OPCODE_J), + makeNopLike(0x1804u) + }; + + size_t discovered = PS2Recompiler::DiscoverAdditionalEntryPoints( + functions, decodedFunctions, sections); + t.Equals(discovered, static_cast(0), + "non-executable targets should not produce additional entries"); + + const bool hasDataEntry = std::any_of(functions.begin(), functions.end(), + [](const Function &fn) { return fn.start == 0x3004u; }); + t.IsFalse(hasDataEntry, "target in data section must not produce entry wrapper"); + }); }); +}