From a5e4a6a8b8d3486aa1a230146882cb70bf0e66bb Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Mon, 6 Jul 2026 21:59:40 -0400 Subject: [PATCH 1/7] feat(recomp): discover packed jal-only entries via cross-unit manifests and thread-entry scan A callee reached only by a jal/j immediate from a caller outside the byte range grouped into one function unit is disassembled as unreachable tail code inside whichever unit contains it, but gets no callable entry of its own. At runtime lookupFunction() reports it not-found and the recovery path silently substitutes broken behavior instead of failing loudly. The existing post-pass discoverAdditionalEntryPoints() already handles same-invocation cross-function targets via resume mapping; two forms survived. Part A - cross-compilation-unit / overlay callers: each invocation now emits external_call_targets.txt (jal/j targets in an executable section but outside its own recompiled ranges) into the output directory, and a new [general] external_call_target_manifests config array ingests manifests produced by other invocations. Ingested targets that land inside this invocation's function ranges are registered through the existing resume-entry mapping (m_resumeEntryTargetsByOwner), so lookupFunction(target) dispatches into the owner unit at the right pc. The emit-selection logic is a public static (CollectExternalCallTargets) for unit testing. Part B - data-embedded thread entries: a new analysis pass locates CreateThread (syscall 0x20) call sites (direct syscall or jal to a libkernel wrapper), recovers the static ThreadParam pointer in $a0 via a bounded lui/addiu/ori constant walk (including the jal delay slot), reads the entry function pointer from ELF data at param+4, and registers entries that fall inside recompiled function ranges via the same resume mapping. Scope is CreateThread-with-immediate-ThreadParam only: StartThread (0x22), runtime-computed param pointers, and handler-registration variants are not covered. Both discovery paths feed the existing registration machinery; no runtime or emitter behavior changes. Includes unit tests for manifest parsing, the external-target selection helper, and the thread-entry discovery helper, plus end-to-end regression tests that drive the real recompile pipeline and assert registration through the emitted function table. --- README.md | 1 + ps2xRecomp/include/ps2recomp/ps2_recompiler.h | 26 + ps2xRecomp/include/ps2recomp/types.h | 1 + ps2xRecomp/src/lib/config_manager.cpp | 15 + ps2xRecomp/src/lib/ps2_recompiler.cpp | 475 +++++++++ ps2xTest/src/ps2_recompiler_tests.cpp | 972 +++++++++++++++++- 6 files changed, 1488 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b9d3ac840..7022aa7cd 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,7 @@ Main fields in `config.toml`: * `general.patch_cache`: apply configured patches to CACHE instructions. * `general.stubs`: names to force as stubs. Also accepts `handler@0xADDRESS` to bind a stripped function address directly to a runtime syscall/stub handler. Includes generic handlers `ret0`, `ret1`, `reta0`. * `general.skip`: names to force as skipped wrappers. +* `general.external_call_target_manifests`: optional array of `external_call_targets.txt` paths emitted by other recompile invocations (e.g. a separately recompiled overlay); their call targets that land inside this invocation's functions are registered as additional entry points. Each invocation writes its own `external_call_targets.txt` into `general.output`. * `patches.instructions`: raw instruction replacements by address. Address binding for stripped ELFs: diff --git a/ps2xRecomp/include/ps2recomp/ps2_recompiler.h b/ps2xRecomp/include/ps2recomp/ps2_recompiler.h index 4ffeaa0d1..97fc4b64f 100644 --- a/ps2xRecomp/include/ps2recomp/ps2_recompiler.h +++ b/ps2xRecomp/include/ps2recomp/ps2_recompiler.h @@ -11,6 +11,8 @@ #include #include #include +#include +#include namespace ps2recomp { @@ -46,6 +48,27 @@ namespace ps2recomp static std::string ClampFilenameLength(const std::string& baseName, const std::string& extension, std::size_t maxLength); + // Parses a call-target manifest (one "0x%08x" address per line, blank lines and + // '#' comments ignored) into a sorted, de-duplicated list of addresses. Shared by + // the production manifest loader and unit tests. + static std::vector ParseCallTargetManifest(std::istream &input); + + // Discovers thread entry function pointers embedded in static ThreadParam + // structs passed to the CreateThread syscall (0x20). Returns the entry-pointer + // values read from data memory, sorted and de-duplicated. + static std::vector DiscoverDataEmbeddedThreadEntries( + const std::unordered_map> &decodedFunctions, + const std::function &isValidAddress, + const std::function &readWord); + + // Collects jal/j targets that fall in executable sections but outside every + // recompiled local function range - candidate cross-unit call targets to + // publish in the external call-target manifest. Sorted and de-duplicated. + static std::vector CollectExternalCallTargets( + const std::unordered_map> &decodedFunctions, + const std::vector &functions, + const std::vector
§ions); + private: ConfigManager m_configManager; std::unique_ptr m_elfParser; @@ -68,10 +91,13 @@ namespace ps2recomp std::map m_generatedStubs; std::unordered_map m_functionRenames; std::unordered_map> m_resumeEntryTargetsByOwner; + std::vector m_ingestedExternalCallTargets; CodeGenerator::BootstrapInfo m_bootstrapInfo; bool decodeFunction(Function &function); void discoverAdditionalEntryPoints(); + void loadExternalCallTargetManifests(); + void emitExternalCallTargetManifest(); bool shouldSkipFunction(const Function &function) const; bool isStubFunction(const Function &function) const; bool generateFunctionHeader(); diff --git a/ps2xRecomp/include/ps2recomp/types.h b/ps2xRecomp/include/ps2recomp/types.h index 7e03280bd..b46bf5aa6 100644 --- a/ps2xRecomp/include/ps2recomp/types.h +++ b/ps2xRecomp/include/ps2recomp/types.h @@ -183,6 +183,7 @@ namespace ps2recomp std::vector stubImplementations; std::unordered_map mmioByInstructionAddress; std::vector jumpTables; + std::vector externalCallTargetManifests; }; } // namespace ps2recomp diff --git a/ps2xRecomp/src/lib/config_manager.cpp b/ps2xRecomp/src/lib/config_manager.cpp index b6f382d98..b5c2c9983 100644 --- a/ps2xRecomp/src/lib/config_manager.cpp +++ b/ps2xRecomp/src/lib/config_manager.cpp @@ -83,6 +83,17 @@ namespace ps2recomp config.skipFunctions = toml::find>(data, "skip"); } + if (general.contains("external_call_target_manifests") && general.at("external_call_target_manifests").is_array()) + { + config.externalCallTargetManifests = + toml::find>(general, "external_call_target_manifests"); + } + else if (data.contains("external_call_target_manifests") && data.at("external_call_target_manifests").is_array()) + { + config.externalCallTargetManifests = + toml::find>(data, "external_call_target_manifests"); + } + if (data.contains("patches") && data.at("patches").is_table()) { const auto &patches = toml::find(data, "patches"); @@ -276,6 +287,10 @@ namespace ps2recomp general["patch_cache"] = config.patchCache; general["skip"] = config.skipFunctions; general["stubs"] = config.stubImplementations; + if (!config.externalCallTargetManifests.empty()) + { + general["external_call_target_manifests"] = config.externalCallTargetManifests; + } data["general"] = general; if (!config.mmioByInstructionAddress.empty()) diff --git a/ps2xRecomp/src/lib/ps2_recompiler.cpp b/ps2xRecomp/src/lib/ps2_recompiler.cpp index df7e356da..44725dfc9 100644 --- a/ps2xRecomp/src/lib/ps2_recompiler.cpp +++ b/ps2xRecomp/src/lib/ps2_recompiler.cpp @@ -3,10 +3,12 @@ #include "ps2recomp/types.h" #include "ps2recomp/elf_parser.h" #include "ps2recomp/r5900_decoder.h" +#include "ps2recomp/control_flow_utils.h" #include "ps2_runtime_calls.h" #include #include #include +#include #include #include #include @@ -982,6 +984,7 @@ namespace ps2recomp #endif } + loadExternalCallTargetManifests(); discoverAdditionalEntryPoints(); if (failedCount > 0) @@ -1714,6 +1717,125 @@ namespace ps2recomp } } + void PS2Recompiler::loadExternalCallTargetManifests() + { + m_ingestedExternalCallTargets.clear(); + + std::vector merged; + for (const auto &manifestPath : m_config.externalCallTargetManifests) + { + std::ifstream manifestFile(manifestPath); + if (!manifestFile) + { + m_reporter.warning("external-call-targets", "Failed to open manifest for reading: " + manifestPath); + continue; + } + + const std::vector parsed = ParseCallTargetManifest(manifestFile); + merged.insert(merged.end(), parsed.begin(), parsed.end()); + + std::ostringstream msg; + msg << "ingested " << parsed.size() << " external call target(s) from " << manifestPath; + m_reporter.progress(msg.str()); + } + + std::sort(merged.begin(), merged.end()); + merged.erase(std::unique(merged.begin(), merged.end()), merged.end()); + m_ingestedExternalCallTargets = std::move(merged); + } + + std::vector PS2Recompiler::CollectExternalCallTargets( + const std::unordered_map> &decodedFunctions, + const std::vector &functions, + const std::vector
§ions) + { + std::vector externalTargets; + + auto isInsideExecutableSection = [&](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 isInsideRecompiledFunction = [&](uint32_t address) -> bool + { + for (const auto &function : functions) + { + if (!function.isRecompiled || function.isStub || function.isSkipped) + { + continue; + } + + if (address >= function.start && address < function.end) + { + return true; + } + } + return false; + }; + + for (const auto &[functionStart, instructions] : decodedFunctions) + { + for (const auto &inst : instructions) + { + if (inst.opcode != OPCODE_JAL && inst.opcode != OPCODE_J) + { + continue; + } + + const uint32_t target = buildAbsoluteJumpTarget(inst.address, inst.target); + if (!isInsideExecutableSection(target)) + { + continue; + } + + if (isInsideRecompiledFunction(target)) + { + continue; + } + + externalTargets.push_back(target); + } + } + + std::sort(externalTargets.begin(), externalTargets.end()); + externalTargets.erase(std::unique(externalTargets.begin(), externalTargets.end()), externalTargets.end()); + + return externalTargets; + } + + void PS2Recompiler::emitExternalCallTargetManifest() + { + const std::vector externalTargets = + CollectExternalCallTargets(m_decodedFunctions, m_functions, m_sections); + + std::ostringstream ss; + for (uint32_t target : externalTargets) + { + ss << "0x" << std::hex << std::setw(8) << std::setfill('0') << target << std::dec << "\n"; + } + + const fs::path manifestPath = fs::path(m_config.outputPath) / "external_call_targets.txt"; + writeToFile(manifestPath.string(), ss.str()); + + { + std::ostringstream msg; + msg << "wrote " << externalTargets.size() << " candidate cross-unit call target(s) to " << manifestPath; + m_reporter.progress(msg.str()); + } + } + void PS2Recompiler::discoverAdditionalEntryPoints() { m_resumeEntryTargetsByOwner.clear(); @@ -1813,6 +1935,64 @@ namespace ps2recomp } } + for (uint32_t target : m_ingestedExternalCallTargets) + { + const Function *owner = findContainingFunction(target); + if (!owner) + { + continue; + } + + if (owner->start == target) + { + continue; + } + + auto &targets = m_resumeEntryTargetsByOwner[owner->start]; + targets.push_back(target); + } + + if (m_elfParser) + { + try + { + const std::vector threadEntries = DiscoverDataEmbeddedThreadEntries( + m_decodedFunctions, + [this](uint32_t address) { return m_elfParser->isValidAddress(address); }, + [this](uint32_t address) { return m_elfParser->readWord(address); }); + + size_t mappedThreadEntries = 0u; + for (uint32_t target : threadEntries) + { + const Function *owner = findContainingFunction(target); + if (!owner) + { + continue; + } + + if (owner->start == target) + { + continue; + } + + auto &targets = m_resumeEntryTargetsByOwner[owner->start]; + targets.push_back(target); + ++mappedThreadEntries; + } + + if (mappedThreadEntries > 0u) + { + std::ostringstream msg; + msg << "mapped " << mappedThreadEntries << " data-embedded thread entry point(s)"; + m_reporter.progress(msg.str()); + } + } + catch (const std::exception &ex) + { + m_reporter.warning("thread-entries", std::string("failed to discover data-embedded thread entries: ") + ex.what()); + } + } + size_t totalTargets = 0u; for (auto it = m_resumeEntryTargetsByOwner.begin(); it != m_resumeEntryTargetsByOwner.end();) { @@ -1841,6 +2021,8 @@ namespace ps2recomp << " owner function(s)"; m_reporter.progress(msg.str()); } + + emitExternalCallTargetManifest(); } bool PS2Recompiler::decodeFunction(Function &function) @@ -2120,4 +2302,297 @@ namespace ps2recomp { return clampFilenameLength(baseName, extension, maxLength); } + + std::vector PS2Recompiler::ParseCallTargetManifest(std::istream &input) + { + std::vector targets; + std::string line; + + while (std::getline(input, line)) + { + const size_t firstNonSpace = line.find_first_not_of(" \t\r\n"); + if (firstNonSpace == std::string::npos) + { + continue; + } + + if (line[firstNonSpace] == '#') + { + continue; + } + + try + { + uint32_t target = static_cast(std::stoul(line.substr(firstNonSpace), nullptr, 0)); + targets.push_back(target); + } + catch (const std::exception &) + { + continue; + } + } + + std::sort(targets.begin(), targets.end()); + targets.erase(std::unique(targets.begin(), targets.end()), targets.end()); + return targets; + } + + namespace + { + constexpr uint32_t kSyscallCreateThreadNumber = 0x20u; + constexpr uint32_t kThreadEntryRegV1 = 3u; + constexpr uint32_t kThreadEntryRegA0 = 4u; + constexpr size_t kThreadWrapperScanWindow = 8u; + constexpr size_t kThreadSyscallBackScanWindow = 8u; + constexpr size_t kThreadConstantScanWindow = 12u; + + bool isAddiuV1SyscallImm(const Instruction &inst) + { + return inst.opcode == OPCODE_ADDIU && inst.rt == kThreadEntryRegV1 && inst.rs == 0u && + (inst.immediate & 0xFFFFu) == kSyscallCreateThreadNumber; + } + + bool isSyscallInstruction(const Instruction &inst) + { + return inst.opcode == OPCODE_SPECIAL && inst.function == SPECIAL_SYSCALL; + } + + // Simplified "who writes this register" model shared by the $v1 clobber check + // and the $a0 constant-propagation walk below: for I-type opcodes the written + // register is rt, for OPCODE_SPECIAL it is rd. $zero is never considered written. + // J-type instructions (j/jal) are excluded: the decoder unconditionally populates + // rs/rt/rd from the raw instruction bits (see R5900Decoder::decodeInstruction), + // but for a 26-bit jump target those bit positions are part of the target, not a + // register field, so treating them as a register write would be spurious. + uint32_t writtenRegisterOrZero(const Instruction &inst) + { + if (inst.opcode == OPCODE_J || inst.opcode == OPCODE_JAL) + { + return 0u; + } + if (inst.opcode == OPCODE_SPECIAL) + { + return inst.rd; + } + return inst.rt; + } + + bool writesTrackedRegister(const Instruction &inst, uint32_t reg) + { + if (reg == 0u) + { + return false; + } + return writtenRegisterOrZero(inst) == reg; + } + + struct ThreadCreateCallSite + { + const std::vector *instructions; + size_t index; + bool isJal; + }; + } + + std::vector PS2Recompiler::DiscoverDataEmbeddedThreadEntries( + const std::unordered_map> &decodedFunctions, + const std::function &isValidAddress, + const std::function &readWord) + { + std::vector results; + + // Step 1: identify CreateThread (syscall 0x20) wrapper function entry points - + // functions whose first few instructions set $v1 = 0x20 and execute a syscall. + std::unordered_set wrapperStarts; + for (const auto &[functionStart, instructions] : decodedFunctions) + { + const size_t window = std::min(instructions.size(), kThreadWrapperScanWindow); + bool sawAddiuV1Syscall = false; + bool isWrapper = false; + for (size_t idx = 0; idx < window; ++idx) + { + const Instruction &inst = instructions[idx]; + if (isAddiuV1SyscallImm(inst)) + { + sawAddiuV1Syscall = true; + continue; + } + if (sawAddiuV1Syscall && isSyscallInstruction(inst)) + { + isWrapper = true; + break; + } + } + + if (isWrapper) + { + wrapperStarts.insert(functionStart); + } + } + + // Step 2: find CreateThread invocation sites - either a jal to a wrapper found + // above, or a direct inline syscall preceded (within a small window, with no + // intervening write to $v1) by addiu $v1, $zero, 0x20. + std::vector callSites; + for (const auto &[functionStart, instructions] : decodedFunctions) + { + (void)functionStart; + for (size_t idx = 0; idx < instructions.size(); ++idx) + { + const Instruction &inst = instructions[idx]; + + if (inst.opcode == OPCODE_JAL) + { + const uint32_t target = buildAbsoluteJumpTarget(inst.address, inst.target); + if (wrapperStarts.count(target) != 0u) + { + callSites.push_back(ThreadCreateCallSite{&instructions, idx, true}); + } + continue; + } + + if (!isSyscallInstruction(inst)) + { + continue; + } + + // Note: this back-scan can double-attribute a single `addiu $v1,$zero,0x20` + // write to two syscall instructions if both sit within the scan window with no + // intervening $v1 clobber. That is harmless here: the worst case is one extra + // in-range candidate entry, which is de-duplicated downstream (results are sorted + // and uniqued, and the resume-target map dedups per owner), so no logic change. + const size_t lowerBound = (idx >= kThreadSyscallBackScanWindow) ? idx - kThreadSyscallBackScanWindow : 0u; + bool foundAddiuV1 = false; + for (size_t p = idx; p > lowerBound;) + { + --p; + const Instruction &prior = instructions[p]; + if (isAddiuV1SyscallImm(prior)) + { + foundAddiuV1 = true; + break; + } + if (writesTrackedRegister(prior, kThreadEntryRegV1)) + { + break; + } + } + + if (foundAddiuV1) + { + callSites.push_back(ThreadCreateCallSite{&instructions, idx, false}); + } + } + } + + // Step 3 + 4: recover the static $a0 (ThreadParam*) value at each call site by + // walking a small constant-propagation window forward, then read the embedded + // entry function pointer (word offset +4 in the ThreadParam struct) from ELF data. + for (const ThreadCreateCallSite &site : callSites) + { + const std::vector &instructions = *site.instructions; + + size_t windowEnd; + if (site.isJal) + { + windowEnd = (site.index + 1u < instructions.size()) ? site.index + 1u : site.index; + } + else + { + if (site.index == 0u) + { + continue; + } + windowEnd = site.index - 1u; + } + const size_t windowStart = + (site.index >= kThreadConstantScanWindow) ? site.index - kThreadConstantScanWindow : 0u; + + std::unordered_map resolved; + auto invalidate = [&resolved](uint32_t reg) + { + if (reg != 0u) + { + resolved.erase(reg); + } + }; + + for (size_t idx = windowStart; idx <= windowEnd; ++idx) + { + const Instruction &inst = instructions[idx]; + + if (inst.opcode == OPCODE_LUI) + { + if (inst.rt != 0u) + { + resolved[inst.rt] = (inst.immediate & 0xFFFFu) << 16; + } + continue; + } + + if (inst.opcode == OPCODE_ADDIU) + { + auto srcIt = resolved.find(inst.rs); + if (inst.rs != 0u && srcIt != resolved.end()) + { + const int32_t lo = static_cast(static_cast(inst.immediate & 0xFFFFu)); + if (inst.rt != 0u) + { + resolved[inst.rt] = static_cast(static_cast(srcIt->second) + lo); + } + continue; + } + } + else if (inst.opcode == OPCODE_ORI) + { + auto srcIt = resolved.find(inst.rs); + if (inst.rs != 0u && srcIt != resolved.end()) + { + const uint32_t lo = inst.immediate & 0xFFFFu; + if (inst.rt != 0u) + { + resolved[inst.rt] = srcIt->second | lo; + } + continue; + } + } + + invalidate(writtenRegisterOrZero(inst)); + } + + auto a0It = resolved.find(kThreadEntryRegA0); + if (a0It == resolved.end()) + { + continue; + } + + const uint32_t paramAddress = a0It->second; + if (!isValidAddress(paramAddress) || !isValidAddress(paramAddress + 4u)) + { + continue; + } + + uint32_t entry = 0u; + try + { + entry = readWord(paramAddress + 4u); + } + catch (const std::exception &) + { + // A section can satisfy isValidAddress's byte-range check yet still have + // no backing data (e.g. BSS), in which case readWord throws. Skip this + // candidate rather than losing the whole discovery pass. + continue; + } + + if (entry != 0u) + { + results.push_back(entry); + } + } + + std::sort(results.begin(), results.end()); + results.erase(std::unique(results.begin(), results.end()), results.end()); + return results; + } } diff --git a/ps2xTest/src/ps2_recompiler_tests.cpp b/ps2xTest/src/ps2_recompiler_tests.cpp index 3980bde44..9669a8436 100644 --- a/ps2xTest/src/ps2_recompiler_tests.cpp +++ b/ps2xTest/src/ps2_recompiler_tests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,28 @@ static Instruction makeAbsJump(uint32_t address, uint32_t target, uint32_t opcod return inst; } +// Mirrors R5900Decoder::decodeInstruction, which unconditionally populates +// rs/rt/rd/immediate from the raw instruction bits regardless of instruction +// format. For a J-type instruction (j/jal) those bit positions are actually +// part of the 26-bit jump target, not real register fields, so a naive "does +// this instruction write rt" check can alias against the jal's own encoded +// target. Used to reproduce that scenario in tests. +static Instruction makeAbsJumpDecoded(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; + inst.rs = RS(inst.raw); + inst.rt = RT(inst.raw); + inst.rd = RD(inst.raw); + inst.immediate = IMMEDIATE(inst.raw); + inst.simmediate = static_cast(SIMMEDIATE(inst.raw)); + return inst; +} + static Instruction makeJrRa(uint32_t address) { Instruction inst{}; @@ -49,6 +72,64 @@ static Instruction makeJrRa(uint32_t address) return inst; } +static Instruction makeLui(uint32_t address, uint32_t rt, uint32_t imm) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = OPCODE_LUI; + inst.rt = rt; + inst.immediate = imm & 0xFFFFu; + inst.raw = (OPCODE_LUI << 26) | (rt << 16) | (imm & 0xFFFFu); + return inst; +} + +static Instruction makeAddiu(uint32_t address, uint32_t rt, uint32_t rs, uint32_t imm) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = OPCODE_ADDIU; + inst.rt = rt; + inst.rs = rs; + inst.immediate = imm & 0xFFFFu; + inst.simmediate = static_cast(static_cast(static_cast(imm & 0xFFFFu))); + inst.raw = (OPCODE_ADDIU << 26) | (rs << 21) | (rt << 16) | (imm & 0xFFFFu); + return inst; +} + +static Instruction makeOri(uint32_t address, uint32_t rt, uint32_t rs, uint32_t imm) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = OPCODE_ORI; + inst.rt = rt; + inst.rs = rs; + inst.immediate = imm & 0xFFFFu; + inst.raw = (OPCODE_ORI << 26) | (rs << 21) | (rt << 16) | (imm & 0xFFFFu); + return inst; +} + +static Instruction makeSyscall(uint32_t address) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = OPCODE_SPECIAL; + inst.function = SPECIAL_SYSCALL; + inst.raw = SPECIAL_SYSCALL; + return inst; +} + +static Instruction makeLw(uint32_t address, uint32_t rt, uint32_t rs) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = OPCODE_LW; + inst.rt = rt; + inst.rs = rs; + inst.isLoad = true; + inst.raw = (OPCODE_LW << 26) | (rs << 21) | (rt << 16); + return inst; +} + static Function makeFunction(const std::string &name, uint32_t start, uint32_t end) { Function fn{}; @@ -155,6 +236,201 @@ static bool writeMinimalMipsElfWithJalFallbackTarget(const std::filesystem::path return writer.save(elfPath.string()); } +// Builds a fixture ELF containing a single "container_fn" function +// [containerStart, containerEnd) that is NOP-filled and ends in jr $ra + delay-slot nop. +// No other code exists anywhere in the ELF, so no jal/j instruction anywhere can ever +// target a mid-body address of this function - the only way a mid-body address can be +// discovered as an entry point is via an ingested external-call-target manifest. The +// ELF entry point is set to containerStart, so bootstrap registration only covers the +// function head. +static bool writeContainerOnlyElf(const std::filesystem::path &elfPath, + uint32_t containerStart, + uint32_t containerEnd) +{ + ELFIO::elfio writer; + writer.create(ELFIO::ELFCLASS32, ELFIO::ELFDATA2LSB); + writer.set_os_abi(ELFIO::ELFOSABI_NONE); + writer.set_type(ELFIO::ET_EXEC); + writer.set_machine(ELFIO::EM_MIPS); + writer.set_entry(containerStart); + + ELFIO::section *text = writer.sections.add(".text"); + text->set_type(ELFIO::SHT_PROGBITS); + text->set_flags(ELFIO::SHF_ALLOC | ELFIO::SHF_EXECINSTR); + text->set_addr_align(4); + text->set_address(containerStart); + + const uint32_t size = containerEnd - containerStart; + const size_t wordCount = size / sizeof(uint32_t); + if (wordCount < 2) + { + return false; + } + std::vector textWords(wordCount, 0x00000000u); // NOP-fill the whole body + textWords[wordCount - 2] = 0x03E00008u; // jr $ra + textWords[wordCount - 1] = 0x00000000u; // nop (delay slot) + text->set_data(reinterpret_cast(textWords.data()), + static_cast(textWords.size() * sizeof(uint32_t))); + + ELFIO::section *strtab = writer.sections.add(".strtab"); + strtab->set_type(ELFIO::SHT_STRTAB); + strtab->set_addr_align(1); + + ELFIO::section *symtab = writer.sections.add(".symtab"); + symtab->set_type(ELFIO::SHT_SYMTAB); + symtab->set_info(1); + symtab->set_link(strtab->get_index()); + symtab->set_addr_align(4); + symtab->set_entry_size(writer.get_default_entry_size(ELFIO::SHT_SYMTAB)); + + ELFIO::symbol_section_accessor symbols(writer, symtab); + ELFIO::string_section_accessor strings(strtab); + symbols.add_symbol(strings, "", 0, 0, ELFIO::STB_LOCAL, ELFIO::STT_NOTYPE, 0, ELFIO::SHN_UNDEF); + symbols.add_symbol(strings, "container_fn", containerStart, size, + ELFIO::STB_GLOBAL, ELFIO::STT_FUNC, 0, text->get_index()); + + ELFIO::segment *textSegment = writer.segments.add(); + textSegment->set_type(ELFIO::PT_LOAD); + textSegment->set_flags(ELFIO::PF_R | ELFIO::PF_X); + textSegment->set_align(0x1000); + textSegment->add_section_index(text->get_index(), text->get_addr_align()); + + return writer.save(elfPath.string()); +} + +// Builds a fixture ELF exercising the data-embedded thread-entry discovery path through +// real ELF/.data plumbing: +// caller @ 0x00100000: nop; lui $a0,hi(P); jal CreateThread; addiu $a0,$a0,lo(P) (delay +// slot); jr $ra; nop +// CreateThread @ 0x00100018 (syscall 0x20 wrapper): addiu $v1,$zero,0x20; syscall; +// jr $ra; nop +// worker_container @ 0x00100028: nop; nop; nop (this is E, the thread entry pointer, +// strictly inside and not the head); nop; jr $ra; nop +// .data @ 0x00200000 (P, the ThreadParam struct): word0 = 0 (unused by this test), +// word1 (P+4) = E = 0x00100030 (PS2 ABI: entry fn ptr is the +// second word of ThreadParam). +static bool writeThreadEntryDataElf(const std::filesystem::path &elfPath) +{ + ELFIO::elfio writer; + writer.create(ELFIO::ELFCLASS32, ELFIO::ELFDATA2LSB); + writer.set_os_abi(ELFIO::ELFOSABI_NONE); + writer.set_type(ELFIO::ET_EXEC); + writer.set_machine(ELFIO::EM_MIPS); + writer.set_entry(0x00100000u); + + ELFIO::section *text = writer.sections.add(".text"); + text->set_type(ELFIO::SHT_PROGBITS); + text->set_flags(ELFIO::SHF_ALLOC | ELFIO::SHF_EXECINSTR); + text->set_addr_align(4); + text->set_address(0x00100000u); + + const std::array textWords = { + // caller @ 0x00100000 + 0x00000000u, // 0x00100000: nop + 0x3C040020u, // 0x00100004: lui $a0, 0x0020 -> $a0 = 0x00200000 (P) + 0x0C040006u, // 0x00100008: jal 0x00100018 (CreateThread) + 0x24840000u, // 0x0010000C: addiu $a0,$a0,0 (delay slot; lo(P) == 0) + 0x03E00008u, // 0x00100010: jr $ra + 0x00000000u, // 0x00100014: nop + // CreateThread @ 0x00100018 + 0x24030020u, // 0x00100018: addiu $v1,$zero,0x20 + 0x0000000Cu, // 0x0010001C: syscall + 0x03E00008u, // 0x00100020: jr $ra + 0x00000000u, // 0x00100024: nop + // worker_container @ 0x00100028 + 0x00000000u, // 0x00100028: nop + 0x00000000u, // 0x0010002C: nop + 0x00000000u, // 0x00100030: nop <- E, the data-embedded thread entry point + 0x00000000u, // 0x00100034: nop + 0x03E00008u, // 0x00100038: jr $ra + 0x00000000u // 0x0010003C: nop + }; + text->set_data(reinterpret_cast(textWords.data()), + static_cast(textWords.size() * sizeof(uint32_t))); + + ELFIO::section *data = writer.sections.add(".data"); + data->set_type(ELFIO::SHT_PROGBITS); + data->set_flags(ELFIO::SHF_ALLOC | ELFIO::SHF_WRITE); + data->set_addr_align(4); + data->set_address(0x00200000u); + const std::array dataWords = { + 0x00000000u, // P + 0: unused by this test + 0x00100030u // P + 4: thread entry function pointer == E + }; + data->set_data(reinterpret_cast(dataWords.data()), + static_cast(dataWords.size() * sizeof(uint32_t))); + + ELFIO::section *strtab = writer.sections.add(".strtab"); + strtab->set_type(ELFIO::SHT_STRTAB); + strtab->set_addr_align(1); + + ELFIO::section *symtab = writer.sections.add(".symtab"); + symtab->set_type(ELFIO::SHT_SYMTAB); + symtab->set_info(1); + symtab->set_link(strtab->get_index()); + symtab->set_addr_align(4); + symtab->set_entry_size(writer.get_default_entry_size(ELFIO::SHT_SYMTAB)); + + ELFIO::symbol_section_accessor symbols(writer, symtab); + ELFIO::string_section_accessor strings(strtab); + symbols.add_symbol(strings, "", 0, 0, ELFIO::STB_LOCAL, ELFIO::STT_NOTYPE, 0, ELFIO::SHN_UNDEF); + symbols.add_symbol(strings, "caller", 0x00100000u, 0x18u, + ELFIO::STB_GLOBAL, ELFIO::STT_FUNC, 0, text->get_index()); + symbols.add_symbol(strings, "CreateThread", 0x00100018u, 0x10u, + ELFIO::STB_GLOBAL, ELFIO::STT_FUNC, 0, text->get_index()); + symbols.add_symbol(strings, "worker_container", 0x00100028u, 0x18u, + ELFIO::STB_GLOBAL, ELFIO::STT_FUNC, 0, text->get_index()); + + ELFIO::segment *textSegment = writer.segments.add(); + textSegment->set_type(ELFIO::PT_LOAD); + textSegment->set_flags(ELFIO::PF_R | ELFIO::PF_X); + textSegment->set_align(0x1000); + textSegment->add_section_index(text->get_index(), text->get_addr_align()); + + ELFIO::segment *dataSegment = writer.segments.add(); + dataSegment->set_type(ELFIO::PT_LOAD); + dataSegment->set_flags(ELFIO::PF_R | ELFIO::PF_W); + dataSegment->set_align(0x1000); + dataSegment->add_section_index(data->get_index(), data->get_addr_align()); + + return writer.save(elfPath.string()); +} + +// Returns every line of `content` containing `needle` - used to inspect the emitted +// register_functions.cpp function-table initializer, whose lines look like: +// g_ps2RecompiledFunctionTable[] = ; // 0x
+static std::vector findLinesContaining(const std::string &content, const std::string &needle) +{ + std::vector matches; + std::istringstream iss(content); + std::string line; + while (std::getline(iss, line)) + { + if (line.find(needle) != std::string::npos) + { + matches.push_back(line); + } + } + return matches; +} + +// Extracts from a "g_ps2RecompiledFunctionTable[] = ; // 0x" line. +static std::string extractOwnerNameFromRegistrationLine(const std::string &line) +{ + const size_t eqPos = line.find("= "); + if (eqPos == std::string::npos) + { + return {}; + } + const size_t start = eqPos + 2; + const size_t semiPos = line.find(';', start); + if (semiPos == std::string::npos) + { + return {}; + } + return line.substr(start, semiPos - start); +} + void register_ps2_recompiler_tests() { MiniTest::Case("PS2Recompiler", [](TestCase &tc) @@ -891,11 +1167,703 @@ void register_ps2_recompiler_tests() }); tc.Run("respect max length for .cpp filenames", [](TestCase& t) { - + t.IsTrue(PS2Recompiler::ClampFilenameLength("ReallyLongFunctionNameReallyLongFunctionNameReallyLongFunctionName_0x12345678",".cpp",50).length() <= 50,"Function name must be max 50 characters"); t.IsTrue(PS2Recompiler::ClampFilenameLength("ReallyLongFunctionNameReallyLongFunctionNameReallyLongFunctionName_0x12345678", ".cpp", 50).rfind("0x12345678") != std::string::npos, "Function name must mantain the function address at the end, if present"); - + + }); + + tc.Run("external call target manifest parsing sorts, dedupes, and ignores comments/blanks", [](TestCase &t) { + std::istringstream manifest( + "0x00462df4\n" + "\n" + "# a comment line\n" + "0x001000A0\n" + "0x00462df4\n" + " \n" + "0x1000a0\n" + " # indented comment\n" + "0x00500000\n"); + + const std::vector parsed = PS2Recompiler::ParseCallTargetManifest(manifest); + + t.Equals(parsed.size(), static_cast(3), + "duplicate and case-variant addresses should collapse to unique targets"); + if (parsed.size() == 3) + { + t.Equals(parsed[0], 0x001000A0u, "targets should be sorted ascending"); + t.Equals(parsed[1], 0x00462df4u, "second target should be the mid-range address"); + t.Equals(parsed[2], 0x00500000u, "third target should be the highest address"); + } + }); + + tc.Run("external call target manifest parsing handles empty input", [](TestCase &t) { + std::istringstream manifest(""); + const std::vector parsed = PS2Recompiler::ParseCallTargetManifest(manifest); + t.Equals(parsed.size(), static_cast(0), "empty manifest should parse to an empty target list"); + }); + + // Tier 1 end-to-end regression test: drives a REAL PS2Recompiler instance through + // initialize() -> recompile() -> generateOutput() over a real ELF fixture and a real + // manifest file, and inspects the actual register_functions.cpp produced by the + // production FunctionTableEmitter. This exercises the full ingestion path - + // loadExternalCallTargetManifests() -> discoverAdditionalEntryPoints()'s + // m_ingestedExternalCallTargets -> owner mapping -> resume-target push (the code + // at ps2_recompiler.cpp around lines 1938-1953) - and nothing else in this fixture + // is capable of discovering the mid-body target, so this test fails if that code + // path is disabled or removed. + tc.Run("full pipeline: manifest-ingested packed jal-only entry registers into its owning unit", [](TestCase &t) { + const auto uniqueSuffix = std::to_string( + static_cast(std::chrono::steady_clock::now().time_since_epoch().count())); + const std::filesystem::path workDir = + std::filesystem::temp_directory_path() / ("ps2recomp-packed-jal-" + uniqueSuffix); + std::error_code mkdirError; + std::filesystem::create_directories(workDir, mkdirError); + t.IsTrue(!mkdirError, "work directory should be created"); + + const std::filesystem::path elfPath = workDir / "fixture.elf"; + constexpr uint32_t containerStart = 0x00100000u; + constexpr uint32_t containerEnd = 0x00100018u; // container_fn: 6 NOP-filled words, jr $ra tail + constexpr uint32_t midBodyTarget = 0x00100008u; // T: not the head, not any jal's target anywhere + + const bool wroteElf = writeContainerOnlyElf(elfPath, containerStart, containerEnd); + t.IsTrue(wroteElf, "container-only fixture ELF should be generated"); + if (!wroteElf) + { + std::error_code cleanupError; + std::filesystem::remove_all(workDir, cleanupError); + return; + } + + const std::filesystem::path manifestPath = workDir / "manifest.txt"; + { + std::ofstream manifestFile(manifestPath); + t.IsTrue(static_cast(manifestFile), "manifest file should be writable"); + manifestFile << "0x00100008\n"; + } + + const std::filesystem::path outWithManifest = workDir / "out_with"; + const std::filesystem::path outWithoutManifest = workDir / "out_without"; + + const std::filesystem::path configWithManifestPath = workDir / "with_manifest.toml"; + { + std::ofstream cfg(configWithManifestPath); + t.IsTrue(static_cast(cfg), "config (with manifest) should be writable"); + cfg << "[general]\n"; + // generic_string(): backslashes from path::string() on Windows are + // escape introducers inside a TOML basic string and break the parse. + cfg << "input = \"" << elfPath.generic_string() << "\"\n"; + cfg << "output = \"" << outWithManifest.generic_string() << "\"\n"; + cfg << "external_call_target_manifests = [\"" << manifestPath.generic_string() << "\"]\n"; + } + + // "Without manifest" case omits external_call_target_manifests entirely. + const std::filesystem::path configWithoutManifestPath = workDir / "without_manifest.toml"; + { + std::ofstream cfg(configWithoutManifestPath); + t.IsTrue(static_cast(cfg), "config (without manifest) should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfPath.generic_string() << "\"\n"; + cfg << "output = \"" << outWithoutManifest.generic_string() << "\"\n"; + } + + std::string headOwnerWithManifest; + std::string targetOwnerWithManifest; + + // --- Run WITH the manifest --- + { + PS2Recompiler recompiler(configWithManifestPath.string()); + t.IsTrue(recompiler.initialize(), "initialize() should succeed for the with-manifest run"); + t.IsTrue(recompiler.recompile(), "recompile() should succeed for the with-manifest run"); + recompiler.generateOutput(); + + const std::filesystem::path registerPath = outWithManifest / "register_functions.cpp"; + std::ifstream registerFile(registerPath); + t.IsTrue(static_cast(registerFile), + "register_functions.cpp should be written for the with-manifest run"); + std::ostringstream contentStream; + contentStream << registerFile.rdbuf(); + const std::string content = contentStream.str(); + + const auto headLines = findLinesContaining(content, "// 0x100000"); + const auto targetLines = findLinesContaining(content, "// 0x100008"); + + t.IsTrue(!headLines.empty(), "container head 0x100000 should be registered (sanity)"); + t.IsTrue(!targetLines.empty(), + "manifest-ingested target 0x100008 must be registered when the manifest is supplied - " + "this is the line that disappears if the ingestion->owner-mapping->resume-target push " + "(ps2_recompiler.cpp ~1938-1953) is disabled"); + + if (!headLines.empty()) + { + headOwnerWithManifest = extractOwnerNameFromRegistrationLine(headLines.front()); + t.IsTrue(headOwnerWithManifest.find("container_fn") != std::string::npos, + "container head should register under a container_fn-derived name"); + } + if (!targetLines.empty()) + { + targetOwnerWithManifest = extractOwnerNameFromRegistrationLine(targetLines.front()); + t.IsTrue(targetOwnerWithManifest.find("container_fn") != std::string::npos, + "0x100008 must be mapped to the container_fn owner, not left unresolved"); + } + + // Test C (boundary/owner-integrity invariant), folded in here so it shares + // this fixture and this pipeline run: the manifest-ingested entry must + // dispatch INTO the owning unit - i.e. resolve to the exact same generated + // owner name as the container's own head - rather than being sliced into a + // truncated standalone function ending at an interior label. The + // resume-mapping path (ps2_recompiler.cpp ~1938-1953) pushes into + // m_resumeEntryTargetsByOwner and creates no per-entry Function::end, so + // there is no "->end" boundary to assert here the way there is for the + // slicer path. That slicer-path End boundary is already covered by the + // existing tests "additional entries split at nearest discovered boundary" + // (~line 280) and "entry reslice trims earlier entries after late + // discovery" (~line 374). + if (!headOwnerWithManifest.empty() && !targetOwnerWithManifest.empty()) + { + t.Equals(targetOwnerWithManifest, headOwnerWithManifest, + "0x100008 must resolve to the SAME owner name as the container head, proving " + "dispatch resumes into the owning unit (which retains its full declared range) " + "rather than being sliced into a separate standalone function"); + } + } + + // --- Run WITHOUT the manifest: reproduces the original bug. Nothing else in + // this fixture can discover 0x100008 (no jal/branch anywhere targets it), so it + // must be left unregistered. + { + PS2Recompiler recompiler(configWithoutManifestPath.string()); + t.IsTrue(recompiler.initialize(), "initialize() should succeed for the without-manifest run"); + t.IsTrue(recompiler.recompile(), "recompile() should succeed for the without-manifest run"); + recompiler.generateOutput(); + + const std::filesystem::path registerPath = outWithoutManifest / "register_functions.cpp"; + std::ifstream registerFile(registerPath); + t.IsTrue(static_cast(registerFile), + "register_functions.cpp should be written for the without-manifest run"); + std::ostringstream contentStream; + contentStream << registerFile.rdbuf(); + const std::string content = contentStream.str(); + + const auto headLines = findLinesContaining(content, "// 0x100000"); + const auto targetLines = findLinesContaining(content, "// 0x100008"); + + t.IsTrue(!headLines.empty(), + "container head 0x100000 should still be registered without a manifest (sanity)"); + t.IsTrue(targetLines.empty(), + "without a manifest, 0x100008 must NOT be registered - this reproduces the original bug"); + } + + std::error_code removeError; + std::filesystem::remove_all(workDir, removeError); + }); + + // Tier 1 end-to-end regression test for data-embedded thread entry discovery: drives + // a REAL PS2Recompiler instance over a real ELF whose .data section contains a + // ThreadParam struct read via the production m_elfParser->readWord/isValidAddress + // path, proving both that the analyzer reads the pointer from real ELF data AND + // that the resulting entry gets registered into its owning unit through the full + // recompile()/generateOutput() pipeline. + tc.Run("full pipeline: data-embedded thread entry (CreateThread ThreadParam) registers into its owning unit", [](TestCase &t) { + const auto uniqueSuffix = std::to_string( + static_cast(std::chrono::steady_clock::now().time_since_epoch().count())); + const std::filesystem::path workDir = + std::filesystem::temp_directory_path() / ("ps2recomp-thread-entry-" + uniqueSuffix); + std::error_code mkdirError; + std::filesystem::create_directories(workDir, mkdirError); + t.IsTrue(!mkdirError, "work directory should be created"); + + const std::filesystem::path elfPath = workDir / "fixture.elf"; + const bool wroteElf = writeThreadEntryDataElf(elfPath); + t.IsTrue(wroteElf, "thread-entry fixture ELF should be generated"); + if (!wroteElf) + { + std::error_code cleanupError; + std::filesystem::remove_all(workDir, cleanupError); + return; + } + + const std::filesystem::path outDir = workDir / "out"; + const std::filesystem::path configPath = workDir / "config.toml"; + { + std::ofstream cfg(configPath); + t.IsTrue(static_cast(cfg), "config should be writable"); + cfg << "[general]\n"; + // generic_string(): see the manifest test above - native Windows + // separators are invalid escapes inside a TOML basic string. + cfg << "input = \"" << elfPath.generic_string() << "\"\n"; + cfg << "output = \"" << outDir.generic_string() << "\"\n"; + } + + PS2Recompiler recompiler(configPath.string()); + t.IsTrue(recompiler.initialize(), "initialize() should succeed"); + t.IsTrue(recompiler.recompile(), "recompile() should succeed"); + recompiler.generateOutput(); + + const std::filesystem::path registerPath = outDir / "register_functions.cpp"; + std::ifstream registerFile(registerPath); + t.IsTrue(static_cast(registerFile), "register_functions.cpp should be written"); + std::ostringstream contentStream; + contentStream << registerFile.rdbuf(); + const std::string content = contentStream.str(); + + const auto entryLines = findLinesContaining(content, "// 0x100030"); + t.IsTrue(!entryLines.empty(), + "data-embedded thread entry 0x100030 (read from the .data ThreadParam struct via " + "the real ELF parser) must be registered"); + if (!entryLines.empty()) + { + const std::string owner = extractOwnerNameFromRegistrationLine(entryLines.front()); + t.IsTrue(owner.find("worker_container") != std::string::npos, + "0x100030 must be mapped to the worker_container owner that actually contains it"); + } + + std::error_code removeError; + std::filesystem::remove_all(workDir, removeError); + }); + + tc.Run("collect external call targets: excludes jal into a local recompiled function, includes jal outside all local functions", [](TestCase &t) { + std::vector
sections = { + {".text", 0x1000u, 0x100000u - 0x1000u, 0u, true, false, false, true, nullptr} + }; + + std::vector functions = { + makeFunction("functionA", 0x1000u, 0x1020u), + makeFunction("functionB", 0x2000u, 0x2020u) + }; + + std::unordered_map> decodedFunctions; + decodedFunctions[0x1000u] = { + makeAbsJump(0x1000u, 0x2010u, OPCODE_JAL), + makeNopLike(0x1004u), + makeAbsJump(0x1008u, 0x50000u, OPCODE_JAL), + makeNopLike(0x100Cu) + }; + + const std::vector targets = + PS2Recompiler::CollectExternalCallTargets(decodedFunctions, functions, sections); + + t.Equals(targets.size(), static_cast(1), + "only the target outside every local function range should be collected"); + if (!targets.empty()) + { + t.Equals(targets[0], 0x50000u, + "jal into functionB's range should be excluded; jal to 0x50000 should be included"); + } + }); + + tc.Run("collect external call targets: excludes targets outside any executable section", [](TestCase &t) { + std::vector
sections = { + {".text", 0x1000u, 0x100000u - 0x1000u, 0u, true, false, false, true, nullptr} + }; + + std::vector functions = { + makeFunction("functionA", 0x1000u, 0x1020u) + }; + + std::unordered_map> decodedFunctions; + decodedFunctions[0x1000u] = { + makeAbsJump(0x1000u, 0x300000u, OPCODE_JAL), + makeNopLike(0x1004u) + }; + + const std::vector targets = + PS2Recompiler::CollectExternalCallTargets(decodedFunctions, functions, sections); + + t.Equals(targets.size(), static_cast(0), + "a jal target past the end of every code section should not be collected"); + }); + + tc.Run("collect external call targets: duplicates collapse and results are sorted", [](TestCase &t) { + std::vector
sections = { + {".text", 0x1000u, 0x100000u - 0x1000u, 0u, true, false, false, true, nullptr} + }; + + std::vector functions = { + makeFunction("functionA", 0x1000u, 0x1020u) + }; + + std::unordered_map> decodedFunctions; + decodedFunctions[0x1000u] = { + makeAbsJump(0x1000u, 0x50000u, OPCODE_JAL), + makeNopLike(0x1004u), + makeAbsJump(0x1008u, 0x50000u, OPCODE_JAL), + makeNopLike(0x100Cu), + makeAbsJump(0x1010u, 0x50000u, OPCODE_J), + makeNopLike(0x1014u), + makeAbsJump(0x1018u, 0x60000u, OPCODE_JAL), + makeNopLike(0x101Cu), + makeAbsJump(0x1020u, 0x40000u, OPCODE_JAL), + makeNopLike(0x1024u) + }; + + const std::vector targets = + PS2Recompiler::CollectExternalCallTargets(decodedFunctions, functions, sections); + + t.Equals(targets.size(), static_cast(3), + "duplicate targets should collapse to unique entries"); + if (targets.size() == 3) + { + t.Equals(targets[0], 0x40000u, "targets should be sorted ascending"); + t.Equals(targets[1], 0x50000u, "second target should be the mid-range address"); + t.Equals(targets[2], 0x60000u, "third target should be the highest address"); + } + }); + + tc.Run("collect external call targets: targets inside skipped or stub local functions are still emitted", [](TestCase &t) { + std::vector
sections = { + {".text", 0x1000u, 0x100000u - 0x1000u, 0u, true, false, false, true, nullptr} + }; + + Function skippedFunction = makeFunction("functionB", 0x2000u, 0x2020u); + skippedFunction.isSkipped = true; + + std::vector functions = { + makeFunction("functionA", 0x1000u, 0x1020u), + skippedFunction + }; + + std::unordered_map> decodedFunctions; + decodedFunctions[0x1000u] = { + makeAbsJump(0x1000u, 0x2010u, OPCODE_JAL), + makeNopLike(0x1004u) + }; + + const std::vector targets = + PS2Recompiler::CollectExternalCallTargets(decodedFunctions, functions, sections); + + t.Equals(targets.size(), static_cast(1), + "a jal landing inside a skipped local function should still be collected"); + if (!targets.empty()) + { + t.Equals(targets[0], 0x2010u, + "only recompiled, non-stub, non-skipped functions should suppress emission"); + } + }); + + tc.Run("collect external call targets: conditional branches are not collected", [](TestCase &t) { + std::vector
sections = { + {".text", 0x1000u, 0x100000u - 0x1000u, 0u, true, false, false, true, nullptr} + }; + + std::vector functions = { + makeFunction("functionA", 0x1000u, 0x1020u) + }; + + // Target (0x50000) is inside the code section and outside every local function + // range, i.e. it would be collected if this were a jal/j - isolating that the + // opcode check, not the address itself, is what excludes it. + Instruction branch = makeAbsJump(0x1000u, 0x50000u, OPCODE_BEQ); + branch.isBranch = true; + + std::unordered_map> decodedFunctions; + decodedFunctions[0x1000u] = { + branch, + makeNopLike(0x1004u) + }; + + const std::vector targets = + PS2Recompiler::CollectExternalCallTargets(decodedFunctions, functions, sections); + + t.Equals(targets.size(), static_cast(0), + "conditional branches are not jal/j and should not be collected even though the address would otherwise qualify"); + }); + + tc.Run("data-embedded thread entries: jal to CreateThread wrapper resolves delay-slot $a0", [](TestCase &t) { + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 8u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddr + 4u, 4, 4, 0x1234), + }}, + }; + + const uint32_t paramAddress = 0x00301234u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(1), "expected exactly one discovered thread entry"); + if (result.size() == 1) + { + t.Equals(result[0], 0x00280000u, "thread entry pointer should be read from the ThreadParam struct"); + } + }); + + tc.Run("data-embedded thread entries: jal's decoder-populated rt field must not clobber $a0", [](TestCase &t) { + // R5900Decoder::decodeInstruction populates rs/rt/rd unconditionally from the + // raw instruction bits, even for J-type (j/jal) instructions where those bit + // positions are actually part of the 26-bit jump target rather than a real + // register field. For wrapperStart = 0x00100200, the jal's encoded target bits + // happen to alias rt = 4 ($a0). If the constant-propagation walk treated that + // as a real write to $a0, it would wrongly erase the $a0 the lui just set, + // causing the delay-slot addiu to fail to resolve. This must still resolve. + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 8u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), + makeAbsJumpDecoded(jalAddr, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddr + 4u, 4, 4, 0x1234), + }}, + }; + + t.Equals(static_cast(RT(decoded.at(callerStart)[2].raw)), 4u, + "test setup sanity check: this jal's decoder-populated rt must alias $a0"); + + const uint32_t paramAddress = 0x00301234u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(1), + "a jal's incidental rt bits must not be treated as a register write"); + if (result.size() == 1) + { + t.Equals(result[0], 0x00280000u, "thread entry pointer should still be read from the ThreadParam struct"); + } + }); + + tc.Run("data-embedded thread entries: direct inline syscall resolves static $a0", [](TestCase &t) { + constexpr uint32_t callerStart = 0x00101000u; + std::unordered_map> decoded = { + {callerStart, { + makeLui(callerStart, 4, 0x0041), + makeAddiu(callerStart + 4u, 4, 4, 0x0100), + makeAddiu(callerStart + 8u, 3, 0, 0x20), + makeSyscall(callerStart + 12u), + }}, + }; + + const uint32_t paramAddress = 0x00410100u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00420000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(1), "expected the inline syscall call site to resolve"); + if (result.size() == 1) + { + t.Equals(result[0], 0x00420000u, "thread entry pointer should be read via the direct syscall path"); + } + }); + + tc.Run("data-embedded thread entries: clobbered $a0 before call yields no result", [](TestCase &t) { + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 12u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), + makeLw(callerStart + 8u, 4, 5), // clobbers $a0 with an unresolved load + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeNopLike(jalAddr + 4u), // delay slot does not re-materialize $a0 + }}, + }; + + std::unordered_map fakeMemory = { + {0x00301234u, 0u}, + {0x00301238u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(0), "a clobbered $a0 with no re-materialization should not resolve"); + }); + + tc.Run("data-embedded thread entries: wrapper with wrong syscall number is not registered", [](TestCase &t) { + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 8u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x21), // wrong syscall number + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddr + 4u, 4, 4, 0x1234), + }}, + }; + + std::unordered_map fakeMemory = { + {0x00301234u, 0u}, + {0x00301238u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(0), "a wrapper using a non-CreateThread syscall number should be ignored"); + }); + + tc.Run("data-embedded thread entries: zero entry pointer is filtered out", [](TestCase &t) { + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 8u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddr + 4u, 4, 4, 0x1234), + }}, + }; + + std::unordered_map fakeMemory = { + {0x00301234u, 0u}, + {0x00301238u, 0u}, // entry pointer is zero, should be excluded + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(0), "a zero entry pointer should never be reported as a thread entry"); + }); + + tc.Run("data-embedded thread entries: multiple call sites to the same struct dedupe", [](TestCase &t) { + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerAStart = 0x00100000u; + constexpr uint32_t callerBStart = 0x00100100u; + constexpr uint32_t jalAddrA = callerAStart + 8u; + constexpr uint32_t jalAddrB = callerBStart + 8u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerAStart, { + makeNopLike(callerAStart), + makeLui(callerAStart + 4u, 4, 0x0030), + makeAbsJump(jalAddrA, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddrA + 4u, 4, 4, 0x1234), + }}, + {callerBStart, { + makeNopLike(callerBStart), + makeLui(callerBStart + 4u, 4, 0x0030), + makeAbsJump(jalAddrB, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddrB + 4u, 4, 4, 0x1234), + }}, + }; + + std::unordered_map fakeMemory = { + {0x00301234u, 0u}, + {0x00301238u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(1), + "two call sites referencing the same ThreadParam struct should dedupe to one entry"); + if (result.size() == 1) + { + t.Equals(result[0], 0x00280000u, "the deduped entry should still be the correct thread entry pointer"); + } + }); + + tc.Run("data-embedded thread entries: addiu LO with sign bit set is sign-extended, not OR'd", [](TestCase &t) { + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 12u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0031), + // sign-extended LO: address = 0x00310000 + sign_ext16(0x8000) = 0x00308000 + makeAddiu(callerStart + 8u, 4, 4, 0x8000), + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeNopLike(jalAddr + 4u), + }}, + }; + + const uint32_t paramAddress = 0x00308000u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00290000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(1), + "the sign-extended address should resolve to the correct ThreadParam struct"); + if (result.size() == 1) + { + t.Equals(result[0], 0x00290000u, "entry pointer should come from address 0x00308000, not the OR'd 0x00318000"); + } }); }); } From 8fdb75602c255da8f6b068b478e4f346571976c6 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Wed, 15 Jul 2026 15:04:18 -0400 Subject: [PATCH 2/7] fix: add missing include before elfio header to fix build on newer GCC --- ps2xRecomp/include/ps2recomp/elf_parser.h | 1 + 1 file changed, 1 insertion(+) diff --git a/ps2xRecomp/include/ps2recomp/elf_parser.h b/ps2xRecomp/include/ps2recomp/elf_parser.h index c8cafe5ee..10f9e4ce8 100644 --- a/ps2xRecomp/include/ps2recomp/elf_parser.h +++ b/ps2xRecomp/include/ps2recomp/elf_parser.h @@ -1,6 +1,7 @@ #ifndef PS2RECOMP_ELF_PARSER_H #define PS2RECOMP_ELF_PARSER_H +#include #include #include #include From 9a2c39d3a2edc21e332aca523b42a5fceb33b719 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 07:56:48 -0400 Subject: [PATCH 3/7] fix(recomp): make external-call-target emission a permissive candidate collector CollectExternalCallTargets gated inclusion on the target landing inside one of the caller's own executable sections. That drops every genuine cross-unit/overlay target by construction, since a callee living in a separately recompiled unit is never inside the caller's own sections - exactly the case this manifest mechanism exists to support. The caller cannot know a callee unit's section layout, so emission must not filter on the caller's own code sections. Replace the inclusion gate with a narrow exclusion: drop only targets landing inside the caller's own data/bss (the one real garbage source - a mis-decoded jal into non-code bytes), and otherwise emit every jal/j target outside every local recompiled function. The ingesting unit's findContainingFunction stays the authoritative filter over these candidates. --- README.md | 2 +- ps2xRecomp/include/ps2recomp/ps2_recompiler.h | 10 ++++-- ps2xRecomp/src/lib/ps2_recompiler.cpp | 13 +++++--- ps2xTest/src/ps2_recompiler_tests.cpp | 33 +++++++++++++++++-- 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index f11518a45..1b3768089 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ Main fields in `config.toml`: * `general.patch_cache`: apply configured patches to CACHE instructions. * `general.stubs`: names to force as stubs. Also accepts `handler@0xADDRESS` to bind a stripped function address directly to a runtime syscall/stub handler. Includes generic handlers `ret0`, `ret1`, `reta0`. * `general.skip`: names to force as skipped wrappers. -* `general.external_call_target_manifests`: optional array of `external_call_targets.txt` paths emitted by other recompile invocations (e.g. a separately recompiled overlay); their call targets that land inside this invocation's functions are registered as additional entry points. Each invocation writes its own `external_call_targets.txt` into `general.output`. +* `general.external_call_target_manifests`: optional array of `external_call_targets.txt` paths emitted by other recompile invocations (e.g. a separately recompiled overlay); their call targets that land inside this invocation's functions are registered as additional entry points. Each invocation writes its own `external_call_targets.txt` into `general.output`, and the emitted candidates include targets outside the emitting unit's own sections entirely (cross-ELF/overlay calls) - the emitting unit cannot know the callee unit's layout, so the ingesting unit is authoritative about which candidates actually resolve. * `patches.instructions`: raw instruction replacements by address. Address binding for stripped ELFs: diff --git a/ps2xRecomp/include/ps2recomp/ps2_recompiler.h b/ps2xRecomp/include/ps2recomp/ps2_recompiler.h index 97fc4b64f..51ecdb5d7 100644 --- a/ps2xRecomp/include/ps2recomp/ps2_recompiler.h +++ b/ps2xRecomp/include/ps2recomp/ps2_recompiler.h @@ -61,9 +61,13 @@ namespace ps2recomp const std::function &isValidAddress, const std::function &readWord); - // Collects jal/j targets that fall in executable sections but outside every - // recompiled local function range - candidate cross-unit call targets to - // publish in the external call-target manifest. Sorted and de-duplicated. + // Collects jal/j targets that fall outside every recompiled local function + // range and outside this unit's own data/bss sections - candidate cross-unit + // call targets (including cross-ELF/overlay targets outside every one of this + // unit's sections) to publish in the external call-target manifest. The + // caller cannot know a callee unit's section layout, so this is a permissive + // collector: the ingesting unit's findContainingFunction is the authoritative + // filter. Sorted and de-duplicated. static std::vector CollectExternalCallTargets( const std::unordered_map> &decodedFunctions, const std::vector &functions, diff --git a/ps2xRecomp/src/lib/ps2_recompiler.cpp b/ps2xRecomp/src/lib/ps2_recompiler.cpp index 44725dfc9..700c9fab9 100644 --- a/ps2xRecomp/src/lib/ps2_recompiler.cpp +++ b/ps2xRecomp/src/lib/ps2_recompiler.cpp @@ -1751,11 +1751,16 @@ namespace ps2recomp { std::vector externalTargets; - auto isInsideExecutableSection = [&](uint32_t address) -> bool + // Excludes only the caller's own data/bss - not its code sections. The caller + // cannot know a callee unit's section layout, so emission must not filter on + // the caller's own code sections; the one real garbage source (a mis-decoded + // jal landing in this unit's own data/bss) is still worth dropping. The + // ingesting unit's findContainingFunction is the authoritative filter. + auto isInsideCallerDataSection = [&](uint32_t address) -> bool { for (const auto §ion : sections) { - if (!section.isCode) + if (section.isCode || !(section.isData || section.isBSS)) { continue; } @@ -1795,12 +1800,12 @@ namespace ps2recomp } const uint32_t target = buildAbsoluteJumpTarget(inst.address, inst.target); - if (!isInsideExecutableSection(target)) + if (isInsideRecompiledFunction(target)) { continue; } - if (isInsideRecompiledFunction(target)) + if (isInsideCallerDataSection(target)) { continue; } diff --git a/ps2xTest/src/ps2_recompiler_tests.cpp b/ps2xTest/src/ps2_recompiler_tests.cpp index 9669a8436..1b569a292 100644 --- a/ps2xTest/src/ps2_recompiler_tests.cpp +++ b/ps2xTest/src/ps2_recompiler_tests.cpp @@ -1453,7 +1453,7 @@ void register_ps2_recompiler_tests() } }); - tc.Run("collect external call targets: excludes targets outside any executable section", [](TestCase &t) { + tc.Run("collect external call targets: foreign/overlay target outside all sections IS collected", [](TestCase &t) { std::vector
sections = { {".text", 0x1000u, 0x100000u - 0x1000u, 0u, true, false, false, true, nullptr} }; @@ -1468,11 +1468,40 @@ void register_ps2_recompiler_tests() makeNopLike(0x1004u) }; + const std::vector targets = + PS2Recompiler::CollectExternalCallTargets(decodedFunctions, functions, sections); + + t.Equals(targets.size(), static_cast(1), + "a jal target past the end of every section of this unit is a candidate cross-unit/overlay " + "target and must be collected - the emitting unit cannot know the callee unit's layout"); + if (!targets.empty()) + { + t.Equals(targets[0], 0x300000u, + "the collected target should be the foreign/overlay address itself"); + } + }); + + tc.Run("collect external call targets: target inside the caller's own data section is excluded", [](TestCase &t) { + std::vector
sections = { + {".text", 0x1000u, 0x100000u - 0x1000u, 0u, true, false, false, true, nullptr}, + {".data", 0x200000u, 0x10000u, 0u, false, true, false, false, nullptr} + }; + + std::vector functions = { + makeFunction("functionA", 0x1000u, 0x1020u) + }; + + std::unordered_map> decodedFunctions; + decodedFunctions[0x1000u] = { + makeAbsJump(0x1000u, 0x200008u, OPCODE_JAL), + makeNopLike(0x1004u) + }; + const std::vector targets = PS2Recompiler::CollectExternalCallTargets(decodedFunctions, functions, sections); t.Equals(targets.size(), static_cast(0), - "a jal target past the end of every code section should not be collected"); + "a jal landing inside the caller's own data section is garbage and must be dropped"); }); tc.Run("collect external call targets: duplicates collapse and results are sorted", [](TestCase &t) { From df1aacdd2e80a1a5b6d43b1cae5242fea32a7ace Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 08:02:08 -0400 Subject: [PATCH 4/7] feat(recomp): split external-call-target analysis into its own phase, hard-fail on a missing configured manifest Manifest emission depends only on this unit's own decoded functions and sections, never on any ingested sibling manifest, so a unit's emitted manifest is a fixpoint after one emission pass regardless of build order. Only ingestion depends on siblings having already run. A single build invocation could previously race a multi-unit build: unit A's generate pass would silently drop unit B's targets if B's manifest hadn't been emitted yet, and a genuinely missing manifest only produced a warning. Split recompile() into an emit-manifest-only analysis phase and the existing generate phase (recompile(bool emitManifestOnly = false)), wired to a new --emit-manifest-only CLI flag. A multi-unit build now runs every unit's analysis phase first (any order), then every unit's generate phase, by which point every configured sibling manifest is guaranteed to exist. Emission moves out of discoverAdditionalEntryPoints and into recompile() itself, running unconditionally in both phases. Since the two-phase flow removes any legitimate reason for a configured manifest to still be missing by generate time, loadExternalCallTargetManifests now hard-fails (propagating through recompile() to a non-zero process exit) instead of warning and continuing on an unreadable manifest path. --- README.md | 2 + ps2xRecomp/include/ps2recomp/ps2_recompiler.h | 13 +- ps2xRecomp/src/lib/ps2_recompiler.cpp | 33 +- ps2xRecomp/src/runner/main.cpp | 23 +- ps2xTest/src/ps2_recompiler_tests.cpp | 547 ++++++++++++++++++ 5 files changed, 607 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 1b3768089..429ac089b 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,8 @@ Main fields in `config.toml`: * `general.stubs`: names to force as stubs. Also accepts `handler@0xADDRESS` to bind a stripped function address directly to a runtime syscall/stub handler. Includes generic handlers `ret0`, `ret1`, `reta0`. * `general.skip`: names to force as skipped wrappers. * `general.external_call_target_manifests`: optional array of `external_call_targets.txt` paths emitted by other recompile invocations (e.g. a separately recompiled overlay); their call targets that land inside this invocation's functions are registered as additional entry points. Each invocation writes its own `external_call_targets.txt` into `general.output`, and the emitted candidates include targets outside the emitting unit's own sections entirely (cross-ELF/overlay calls) - the emitting unit cannot know the callee unit's layout, so the ingesting unit is authoritative about which candidates actually resolve. + +For a multi-unit build (each unit listing one or more of the others in `external_call_target_manifests`), run every unit twice: first with `ps2recomp --emit-manifest-only` for every unit (any order - manifest emission depends only on that unit's own decoded functions, not on any sibling), then run every unit normally (`ps2recomp `) so every configured sibling manifest already exists. A configured manifest that is still missing at generate time is a hard error. * `patches.instructions`: raw instruction replacements by address. Address binding for stripped ELFs: diff --git a/ps2xRecomp/include/ps2recomp/ps2_recompiler.h b/ps2xRecomp/include/ps2recomp/ps2_recompiler.h index 51ecdb5d7..7fbdf81f7 100644 --- a/ps2xRecomp/include/ps2recomp/ps2_recompiler.h +++ b/ps2xRecomp/include/ps2recomp/ps2_recompiler.h @@ -33,7 +33,13 @@ namespace ps2recomp ~PS2Recompiler(); bool initialize(); - bool recompile(); + // emitManifestOnly: run only the input-independent analysis phase (decode + + // emit this unit's external_call_targets.txt) and return before ingesting any + // sibling manifest or discovering entry points. Used to drive a two-phase + // multi-unit build: run every unit with emitManifestOnly=true first (any + // order), then run every unit normally so every configured sibling manifest + // is guaranteed to already exist. + bool recompile(bool emitManifestOnly = false); void generateOutput(); void printReport() const; @@ -100,7 +106,10 @@ namespace ps2recomp bool decodeFunction(Function &function); void discoverAdditionalEntryPoints(); - void loadExternalCallTargetManifests(); + // Returns false (after reporting an error) when a configured manifest path + // cannot be opened - there is no legitimate reason for a configured manifest + // to be missing once every unit's analysis phase has run. + bool loadExternalCallTargetManifests(); void emitExternalCallTargetManifest(); bool shouldSkipFunction(const Function &function) const; bool isStubFunction(const Function &function) const; diff --git a/ps2xRecomp/src/lib/ps2_recompiler.cpp b/ps2xRecomp/src/lib/ps2_recompiler.cpp index 700c9fab9..1118623bd 100644 --- a/ps2xRecomp/src/lib/ps2_recompiler.cpp +++ b/ps2xRecomp/src/lib/ps2_recompiler.cpp @@ -927,7 +927,7 @@ namespace ps2recomp } } - bool PS2Recompiler::recompile() + bool PS2Recompiler::recompile(bool emitManifestOnly) { try { @@ -984,7 +984,23 @@ namespace ps2recomp #endif } - loadExternalCallTargetManifests(); + // Emission depends only on this unit's own decoded functions and sections, + // never on any ingested manifest - every unit's emitted manifest is a + // fixpoint after one emission pass, independent of build order. Emit + // unconditionally, then in analysis-only mode stop before ingesting any + // sibling manifest so a multi-unit clean build can run every unit's + // analysis phase first without any sibling manifest existing yet. + emitExternalCallTargetManifest(); + if (emitManifestOnly) + { + m_reporter.progress("analysis phase complete (manifest emitted)"); + return true; + } + + if (!loadExternalCallTargetManifests()) + { + return false; + } discoverAdditionalEntryPoints(); if (failedCount > 0) @@ -1717,7 +1733,7 @@ namespace ps2recomp } } - void PS2Recompiler::loadExternalCallTargetManifests() + bool PS2Recompiler::loadExternalCallTargetManifests() { m_ingestedExternalCallTargets.clear(); @@ -1727,8 +1743,12 @@ namespace ps2recomp std::ifstream manifestFile(manifestPath); if (!manifestFile) { - m_reporter.warning("external-call-targets", "Failed to open manifest for reading: " + manifestPath); - continue; + // The two-phase build (recompile(true) for every unit, then + // recompile(false) for every unit) removes any legitimate reason for a + // configured manifest to be missing by generate time, so this is a hard + // error rather than a warn-and-continue. + m_reporter.error("external-call-targets", "Failed to open manifest for reading: " + manifestPath); + return false; } const std::vector parsed = ParseCallTargetManifest(manifestFile); @@ -1742,6 +1762,7 @@ namespace ps2recomp std::sort(merged.begin(), merged.end()); merged.erase(std::unique(merged.begin(), merged.end()), merged.end()); m_ingestedExternalCallTargets = std::move(merged); + return true; } std::vector PS2Recompiler::CollectExternalCallTargets( @@ -2026,8 +2047,6 @@ namespace ps2recomp << " owner function(s)"; m_reporter.progress(msg.str()); } - - emitExternalCallTargetManifest(); } bool PS2Recompiler::decodeFunction(Function &function) diff --git a/ps2xRecomp/src/runner/main.cpp b/ps2xRecomp/src/runner/main.cpp index 64c77973d..c81b3ca96 100644 --- a/ps2xRecomp/src/runner/main.cpp +++ b/ps2xRecomp/src/runner/main.cpp @@ -7,8 +7,12 @@ using namespace ps2recomp; void printUsage() { std::cout << "PS2Recomp - A static recompiler for PlayStation 2 ELF files\n"; - std::cout << "Usage: ps2recomp \n"; + std::cout << "Usage: ps2recomp [--emit-manifest-only]\n"; std::cout << " config.toml: Configuration file for the recompiler\n"; + std::cout << " --emit-manifest-only: run only the analysis phase (decode + emit this\n"; + std::cout << " unit's external_call_targets.txt) and exit before ingesting any sibling\n"; + std::cout << " manifest. For a multi-unit build, run every unit with this flag first\n"; + std::cout << " (any order), then run every unit normally.\n"; } int main(int argc, char *argv[]) @@ -20,6 +24,14 @@ int main(int argc, char *argv[]) } std::string configPath = argv[1]; + bool emitManifestOnly = false; + for (int i = 2; i < argc; ++i) + { + if (std::string(argv[i]) == "--emit-manifest-only") + { + emitManifestOnly = true; + } + } try { @@ -32,13 +44,20 @@ int main(int argc, char *argv[]) return 1; } - if (!recompiler.recompile()) + if (!recompiler.recompile(emitManifestOnly)) { std::cerr << "Recompilation failed\n"; recompiler.printReport(); return 1; } + if (emitManifestOnly) + { + recompiler.printReport(); + std::cout << "Analysis phase completed successfully\n"; + return 0; + } + recompiler.generateOutput(); recompiler.printReport(); diff --git a/ps2xTest/src/ps2_recompiler_tests.cpp b/ps2xTest/src/ps2_recompiler_tests.cpp index 1b569a292..af1fb0a3f 100644 --- a/ps2xTest/src/ps2_recompiler_tests.cpp +++ b/ps2xTest/src/ps2_recompiler_tests.cpp @@ -396,6 +396,133 @@ static bool writeThreadEntryDataElf(const std::filesystem::path &elfPath) return writer.save(elfPath.string()); } +// Builds a fixture ELF with a single "caller" function [callerStart, callerStart+0x10) +// that JALs to jalTarget - an address that lies entirely outside every section of +// this ELF (that is the point: it exists only in a sibling unit's ELF). Pairs with +// writeContainerOnlyElf (the callee side) to build a genuine two-ELF cross-unit +// regression: this unit's own CollectExternalCallTargets sees a jal landing outside +// every one of its own sections, which the pre-fix inclusion gate dropped. +static bool writeJalToForeignTargetElf(const std::filesystem::path &elfPath, + uint32_t callerStart, + uint32_t jalTarget) +{ + ELFIO::elfio writer; + writer.create(ELFIO::ELFCLASS32, ELFIO::ELFDATA2LSB); + writer.set_os_abi(ELFIO::ELFOSABI_NONE); + writer.set_type(ELFIO::ET_EXEC); + writer.set_machine(ELFIO::EM_MIPS); + writer.set_entry(callerStart); + + ELFIO::section *text = writer.sections.add(".text"); + text->set_type(ELFIO::SHT_PROGBITS); + text->set_flags(ELFIO::SHF_ALLOC | ELFIO::SHF_EXECINSTR); + text->set_addr_align(4); + text->set_address(callerStart); + + const uint32_t jalWord = (static_cast(OPCODE_JAL) << 26) | ((jalTarget >> 2) & 0x03FFFFFFu); + const std::array textWords = { + jalWord, // jal jalTarget + 0x00000000u, // nop (delay slot) + 0x03E00008u, // jr $ra + 0x00000000u // nop (delay slot) + }; + text->set_data(reinterpret_cast(textWords.data()), + static_cast(textWords.size() * sizeof(uint32_t))); + + ELFIO::section *strtab = writer.sections.add(".strtab"); + strtab->set_type(ELFIO::SHT_STRTAB); + strtab->set_addr_align(1); + + ELFIO::section *symtab = writer.sections.add(".symtab"); + symtab->set_type(ELFIO::SHT_SYMTAB); + symtab->set_info(1); + symtab->set_link(strtab->get_index()); + symtab->set_addr_align(4); + symtab->set_entry_size(writer.get_default_entry_size(ELFIO::SHT_SYMTAB)); + + ELFIO::symbol_section_accessor symbols(writer, symtab); + ELFIO::string_section_accessor strings(strtab); + symbols.add_symbol(strings, "", 0, 0, ELFIO::STB_LOCAL, ELFIO::STT_NOTYPE, 0, ELFIO::SHN_UNDEF); + symbols.add_symbol(strings, "caller_fn", callerStart, + static_cast(textWords.size() * sizeof(uint32_t)), + ELFIO::STB_GLOBAL, ELFIO::STT_FUNC, 0, text->get_index()); + + ELFIO::segment *textSegment = writer.segments.add(); + textSegment->set_type(ELFIO::PT_LOAD); + textSegment->set_flags(ELFIO::PF_R | ELFIO::PF_X); + textSegment->set_align(0x1000); + textSegment->add_section_index(text->get_index(), text->get_addr_align()); + + return writer.save(elfPath.string()); +} + +// Builds a fixture ELF with a single container_fn [containerStart, containerEnd), +// NOP-filled and jr $ra-terminated like writeContainerOnlyElf, except word index 0 +// is a jal to foreignJalTarget (assumed to land outside every section of this ELF, +// i.e. inside a sibling unit) instead of a nop. This unit therefore both exposes a +// mid-body target (containerStart+8, a real decoded instruction boundary that is not +// the function head) for a sibling to call into, AND itself calls into a sibling's +// mid-body target - used to build the mutually-calling two-phase multi-unit test. +static bool writeMutualCallElf(const std::filesystem::path &elfPath, + uint32_t containerStart, + uint32_t containerEnd, + uint32_t foreignJalTarget) +{ + ELFIO::elfio writer; + writer.create(ELFIO::ELFCLASS32, ELFIO::ELFDATA2LSB); + writer.set_os_abi(ELFIO::ELFOSABI_NONE); + writer.set_type(ELFIO::ET_EXEC); + writer.set_machine(ELFIO::EM_MIPS); + writer.set_entry(containerStart); + + ELFIO::section *text = writer.sections.add(".text"); + text->set_type(ELFIO::SHT_PROGBITS); + text->set_flags(ELFIO::SHF_ALLOC | ELFIO::SHF_EXECINSTR); + text->set_addr_align(4); + text->set_address(containerStart); + + const uint32_t size = containerEnd - containerStart; + const size_t wordCount = size / sizeof(uint32_t); + if (wordCount < 4) + { + return false; + } + const uint32_t jalWord = (static_cast(OPCODE_JAL) << 26) | ((foreignJalTarget >> 2) & 0x03FFFFFFu); + std::vector textWords(wordCount, 0x00000000u); // NOP-fill the whole body + textWords[0] = jalWord; // jal foreignJalTarget + // textWords[1] stays the delay-slot nop; textWords[2] (containerStart+8) is the + // mid-body target this unit exposes to a sibling. + textWords[wordCount - 2] = 0x03E00008u; // jr $ra + textWords[wordCount - 1] = 0x00000000u; // nop (delay slot) + text->set_data(reinterpret_cast(textWords.data()), + static_cast(textWords.size() * sizeof(uint32_t))); + + ELFIO::section *strtab = writer.sections.add(".strtab"); + strtab->set_type(ELFIO::SHT_STRTAB); + strtab->set_addr_align(1); + + ELFIO::section *symtab = writer.sections.add(".symtab"); + symtab->set_type(ELFIO::SHT_SYMTAB); + symtab->set_info(1); + symtab->set_link(strtab->get_index()); + symtab->set_addr_align(4); + symtab->set_entry_size(writer.get_default_entry_size(ELFIO::SHT_SYMTAB)); + + ELFIO::symbol_section_accessor symbols(writer, symtab); + ELFIO::string_section_accessor strings(strtab); + symbols.add_symbol(strings, "", 0, 0, ELFIO::STB_LOCAL, ELFIO::STT_NOTYPE, 0, ELFIO::SHN_UNDEF); + symbols.add_symbol(strings, "container_fn", containerStart, size, + ELFIO::STB_GLOBAL, ELFIO::STT_FUNC, 0, text->get_index()); + + ELFIO::segment *textSegment = writer.segments.add(); + textSegment->set_type(ELFIO::PT_LOAD); + textSegment->set_flags(ELFIO::PF_R | ELFIO::PF_X); + textSegment->set_align(0x1000); + textSegment->add_section_index(text->get_index(), text->get_addr_align()); + + return writer.save(elfPath.string()); +} + // Returns every line of `content` containing `needle` - used to inspect the emitted // register_functions.cpp function-table initializer, whose lines look like: // g_ps2RecompiledFunctionTable[] = ; // 0x
@@ -1359,6 +1486,426 @@ void register_ps2_recompiler_tests() std::filesystem::remove_all(workDir, removeError); }); + // Headline two-ELF cross-unit regression: drives two REAL, independent + // PS2Recompiler instances (A and B) over two separate ELF fixtures with no + // shared address space overlap in intent - A's jal target T exists only + // inside B and lies entirely outside every section of A. Proves the full + // chain: A's analysis phase emits T (Fix 1, the permissive collector) into + // A's manifest, independent of build order relative to B; B then ingests + // A's manifest and registers T into its own owning function. + tc.Run("two-ELF: A emits T, B ingests T", [](TestCase &t) { + const auto uniqueSuffix = std::to_string( + static_cast(std::chrono::steady_clock::now().time_since_epoch().count())); + const std::filesystem::path workDir = + std::filesystem::temp_directory_path() / ("ps2recomp-two-elf-" + uniqueSuffix); + std::error_code mkdirError; + std::filesystem::create_directories(workDir, mkdirError); + t.IsTrue(!mkdirError, "work directory should be created"); + + constexpr uint32_t callerStart = 0x00300000u; // A's own base range + constexpr uint32_t containerStart = 0x00100000u; // B's own base range + constexpr uint32_t containerEnd = 0x00100018u; // container_fn: 6 NOP-filled words, jr $ra tail + constexpr uint32_t targetT = 0x00100008u; // T: inside B, not B's head, not any of A's own sections + + const std::filesystem::path elfAPath = workDir / "a.elf"; + const std::filesystem::path elfBPath = workDir / "b.elf"; + const bool wroteA = writeJalToForeignTargetElf(elfAPath, callerStart, targetT); + const bool wroteB = writeContainerOnlyElf(elfBPath, containerStart, containerEnd); + t.IsTrue(wroteA, "ELF A fixture should be generated"); + t.IsTrue(wroteB, "ELF B fixture should be generated"); + if (!wroteA || !wroteB) + { + std::error_code cleanupError; + std::filesystem::remove_all(workDir, cleanupError); + return; + } + + const std::filesystem::path outA = workDir / "out_a"; + const std::filesystem::path outAAgain = workDir / "out_a_again"; + const std::filesystem::path outB = workDir / "out_b"; + const std::filesystem::path outBNoManifest = workDir / "out_b_no_manifest"; + + const std::filesystem::path configAPath = workDir / "a.toml"; + { + std::ofstream cfg(configAPath); + t.IsTrue(static_cast(cfg), "config A should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfAPath.generic_string() << "\"\n"; + cfg << "output = \"" << outA.generic_string() << "\"\n"; + } + const std::filesystem::path configAAgainPath = workDir / "a_again.toml"; + { + std::ofstream cfg(configAAgainPath); + t.IsTrue(static_cast(cfg), "config A (again) should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfAPath.generic_string() << "\"\n"; + cfg << "output = \"" << outAAgain.generic_string() << "\"\n"; + } + + // --- Analysis phase for A, run once, then again after B's own analysis + // phase runs in between - proves emission is order-independent (it depends + // only on A's own decoded functions/sections, never on any sibling). + { + PS2Recompiler recompilerA(configAPath.string()); + t.IsTrue(recompilerA.initialize(), "A: initialize() should succeed"); + t.IsTrue(recompilerA.recompile(true), "A: analysis-phase recompile(true) should succeed"); + } + + const std::filesystem::path configBAnalysisPath = workDir / "b_analysis.toml"; + const std::filesystem::path outBAnalysis = workDir / "out_b_analysis"; + { + std::ofstream cfg(configBAnalysisPath); + t.IsTrue(static_cast(cfg), "config B (analysis) should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfBPath.generic_string() << "\"\n"; + cfg << "output = \"" << outBAnalysis.generic_string() << "\"\n"; + } + { + PS2Recompiler recompilerBAnalysis(configBAnalysisPath.string()); + t.IsTrue(recompilerBAnalysis.initialize(), "B: initialize() should succeed for its own analysis phase"); + t.IsTrue(recompilerBAnalysis.recompile(true), "B: analysis-phase recompile(true) should succeed"); + } + + { + PS2Recompiler recompilerAAgain(configAAgainPath.string()); + t.IsTrue(recompilerAAgain.initialize(), "A (again): initialize() should succeed"); + t.IsTrue(recompilerAAgain.recompile(true), "A (again): analysis-phase recompile(true) should succeed"); + } + + const std::filesystem::path manifestAPath = outA / "external_call_targets.txt"; + const std::filesystem::path manifestAAgainPath = outAAgain / "external_call_targets.txt"; + std::ifstream manifestAFile(manifestAPath, std::ios::binary); + std::ifstream manifestAAgainFile(manifestAAgainPath, std::ios::binary); + t.IsTrue(static_cast(manifestAFile), "A's manifest should be written"); + t.IsTrue(static_cast(manifestAAgainFile), "A's manifest (again) should be written"); + std::ostringstream manifestAStream; + std::ostringstream manifestAAgainStream; + manifestAStream << manifestAFile.rdbuf(); + manifestAAgainStream << manifestAAgainFile.rdbuf(); + const std::string manifestAContent = manifestAStream.str(); + const std::string manifestAAgainContent = manifestAAgainStream.str(); + + t.IsTrue(manifestAContent.find("0x00100008") != std::string::npos, + "A's emitted manifest must contain T (0x00100008), which lies outside every section of A - " + "this is the case Fix 1's permissive collector restores"); + t.Equals(manifestAContent, manifestAAgainContent, + "A's emitted manifest must be byte-identical whether A's analysis runs before or after " + "B's analysis - emission depends only on A's own decoded functions/sections"); + + // --- B ingests A's manifest and registers T into its own owning unit. + const std::filesystem::path configBPath = workDir / "b.toml"; + { + std::ofstream cfg(configBPath); + t.IsTrue(static_cast(cfg), "config B should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfBPath.generic_string() << "\"\n"; + cfg << "output = \"" << outB.generic_string() << "\"\n"; + cfg << "external_call_target_manifests = [\"" << manifestAPath.generic_string() << "\"]\n"; + } + { + PS2Recompiler recompilerB(configBPath.string()); + t.IsTrue(recompilerB.initialize(), "B: initialize() should succeed"); + t.IsTrue(recompilerB.recompile(false), "B: generate-phase recompile(false) should succeed"); + recompilerB.generateOutput(); + + const std::filesystem::path registerPath = outB / "register_functions.cpp"; + std::ifstream registerFile(registerPath); + t.IsTrue(static_cast(registerFile), "B's register_functions.cpp should be written"); + std::ostringstream contentStream; + contentStream << registerFile.rdbuf(); + const std::string content = contentStream.str(); + + const auto headLines = findLinesContaining(content, "// 0x100000"); + const auto targetLines = findLinesContaining(content, "// 0x100008"); + + t.IsTrue(!headLines.empty(), "B's container head 0x100000 should be registered (sanity)"); + t.IsTrue(!targetLines.empty(), + "T (0x100008) must be registered in B once B ingests A's manifest"); + + if (!headLines.empty() && !targetLines.empty()) + { + const std::string headOwner = extractOwnerNameFromRegistrationLine(headLines.front()); + const std::string targetOwner = extractOwnerNameFromRegistrationLine(targetLines.front()); + t.Equals(targetOwner, headOwner, + "T must resolve to the SAME owner name as B's container head, proving dispatch " + "resumes into B's owning unit"); + } + } + + // --- Negative arm: B with no manifest configured must not discover T - + // nothing in B's own fixture can discover a mid-body target on its own. + const std::filesystem::path configBNoManifestPath = workDir / "b_no_manifest.toml"; + { + std::ofstream cfg(configBNoManifestPath); + t.IsTrue(static_cast(cfg), "config B (no manifest) should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfBPath.generic_string() << "\"\n"; + cfg << "output = \"" << outBNoManifest.generic_string() << "\"\n"; + } + { + PS2Recompiler recompilerBNoManifest(configBNoManifestPath.string()); + t.IsTrue(recompilerBNoManifest.initialize(), "B (no manifest): initialize() should succeed"); + t.IsTrue(recompilerBNoManifest.recompile(false), "B (no manifest): recompile(false) should succeed"); + recompilerBNoManifest.generateOutput(); + + const std::filesystem::path registerPath = outBNoManifest / "register_functions.cpp"; + std::ifstream registerFile(registerPath); + t.IsTrue(static_cast(registerFile), "B's register_functions.cpp should be written"); + std::ostringstream contentStream; + contentStream << registerFile.rdbuf(); + const std::string content = contentStream.str(); + + const auto targetLines = findLinesContaining(content, "// 0x100008"); + t.IsTrue(targetLines.empty(), + "without A's manifest, T (0x100008) must NOT be registered - reproduces the original " + "cross-unit gap"); + } + + std::error_code removeError; + std::filesystem::remove_all(workDir, removeError); + }); + + tc.Run("missing configured manifest hard-fails at generate time", [](TestCase &t) { + const auto uniqueSuffix = std::to_string( + static_cast(std::chrono::steady_clock::now().time_since_epoch().count())); + const std::filesystem::path workDir = + std::filesystem::temp_directory_path() / ("ps2recomp-missing-manifest-" + uniqueSuffix); + std::error_code mkdirError; + std::filesystem::create_directories(workDir, mkdirError); + t.IsTrue(!mkdirError, "work directory should be created"); + + const std::filesystem::path elfPath = workDir / "fixture.elf"; + const bool wroteElf = writeContainerOnlyElf(elfPath, 0x00100000u, 0x00100018u); + t.IsTrue(wroteElf, "fixture ELF should be generated"); + if (!wroteElf) + { + std::error_code cleanupError; + std::filesystem::remove_all(workDir, cleanupError); + return; + } + + const std::filesystem::path missingManifestPath = workDir / "does_not_exist.txt"; + const std::filesystem::path outDir = workDir / "out"; + const std::filesystem::path configPath = workDir / "config.toml"; + { + std::ofstream cfg(configPath); + t.IsTrue(static_cast(cfg), "config should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfPath.generic_string() << "\"\n"; + cfg << "output = \"" << outDir.generic_string() << "\"\n"; + cfg << "external_call_target_manifests = [\"" << missingManifestPath.generic_string() << "\"]\n"; + } + + PS2Recompiler recompiler(configPath.string()); + t.IsTrue(recompiler.initialize(), "initialize() should succeed"); + t.IsFalse(recompiler.recompile(false), + "recompile(false) must hard-fail when a configured manifest cannot be opened"); + + std::error_code removeError; + std::filesystem::remove_all(workDir, removeError); + }); + + tc.Run("existing (even empty) configured manifest does not hard-fail", [](TestCase &t) { + const auto uniqueSuffix = std::to_string( + static_cast(std::chrono::steady_clock::now().time_since_epoch().count())); + const std::filesystem::path workDir = + std::filesystem::temp_directory_path() / ("ps2recomp-empty-manifest-" + uniqueSuffix); + std::error_code mkdirError; + std::filesystem::create_directories(workDir, mkdirError); + t.IsTrue(!mkdirError, "work directory should be created"); + + const std::filesystem::path elfPath = workDir / "fixture.elf"; + const bool wroteElf = writeContainerOnlyElf(elfPath, 0x00100000u, 0x00100018u); + t.IsTrue(wroteElf, "fixture ELF should be generated"); + if (!wroteElf) + { + std::error_code cleanupError; + std::filesystem::remove_all(workDir, cleanupError); + return; + } + + const std::filesystem::path emptyManifestPath = workDir / "empty_manifest.txt"; + { + std::ofstream manifestFile(emptyManifestPath); + t.IsTrue(static_cast(manifestFile), "empty manifest file should be writable"); + } + + const std::filesystem::path outDir = workDir / "out"; + const std::filesystem::path configPath = workDir / "config.toml"; + { + std::ofstream cfg(configPath); + t.IsTrue(static_cast(cfg), "config should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfPath.generic_string() << "\"\n"; + cfg << "output = \"" << outDir.generic_string() << "\"\n"; + cfg << "external_call_target_manifests = [\"" << emptyManifestPath.generic_string() << "\"]\n"; + } + + PS2Recompiler recompiler(configPath.string()); + t.IsTrue(recompiler.initialize(), "initialize() should succeed"); + t.IsTrue(recompiler.recompile(false), + "recompile(false) must NOT hard-fail when a configured manifest exists (even if empty) - " + "this half-guard catches a mutation that makes the hard-fail unconditional"); + + std::error_code removeError; + std::filesystem::remove_all(workDir, removeError); + }); + + tc.Run("analysis phase never hard-fails on a missing sibling manifest", [](TestCase &t) { + const auto uniqueSuffix = std::to_string( + static_cast(std::chrono::steady_clock::now().time_since_epoch().count())); + const std::filesystem::path workDir = + std::filesystem::temp_directory_path() / ("ps2recomp-analysis-safe-" + uniqueSuffix); + std::error_code mkdirError; + std::filesystem::create_directories(workDir, mkdirError); + t.IsTrue(!mkdirError, "work directory should be created"); + + const std::filesystem::path elfAPath = workDir / "a.elf"; + const bool wroteA = writeContainerOnlyElf(elfAPath, 0x00100000u, 0x00100018u); + t.IsTrue(wroteA, "ELF A fixture should be generated"); + if (!wroteA) + { + std::error_code cleanupError; + std::filesystem::remove_all(workDir, cleanupError); + return; + } + + // B's manifest has not been emitted anywhere in this test - it is a sibling + // that simply has not run its own analysis phase yet. + const std::filesystem::path notYetEmittedBManifestPath = workDir / "b_out" / "external_call_targets.txt"; + + const std::filesystem::path outA = workDir / "out_a"; + const std::filesystem::path configAPath = workDir / "a.toml"; + { + std::ofstream cfg(configAPath); + t.IsTrue(static_cast(cfg), "config A should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfAPath.generic_string() << "\"\n"; + cfg << "output = \"" << outA.generic_string() << "\"\n"; + cfg << "external_call_target_manifests = [\"" << notYetEmittedBManifestPath.generic_string() << "\"]\n"; + } + + PS2Recompiler recompilerA(configAPath.string()); + t.IsTrue(recompilerA.initialize(), "A: initialize() should succeed"); + t.IsTrue(recompilerA.recompile(true), + "A: analysis-phase recompile(true) must succeed even though its configured sibling " + "manifest does not exist yet - the split makes the hard-fail safe"); + + const std::filesystem::path manifestAPath = outA / "external_call_targets.txt"; + std::ifstream manifestAFile(manifestAPath); + t.IsTrue(static_cast(manifestAFile), "A's own manifest should still be emitted"); + + std::error_code removeError; + std::filesystem::remove_all(workDir, removeError); + }); + + tc.Run("two-phase clean build of mutually-calling units", [](TestCase &t) { + const auto uniqueSuffix = std::to_string( + static_cast(std::chrono::steady_clock::now().time_since_epoch().count())); + const std::filesystem::path workDir = + std::filesystem::temp_directory_path() / ("ps2recomp-mutual-two-phase-" + uniqueSuffix); + std::error_code mkdirError; + std::filesystem::create_directories(workDir, mkdirError); + t.IsTrue(!mkdirError, "work directory should be created"); + + constexpr uint32_t containerStartA = 0x00100000u; + constexpr uint32_t containerEndA = 0x00100020u; + constexpr uint32_t targetA = 0x00100008u; // A's mid-body target, exposed to B + + constexpr uint32_t containerStartB = 0x00200000u; + constexpr uint32_t containerEndB = 0x00200020u; + constexpr uint32_t targetB = 0x00200008u; // B's mid-body target, exposed to A + + const std::filesystem::path elfAPath = workDir / "a.elf"; + const std::filesystem::path elfBPath = workDir / "b.elf"; + const bool wroteA = writeMutualCallElf(elfAPath, containerStartA, containerEndA, targetB); + const bool wroteB = writeMutualCallElf(elfBPath, containerStartB, containerEndB, targetA); + t.IsTrue(wroteA, "ELF A fixture should be generated"); + t.IsTrue(wroteB, "ELF B fixture should be generated"); + if (!wroteA || !wroteB) + { + std::error_code cleanupError; + std::filesystem::remove_all(workDir, cleanupError); + return; + } + + const std::filesystem::path outA = workDir / "out_a"; + const std::filesystem::path outB = workDir / "out_b"; + const std::filesystem::path manifestAPath = outA / "external_call_targets.txt"; + const std::filesystem::path manifestBPath = outB / "external_call_targets.txt"; + + const std::filesystem::path configAPath = workDir / "a.toml"; + { + std::ofstream cfg(configAPath); + t.IsTrue(static_cast(cfg), "config A should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfAPath.generic_string() << "\"\n"; + cfg << "output = \"" << outA.generic_string() << "\"\n"; + cfg << "external_call_target_manifests = [\"" << manifestBPath.generic_string() << "\"]\n"; + } + const std::filesystem::path configBPath = workDir / "b.toml"; + { + std::ofstream cfg(configBPath); + t.IsTrue(static_cast(cfg), "config B should be writable"); + cfg << "[general]\n"; + cfg << "input = \"" << elfBPath.generic_string() << "\"\n"; + cfg << "output = \"" << outB.generic_string() << "\"\n"; + cfg << "external_call_target_manifests = [\"" << manifestAPath.generic_string() << "\"]\n"; + } + + // --- Phase 1: analysis phase for both units, neither sibling manifest + // exists yet at the time either analysis phase runs. + { + PS2Recompiler recompilerA(configAPath.string()); + t.IsTrue(recompilerA.initialize(), "A: initialize() should succeed"); + t.IsTrue(recompilerA.recompile(true), "A: phase 1 recompile(true) should succeed"); + } + { + PS2Recompiler recompilerB(configBPath.string()); + t.IsTrue(recompilerB.initialize(), "B: initialize() should succeed"); + t.IsTrue(recompilerB.recompile(true), "B: phase 1 recompile(true) should succeed"); + } + + std::ifstream manifestAFile(manifestAPath); + std::ifstream manifestBFile(manifestBPath); + t.IsTrue(static_cast(manifestAFile), "A's manifest should exist after phase 1"); + t.IsTrue(static_cast(manifestBFile), "B's manifest should exist after phase 1"); + + // --- Phase 2: generate phase for both units, each now ingesting the + // other's phase-1 manifest with no missing-manifest failure. + { + PS2Recompiler recompilerA(configAPath.string()); + t.IsTrue(recompilerA.initialize(), "A: initialize() should succeed for phase 2"); + t.IsTrue(recompilerA.recompile(false), "A: phase 2 recompile(false) should succeed"); + recompilerA.generateOutput(); + + std::ifstream registerFile(outA / "register_functions.cpp"); + t.IsTrue(static_cast(registerFile), "A's register_functions.cpp should be written"); + std::ostringstream contentStream; + contentStream << registerFile.rdbuf(); + const auto targetLines = findLinesContaining(contentStream.str(), "// 0x100008"); + t.IsTrue(!targetLines.empty(), + "A must register its own mid-body target 0x100008 once it ingests B's manifest"); + } + { + PS2Recompiler recompilerB(configBPath.string()); + t.IsTrue(recompilerB.initialize(), "B: initialize() should succeed for phase 2"); + t.IsTrue(recompilerB.recompile(false), "B: phase 2 recompile(false) should succeed"); + recompilerB.generateOutput(); + + std::ifstream registerFile(outB / "register_functions.cpp"); + t.IsTrue(static_cast(registerFile), "B's register_functions.cpp should be written"); + std::ostringstream contentStream; + contentStream << registerFile.rdbuf(); + const auto targetLines = findLinesContaining(contentStream.str(), "// 0x200008"); + t.IsTrue(!targetLines.empty(), + "B must register its own mid-body target 0x200008 once it ingests A's manifest"); + } + + std::error_code removeError; + std::filesystem::remove_all(workDir, removeError); + }); + // Tier 1 end-to-end regression test for data-embedded thread entry discovery: drives // a REAL PS2Recompiler instance over a real ELF whose .data section contains a // ThreadParam struct read via the production m_elfParser->readWord/isValidAddress From f36a152d6c374167861947a5edf3e812ff913b6d Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 08:09:53 -0400 Subject: [PATCH 5/7] fix(recomp): opcode-keyed conservative def model + basic-block restriction for the thread-entry scan The $a0 constant-propagation walk tracked "who writes this register" via a single writtenRegisterOrZero helper (I-type -> rt, SPECIAL -> rd) with no concept of what an instruction actually reads vs. writes, and no bound on how far it could walk relative to control flow. That let a store of $a0 (which reads it) spuriously erase a resolved value, and let the walk trust a value across an unrelated intervening call or branch that does not dominate the CreateThread site - a walk with no block/dominance check can easily accept a stale register value as if it were still live. Replace it with mayWriteGprs: a small conservative superset of the GPRs an instruction may write, keyed off opcode (+ SPECIAL function) so unit tests built by hand behave identically to real decoded ELF instructions. Stores and coprocessor stores read their operand and are excluded; COP0/COP1/ COP2/MMI are treated conservatively as writing both rt and rd, since an MFC*-style GPR write can't be distinguished from an MTC*-style GPR read without sub-decoding fmt - always the safe direction (a missed entry, never a false survival). Bound the constant-propagation walk (and the Step-2 $v1 back-scan) to the call site's basic block: scan backward for the nearest preceding control-transfer instruction and start the walk after its delay slot. A basic block has no interior control transfer, so a call can never sit mid-block - a resolved $a0 can no longer survive across an unrelated call, and the walk never trusts a value across a branch/jump it doesn't dominate. Also close the one remaining gap this leaves: a forward jal/j elsewhere in the function landing inside the walked window is a join point the backward scan can't see, so shrink the walk to start at that join too. --- ps2xRecomp/src/lib/ps2_recompiler.cpp | 258 ++++++++++++++++++++++++-- ps2xTest/src/ps2_recompiler_tests.cpp | 242 ++++++++++++++++++++++++ 2 files changed, 481 insertions(+), 19 deletions(-) diff --git a/ps2xRecomp/src/lib/ps2_recompiler.cpp b/ps2xRecomp/src/lib/ps2_recompiler.cpp index 1118623bd..92897b110 100644 --- a/ps2xRecomp/src/lib/ps2_recompiler.cpp +++ b/ps2xRecomp/src/lib/ps2_recompiler.cpp @@ -2381,33 +2381,181 @@ namespace ps2recomp return inst.opcode == OPCODE_SPECIAL && inst.function == SPECIAL_SYSCALL; } - // Simplified "who writes this register" model shared by the $v1 clobber check - // and the $a0 constant-propagation walk below: for I-type opcodes the written - // register is rt, for OPCODE_SPECIAL it is rd. $zero is never considered written. - // J-type instructions (j/jal) are excluded: the decoder unconditionally populates - // rs/rt/rd from the raw instruction bits (see R5900Decoder::decodeInstruction), - // but for a 26-bit jump target those bit positions are part of the target, not a - // register field, so treating them as a register write would be spurious. - uint32_t writtenRegisterOrZero(const Instruction &inst) + // A small fixed-capacity set of GPR numbers, used instead of a heap-allocating + // container since mayWriteGprs never needs to report more than two registers. + struct GprWriteSet { - if (inst.opcode == OPCODE_J || inst.opcode == OPCODE_JAL) + uint32_t regs[2] = {0u, 0u}; + uint8_t count = 0u; + + void add(uint32_t reg) { - return 0u; + if (reg != 0u && count < 2u) + { + regs[count++] = reg; + } } - if (inst.opcode == OPCODE_SPECIAL) - { - return inst.rd; + + const uint32_t *begin() const { return regs; } + const uint32_t *end() const { return regs + count; } + }; + + // Conservative superset of the GPRs `inst` may write, keyed off opcode (+ + // SPECIAL function) rather than the decoder-derived modifiesGPR/isStore/... + // booleans: the unit-test fixtures build Instructions by hand and populate + // only raw fields, so deriving from opcode keeps unit-test and real-ELF + // behavior identical. Only $v1 and $a0 are ever tracked by callers of this + // helper, so this need not be a disassembler-complete def model - it only has + // to never miss a write to one of those two registers (soundness target: a + // tracked register must never falsely survive a clobber; over-invalidating is + // always safe and only costs a missed entry). + GprWriteSet mayWriteGprs(const Instruction &inst) + { + GprWriteSet result; + + switch (inst.opcode) + { + // Stores (including coprocessor stores) read rt, they don't write it; + // CACHE/PREF and coprocessor loads that target a coprocessor register + // (not a GPR) write nothing tracked; branches and jumps/calls write + // nothing tracked ($ra=31 for jal/…AL REGIMM variants is untracked). + case OPCODE_SB: + case OPCODE_SH: + case OPCODE_SWL: + case OPCODE_SW: + case OPCODE_SDL: + case OPCODE_SDR: + case OPCODE_SWR: + case OPCODE_SC: + case OPCODE_SCD: + case OPCODE_SQ: + case OPCODE_SD: + case OPCODE_SWC1: + case OPCODE_SWC2: + case OPCODE_SDC1: + case OPCODE_SDC2: // SQC2 aliases SDC2 (same enum value) + case OPCODE_CACHE: + case OPCODE_PREF: + case OPCODE_LWC1: + case OPCODE_LWC2: + case OPCODE_LDC1: + case OPCODE_LDC2: // LQC2 aliases LDC2 (same enum value) + case OPCODE_BEQ: + case OPCODE_BNE: + case OPCODE_BLEZ: + case OPCODE_BGTZ: + case OPCODE_BEQL: + case OPCODE_BNEL: + case OPCODE_BLEZL: + case OPCODE_BGTZL: + case OPCODE_REGIMM: + case OPCODE_J: + case OPCODE_JAL: + return result; + + // I-type ALU and GPR loads write rt. + case OPCODE_ADDI: + case OPCODE_ADDIU: + case OPCODE_SLTI: + case OPCODE_SLTIU: + case OPCODE_ANDI: + case OPCODE_ORI: + case OPCODE_XORI: + case OPCODE_LUI: + case OPCODE_DADDI: + case OPCODE_DADDIU: + case OPCODE_LB: + case OPCODE_LH: + case OPCODE_LWL: + case OPCODE_LW: + case OPCODE_LBU: + case OPCODE_LHU: + case OPCODE_LWR: + case OPCODE_LWU: + case OPCODE_LD: + case OPCODE_LDL: + case OPCODE_LDR: + case OPCODE_LL: + case OPCODE_LLD: + case OPCODE_LQ: + result.add(inst.rt); + return result; + + case OPCODE_SPECIAL: + // JR/JALR/SYSCALL/BREAK/SYNC write nothing tracked (JALR's rd is a + // link register, conventionally $ra=31, untracked). Every other + // SPECIAL form (arithmetic/logical/shift/move) writes rd; the forms + // that write HI/LO/SA or nothing instead (MULT/DIV/MTHI/MTLO/MTSA, + // traps, …) encode rd=0 in valid encodings, so {rd} degenerates to + // {} naturally. + if (inst.function == SPECIAL_JR || inst.function == SPECIAL_JALR || + inst.function == SPECIAL_SYSCALL || inst.function == SPECIAL_BREAK || + inst.function == SPECIAL_SYNC) + { + return result; + } + result.add(inst.rd); + return result; + + case OPCODE_COP0: + case OPCODE_COP1: + case OPCODE_COP2: + case OPCODE_MMI: + // Unknown/complex: an MFC*/CFC*/QMFC2 writes GPR rt; other forms + // (e.g. mtc1, which only reads rt) do not. Rather than sub-decode + // fmt, conservatively invalidate both rt and rd - the safe direction + // (a missed entry, never a false survival). + result.add(inst.rt); + result.add(inst.rd); + return result; + + default: + return result; } - return inst.rt; } - bool writesTrackedRegister(const Instruction &inst, uint32_t reg) + bool mayClobber(const Instruction &inst, uint32_t reg) { if (reg == 0u) { return false; } - return writtenRegisterOrZero(inst) == reg; + for (uint32_t written : mayWriteGprs(inst)) + { + if (written == reg) + { + return true; + } + } + return false; + } + + // True for instructions that transfer control - used to bound the + // constant-propagation walk and the $v1 back-scan to a single basic block, so + // neither can cross a branch/jump and trust register state that does not + // dominate the site being analyzed. + bool isControlTransfer(const Instruction &inst) + { + switch (inst.opcode) + { + case OPCODE_J: + case OPCODE_JAL: + case OPCODE_BEQ: + case OPCODE_BNE: + case OPCODE_BLEZ: + case OPCODE_BGTZ: + case OPCODE_BEQL: + case OPCODE_BNEL: + case OPCODE_BLEZL: + case OPCODE_BGTZL: + case OPCODE_REGIMM: + return true; + case OPCODE_SPECIAL: + return inst.function == SPECIAL_JR || inst.function == SPECIAL_JALR || + inst.function == SPECIAL_SYSCALL || inst.function == SPECIAL_BREAK; + default: + return false; + } } struct ThreadCreateCallSite @@ -2496,7 +2644,16 @@ namespace ps2recomp foundAddiuV1 = true; break; } - if (writesTrackedRegister(prior, kThreadEntryRegV1)) + // A branch/jump between the candidate materialization and the + // syscall means the materialization does not dominate this site - + // stop without accepting it (this instruction is also excluded + // from the block, so it can't be the found materialization even + // if it happened to match isAddiuV1SyscallImm). + if (isControlTransfer(prior)) + { + break; + } + if (mayClobber(prior, kThreadEntryRegV1)) { break; } @@ -2532,6 +2689,66 @@ namespace ps2recomp const size_t windowStart = (site.index >= kThreadConstantScanWindow) ? site.index - kThreadConstantScanWindow : 0u; + // Restrict the walk to the call site's basic block (+ the retained delay + // slot in windowEnd): scan backward from immediately before the call for + // the nearest preceding control transfer. A basic block has no interior + // control transfer, so a jal/inline syscall can never sit mid-block - a + // resolved $a0 structurally cannot survive across an unrelated call, and + // the walk never crosses a branch/jump it doesn't dominate. + size_t blockStart = windowStart; + if (site.index > 0u) + { + size_t k = site.index - 1u; + while (true) + { + if (isControlTransfer(instructions[k])) + { + blockStart = k + 2u; // block begins after the terminator's delay slot + break; + } + if (k <= windowStart) + { + break; + } + --k; + } + } + size_t walkStart = std::max(blockStart, windowStart); + + // --- Optional dominance hardening: a forward jal/j from elsewhere in the + // owner function whose absolute target lands inside [walkStart, windowEnd] + // is a join point the backward-only scan above cannot see - the "block" it + // found would then have more than one entry, so state resolved before the + // join cannot be trusted to dominate the call. Move walkStart forward to + // the join (over-invalidation - the safe direction: this can only lose a + // candidate, never let a stale value survive). Self-contained and + // independently removable without affecting the required backward-only + // core above. + { + const uint32_t walkStartAddress = instructions[walkStart].address; + const uint32_t windowEndAddress = instructions[windowEnd].address; + for (const Instruction &candidate : instructions) + { + if (candidate.opcode != OPCODE_J && candidate.opcode != OPCODE_JAL) + { + continue; + } + const uint32_t joinTarget = buildAbsoluteJumpTarget(candidate.address, candidate.target); + if (joinTarget <= walkStartAddress || joinTarget > windowEndAddress) + { + continue; + } + for (size_t j = walkStart; j <= windowEnd; ++j) + { + if (instructions[j].address == joinTarget) + { + walkStart = std::max(walkStart, j); + break; + } + } + } + } + std::unordered_map resolved; auto invalidate = [&resolved](uint32_t reg) { @@ -2541,7 +2758,7 @@ namespace ps2recomp } }; - for (size_t idx = windowStart; idx <= windowEnd; ++idx) + for (size_t idx = walkStart; idx <= windowEnd; ++idx) { const Instruction &inst = instructions[idx]; @@ -2581,7 +2798,10 @@ namespace ps2recomp } } - invalidate(writtenRegisterOrZero(inst)); + for (uint32_t reg : mayWriteGprs(inst)) + { + invalidate(reg); + } } auto a0It = resolved.find(kThreadEntryRegA0); diff --git a/ps2xTest/src/ps2_recompiler_tests.cpp b/ps2xTest/src/ps2_recompiler_tests.cpp index af1fb0a3f..773024153 100644 --- a/ps2xTest/src/ps2_recompiler_tests.cpp +++ b/ps2xTest/src/ps2_recompiler_tests.cpp @@ -130,6 +130,49 @@ static Instruction makeLw(uint32_t address, uint32_t rt, uint32_t rs) return inst; } +static Instruction makeSw(uint32_t address, uint32_t rt, uint32_t rs) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = OPCODE_SW; + inst.rt = rt; + inst.rs = rs; + inst.isStore = true; + inst.raw = (OPCODE_SW << 26) | (rs << 21) | (rt << 16); + return inst; +} + +static Instruction makeBeq(uint32_t address, uint32_t rs, uint32_t rt, uint32_t target) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = OPCODE_BEQ; + inst.rs = rs; + inst.rt = rt; + // Test-fixture convention (shared with makeAbsJump): store the branch target the + // same way a J-type target is stored, since this scan's helpers never resolve a + // conditional branch's real PC-relative target. + inst.target = (target >> 2) & 0x03FFFFFFu; + inst.isBranch = true; + inst.hasDelaySlot = true; + inst.raw = (OPCODE_BEQ << 26) | (rs << 21) | (rt << 16); + return inst; +} + +// fmt is the COP "sub-opcode" field (bits 25-21, decoded into rs); rt is the GPR +// operand. Used to build e.g. an `mtc1 $a0,$fN` (opcode=OPCODE_COP1, fmt=COP1_MT, +// rt=4), which only READS rt. +static Instruction makeCopMove(uint32_t address, uint32_t opcode, uint32_t fmt, uint32_t rt) +{ + Instruction inst{}; + inst.address = address; + inst.opcode = opcode; + inst.rs = fmt; + inst.rt = rt; + inst.raw = (opcode << 26) | (fmt << 21) | (rt << 16); + return inst; +} + static Function makeFunction(const std::string &name, uint32_t start, uint32_t end) { Function fn{}; @@ -2293,6 +2336,205 @@ void register_ps2_recompiler_tests() t.Equals(result.size(), static_cast(0), "a clobbered $a0 with no re-materialization should not resolve"); }); + tc.Run("data-embedded thread entries: store between materialization and call does not clobber $a0", [](TestCase &t) { + // Maintainer's exact sequence: lui $a0,hi(P); sw $a0,0($sp); jal + // CreateThread; addiu $a0,$a0,lo(P) (delay slot). A store reads its rt + // operand, it does not write it - sw $a0 must not erase the $a0 the lui + // just resolved. Strong pin for "stores don't clobber rt": mutating SW to + // {rt} in mayWriteGprs makes this test fail. + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 12u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), + makeSw(callerStart + 8u, 4, 29), // sw $a0, 0($sp) - reads $a0, does not clobber it + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddr + 4u, 4, 4, 0x1234), // delay slot: addiu $a0,$a0,lo(P) + }}, + }; + + const uint32_t paramAddress = 0x00301234u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(1), + "a store of $a0 between materialization and the call must not erase the resolved $a0"); + if (result.size() == 1) + { + t.Equals(result[0], 0x00280000u, "thread entry pointer should still resolve through the store"); + } + }); + + tc.Run("data-embedded thread entries: unrelated jal between materialization and CreateThread is not resolved", [](TestCase &t) { + // Negative case: lui $a0,hi(P); addiu $a0,$a0,lo(P); jal unrelated_fn; + // nop; addiu $v1,$zero,0x20; syscall. $a0 is fully resolved before the + // unrelated call, but the unrelated jal is a control transfer sitting + // between the materialization and the CreateThread invocation, so it + // cannot dominate - the basic-block restriction, not mayWriteGprs + // (a jal writes only $ra=31, untracked), is what must drop this. Pin: + // removing the isControlTransfer backward-scan (walking the full window + // instead) makes this test fail. + constexpr uint32_t callerStart = 0x00101000u; + constexpr uint32_t unrelatedFn = 0x00109000u; + + std::unordered_map> decoded = { + {callerStart, { + makeLui(callerStart, 4, 0x0040), // lui $a0, 0x0040 + makeAddiu(callerStart + 4u, 4, 4, 0x0100), // addiu $a0,$a0,0x0100 -> $a0 = P (fully resolved) + makeAbsJump(callerStart + 8u, unrelatedFn, OPCODE_JAL), // jal unrelated_fn + makeNopLike(callerStart + 12u), // delay slot + makeAddiu(callerStart + 16u, 3, 0, 0x20), // addiu $v1,$zero,0x20 + makeSyscall(callerStart + 20u), + }}, + }; + + const uint32_t paramAddress = 0x00400100u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00450000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(0), + "an unrelated call sitting between the materialization and CreateThread must not let the " + "stale $a0 survive - the basic-block restriction must drop it"); + }); + + tc.Run("data-embedded thread entries: strong load-clobber pin", [](TestCase &t) { + // lui $a0,hi(P); lw $a0,0($t0); jal CreateThread; addiu $a0,$a0,0 (delay + // slot). hi(P) alone (0x00300000) IS a valid param address in fakeMemory + // and the delay-slot addiu's immediate is 0, so if LW failed to + // invalidate $a0 the stale lui-only value would survive unchanged and + // still resolve - unlike the existing weak "clobbered $a0" test (whose + // lui-only value is not a valid param either way), this assertion truly + // depends on LW invalidating $a0. Pin: mutating LW to {} in mayWriteGprs + // makes this test fail. + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00102000u; + constexpr uint32_t jalAddr = callerStart + 12u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeSyscall(wrapperStart + 4u), + makeJrRa(wrapperStart + 8u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), // lui $a0, 0x0030 -> $a0 = 0x00300000 (a valid param on its own) + makeLw(callerStart + 8u, 4, 6), // lw $a0, 0($t0) - must invalidate $a0 + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddr + 4u, 4, 4, 0x0000), // delay slot: addiu $a0,$a0,0 + }}, + }; + + const uint32_t paramAddress = 0x00300000u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(0), + "the reloaded $a0 must not resolve through a load that invalidated it, even though the " + "pre-load lui-only value happens to be a valid param address on its own"); + }); + + tc.Run("data-embedded thread entries: mtc1 reading $a0 is a documented safe-direction false negative", [](TestCase &t) { + // The conservative COP0/COP1/COP2/MMI {rt,rd} invalidation intentionally + // reproduces a false negative here: mtc1 $a0,$f0 only READS $a0, but the + // helper cannot distinguish that from an MFC1-style GPR write without + // sub-decoding fmt, so it invalidates $a0 anyway. This is the accepted, + // safe-direction tradeoff (over-invalidation only costs a missed entry, + // never a false survival) - documented here as a guard on that class of + // mayWriteGprs, not one of the strong pins. + constexpr uint32_t callerStart = 0x00103000u; + + std::unordered_map> decoded = { + {callerStart, { + makeLui(callerStart, 4, 0x0050), // lui $a0, 0x0050 + makeAddiu(callerStart + 4u, 4, 4, 0x0060), // addiu $a0,$a0,0x0060 -> $a0 = P (fully resolved) + makeCopMove(callerStart + 8u, OPCODE_COP1, COP1_MT, 4), // mtc1 $a0, $f0 - reads $a0 only + makeAddiu(callerStart + 12u, 3, 0, 0x20), // addiu $v1,$zero,0x20 + makeSyscall(callerStart + 16u), + }}, + }; + + const uint32_t paramAddress = 0x00500060u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00550000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(0), + "the conservative COP invalidation is expected to miss this entry - documented false negative, " + "the safe direction"); + }); + + tc.Run("data-embedded thread entries: branch between materialization and inline CreateThread is a block boundary", [](TestCase &t) { + // A real branch is itself a block terminator: even though a branch writes + // nothing tracked, positioning one between the materialization and a + // same-block inline CreateThread must still fail to resolve, because the + // basic-block restriction stops the backward scan at the branch. Doubles + // as a block-boundary check distinct from the unrelated-jal negative + // above (which pins the mechanism via an unconditional jal). + constexpr uint32_t callerStart = 0x00104000u; + + std::unordered_map> decoded = { + {callerStart, { + makeLui(callerStart, 4, 0x0060), // lui $a0, 0x0060 + makeAddiu(callerStart + 4u, 4, 4, 0x0070), // addiu $a0,$a0,0x0070 -> $a0 = P (fully resolved) + makeBeq(callerStart + 8u, 0, 0, callerStart + 0x100), // beq $zero,$zero,elsewhere + makeNopLike(callerStart + 12u), // delay slot + makeAddiu(callerStart + 16u, 3, 0, 0x20), // addiu $v1,$zero,0x20 + makeSyscall(callerStart + 20u), + }}, + }; + + const uint32_t paramAddress = 0x00600070u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00650000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(0), + "a branch between the materialization and the call is a block boundary, even inline " + "within the same function"); + }); + tc.Run("data-embedded thread entries: wrapper with wrong syscall number is not registered", [](TestCase &t) { constexpr uint32_t wrapperStart = 0x00100200u; constexpr uint32_t callerStart = 0x00100000u; From 6f36042b21787c062a405c64013ab5a9d94f72ed Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 08:11:26 -0400 Subject: [PATCH 6/7] fix(recomp): reset the CreateThread wrapper scan's $v1 state on a later clobber Step 1's wrapper-detection scan set sawAddiuV1Syscall on the first addiu $v1,$zero,0x20 seen and never cleared it, so a candidate wrapper whose $v1 was reassigned again before the syscall (e.g. addiu $v1,$zero,0x20; addiu $v1,$zero,0x21; syscall) was still misclassified as a CreateThread wrapper using the earlier, no-longer-live 0x20. Mirror the inline scan's own clobber handling: reset the flag on any later write to $v1 that is not itself the 0x20 materialization, via the same mayClobber predicate Step 2 already uses. --- ps2xRecomp/src/lib/ps2_recompiler.cpp | 9 +++ ps2xTest/src/ps2_recompiler_tests.cpp | 86 +++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/ps2xRecomp/src/lib/ps2_recompiler.cpp b/ps2xRecomp/src/lib/ps2_recompiler.cpp index 92897b110..32ae73884 100644 --- a/ps2xRecomp/src/lib/ps2_recompiler.cpp +++ b/ps2xRecomp/src/lib/ps2_recompiler.cpp @@ -2589,6 +2589,15 @@ namespace ps2recomp sawAddiuV1Syscall = true; continue; } + // A later write to $v1 that is not itself the 0x20 materialization + // (e.g. a second addiu $v1,... with a different immediate) means the + // 0x20 seen earlier no longer holds by the time any subsequent syscall + // runs - reset before checking, mirroring the inline scan's clobber + // check in Step 2. + if (sawAddiuV1Syscall && mayClobber(inst, kThreadEntryRegV1)) + { + sawAddiuV1Syscall = false; + } if (sawAddiuV1Syscall && isSyscallInstruction(inst)) { isWrapper = true; diff --git a/ps2xTest/src/ps2_recompiler_tests.cpp b/ps2xTest/src/ps2_recompiler_tests.cpp index 773024153..ef99533e7 100644 --- a/ps2xTest/src/ps2_recompiler_tests.cpp +++ b/ps2xTest/src/ps2_recompiler_tests.cpp @@ -2567,6 +2567,92 @@ void register_ps2_recompiler_tests() t.Equals(result.size(), static_cast(0), "a wrapper using a non-CreateThread syscall number should be ignored"); }); + tc.Run("data-embedded thread entries: wrapper scan resets on a later $v1 clobber", [](TestCase &t) { + // Negative case (maintainer's exact sequence): addiu $v1,$zero,0x20; + // addiu $v1,$zero,0x21; syscall. The second addiu overwrites $v1 with + // 0x21 before the syscall runs, so this is not a CreateThread wrapper - + // the actual syscall number is 0x21. Pin: removing the + // sawAddiuV1Syscall = false reset makes this test fail (it would then be + // misclassified as a wrapper and the jal below would produce a result). + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 8u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeAddiu(wrapperStart + 4u, 3, 0, 0x21), // clobbers $v1 with a different immediate + makeSyscall(wrapperStart + 8u), + makeJrRa(wrapperStart + 12u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddr + 4u, 4, 4, 0x1234), + }}, + }; + + std::unordered_map fakeMemory = { + {0x00301234u, 0u}, + {0x00301238u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(0), + "a $v1 clobber between the 0x20 materialization and the syscall must not be classified as " + "a CreateThread wrapper"); + }); + + tc.Run("data-embedded thread entries: wrapper scan reset is conditional on a $v1 write (positive half-guard)", [](TestCase &t) { + // Positive half-guard: addiu $v1,$zero,0x20; addiu $a1,$zero,1; syscall. + // The intervening instruction writes $a1, not $v1, so the 0x20 + // materialization is still live and this IS a legitimate wrapper. Catches + // a mutation that makes the reset unconditional (fires on every + // non-materialization instruction, not just a $v1 write), which would + // drop this wrapper too. + constexpr uint32_t wrapperStart = 0x00100200u; + constexpr uint32_t callerStart = 0x00100000u; + constexpr uint32_t jalAddr = callerStart + 8u; + + std::unordered_map> decoded = { + {wrapperStart, { + makeAddiu(wrapperStart, 3, 0, 0x20), + makeAddiu(wrapperStart + 4u, 5, 0, 1), // writes $a1, not $v1 - must not reset + makeSyscall(wrapperStart + 8u), + makeJrRa(wrapperStart + 12u), + }}, + {callerStart, { + makeNopLike(callerStart), + makeLui(callerStart + 4u, 4, 0x0030), + makeAbsJump(jalAddr, wrapperStart, OPCODE_JAL), + makeAddiu(jalAddr + 4u, 4, 4, 0x1234), + }}, + }; + + const uint32_t paramAddress = 0x00301234u; + std::unordered_map fakeMemory = { + {paramAddress, 0u}, + {paramAddress + 4u, 0x00280000u}, + }; + auto isValid = [&](uint32_t addr) { return fakeMemory.count(addr) != 0u; }; + auto readWord = [&](uint32_t addr) { return fakeMemory.at(addr); }; + + const std::vector result = + PS2Recompiler::DiscoverDataEmbeddedThreadEntries(decoded, isValid, readWord); + + t.Equals(result.size(), static_cast(1), + "an intervening write to a register other than $v1 must not reset the wrapper scan"); + if (result.size() == 1) + { + t.Equals(result[0], 0x00280000u, "thread entry pointer should resolve through the legitimate wrapper"); + } + }); + tc.Run("data-embedded thread entries: zero entry pointer is filtered out", [](TestCase &t) { constexpr uint32_t wrapperStart = 0x00100200u; constexpr uint32_t callerStart = 0x00100000u; From 9bc317e2b23ef8993424c957f05d4e75c7bcc882 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 09:15:39 -0400 Subject: [PATCH 7/7] fix(recomp): drop the unpinned forward-join hardening from the thread-entry walk The basic-block backward-scan restriction already answers the dominance concern the maintainer raised. The extra forward jal/j join scan was incomplete (it can only see J/JAL joins; branch targets are PC-relative and never expressible as an absolute jump target), unpinned by any test, and contained an out-of-bounds instruction access when the call site was the last decoded instruction and its predecessor was a control transfer (walkStart == instructions.size()). Remove it; walkStart's only remaining consumer is the bounded resolution loop. --- ps2xRecomp/src/lib/ps2_recompiler.cpp | 34 --------------------------- 1 file changed, 34 deletions(-) diff --git a/ps2xRecomp/src/lib/ps2_recompiler.cpp b/ps2xRecomp/src/lib/ps2_recompiler.cpp index 32ae73884..a899b3de7 100644 --- a/ps2xRecomp/src/lib/ps2_recompiler.cpp +++ b/ps2xRecomp/src/lib/ps2_recompiler.cpp @@ -2724,40 +2724,6 @@ namespace ps2recomp } size_t walkStart = std::max(blockStart, windowStart); - // --- Optional dominance hardening: a forward jal/j from elsewhere in the - // owner function whose absolute target lands inside [walkStart, windowEnd] - // is a join point the backward-only scan above cannot see - the "block" it - // found would then have more than one entry, so state resolved before the - // join cannot be trusted to dominate the call. Move walkStart forward to - // the join (over-invalidation - the safe direction: this can only lose a - // candidate, never let a stale value survive). Self-contained and - // independently removable without affecting the required backward-only - // core above. - { - const uint32_t walkStartAddress = instructions[walkStart].address; - const uint32_t windowEndAddress = instructions[windowEnd].address; - for (const Instruction &candidate : instructions) - { - if (candidate.opcode != OPCODE_J && candidate.opcode != OPCODE_JAL) - { - continue; - } - const uint32_t joinTarget = buildAbsoluteJumpTarget(candidate.address, candidate.target); - if (joinTarget <= walkStartAddress || joinTarget > windowEndAddress) - { - continue; - } - for (size_t j = walkStart; j <= windowEnd; ++j) - { - if (instructions[j].address == joinTarget) - { - walkStart = std::max(walkStart, j); - break; - } - } - } - } - std::unordered_map resolved; auto invalidate = [&resolved](uint32_t reg) {