Skip to content

Copy byval argument to local stackslot if alignment is insufficient#1641

Open
0xmuon wants to merge 3 commits intorust-lang:mainfrom
0xmuon:fix1
Open

Copy byval argument to local stackslot if alignment is insufficient#1641
0xmuon wants to merge 3 commits intorust-lang:mainfrom
0xmuon:fix1

Conversation

@0xmuon
Copy link
Copy Markdown

@0xmuon 0xmuon commented Apr 13, 2026

fix: #1465

Copy the underaligned byval (indirect) arguments into a local stackslot when the incoming pointer alignment is less than the Rust ABI alignment. This avoids miscompiles from assuming stronger alignment than the ABI guarantees.

flags,
);
} else {
place.write_cvalue(fx, param);
Copy link
Copy Markdown
Member

@bjorn3 bjorn3 Apr 13, 2026

Choose a reason for hiding this comment

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

View changes since the review

Hang on, doesn't this write_cvalue already copy to a stack slot of sufficient alignment? I think really the only thing necessary is having if let ArgKind::Normal(Some(val)) = arg_kind only apply when the argument is correctly aligned. Or to handle alignment correctly, when the alignment of the param doesn't match the alignment of the type, use a type of [u8; size_of::<T>] for param in cvalue_for_param and use write_cvalue_maybe_transmute to write it to place in that case. That would also ensure the case where the ABI passes it byval while inside the function it is stored as SSA value rather than in a stack slot.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Do you agree this covers the SSA-local case correctly, and should we also handle meta_attrs: Some(_) (unsized indirect) or leave that unchanged for now?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

so, in abi::codegen_fn_prelude, the if let ArgKind::Normal(Some(val)) fast-path to be gated on underaligned_pointee_align.is_none() (like only when attrs.pointee_align >= layout.align.abi or pointee_align is None) and in abi::pass_mode::cvalue_for_param, for PassMode::Indirect { attrs, meta_attrs: None, ......} where attrs.pointee_align < arg_abi.layout.align.abi, we return CValue::by_ref(ptr, layout_of([u8; size])) instead of layout_of(T) and when initializing locals from such params we use place.write_cvalue_transmute(fx, param) (and similarly for spread fields). Am i right'?

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.

Copy byval argument to local stackslot if alignment is insufficient

2 participants