Skip to content

Commit 3c795e0

Browse files
committed
Auto merge of #33872 - nagisa:undef-is-llvm-for-sigsegv, r=eddyb
Fix handling of FFI arguments r? @eddyb @nikomatsakis or whoever else. cc @alexcrichton @rust-lang/core The strategy employed here was to essentially change code we generate from ```llvm %s = alloca %S ; potentially smaller than argument, but never larger %1 = bitcast %S* %s to { i64, i64 }* store { i64, i64 } %0, { i64, i64 }* %1, align 4 ``` to ```llvm %1 = alloca { i64, i64 } ; the copy of argument itself store { i64, i64 } %0, { i64, i64 }* %1, align 4 %s = bitcast { i64, i64 }* %1 to %S* ; potentially truncate by casting to a pointer of smaller type. ```
2 parents d5759a3 + 5b40452 commit 3c795e0

File tree

4 files changed

+81
-36
lines changed

4 files changed

+81
-36
lines changed

src/librustc_trans/base.rs

+37-26
Original file line numberDiff line numberDiff line change
@@ -1660,24 +1660,32 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
16601660
self.schedule_drop_mem(arg_scope_id, llarg, arg_ty, None);
16611661

16621662
datum::Datum::new(llarg,
1663-
arg_ty,
1664-
datum::Lvalue::new("FunctionContext::bind_args"))
1663+
arg_ty,
1664+
datum::Lvalue::new("FunctionContext::bind_args"))
16651665
} else {
1666-
unpack_datum!(bcx, datum::lvalue_scratch_datum(bcx, arg_ty, "",
1667-
uninit_reason,
1668-
arg_scope_id, |bcx, dst| {
1669-
debug!("FunctionContext::bind_args: {:?}: {:?}", hir_arg, arg_ty);
1666+
let lltmp = if common::type_is_fat_ptr(bcx.tcx(), arg_ty) {
1667+
let lltemp = alloc_ty(bcx, arg_ty, "");
16701668
let b = &bcx.build();
1671-
if common::type_is_fat_ptr(bcx.tcx(), arg_ty) {
1672-
let meta = &self.fn_ty.args[idx];
1673-
idx += 1;
1674-
arg.store_fn_arg(b, &mut llarg_idx, expr::get_dataptr(bcx, dst));
1675-
meta.store_fn_arg(b, &mut llarg_idx, expr::get_meta(bcx, dst));
1676-
} else {
1677-
arg.store_fn_arg(b, &mut llarg_idx, dst);
1678-
}
1679-
bcx
1680-
}))
1669+
// we pass fat pointers as two words, but we want to
1670+
// represent them internally as a pointer to two words,
1671+
// so make an alloca to store them in.
1672+
let meta = &self.fn_ty.args[idx];
1673+
idx += 1;
1674+
arg.store_fn_arg(b, &mut llarg_idx, expr::get_dataptr(bcx, lltemp));
1675+
meta.store_fn_arg(b, &mut llarg_idx, expr::get_meta(bcx, lltemp));
1676+
lltemp
1677+
} else {
1678+
// otherwise, arg is passed by value, so store it into a temporary.
1679+
let llarg_ty = arg.cast.unwrap_or(arg.memory_ty(bcx.ccx()));
1680+
let lltemp = alloca(bcx, llarg_ty, "");
1681+
let b = &bcx.build();
1682+
arg.store_fn_arg(b, &mut llarg_idx, lltemp);
1683+
// And coerce the temporary into the type we expect.
1684+
b.pointercast(lltemp, arg.memory_ty(bcx.ccx()).ptr_to())
1685+
};
1686+
bcx.fcx.schedule_drop_mem(arg_scope_id, lltmp, arg_ty, None);
1687+
datum::Datum::new(lltmp, arg_ty,
1688+
datum::Lvalue::new("bind_args"))
16811689
}
16821690
} else {
16831691
// FIXME(pcwalton): Reduce the amount of code bloat this is responsible for.
@@ -1712,16 +1720,19 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
17121720
};
17131721

17141722
let pat = &hir_arg.pat;
1715-
bcx = if let Some(name) = simple_name(pat) {
1716-
// Generate nicer LLVM for the common case of fn a pattern
1717-
// like `x: T`
1718-
set_value_name(arg_datum.val, &bcx.name(name));
1719-
self.lllocals.borrow_mut().insert(pat.id, arg_datum);
1720-
bcx
1721-
} else {
1722-
// General path. Copy out the values that are used in the
1723-
// pattern.
1724-
_match::bind_irrefutable_pat(bcx, pat, arg_datum.match_input(), arg_scope_id)
1723+
bcx = match simple_name(pat) {
1724+
// The check for alloca is necessary because above for the immediate argument case
1725+
// we had to cast. At this point arg_datum is not an alloca anymore and thus
1726+
// breaks debuginfo if we allow this optimisation.
1727+
Some(name)
1728+
if unsafe { llvm::LLVMIsAAllocaInst(arg_datum.val) != ::std::ptr::null_mut() } => {
1729+
// Generate nicer LLVM for the common case of fn a pattern
1730+
// like `x: T`
1731+
set_value_name(arg_datum.val, &bcx.name(name));
1732+
self.lllocals.borrow_mut().insert(pat.id, arg_datum);
1733+
bcx
1734+
},
1735+
_ => _match::bind_irrefutable_pat(bcx, pat, arg_datum.match_input(), arg_scope_id)
17251736
};
17261737
debuginfo::create_argument_metadata(bcx, hir_arg);
17271738
}

src/librustc_trans/mir/mod.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -327,23 +327,28 @@ fn arg_value_refs<'bcx, 'tcx>(bcx: &BlockAndBuilder<'bcx, 'tcx>,
327327
llarg_idx += 1;
328328
llarg
329329
} else {
330-
let lltemp = bcx.with_block(|bcx| {
331-
base::alloc_ty(bcx, arg_ty, &format!("arg{}", arg_index))
332-
});
333330
if common::type_is_fat_ptr(tcx, arg_ty) {
331+
let lltemp = bcx.with_block(|bcx| {
332+
base::alloc_ty(bcx, arg_ty, &format!("arg{}", arg_index))
333+
});
334334
// we pass fat pointers as two words, but we want to
335335
// represent them internally as a pointer to two words,
336336
// so make an alloca to store them in.
337337
let meta = &fcx.fn_ty.args[idx];
338338
idx += 1;
339339
arg.store_fn_arg(bcx, &mut llarg_idx, get_dataptr(bcx, lltemp));
340340
meta.store_fn_arg(bcx, &mut llarg_idx, get_meta(bcx, lltemp));
341+
lltemp
341342
} else {
342-
// otherwise, arg is passed by value, so make a
343-
// temporary and store it there
343+
// otherwise, arg is passed by value, so store it into a temporary.
344+
let llarg_ty = arg.cast.unwrap_or(arg.memory_ty(bcx.ccx()));
345+
let lltemp = bcx.with_block(|bcx| {
346+
base::alloca(bcx, llarg_ty, &format!("arg{}", arg_index))
347+
});
344348
arg.store_fn_arg(bcx, &mut llarg_idx, lltemp);
349+
// And coerce the temporary into the type we expect.
350+
bcx.pointercast(lltemp, arg.memory_ty(bcx.ccx()).ptr_to())
345351
}
346-
lltemp
347352
};
348353
bcx.with_block(|bcx| arg_scope.map(|scope| {
349354
// Is this a regular argument?

src/test/codegen/stores.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ pub struct Bytes {
2626
#[no_mangle]
2727
#[rustc_no_mir] // FIXME #27840 MIR has different codegen.
2828
pub fn small_array_alignment(x: &mut [i8; 4], y: [i8; 4]) {
29-
// CHECK: [[VAR:%[0-9]+]] = bitcast [4 x i8]* %y to i32*
30-
// CHECK: store i32 %{{.*}}, i32* [[VAR]], align 1
29+
// CHECK: store i32 %{{.*}}, i32* %{{.*}}, align 1
30+
// CHECK: [[VAR:%[0-9]+]] = bitcast i32* %{{.*}} to [4 x i8]*
3131
*x = y;
3232
}
3333

@@ -37,7 +37,7 @@ pub fn small_array_alignment(x: &mut [i8; 4], y: [i8; 4]) {
3737
#[no_mangle]
3838
#[rustc_no_mir] // FIXME #27840 MIR has different codegen.
3939
pub fn small_struct_alignment(x: &mut Bytes, y: Bytes) {
40-
// CHECK: [[VAR:%[0-9]+]] = bitcast %Bytes* %y to i32*
41-
// CHECK: store i32 %{{.*}}, i32* [[VAR]], align 1
40+
// CHECK: store i32 %{{.*}}, i32* %{{.*}}, align 1
41+
// CHECK: [[VAR:%[0-9]+]] = bitcast i32* %{{.*}} to %Bytes*
4242
*x = y;
4343
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// compile-flags: -O
12+
// Regression test for https://github.com/rust-lang/rust/issues/33868
13+
14+
#[repr(C)]
15+
pub struct S {
16+
a: u32,
17+
b: f32,
18+
c: u32
19+
}
20+
21+
#[no_mangle]
22+
#[inline(never)]
23+
pub extern "C" fn test(s: S) -> u32 {
24+
s.c
25+
}
26+
27+
fn main() {
28+
assert_eq!(test(S{a: 0, b: 0.0, c: 42}), 42);
29+
}

0 commit comments

Comments
 (0)