Copy byval argument to local stackslot if alignment is insufficient#1641
Copy byval argument to local stackslot if alignment is insufficient#16410xmuon wants to merge 3 commits intorust-lang:mainfrom
Conversation
| flags, | ||
| ); | ||
| } else { | ||
| place.write_cvalue(fx, param); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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'?
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.