Add missing attributes to indirect calls for foreign functions

When calling a foreign function, some arguments and/or return value
attributes are required to conform to the foreign ABI. Currently those
attributes are only added to the declaration of foreign functions. With
direct calls, this is no problem, because LLVM can see that those
attributes apply to the call. But with an indirect call, LLVM cannot do
that and the attribute is missing.

To fix that, we have to add those attribute to the calls to foreign
functions as well.

This also allows to remove the special handling of the SRet attribute,
which is ABI-dependent and will be set via the `attr` field of the
return type's `ArgType`.

Author: dotdash

src/librustc/middle/trans/foreign.rs

@@ -11,7 +11,7 @@
 
 use back::{link};
 use lib::llvm::llvm;
-use lib::llvm::{ValueRef, CallConv, StructRetAttribute, Linkage};
+use lib::llvm::{ValueRef, CallConv, Linkage};
 use lib;
 use middle::weak_lang_items;
 use middle::trans::base::push_ctxt;
@@ -373,18 +373,41 @@ pub fn trans_native_call<'a>(
     };
 
     // A function pointer is called without the declaration available, so we have to apply
-    // any attributes with ABI implications directly to the call instruction. Right now, the
-    // only attribute we need to worry about is `sret`.
+    // any attributes with ABI implications directly to the call instruction.
     let mut attrs = Vec::new();
-    if fn_type.ret_ty.is_indirect() {
-        attrs.push((1, lib::llvm::StructRetAttribute as u64));
 
+    // Add attributes that are always applicable, independent of the concrete foreign ABI
+    if fn_type.ret_ty.is_indirect() {
         // The outptr can be noalias and nocapture because it's entirely
         // invisible to the program. We can also mark it as nonnull
         attrs.push((1, lib::llvm::NoAliasAttribute as u64));
         attrs.push((1, lib::llvm::NoCaptureAttribute as u64));
         attrs.push((1, lib::llvm::NonNullAttribute as u64));
     };
+
+    // Add attributes that depend on the concrete foreign ABI
+    let mut arg_idx = if fn_type.ret_ty.is_indirect() { 1 } else { 0 };
+    match fn_type.ret_ty.attr {
+        Some(attr) => attrs.push((arg_idx, attr as u64)),
+        _ => ()
+    }
+
+    arg_idx += 1;
+    for arg_ty in fn_type.arg_tys.iter() {
+        if arg_ty.is_ignore() {
+            continue;
+        }
+        // skip padding
+        if arg_ty.pad.is_some() { arg_idx += 1; }
+
+        match arg_ty.attr {
+            Some(attr) => attrs.push((arg_idx, attr as u64)),
+            _ => {}
+        }
+
+        arg_idx += 1;
+    }
+
     let llforeign_retval = CallWithConv(bcx,
                                         llfn,
                                         llargs_foreign.as_slice(),

src/rt/rust_test_helpers.c

@@ -199,3 +199,21 @@ void
 rust_dbg_static_mut_check_four() {
     assert(rust_dbg_static_mut == 4);
 }
+
+struct S {
+    uint64_t x;
+    uint64_t y;
+    uint64_t z;
+};
+
+uint64_t get_x(struct S s) {
+    return s.x;
+}
+
+uint64_t get_y(struct S s) {
+    return s.y;
+}
+
+uint64_t get_z(struct S s) {
+    return s.z;
+}

src/test/run-pass/foreign-fn-with-byval.rs

@@ -0,0 +1,36 @@
+// Copyright 2014 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.
+
+pub struct S {
+    x: u64,
+    y: u64,
+    z: u64,
+}
+
+#[link(name = "rust_test_helpers")]
+extern {
+    pub fn get_x(x: S) -> u64;
+    pub fn get_y(x: S) -> u64;
+    pub fn get_z(x: S) -> u64;
+}
+
+#[inline(never)]
+fn indirect_call(func: unsafe extern fn(s: S) -> u64, s: S) -> u64 {
+    unsafe {
+        func(s)
+    }
+}
+
+fn main() {
+    let s = S { x: 1, y: 2, z: 3 };
+    assert_eq!(s.x, indirect_call(get_x, s));
+    assert_eq!(s.y, indirect_call(get_y, s));
+    assert_eq!(s.z, indirect_call(get_z, s));
+}