feat(recomp): discover packed jal-only entries via cross-unit manifests and thread-entry scan - #150
Conversation
f96656a to
1b4571f
Compare
…ts 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.
1b4571f to
a5e4a6a
Compare
| // 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<uint32_t> CollectExternalCallTargets( |
There was a problem hiding this comment.
This method currently requires the target to be inside an executable section of the caller's ELF. This appears to exclude the main use case described by the PR: a direct jal from a main ELF to a separately compiled overlay whose target range is not represented by any executable section in the main ELF.
The current unit test explicitly expects targets outside the current ELF's executable sections to be discarded, while the end-to-end test starts from a manually pre-created manifest and therefore never exercises cross-ELF manifest emission.
Could we add a real two-ELF regression test where ELF A contains a jal to code that exists only in ELF B, then verify that A emits the target and B ingests it? Unless both ELFs happen to have overlapping executable section ranges, the current filter seems to drop exactly the cross-unit target this feature is intended to preserve.
| } | ||
|
|
||
| loadExternalCallTargetManifests(); | ||
| discoverAdditionalEntryPoints(); |
There was a problem hiding this comment.
There seems to be a build-order problem here. Each invocation loads sibling manifests before discoverAdditionalEntryPoints(), but only emits its own manifest at the end of that pass.
For mutually calling units, such as a main ELF and an overlay that call into each other, a clean build creates a dependency cycle: each output requires the other unit's manifest before that manifest has been produced. The first invocation only logs a warning and continues, leaving its generated registration table incomplete until it is run again.
The end-to-end test pre-creates the manifest, so it does not exercise this clean-build scenario. Could manifest generation be split into a separate analysis phase, or could the project provide an explicit deterministic two-pass orchestration? At minimum, silently continuing when a configured manifest is missing seems unsafe because it produces valid-looking but incomplete output.
| // 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) |
There was a problem hiding this comment.
this method treats rt as the destination of every non-SPECIAL, non-J-type instruction. That is not valid for stores, branches, and several coprocessor instructions. For example, sw $a0, ... reads $a0 but would currently invalidate the resolved $a0, causing a false negative.
There is also a false-positive direction: previous jal instructions are treated as writing no tracked register, so a resolved $a0 can survive across an arbitrary function call even though $a0 is caller-saved and may have been clobbered by the callee. The linear propagation walk also crosses control-flow instructions without checking whether the materialization dominates the CreateThread call.
Could this use an opcode-aware register-def helper and stop or invalidate state at earlier calls and control-flow boundaries? Restricting the analysis to the call site's basic block plus the delay slot would be more conservative.
Please add regression tests for at least:
lui $a0, ...
sw $a0, 0($sp)
jal CreateThread
addiu $a0, $a0, ...
and for an unrelated jal between the $a0 materialization and the CreateThread call.
| for (const auto &[functionStart, instructions] : decodedFunctions) | ||
| { | ||
| const size_t window = std::min(instructions.size(), kThreadWrapperScanWindow); | ||
| bool sawAddiuV1Syscall = false; |
There was a problem hiding this comment.
sawAddiuV1Syscall becomes true, the wrapper scan accepts any later syscall in the scan window, even if $v1 was overwritten in between.
For example, this is currently classified as a CreateThread wrapper even though the executed syscall number is 0x21:
addiu $v1, $zero, 0x20
addiu $v1, $zero, 0x21
syscall
That can cause ordinary calls to the function to be interpreted as CreateThread calls and arbitrary memory to be read as a ThreadParam.
Could the wrapper scan apply the same $v1 clobber-aware logic used by the inline-syscall scan and include a negative regression test for this sequence?
…e 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.
… 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.
…ction 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.
…er 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.
…-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.
|
All four points were correct, thanks especially for catching that the cross-ELF emit path was never actually exercised. Reworked (follow-up commits; body updated):
|
2 participants
Problem
A callee reached only by a
jal/jimmediate from a caller outside its own byte range isdisassembled as unreachable tail code inside whichever function unit contains it, with no callable
entry of its own. At runtime
hasFunction(target)fails anddispatchGuestBranchfallsinto the missing-function policy; the default,
ContinueToTarget, movespcto the target withoutexecuting the callee.
discoverAdditionalEntryPoints()already recovers same-invocationcross-function targets. Two forms are not recovered: callers in a separately recompiled unit or
overlay, and thread entry pointers stored in a static
ThreadParamstruct.Fix
Two discovery paths, both registering through the existing
m_resumeEntryTargetsByOwnermapping.No runtime or code-generator behavior changes.
Cross-unit manifests. Each invocation writes
external_call_targets.txtinto its outputdirectory.
CollectExternalCallTargetsemitsjal/jtargets that land outside all of this unit'srecompiled, non-stub, non-skipped functions, dropping only targets inside this unit's own data/bss
sections — a mis-decoded
jalinto non-code bytes. A caller cannot know a callee unit's sectionlayout, so emission is a permissive candidate collector and the ingesting unit decides.
findContainingFunctionaccepts a candidate only at a decoded instruction boundary inside arecompiled, non-stub, non-skipped, non-entry function it does not start. A new
[general] external_call_target_manifestsnames the sibling manifests to ingest.Two-phase build.
recompile(bool emitManifestOnly = false)— the default preserves existingcall sites — plus a
--emit-manifest-onlyflag. Emission runs at the top ofrecompile()in bothmodes and reads only this unit's own decode, so analysis order does not matter. Run every unit's
analysis phase, then every unit's generate phase. A configured manifest that cannot be opened at
generate time is now a hard error reaching a non-zero exit, replacing a warn-and-continue.
Thread entries.
DiscoverDataEmbeddedThreadEntrieslocatesCreateThreadsites: an inlinesyscall, or ajalto a wrapper whose opening instructions set$v1 = 0x20. It recovers thestatic
ThreadParam*in$a0through a boundedlui/addiu/oriwalk that includes the delayslot, then reads the entry pointer at
param+4.Basis
jal/jencode an absolute 26-bit target, so a cross-unit call target is recoverable from thecaller's decode alone, with no relocation or symbol.
CreateThreadis syscall0x20in$v1, itsThreadParam*argument arrives in$a0, and thestruct's entry pointer is the word at
+4.mayWriteGprsderives writable registers from opcode (andSPECIALfunction) rather thandecoder-populated booleans, so hand-built and decoded instructions behave alike. Stores read
rtand write no GPR;
COP0/COP1/COP2/MMIinvalidate bothrtandrdrather than sub-decodefmt. Over-invalidating costs a missed entry, never a stale value.$v1back-scan stop at the call site's basic-block boundary. A basicblock has no interior control transfer, so no value survives a branch or an unrelated call.
Testing
Read the runner's
Failed:line rather than a pasted count; each test below runs by name.Test roster: two-ELF cross-unit regression, two-phase build and hard-fail guards, target-selection cases, thread-entry def-model and basic-block pins, wrapper-scan reset pair, manifest parsing
two-ELF: A emits T, B ingests T— drives two independentPS2Recompilerinstances over two ELFfixtures. A's
jaltargetsT, which exists only inside B and lies outside every section of A.A's analysis phase emits
T; A's manifest is asserted byte-identical whether A's analysis runsbefore or after B's; B then ingests A's actual emitted file and registers
Tunder its owningfunction. A negative arm asserts
Tis not registered when B configures no manifest.full pipeline: manifest-ingested packed jal-only entry registers into its owning unit— thesingle-ELF pipeline with and without a manifest; the without-manifest arm reproduces the
unregistered target.
missing configured manifest hard-fails at generate time/existing (even empty) configured manifest does not hard-fail.analysis phase never hard-fails on a missing sibling manifest, andtwo-phase clean build of mutually-calling units, which drives two ELFs calling into each other's mid-body targets througha full analysis-then-generate cycle for both.
collect external call targets: …— a foreign/overlay target outside all sections is collected; atarget inside the caller's own data section is excluded; a
jalinto a local recompiled functionis excluded while one outside all local functions is included; targets inside skipped or stub
local functions are still emitted; conditional branches are not collected; duplicates collapse and
results sort.
data-embedded thread entries: …—store between materialization and call does not clobber $a0,strong load-clobber pin,unrelated jal between materialization and CreateThread is not resolved,branch between materialization and inline CreateThread is a block boundary,mtc1 reading $a0 is a documented safe-direction false negative,wrapper scan resets on a later $v1 clobberwith its positive half-guard,wrapper with wrong syscall number is not registered,zero entry pointer is filtered out,multiple call sites to the same struct dedupe,addiu LO with sign bit set is sign-extended, not OR'd, plus the wrapper and inline resolution cases andthe
jaldecoder-populatedrtcase.full pipeline: data-embedded thread entry (CreateThread ThreadParam) registers into its owning unit.external call target manifest parsing sorts, dedupes, and ignores comments/blanksand…handles empty input.Risk and not in scope
external_call_targets.txtinto its output directory, includingsingle-unit builds that ingest nothing — one extra file for integrators packaging that output.
failures warn rather than abort the pass.
external_call_target_manifests.StartThread(0x22), runtime-computedThreadParampointers, handler-registration variants. An
mtc1 $a0before the call over-invalidates and losesthe entry, asserted as a known false negative.
addiu $v1,$zero,0x20inside the back-scan window can be attributed to two syscalls; theextra candidate is de-duplicated downstream.
<cstdint>is added ahead of the elfio include, fixing the build onnewer GCC.