Skip to content

Commit

Permalink
Fix transmute::<T, U> where T requires a bigger alignment than U
Browse files Browse the repository at this point in the history
For transmute::<T, U> we simply pointercast the destination from a U
pointer to a T pointer, without providing any alignment information,
thus LLVM assumes that the destination is aligned to hold a value of
type T, which is not necessarily true. This can lead to LLVM emitting
machine instructions that assume said alignment, and thus cause aborts.

To fix this, we need to provide the actual alignment to store_operand()
and in turn to store() so they can set the proper alignment information
on the stores and LLVM can emit the proper machine instructions.

Fixes #32947
  • Loading branch information
dotdash committed Dec 31, 2016
1 parent 82801b5 commit 71a11a0
Show file tree
Hide file tree
Showing 11 changed files with 97 additions and 65 deletions.
9 changes: 3 additions & 6 deletions src/librustc_trans/abi.rs
Expand Up @@ -248,11 +248,8 @@ impl ArgType {
let can_store_through_cast_ptr = false;
if can_store_through_cast_ptr {
let cast_dst = bcx.pointercast(dst, ty.ptr_to());
let store = bcx.store(val, cast_dst);
let llalign = llalign_of_min(ccx, self.ty);
unsafe {
llvm::LLVMSetAlignment(store, llalign);
}
bcx.store(val, cast_dst, Some(llalign));
} else {
// The actual return type is a struct, but the ABI
// adaptation code has cast it into some scalar type. The
Expand All @@ -273,7 +270,7 @@ impl ArgType {
base::Lifetime::Start.call(bcx, llscratch);

// ...where we first store the value...
bcx.store(val, llscratch);
bcx.store(val, llscratch, None);

// ...and then memcpy it to the intended destination.
base::call_memcpy(bcx,
Expand All @@ -289,7 +286,7 @@ impl ArgType {
if self.original_ty == Type::i1(ccx) {
val = bcx.zext(val, Type::i8(ccx));
}
bcx.store(val, dst);
bcx.store(val, dst, None);
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/librustc_trans/adt.rs
Expand Up @@ -443,11 +443,11 @@ pub fn trans_set_discr<'a, 'tcx>(
layout::CEnum{ discr, min, max, .. } => {
assert_discr_in_range(Disr(min), Disr(max), to);
bcx.store(C_integral(Type::from_integer(bcx.ccx, discr), to.0, true),
val);
val, None);
}
layout::General{ discr, .. } => {
bcx.store(C_integral(Type::from_integer(bcx.ccx, discr), to.0, true),
bcx.struct_gep(val, 0));
bcx.struct_gep(val, 0), None);
}
layout::Univariant { .. }
| layout::UntaggedUnion { .. }
Expand All @@ -458,7 +458,7 @@ pub fn trans_set_discr<'a, 'tcx>(
let nnty = compute_fields(bcx.ccx, t, nndiscr as usize, false)[0];
if to.0 != nndiscr {
let llptrty = type_of::sizing_type_of(bcx.ccx, nnty);
bcx.store(C_null(llptrty), val);
bcx.store(C_null(llptrty), val, None);
}
}
layout::StructWrappedNullablePointer { nndiscr, ref discrfield, ref nonnull, .. } => {
Expand All @@ -476,7 +476,7 @@ pub fn trans_set_discr<'a, 'tcx>(
let path = discrfield.iter().map(|&i| i as usize).collect::<Vec<_>>();
let llptrptr = bcx.gepi(val, &path[..]);
let llptrty = val_ty(llptrptr).element_type();
bcx.store(C_null(llptrty), llptrptr);
bcx.store(C_null(llptrty), llptrptr, None);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/asm.rs
Expand Up @@ -105,7 +105,7 @@ pub fn trans_inline_asm<'a, 'tcx>(
let outputs = ia.outputs.iter().zip(&outputs).filter(|&(ref o, _)| !o.is_indirect);
for (i, (_, &(val, _))) in outputs.enumerate() {
let v = if num_outputs == 1 { r } else { bcx.extract_value(r, i) };
bcx.store(v, val);
bcx.store(v, val, None);
}

// Store expn_id in a metadata node so we can map LLVM errors
Expand Down
31 changes: 13 additions & 18 deletions src/librustc_trans/base.rs
Expand Up @@ -290,7 +290,7 @@ pub fn coerce_unsized_into<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
let src_f = adt::trans_field_ptr(bcx, src_ty, src, Disr(0), i);
let dst_f = adt::trans_field_ptr(bcx, dst_ty, dst, Disr(0), i);
if src_fty == dst_fty {
memcpy_ty(bcx, dst_f, src_f, src_fty);
memcpy_ty(bcx, dst_f, src_f, src_fty, None);
} else {
coerce_unsized_into(bcx, src_f, src_fty, dst_f, dst_fty);
}
Expand Down Expand Up @@ -429,7 +429,7 @@ pub fn store_ty<'a, 'tcx>(cx: &BlockAndBuilder<'a, 'tcx>, v: ValueRef, dst: Valu
let llextra = cx.extract_value(v, abi::FAT_PTR_EXTRA);
store_fat_ptr(cx, lladdr, llextra, dst, t);
} else {
cx.store(from_immediate(cx, v), dst);
cx.store(from_immediate(cx, v), dst, None);
}
}

Expand All @@ -439,8 +439,8 @@ pub fn store_fat_ptr<'a, 'tcx>(cx: &BlockAndBuilder<'a, 'tcx>,
dst: ValueRef,
_ty: Ty<'tcx>) {
// FIXME: emit metadata
cx.store(data, get_dataptr(cx, dst));
cx.store(extra, get_meta(cx, dst));
cx.store(data, get_dataptr(cx, dst), None);
cx.store(extra, get_meta(cx, dst), None);
}

pub fn load_fat_ptr<'a, 'tcx>(
Expand Down Expand Up @@ -523,26 +523,21 @@ pub fn call_memcpy<'a, 'tcx>(b: &Builder<'a, 'tcx>,
b.call(memcpy, &[dst_ptr, src_ptr, size, align, volatile], None);
}

pub fn memcpy_ty<'a, 'tcx>(
bcx: &BlockAndBuilder<'a, 'tcx>, dst: ValueRef, src: ValueRef, t: Ty<'tcx>
) {
pub fn memcpy_ty<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
dst: ValueRef,
src: ValueRef,
t: Ty<'tcx>,
align: Option<u32>) {
let ccx = bcx.ccx;

if type_is_zero_size(ccx, t) {
return;
}

if t.is_structural() {
let llty = type_of::type_of(ccx, t);
let llsz = llsize_of(ccx, llty);
let llalign = type_of::align_of(ccx, t);
call_memcpy(bcx, dst, src, llsz, llalign as u32);
} else if common::type_is_fat_ptr(bcx.ccx, t) {
let (data, extra) = load_fat_ptr(bcx, src, t);
store_fat_ptr(bcx, data, extra, dst, t);
} else {
store_ty(bcx, load_ty(bcx, src, t), dst, t);
}
let llty = type_of::type_of(ccx, t);
let llsz = llsize_of(ccx, llty);
let llalign = align.unwrap_or_else(|| type_of::align_of(ccx, t));
call_memcpy(bcx, dst, src, llsz, llalign as u32);
}

pub fn call_memset<'a, 'tcx>(b: &Builder<'a, 'tcx>,
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_trans/builder.rs
Expand Up @@ -512,13 +512,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
value
}

pub fn store(&self, val: ValueRef, ptr: ValueRef) -> ValueRef {
pub fn store(&self, val: ValueRef, ptr: ValueRef, align: Option<u32>) -> ValueRef {
debug!("Store {:?} -> {:?}", Value(val), Value(ptr));
assert!(!self.llbuilder.is_null());
self.count_insn("store");
let ptr = self.check_store(val, ptr);
unsafe {
llvm::LLVMBuildStore(self.llbuilder, val, ptr)
let store = llvm::LLVMBuildStore(self.llbuilder, val, ptr);
if let Some(align) = align {
llvm::LLVMSetAlignment(store, align as c_uint);
}
store
}
}

Expand Down
27 changes: 12 additions & 15 deletions src/librustc_trans/intrinsic.rs
Expand Up @@ -288,8 +288,8 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
let val = bcx.call(llfn, &[llargs[0], llargs[1]], None);
let result = bcx.extract_value(val, 0);
let overflow = bcx.zext(bcx.extract_value(val, 1), Type::bool(ccx));
bcx.store(result, bcx.struct_gep(llresult, 0));
bcx.store(overflow, bcx.struct_gep(llresult, 1));
bcx.store(result, bcx.struct_gep(llresult, 0), None);
bcx.store(overflow, bcx.struct_gep(llresult, 1), None);

C_nil(bcx.ccx)
},
Expand Down Expand Up @@ -407,8 +407,8 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
failorder, weak);
let result = bcx.extract_value(val, 0);
let success = bcx.zext(bcx.extract_value(val, 1), Type::bool(bcx.ccx));
bcx.store(result, bcx.struct_gep(llresult, 0));
bcx.store(success, bcx.struct_gep(llresult, 1));
bcx.store(result, bcx.struct_gep(llresult, 0), None);
bcx.store(success, bcx.struct_gep(llresult, 1), None);
} else {
invalid_monomorphization(sty);
}
Expand Down Expand Up @@ -613,7 +613,7 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,

for i in 0..elems.len() {
let val = bcx.extract_value(val, i);
bcx.store(val, bcx.struct_gep(llresult, i));
bcx.store(val, bcx.struct_gep(llresult, i), None);
}
C_nil(ccx)
}
Expand All @@ -625,10 +625,7 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
if val_ty(llval) != Type::void(ccx) && machine::llsize_of_alloc(ccx, val_ty(llval)) != 0 {
if let Some(ty) = fn_ty.ret.cast {
let ptr = bcx.pointercast(llresult, ty.ptr_to());
let store = bcx.store(llval, ptr);
unsafe {
llvm::LLVMSetAlignment(store, type_of::align_of(ccx, ret_ty));
}
bcx.store(llval, ptr, Some(type_of::align_of(ccx, ret_ty)));
} else {
store_ty(bcx, llval, llresult, ret_ty);
}
Expand Down Expand Up @@ -695,7 +692,7 @@ fn try_intrinsic<'a, 'tcx>(
) {
if bcx.sess().no_landing_pads() {
bcx.call(func, &[data], None);
bcx.store(C_null(Type::i8p(&bcx.ccx)), dest);
bcx.store(C_null(Type::i8p(&bcx.ccx)), dest, None);
} else if wants_msvc_seh(bcx.sess()) {
trans_msvc_try(bcx, func, data, local_ptr, dest);
} else {
Expand Down Expand Up @@ -789,8 +786,8 @@ fn trans_msvc_try<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
let val1 = C_i32(ccx, 1);
let arg2 = catchpad.load(catchpad.inbounds_gep(addr, &[val1]));
let local_ptr = catchpad.bitcast(local_ptr, i64p);
catchpad.store(arg1, local_ptr);
catchpad.store(arg2, catchpad.inbounds_gep(local_ptr, &[val1]));
catchpad.store(arg1, local_ptr, None);
catchpad.store(arg2, catchpad.inbounds_gep(local_ptr, &[val1]), None);
catchpad.catch_ret(tok, caught.llbb());

caught.ret(C_i32(ccx, 1));
Expand All @@ -799,7 +796,7 @@ fn trans_msvc_try<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
// Note that no invoke is used here because by definition this function
// can't panic (that's what it's catching).
let ret = bcx.call(llfn, &[func, data, local_ptr], None);
bcx.store(ret, dest);
bcx.store(ret, dest, None);
}

// Definition of the standard "try" function for Rust using the GNU-like model
Expand Down Expand Up @@ -858,14 +855,14 @@ fn trans_gnu_try<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
let vals = catch.landing_pad(lpad_ty, bcx.ccx.eh_personality(), 1, catch.fcx().llfn);
catch.add_clause(vals, C_null(Type::i8p(ccx)));
let ptr = catch.extract_value(vals, 0);
catch.store(ptr, catch.bitcast(local_ptr, Type::i8p(ccx).ptr_to()));
catch.store(ptr, catch.bitcast(local_ptr, Type::i8p(ccx).ptr_to()), None);
catch.ret(C_i32(ccx, 1));
});

// Note that no invoke is used here because by definition this function
// can't panic (that's what it's catching).
let ret = bcx.call(llfn, &[func, data, local_ptr], None);
bcx.store(ret, dest);
bcx.store(ret, dest, None);
}

// Helper function to give a Block to a closure to translate a shim function.
Expand Down
17 changes: 11 additions & 6 deletions src/librustc_trans/mir/block.rs
Expand Up @@ -23,14 +23,16 @@ use consts;
use Disr;
use machine::{llalign_of_min, llbitsize_of_real};
use meth;
use type_of;
use type_of::{self, align_of};
use glue;
use type_::Type;

use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::fx::FxHashMap;
use syntax::symbol::Symbol;

use std::cmp;

use super::{MirContext, LocalRef};
use super::analyze::CleanupKind;
use super::constant::Const;
Expand Down Expand Up @@ -207,7 +209,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
let llslot = match op.val {
Immediate(_) | Pair(..) => {
let llscratch = bcx.fcx().alloca(ret.original_ty, "ret");
self.store_operand(&bcx, llscratch, op);
self.store_operand(&bcx, llscratch, op, None);
llscratch
}
Ref(llval) => llval
Expand Down Expand Up @@ -426,7 +428,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
// The first argument is a thin destination pointer.
let llptr = self.trans_operand(&bcx, &args[0]).immediate();
let val = self.trans_operand(&bcx, &args[1]);
self.store_operand(&bcx, llptr, val);
self.store_operand(&bcx, llptr, val, None);
funclet_br(self, bcx, target);
return;
}
Expand Down Expand Up @@ -659,7 +661,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
Immediate(_) | Pair(..) => {
if arg.is_indirect() || arg.cast.is_some() {
let llscratch = bcx.fcx().alloca(arg.original_ty, "arg");
self.store_operand(bcx, llscratch, op);
self.store_operand(bcx, llscratch, op, None);
(llscratch, true)
} else {
(op.pack_if_pair(bcx).immediate(), false)
Expand Down Expand Up @@ -801,7 +803,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
let llretval = bcx.landing_pad(llretty, llpersonality, 1, self.fcx.llfn);
bcx.set_cleanup(llretval);
let slot = self.get_personality_slot(&bcx);
bcx.store(llretval, slot);
bcx.store(llretval, slot, None);
bcx.br(target.llbb());
bcx.llbb()
}
Expand Down Expand Up @@ -886,7 +888,10 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {

let llty = type_of::type_of(bcx.ccx, val.ty);
let cast_ptr = bcx.pointercast(dst.llval, llty.ptr_to());
self.store_operand(bcx, cast_ptr, val);
let in_type = val.ty;
let out_type = dst.ty.to_ty(bcx.tcx());;
let llalign = cmp::min(align_of(bcx.ccx, in_type), align_of(bcx.ccx, out_type));
self.store_operand(bcx, cast_ptr, val, Some(llalign));
}


Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/mir/mod.rs
Expand Up @@ -514,7 +514,7 @@ fn arg_local_refs<'a, 'tcx>(bcx: &BlockAndBuilder<'a, 'tcx>,
// environment into its components so it ends up out of bounds.
let env_ptr = if !env_ref {
let alloc = bcx.fcx().alloca(common::val_ty(llval), "__debuginfo_env_ptr");
bcx.store(llval, alloc);
bcx.store(llval, alloc, None);
alloc
} else {
llval
Expand Down
15 changes: 9 additions & 6 deletions src/librustc_trans/mir/operand.rs
Expand Up @@ -244,21 +244,24 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
pub fn store_operand(&mut self,
bcx: &BlockAndBuilder<'a, 'tcx>,
lldest: ValueRef,
operand: OperandRef<'tcx>) {
debug!("store_operand: operand={:?}", operand);
operand: OperandRef<'tcx>,
align: Option<u32>) {
debug!("store_operand: operand={:?}, align={:?}", operand, align);
// Avoid generating stores of zero-sized values, because the only way to have a zero-sized
// value is through `undef`, and store itself is useless.
if common::type_is_zero_size(bcx.ccx, operand.ty) {
return;
}
match operand.val {
OperandValue::Ref(r) => base::memcpy_ty(bcx, lldest, r, operand.ty),
OperandValue::Immediate(s) => base::store_ty(bcx, s, lldest, operand.ty),
OperandValue::Ref(r) => base::memcpy_ty(bcx, lldest, r, operand.ty, align),
OperandValue::Immediate(s) => {
bcx.store(base::from_immediate(bcx, s), lldest, align);
}
OperandValue::Pair(a, b) => {
let a = base::from_immediate(bcx, a);
let b = base::from_immediate(bcx, b);
bcx.store(a, bcx.struct_gep(lldest, 0));
bcx.store(b, bcx.struct_gep(lldest, 1));
bcx.store(a, bcx.struct_gep(lldest, 0), align);
bcx.store(b, bcx.struct_gep(lldest, 1), align);
}
}
}
Expand Down

0 comments on commit 71a11a0

Please sign in to comment.