Skip to content

Don't promote a whole function because of an indirect call - #209

Open
Sinan-Karakaya wants to merge 1 commit into
ran-j:mainfrom
Sinan-Karakaya:fix/indirect-call-fallback
Open

Don't promote a whole function because of an indirect call#209
Sinan-Karakaya wants to merge 1 commit into
ran-j:mainfrom
Sinan-Karakaya:fix/indirect-call-fallback

Conversation

@Sinan-Karakaya

Copy link
Copy Markdown
Contributor

When a computed jump cannot be resolved to a jump table, every instruction in the function becomes an entry point, because the jump could land on any of them. That fallback was also being applied to JALR.

JALR is not a jump but a call: it transfers control to another function and returns to the instruction after the delay slot. That return address is already queued as a resume target a few lines above, so nothing else in the function needs to be reachable from outside.

Indirect calls are ordinary code — function pointers, virtual dispatch, callbacks — so the fallback fires constantly on real games. Classifying every warned site on Dragon Quest VIII (NTSC-U) by decoding the instruction out of the ELF:

kind sites
JALR (indirect call) 2,210
JR (computed jump) 212

with jalr $t9 alone accounting for 1,874. Split by whether the function contains a genuine computed jump at all:

functions promoted entries
only indirect calls 1,078 160,100 (84.3%)
at least one computed jump 204 29,776

This restricts the fallback to JR. Measured on the same binary:

metric before after
promoted fallback entries 189,876 1,688
registered table entries 156,783 75,386
register_functions.cpp 13 MB 6.0 MB
total generated output 180 MB 163 MB
largest function file 1.2 MB 756 KB

I checked this did not silently drop anything needed: all 2,210 indirect-call sites were re-examined for their return-address resume entry (addr + 8) in the regenerated output. 42 lack one, and all 42 are inside a region of interleaved rodata where the "function" is a carving from string and float data and the return address falls outside its bounds. No real code lost an entry point.

All 11,492 generated translation units still pass clang++ -std=c++20 -fsyntax-only.

Copilot AI lite review requested due to automatic review settings August 17, 2026 16:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the control-flow analyzer’s indirect-register jump handling so that unresolved computed jumps (JR) trigger the “promote whole function” fallback, while indirect calls (JALR) do not—reducing excessive entry-point promotion and generated output bloat in real-world binaries.

Changes:

  • Restricts the “unresolved indirect fallback promotes every instruction” behavior to unresolved JR (computed jumps).
  • Documents why JALR (indirect call) should not force whole-function promotion.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +395 to 398
if (!foundTable && jrInst->function != SPECIAL_JALR)
{
needsIndirectFallback = true;
}
Comment on lines +388 to +395
// Only an unresolved computed *jump* can land on an arbitrary
// instruction of this function and therefore force every address to
// become an entry point. JALR is a call: it transfers control to
// another function and comes back to the instruction after the delay
// slot, which is already queued as a resume target above. Treating a
// call like a jump here promotes the whole function for what is
// usually just a function pointer or virtual dispatch.
if (!foundTable && jrInst->function != SPECIAL_JALR)
When a computed jump cannot be resolved to a jump table, every instruction in
the function becomes an entry point, because the jump could land on any of
them. That fallback was also applied to JALR, which is not a jump but a call:
it transfers control to another function and returns to the instruction after
the delay slot. That return address is already queued as a resume target a few
lines above, so nothing else in the function needs to be reachable from
outside.

Indirect calls are ordinary code -- function pointers, virtual dispatch,
callbacks -- so the fallback fired constantly. On a 3 MB PS2 executable, 2,210
of the 2,422 unresolved sites were JALR, and 1,078 of the 1,282 affected
functions contained no unresolved jump at all.

Restrict the fallback to JR. Promoted entries drop from 189,876 to 1,688,
registered table entries from 156,783 to 75,386, the generated registration
file from 13 MB to 6 MB, and total output from 180 MB to 163 MB. Every
indirect call site in real code keeps its return-address resume entry (the
only sites that lose one are bogus functions carved out of rodata, where the
address is outside the function anyway).
@Sinan-Karakaya
Sinan-Karakaya force-pushed the fix/indirect-call-fallback branch from 72489ee to 8b42360 Compare August 17, 2026 16:47
@Sinan-Karakaya

Copy link
Copy Markdown
Contributor Author

The first CI run failed one test, unresolved JALR marks internal labels as indirect fallback resume entries, and I have updated it in the pushed revision. Flagging that explicitly since changing a test alongside the code it covers deserves justification.

The test asserted that 0x320C lands in indirectFallbackEntryPoints for a JALR at 0x3204. That address is the call's return pc (0x3204 + 8), and it is still registered: queueResumeEntryTarget puts it in resumeEntryPoints, and ps2_recompiler.cpp merges both sets into the same per-owner resume-target list:

ownerTargets.insert(ownerTargets.end(),
                    analysisResult.resumeEntryPoints.begin(),
                    analysisResult.resumeEntryPoints.end());
ownerTargets.insert(ownerTargets.end(),
                    analysisResult.indirectFallbackEntryPoints.begin(),
                    analysisResult.indirectFallbackEntryPoints.end());

So the return pc keeps identical downstream treatment; what changes is that the other instructions in the function are no longer promoted with it. The preceding test, JALR should mark its return/fallthrough pc as resumable, already covers the part that matters and still passes untouched.

The updated test now asserts the behaviour rather than the mechanism: the return pc is resumable and emits a label, and unrelated instructions (0x3210, and the function start) are not promoted — which makes it a regression test for the actual fix. The sibling unresolved JR ... test is deliberately left alone, since JR must still promote; I verified locally that it continues to pass.

Empirically, on the binary in the description, all 2,210 indirect-call sites keep their return-address resume entry in the regenerated output. The only 42 without one sit inside a region of interleaved rodata where the "function" is a carving from string data and the return address falls outside its bounds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants