codegen_ssa: copy tupled rust-call constant args before indirect pass#155291
Open
blackms wants to merge 1 commit intorust-lang:mainfrom
Open
codegen_ssa: copy tupled rust-call constant args before indirect pass#155291blackms wants to merge 1 commit intorust-lang:mainfrom
blackms wants to merge 1 commit intorust-lang:mainfrom
Conversation
When `codegen_arguments_untupled` lowered a call to a closure with the `extern "rust-call"` ABI, it used the `OperandRef` returned by `codegen_operand` directly. If the MIR operand was `Copy` or `Constant` and the tuple's backend layout was by-ref (`BackendRepr::Memory`), the returned value was `Ref(..)` pointing into shared, potentially read-only memory -- typically a promoted allocation in `.rodata`. Projecting the fields out of that pointer and handing them to a callee that owns its argument storage by ABI contract leads the callee to write through read-only memory, which aborts with SIGBUS on Linux and `STATUS_HEAP_CORRUPTION` on Windows. This is the exact same hazard that the prior fix for the non-tupled call path in `codegen_call_terminator` addressed: it spills `Copy`/`Constant` operands that arrive by-ref into a fresh alloca before passing a pointer to them. That safety-copy was never applied to the tupled `rust-call` path, so this commit ports the same logic over to `codegen_arguments_untupled` -- spilling the whole tuple before the field projections -- and wires the `lifetime_start`/`lifetime_end` pair into the existing `lifetime_ends_after_call` bookkeeping. The regression test exercises the classic reproducer (a `const VALUE: (Thing,)` invoked through `Fn::call`) at both `-Copt-level=0` (which aborts deterministically without the fix) and `-Copt-level=3` (coverage that the fix does not regress release codegen).
Contributor
|
Is any LLM involved in this PR? Your other PR has an LLM disclaimer. |
Author
|
Good catch, and thanks for asking directly — you're right that I should have put the disclosure on this PR too. It's on #155296 but I missed it here. My policy is proactive disclosure on every public contribution, so this was an oversight, not a judgment call. This PR was roughly a 50/50 collaboration between me and a Claude-based coding agent team. The technical hypothesis and patch originated on the agent side; the repro, scoping, review, and PR hygiene were mine. Agent-side work:
Human-side work:
Happy to answer more questions on any specific part of it. |
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.
Fixes #155241 (the
const-argument / indirect-ABI half of the issue; the MIR-GVN half is tracked separately).Problem
Invoking a closure with the
extern "rust-call"ABI on a constant tuple whose layout is passed indirectly has been miscompiling sincenightly-2017-11-18. On Linux the program SIGBUSes; on Windows it aborts withSTATUS_HEAP_CORRUPTION.Original reproducer (still a smoking gun at
-Copt-level=0today):The root cause is in
codegen_arguments_untupled(compiler/rustc_codegen_ssa/src/mir/block.rs). When the MIR operand isOperand::CopyorOperand::Constantand the tuple's backend layout is by-ref (BackendRepr::Memory),codegen_operandreturnsOperandValue::Ref(alloc_addr)wherealloc_addrpoints into a shared, read-only allocation (typically a promoted constant in.rodata). The field projections downstream load field pointers out of that memory, so theextern "rust-call"callee — which owns its argument storage by ABI contract — writes through read-only memory when it mutates.Fix
This is exactly the hazard that #45996 fixed for the normal (non-tupled) call path in
codegen_call_terminator, by spillingCopy/Constantoperands that arrive by-ref into a fresh alloca before passing a pointer. That safety-copy was never applied to the tupledrust-callpath; this PR ports the same logic across, spilling the whole tuple before the field projections and wiring thelifetime_start/lifetime_endpair into the existinglifetime_ends_after_callbookkeeping. The new block is a line-for-line parallel of the existing code a few hundred lines above in the same file.Test
Adds a
//@ run-passregression test attests/ui/closures/rust-call-const-arg-alias.rsexercising the 2017 reproducer throughFn::call/FnMut::call_mut/FnOnce::call_once, at both-Copt-level=0(where the bug aborts deterministically without the fix) and-Copt-level=3(coverage that the fix does not regress release codegen). The test usesassert_eq!on the observed mutation so a future regression that silently ignores the write, rather than crashing, is also caught.Verification
Bug reproduction was confirmed locally on
rustc 1.97.0-nightly (17584a181 2026-04-13)using the MCVE above: exit 138 (SIGBUS) at-Copt-level=0. A full bootstrap of the patched compiler was not performed; the patch is a mechanical port of a proven safety-copy with no deviation in data-flow aftertuple.valis updated, so the behaviour from that point on is already exercised by every non-constantrust-callinvocation today.r? compiler