fix(codegen): callable-invoker trampoline preserves callee-saved registers (#487)#490
Open
mirchaemanuel wants to merge 1 commit into
Open
Conversation
…sters Fixes illegalstudio#487. The generated descriptor-invoker trampoline saved only fp/lr and then used callee-saved registers as raw scratch (AArch64 x19-x26 via the nested-call register and the mixed/indexed argument loaders; x86_64 r12/rbx/r13-r15). The linear register allocator relies on the platform ABI and parks values that live across a callable_descriptor_invoke in exactly those registers, so any such value was clobbered by the invoke: in '$acc += $f()' inside a loop the loaded accumulator read back as the trampoline's leftover scratch (the empty args-array length, 0) and only the last call's value survived. Every indirect callable form was affected (closures, arrow functions, callable strings, first-class callables, method FCCs), with any operator, compound or not, including property targets; the closure body itself ran every iteration. Direct calls were safe because compiled functions save the allocator-used callee-saved registers in their own prologues - the invoker trampoline was the one generated path missing that treatment. --regalloc=stack masked the bug. The invoker frame grows to hold a save area and the prologue/epilogue now store/reload the callee-saved set it scratches, mirroring compiled-function prologues on both targets. Regression tests in tests/codegen/callables/expr_calls.rs (closure compound assign, callable-string/FCC with -= and *=, non-compound + property target). Broad suites green (2231 codegen + 993 error tests).
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.
Summary
Fixes #487. Compound assignment with an indirect callable RHS silently miscompiled:
$acc += $f()in a loop accumulated only the last call's value (4instead of80), with the closure demonstrably running every iteration. Every indirect form was affected — closures, arrow functions, callable strings, first-class callables, method FCCs — with any operator, compound or not, including property targets.Root cause
An ABI violation in the generated descriptor-invoker trampoline (
src/codegen/runtime_callable_invoker.rs): it saved only fp/lr and then used callee-saved registers as raw scratch (AArch64x19–x26via the nested-call register and the argument loaders; x86_64r12/rbx/r13–r15). The linear register allocator relies on the platform ABI and parks values that live across acallable_descriptor_invokein exactly those registers — so the loaded LHS of$acc += $f()was clobbered by the invoke and read back as the trampoline's leftover scratch (the empty args-array length,0). Each iteration computed0 + f().Diagnosed via IR (byte-identical with the optimizer off — lowering and effects are correct) and assembly (the allocator parks
v6inx21; the trampoline's last write tox21is the args length).--regalloc=stackmasked the bug; direct calls were safe because compiled functions save allocator-used callee-saved registers in their own prologues — the invoker trampoline was the one generated code path missing that treatment. Same defect class as #378 on the runtime-routine side.Fix
The invoker frame grows to hold a save area, and the prologue/epilogue now store/reload the callee-saved set the body scratches — mirroring compiled-function prologues on both targets.
Verification
tests/codegen/callables/expr_calls.rs(closure compound assign; callable-string/FCC with-=/*=; non-compound + property target).ret, no slot collisions, return registers untouched).setjmp/longjmp, which restores callee-saved registers from the try-entry register file, so the skipped trampoline epilogue is irrelevant — verified behaviorally across throw-through-two-trampolines, multiple live values, and partial-loop throw/catch shapes.