Skip to content

Commit dbc51b3

Browse files
0xmuonbjorn3
andauthored
Copy byval argument to local stackslot if alignment is insufficient (#1641)
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. Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
1 parent 9393532 commit dbc51b3

2 files changed

Lines changed: 54 additions & 21 deletions

File tree

src/abi/mod.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ use crate::base::codegen_unwind_terminate;
3131
use crate::debuginfo::EXCEPTION_HANDLER_CLEANUP;
3232
use crate::prelude::*;
3333

34+
struct ArgValue<'tcx> {
35+
value: CValue<'tcx>,
36+
is_underaligned_pointee: bool,
37+
}
38+
3439
fn clif_sig_from_fn_abi<'tcx>(
3540
tcx: TyCtxt<'tcx>,
3641
default_call_conv: CallConv,
@@ -245,8 +250,8 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
245250

246251
// None means pass_mode == NoPass
247252
enum ArgKind<'tcx> {
248-
Normal(Option<CValue<'tcx>>),
249-
Spread(Vec<Option<CValue<'tcx>>>),
253+
Normal(Option<ArgValue<'tcx>>),
254+
Spread(Vec<Option<ArgValue<'tcx>>>),
250255
}
251256

252257
// FIXME implement variadics in cranelift
@@ -299,8 +304,12 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
299304
if fx.instance.def.requires_caller_location(fx.tcx) {
300305
// Store caller location for `#[track_caller]`.
301306
let arg_abi = arg_abis_iter.next().unwrap();
302-
fx.caller_location =
303-
Some(cvalue_for_param(fx, None, None, arg_abi, &mut block_params_iter).unwrap());
307+
let param = cvalue_for_param(fx, None, None, arg_abi, &mut block_params_iter).unwrap();
308+
assert!(
309+
!param.is_underaligned_pointee,
310+
"caller location argument should not be underaligned",
311+
);
312+
fx.caller_location = Some(param.value);
304313
}
305314

306315
assert_eq!(arg_abis_iter.next(), None, "ArgAbi left behind for {:?}", fx.fn_abi);
@@ -311,7 +320,9 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
311320
for (local, arg_kind, ty) in func_params {
312321
// While this is normally an optimization to prevent an unnecessary copy when an argument is
313322
// not mutated by the current function, this is necessary to support unsized arguments.
314-
if let ArgKind::Normal(Some(val)) = arg_kind {
323+
if let ArgKind::Normal(Some(ArgValue { value: val, is_underaligned_pointee: false })) =
324+
arg_kind
325+
{
315326
if let Some((addr, meta)) = val.try_to_ptr() {
316327
// Ownership of the value at the backing storage for an argument is passed to the
317328
// callee per the ABI, so it is fine to borrow the backing storage of this argument
@@ -338,13 +349,22 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
338349
match arg_kind {
339350
ArgKind::Normal(param) => {
340351
if let Some(param) = param {
341-
place.write_cvalue(fx, param);
352+
if param.is_underaligned_pointee {
353+
place.write_cvalue_transmute(fx, param.value);
354+
} else {
355+
place.write_cvalue(fx, param.value);
356+
}
342357
}
343358
}
344359
ArgKind::Spread(params) => {
345360
for (i, param) in params.into_iter().enumerate() {
346361
if let Some(param) = param {
347-
place.place_field(fx, FieldIdx::new(i)).write_cvalue(fx, param);
362+
let field_place = place.place_field(fx, FieldIdx::new(i));
363+
if param.is_underaligned_pointee {
364+
field_place.write_cvalue_transmute(fx, param.value);
365+
} else {
366+
field_place.write_cvalue(fx, param.value);
367+
}
348368
}
349369
}
350370
}

src/abi/pass_mode.rs

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use rustc_target::callconv::{
77
};
88
use smallvec::{SmallVec, smallvec};
99

10+
use super::ArgValue;
1011
use crate::prelude::*;
1112
use crate::value_and_place::assert_assignable;
1213

@@ -286,7 +287,7 @@ pub(super) fn cvalue_for_param<'tcx>(
286287
local_field: Option<usize>,
287288
arg_abi: &ArgAbi<'tcx, Ty<'tcx>>,
288289
block_params_iter: &mut impl Iterator<Item = Value>,
289-
) -> Option<CValue<'tcx>> {
290+
) -> Option<ArgValue<'tcx>> {
290291
let block_params = arg_abi
291292
.get_abi_param(fx.tcx)
292293
.into_iter()
@@ -307,30 +308,42 @@ pub(super) fn cvalue_for_param<'tcx>(
307308
arg_abi.layout,
308309
);
309310

310-
match arg_abi.mode {
311-
PassMode::Ignore => None,
311+
let value = match arg_abi.mode {
312+
PassMode::Ignore => return None,
312313
PassMode::Direct(_) => {
313314
assert_eq!(block_params.len(), 1, "{:?}", block_params);
314-
Some(CValue::by_val(block_params[0], arg_abi.layout))
315+
CValue::by_val(block_params[0], arg_abi.layout)
315316
}
316317
PassMode::Pair(_, _) => {
317318
assert_eq!(block_params.len(), 2, "{:?}", block_params);
318-
Some(CValue::by_val_pair(block_params[0], block_params[1], arg_abi.layout))
319+
CValue::by_val_pair(block_params[0], block_params[1], arg_abi.layout)
319320
}
320321
PassMode::Cast { ref cast, .. } => {
321-
Some(from_casted_value(fx, &block_params, arg_abi.layout, cast))
322+
from_casted_value(fx, &block_params, arg_abi.layout, cast)
322323
}
323-
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => {
324+
PassMode::Indirect { attrs, meta_attrs: None, on_stack: _ } => {
324325
assert_eq!(block_params.len(), 1, "{:?}", block_params);
325-
Some(CValue::by_ref(Pointer::new(block_params[0]), arg_abi.layout))
326+
if let Some(pointee_align) = attrs.pointee_align
327+
&& pointee_align < arg_abi.layout.align.abi
328+
&& arg_abi.layout.is_sized()
329+
&& arg_abi.layout.size != Size::ZERO
330+
{
331+
// Underaligned pointer: treat as `[u8; size]` and transmute-copy into the real type.
332+
let bytes_ty = Ty::new_array(fx.tcx, fx.tcx.types.u8, arg_abi.layout.size.bytes());
333+
let bytes_layout = fx.layout_of(bytes_ty);
334+
return Some(ArgValue {
335+
value: CValue::by_ref(Pointer::new(block_params[0]), bytes_layout),
336+
is_underaligned_pointee: true,
337+
});
338+
} else {
339+
CValue::by_ref(Pointer::new(block_params[0]), arg_abi.layout)
340+
}
326341
}
327342
PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack: _ } => {
328343
assert_eq!(block_params.len(), 2, "{:?}", block_params);
329-
Some(CValue::by_ref_unsized(
330-
Pointer::new(block_params[0]),
331-
block_params[1],
332-
arg_abi.layout,
333-
))
344+
CValue::by_ref_unsized(Pointer::new(block_params[0]), block_params[1], arg_abi.layout)
334345
}
335-
}
346+
};
347+
348+
Some(ArgValue { value, is_underaligned_pointee: false })
336349
}

0 commit comments

Comments
 (0)