about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2016-05-26 02:52:49 -0700
committerbors <bors@rust-lang.org>2016-05-26 02:52:49 -0700
commit3c795e08d6f4a532f12f3f8e1837db5e0647f8b0 (patch)
tree5d1cf1555506d6a4c8d3ab1ebbb43b092103848b
parentd5759a3417fa395d439f4283825504dd4f78dc87 (diff)
parent5b404523dd1741681ba411c48c3df3c4cf5b22ab (diff)
downloadrust-3c795e08d6f4a532f12f3f8e1837db5e0647f8b0.tar.gz
rust-3c795e08d6f4a532f12f3f8e1837db5e0647f8b0.zip
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.
```
-rw-r--r--src/librustc_trans/base.rs63
-rw-r--r--src/librustc_trans/mir/mod.rs17
-rw-r--r--src/test/codegen/stores.rs8
-rw-r--r--src/test/run-pass/foreign-truncated-arguments.rs29
4 files changed, 81 insertions, 36 deletions
diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs
index 03e12c1c8a7..712805061ea 100644
--- a/src/librustc_trans/base.rs
+++ b/src/librustc_trans/base.rs
@@ -1660,24 +1660,32 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
                     self.schedule_drop_mem(arg_scope_id, llarg, arg_ty, None);
 
                     datum::Datum::new(llarg,
-                                    arg_ty,
-                                    datum::Lvalue::new("FunctionContext::bind_args"))
+                                      arg_ty,
+                                      datum::Lvalue::new("FunctionContext::bind_args"))
                 } else {
-                    unpack_datum!(bcx, datum::lvalue_scratch_datum(bcx, arg_ty, "",
-                                                                   uninit_reason,
-                                                                   arg_scope_id, |bcx, dst| {
-                        debug!("FunctionContext::bind_args: {:?}: {:?}", hir_arg, arg_ty);
+                    let lltmp = if common::type_is_fat_ptr(bcx.tcx(), arg_ty) {
+                        let lltemp = alloc_ty(bcx, arg_ty, "");
                         let b = &bcx.build();
-                        if common::type_is_fat_ptr(bcx.tcx(), arg_ty) {
-                            let meta = &self.fn_ty.args[idx];
-                            idx += 1;
-                            arg.store_fn_arg(b, &mut llarg_idx, expr::get_dataptr(bcx, dst));
-                            meta.store_fn_arg(b, &mut llarg_idx, expr::get_meta(bcx, dst));
-                        } else {
-                            arg.store_fn_arg(b, &mut llarg_idx, dst);
-                        }
-                        bcx
-                    }))
+                        // we pass fat pointers as two words, but we want to
+                        // represent them internally as a pointer to two words,
+                        // so make an alloca to store them in.
+                        let meta = &self.fn_ty.args[idx];
+                        idx += 1;
+                        arg.store_fn_arg(b, &mut llarg_idx, expr::get_dataptr(bcx, lltemp));
+                        meta.store_fn_arg(b, &mut llarg_idx, expr::get_meta(bcx, lltemp));
+                        lltemp
+                    } else  {
+                        // otherwise, arg is passed by value, so store it into a temporary.
+                        let llarg_ty = arg.cast.unwrap_or(arg.memory_ty(bcx.ccx()));
+                        let lltemp = alloca(bcx, llarg_ty, "");
+                        let b = &bcx.build();
+                        arg.store_fn_arg(b, &mut llarg_idx, lltemp);
+                        // And coerce the temporary into the type we expect.
+                        b.pointercast(lltemp, arg.memory_ty(bcx.ccx()).ptr_to())
+                    };
+                    bcx.fcx.schedule_drop_mem(arg_scope_id, lltmp, arg_ty, None);
+                    datum::Datum::new(lltmp, arg_ty,
+                                      datum::Lvalue::new("bind_args"))
                 }
             } else {
                 // FIXME(pcwalton): Reduce the amount of code bloat this is responsible for.
@@ -1712,16 +1720,19 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
             };
 
             let pat = &hir_arg.pat;
-            bcx = if let Some(name) = simple_name(pat) {
-                // Generate nicer LLVM for the common case of fn a pattern
-                // like `x: T`
-                set_value_name(arg_datum.val, &bcx.name(name));
-                self.lllocals.borrow_mut().insert(pat.id, arg_datum);
-                bcx
-            } else {
-                // General path. Copy out the values that are used in the
-                // pattern.
-                _match::bind_irrefutable_pat(bcx, pat, arg_datum.match_input(), arg_scope_id)
+            bcx = match simple_name(pat) {
+                // The check for alloca is necessary because above for the immediate argument case
+                // we had to cast. At this point arg_datum is not an alloca anymore and thus
+                // breaks debuginfo if we allow this optimisation.
+                Some(name)
+                if unsafe { llvm::LLVMIsAAllocaInst(arg_datum.val) != ::std::ptr::null_mut() } => {
+                    // Generate nicer LLVM for the common case of fn a pattern
+                    // like `x: T`
+                    set_value_name(arg_datum.val, &bcx.name(name));
+                    self.lllocals.borrow_mut().insert(pat.id, arg_datum);
+                    bcx
+                },
+                _ => _match::bind_irrefutable_pat(bcx, pat, arg_datum.match_input(), arg_scope_id)
             };
             debuginfo::create_argument_metadata(bcx, hir_arg);
         }
diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs
index b98e04e51c0..ffc14b4468b 100644
--- a/src/librustc_trans/mir/mod.rs
+++ b/src/librustc_trans/mir/mod.rs
@@ -327,10 +327,10 @@ fn arg_value_refs<'bcx, 'tcx>(bcx: &BlockAndBuilder<'bcx, 'tcx>,
             llarg_idx += 1;
             llarg
         } else {
-            let lltemp = bcx.with_block(|bcx| {
-                base::alloc_ty(bcx, arg_ty, &format!("arg{}", arg_index))
-            });
             if common::type_is_fat_ptr(tcx, arg_ty) {
+                let lltemp = bcx.with_block(|bcx| {
+                    base::alloc_ty(bcx, arg_ty, &format!("arg{}", arg_index))
+                });
                 // we pass fat pointers as two words, but we want to
                 // represent them internally as a pointer to two words,
                 // so make an alloca to store them in.
@@ -338,12 +338,17 @@ fn arg_value_refs<'bcx, 'tcx>(bcx: &BlockAndBuilder<'bcx, 'tcx>,
                 idx += 1;
                 arg.store_fn_arg(bcx, &mut llarg_idx, get_dataptr(bcx, lltemp));
                 meta.store_fn_arg(bcx, &mut llarg_idx, get_meta(bcx, lltemp));
+                lltemp
             } else  {
-                // otherwise, arg is passed by value, so make a
-                // temporary and store it there
+                // otherwise, arg is passed by value, so store it into a temporary.
+                let llarg_ty = arg.cast.unwrap_or(arg.memory_ty(bcx.ccx()));
+                let lltemp = bcx.with_block(|bcx| {
+                    base::alloca(bcx, llarg_ty, &format!("arg{}", arg_index))
+                });
                 arg.store_fn_arg(bcx, &mut llarg_idx, lltemp);
+                // And coerce the temporary into the type we expect.
+                bcx.pointercast(lltemp, arg.memory_ty(bcx.ccx()).ptr_to())
             }
-            lltemp
         };
         bcx.with_block(|bcx| arg_scope.map(|scope| {
             // Is this a regular argument?
diff --git a/src/test/codegen/stores.rs b/src/test/codegen/stores.rs
index f849a6c9b18..8c425507975 100644
--- a/src/test/codegen/stores.rs
+++ b/src/test/codegen/stores.rs
@@ -26,8 +26,8 @@ pub struct Bytes {
 #[no_mangle]
 #[rustc_no_mir] // FIXME #27840 MIR has different codegen.
 pub fn small_array_alignment(x: &mut [i8; 4], y: [i8; 4]) {
-// CHECK: [[VAR:%[0-9]+]] = bitcast [4 x i8]* %y to i32*
-// CHECK: store i32 %{{.*}}, i32* [[VAR]], align 1
+// CHECK: store i32 %{{.*}}, i32* %{{.*}}, align 1
+// CHECK: [[VAR:%[0-9]+]] = bitcast i32* %{{.*}} to [4 x i8]*
     *x = y;
 }
 
@@ -37,7 +37,7 @@ pub fn small_array_alignment(x: &mut [i8; 4], y: [i8; 4]) {
 #[no_mangle]
 #[rustc_no_mir] // FIXME #27840 MIR has different codegen.
 pub fn small_struct_alignment(x: &mut Bytes, y: Bytes) {
-// CHECK: [[VAR:%[0-9]+]] = bitcast %Bytes* %y to i32*
-// CHECK: store i32 %{{.*}}, i32* [[VAR]], align 1
+// CHECK: store i32 %{{.*}}, i32* %{{.*}}, align 1
+// CHECK: [[VAR:%[0-9]+]] = bitcast i32* %{{.*}} to %Bytes*
     *x = y;
 }
diff --git a/src/test/run-pass/foreign-truncated-arguments.rs b/src/test/run-pass/foreign-truncated-arguments.rs
new file mode 100644
index 00000000000..a983a4a9598
--- /dev/null
+++ b/src/test/run-pass/foreign-truncated-arguments.rs
@@ -0,0 +1,29 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -O
+// Regression test for https://github.com/rust-lang/rust/issues/33868
+
+#[repr(C)]
+pub struct S {
+    a: u32,
+    b: f32,
+    c: u32
+}
+
+#[no_mangle]
+#[inline(never)]
+pub extern "C" fn test(s: S) -> u32 {
+    s.c
+}
+
+fn main() {
+    assert_eq!(test(S{a: 0, b: 0.0, c: 42}), 42);
+}