codegen: force continuation on JAL/JALR resume-PC mismatch instead of aborting caller#117
Closed
jlsandri wants to merge 5 commits intoran-j:mainfrom
Closed
codegen: force continuation on JAL/JALR resume-PC mismatch instead of aborting caller#117jlsandri wants to merge 5 commits intoran-j:mainfrom
jlsandri wants to merge 5 commits intoran-j:mainfrom
Conversation
… only to register_functions
When a callee yields at an internal continuation PC, the caller now re-enters it in a while loop until it exits to the real fallthrough. Prevents nested continuations from leaking as false PC_MISMATCH.
After a JALR call + re-entry loop, if ctx->pc doesn't match the expected fallthrough address, the previous codegen printed [PC_MISMATCH] and aborted the caller with `return`. That cascaded up the entire call stack whenever a nested callee legitimately left ctx->pc at an internal resume target (overlay functions, nested JALR chains, and functions that set ctx->pc for their own resume-PC mechanism). Replace the hard abort with a throttled [JALR_FIXUP] diagnostic (5 warnings per call site) and force ctx->pc = fallthroughPc so the caller's still-valid stack frame and register state can continue executing. Stacks on the continuation-PC handling PR — this hunk replaces the PC_MISMATCH branch that PR introduced.
Extends the JALR_FIXUP to cover all three JAL codegen paths:
1. Direct JAL via findFunctionByAddress (called 0x%x pattern)
2. Direct JAL via lookupFunction (called 0x{literal} pattern)
3. Reloc JAL (called reloc pattern)
All paths now force ctx->pc = fallthroughPc on mismatch instead of
aborting. This removes the cascade failure where a nested callee's
PC_MISMATCH would abort every function up the call stack.
a4153e8 to
bc83180
Compare
Author
|
Closing as part of a batch cleanup after #107 landed. The runtime ecosystem refactor in #107 substantially reworked the files this PR touched, and I would like to re-audit the underlying fix against the new code structure before putting it back in front of you. If the fix is still needed after that re-audit, I will re-open as a focused PR rebased onto current main. Thanks for your patience. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After a
JAL/JALRcall + continuation re-entry loop, ifctx->pcdidn't match the expected fallthrough address, the previous codegen logged[PC_MISMATCH]and aborted the caller withreturn.That abort cascaded up the entire call stack whenever a nested callee legitimately left
ctx->pcat an internal resume target — common with overlay functions, nestedJALRchains, and any function that setsctx->pcas part of its own resume-PC mechanism. The caller's stack frame and register context were still valid; the codegen was throwing them away on a false signal.Fix
Replace the hard
returnwith:[JALR_FIXUP]/[JAL_FIXUP]diagnostic — 5 warnings per call site, so the signal isn't lost.ctx->pc = fallthroughPc;— force continuation at the caller's expected resume point.Two commits:
JALRresume-PC fixup at the single indirect-call site (depends on codegen: handle continuation PCs and nested callee yields #115 for the re-entry loop structure it edits).JALresume-PC fixup extending the same treatment to all three direct-call codegen paths for consistency:JALviafindFunctionByAddress(called 0x%xpattern)JALvialookupFunction(called 0x{literal}pattern)JAL(called relocpattern)Why this is stacked on #115
The JALR hunk rewrites the
PC_MISMATCHbranch that #115 introduces as part of the re-entry loop. Without #115 merged first, there is noPC_MISMATCHto rewrite.Scope
ps2xRecomp/src/lib/code_generator.cppRationale
The old behaviour was conservative — any resume-PC anomaly aborted the whole chain — and that's reasonable as a first line of defence. But in practice the only trigger is legitimate internal resume-PC use by the callee, which the caller should tolerate. Demoting the abort to a throttled warning + forced continuation keeps the diagnostic signal while eliminating the cascade-failure class entirely.