From d17a99c981ab429cab515dc82bf69dda5e152f19 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Fri, 6 Jan 2017 10:49:13 +0200 Subject: [PATCH] Set the expected align for byval arguments The alignment expected by the sysv ABI for i128 (16 bytes) does not match the alignment expected by rustc (8 bytes). Anything containing `i128`, then, when passed across the ABI boundary may result in stack/memory being trashed by the callee. --- src/librustc_llvm/ffi.rs | 2 ++ src/librustc_trans/abi.rs | 12 ++++++++++++ src/librustc_trans/cabi_asmjs.rs | 3 ++- src/librustc_trans/cabi_x86.rs | 3 ++- src/librustc_trans/cabi_x86_64.rs | 3 ++- src/rt/rust_test_helpers.c | 13 ++++++++++--- src/rustllvm/RustWrapper.cpp | 19 +++++++++++++++++++ src/test/run-pass/i128-ffi.rs | 12 ++++++++++++ 8 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/librustc_llvm/ffi.rs b/src/librustc_llvm/ffi.rs index 6815da4cc20fd..46d0469ba47a2 100644 --- a/src/librustc_llvm/ffi.rs +++ b/src/librustc_llvm/ffi.rs @@ -691,6 +691,7 @@ extern "C" { -> ValueRef; pub fn LLVMSetFunctionCallConv(Fn: ValueRef, CC: c_uint); pub fn LLVMRustAddDereferenceableAttr(Fn: ValueRef, index: c_uint, bytes: u64); + pub fn LLVMRustAddAlignAttr(Fn: ValueRef, index: c_uint, bytes: u64); pub fn LLVMRustAddFunctionAttribute(Fn: ValueRef, index: c_uint, attr: Attribute); pub fn LLVMRustAddFunctionAttrStringValue(Fn: ValueRef, index: c_uint, @@ -721,6 +722,7 @@ extern "C" { pub fn LLVMSetInstructionCallConv(Instr: ValueRef, CC: c_uint); pub fn LLVMRustAddCallSiteAttribute(Instr: ValueRef, index: c_uint, attr: Attribute); pub fn LLVMRustAddDereferenceableCallSiteAttr(Instr: ValueRef, index: c_uint, bytes: u64); + pub fn LLVMRustAddAlignCallSiteAttr(Instr: ValueRef, index: c_uint, bytes: u64); // Operations on load/store instructions (only) pub fn LLVMSetVolatile(MemoryAccessInst: ValueRef, volatile: Bool); diff --git a/src/librustc_trans/abi.rs b/src/librustc_trans/abi.rs index ad4bb0fce22ad..fb792a2beb850 100644 --- a/src/librustc_trans/abi.rs +++ b/src/librustc_trans/abi.rs @@ -96,6 +96,7 @@ impl ArgAttribute { pub struct ArgAttributes { regular: ArgAttribute, dereferenceable_bytes: u64, + align: u64, } impl ArgAttributes { @@ -109,6 +110,11 @@ impl ArgAttributes { self } + pub fn set_align(&mut self, align: u64) -> &mut Self { + self.align = align; + self + } + pub fn apply_llfn(&self, idx: AttributePlace, llfn: ValueRef) { unsafe { self.regular.for_each_kind(|attr| attr.apply_llfn(idx, llfn)); @@ -117,6 +123,9 @@ impl ArgAttributes { idx.as_uint(), self.dereferenceable_bytes); } + if self.align != 0 { + llvm::LLVMRustAddAlignAttr(llfn, idx.as_uint(), self.align); + } } } @@ -128,6 +137,9 @@ impl ArgAttributes { idx.as_uint(), self.dereferenceable_bytes); } + if self.align != 0 { + llvm::LLVMRustAddAlignCallSiteAttr(callsite, idx.as_uint(), self.align); + } } } } diff --git a/src/librustc_trans/cabi_asmjs.rs b/src/librustc_trans/cabi_asmjs.rs index f410627400c34..465ffb8d3a22a 100644 --- a/src/librustc_trans/cabi_asmjs.rs +++ b/src/librustc_trans/cabi_asmjs.rs @@ -11,7 +11,7 @@ #![allow(non_upper_case_globals)] use llvm::{Struct, Array}; -use abi::{FnType, ArgType, ArgAttribute}; +use abi::{FnType, ArgType, ArgAttribute, ty_align}; use context::CrateContext; // Data layout: e-p:32:32-i64:64-v128:32:128-n32-S128 @@ -40,6 +40,7 @@ fn classify_arg_ty(ccx: &CrateContext, arg: &mut ArgType) { if arg.ty.is_aggregate() { arg.make_indirect(ccx); arg.attrs.set(ArgAttribute::ByVal); + arg.attrs.set_align(ty_align(arg.ty, 0) as u64); } } diff --git a/src/librustc_trans/cabi_x86.rs b/src/librustc_trans/cabi_x86.rs index fea005f3d77da..47ebdb8c2f1a8 100644 --- a/src/librustc_trans/cabi_x86.rs +++ b/src/librustc_trans/cabi_x86.rs @@ -9,7 +9,7 @@ // except according to those terms. use llvm::*; -use abi::{ArgAttribute, FnType}; +use abi::{ArgAttribute, FnType, ty_align}; use type_::Type; use super::common::*; use super::machine::*; @@ -53,6 +53,7 @@ pub fn compute_abi_info(ccx: &CrateContext, fty: &mut FnType, flavor: Flavor) { if arg.ty.kind() == Struct { arg.make_indirect(ccx); arg.attrs.set(ArgAttribute::ByVal); + arg.attrs.set_align(ty_align(arg.ty, 0) as u64); } else { arg.extend_integer_width_to(32); } diff --git a/src/librustc_trans/cabi_x86_64.rs b/src/librustc_trans/cabi_x86_64.rs index 7f2fdbf000b65..865aa18bcbdde 100644 --- a/src/librustc_trans/cabi_x86_64.rs +++ b/src/librustc_trans/cabi_x86_64.rs @@ -16,7 +16,7 @@ use self::RegClass::*; use llvm::{Integer, Pointer, Float, Double}; use llvm::{Struct, Array, Vector}; -use abi::{self, ArgType, ArgAttribute, FnType}; +use abi::{self, ArgType, ArgAttribute, FnType, ty_align}; use context::CrateContext; use type_::Type; @@ -343,6 +343,7 @@ pub fn compute_abi_info(ccx: &CrateContext, fty: &mut FnType) { arg.make_indirect(ccx); if let Some(attr) = ind_attr { arg.attrs.set(attr); + arg.attrs.set_align(ty_align(arg.ty, 0) as u64); } } else { arg.cast = Some(llreg_ty(ccx, &cls)); diff --git a/src/rt/rust_test_helpers.c b/src/rt/rust_test_helpers.c index f2d9119a7d156..11ab0ad974ec8 100644 --- a/src/rt/rust_test_helpers.c +++ b/src/rt/rust_test_helpers.c @@ -269,10 +269,17 @@ LARGE_INTEGER increment_all_parts(LARGE_INTEGER li) { return li; } -#define DO_INT128_TEST !(defined(WIN32) || defined(_WIN32) || defined(__WIN32)) && \ - defined(__amd64__) +#if !(defined(WIN32) || defined(_WIN32) || defined(__WIN32)) && defined(__amd64__) -#if DO_INT128_TEST +struct Foo { + __int128 a; + int8_t b; + uint16_t c; +}; + +struct Foo adt_id(struct Foo foo) { + return foo; +} unsigned __int128 identity(unsigned __int128 a) { return a; diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index ea31e8dab678f..593cc4972bd02 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -174,6 +174,17 @@ extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, AttributeSet::get(Call->getContext(), Index, B))); } +extern "C" void LLVMRustAddAlignCallSiteAttr(LLVMValueRef Instr, + unsigned Index, + uint64_t Bytes) { + CallSite Call = CallSite(unwrap(Instr)); + AttrBuilder B; + B.addAlignmentAttr(Bytes); + Call.setAttributes(Call.getAttributes().addAttributes( + Call->getContext(), Index, + AttributeSet::get(Call->getContext(), Index, B))); +} + extern "C" void LLVMRustAddFunctionAttribute(LLVMValueRef Fn, unsigned Index, LLVMRustAttribute RustAttr) { Function *A = unwrap(Fn); @@ -190,6 +201,14 @@ extern "C" void LLVMRustAddDereferenceableAttr(LLVMValueRef Fn, unsigned Index, A->addAttributes(Index, AttributeSet::get(A->getContext(), Index, B)); } +extern "C" void LLVMRustAddAlignAttr(LLVMValueRef Fn, unsigned Index, + uint64_t Bytes) { + Function *A = unwrap(Fn); + AttrBuilder B; + B.addAlignmentAttr(Bytes); + A->addAttributes(Index, AttributeSet::get(A->getContext(), Index, B)); +} + extern "C" void LLVMRustAddFunctionAttrStringValue(LLVMValueRef Fn, unsigned Index, const char *Name, diff --git a/src/test/run-pass/i128-ffi.rs b/src/test/run-pass/i128-ffi.rs index 3b5f4884d21e7..a237dd1c392f4 100644 --- a/src/test/run-pass/i128-ffi.rs +++ b/src/test/run-pass/i128-ffi.rs @@ -22,11 +22,20 @@ #![feature(i128_type)] +#[repr(C)] +#[derive(Copy, Clone, PartialEq, Debug)] +struct Foo { + a: i128, + b: i8, + c: u16, +} + #[link(name = "rust_test_helpers", kind = "static")] extern "C" { fn identity(f: u128) -> u128; fn square(f: i128) -> i128; fn sub(f: i128, f: i128) -> i128; + fn adt_id(f: Foo) -> Foo; } fn main() { @@ -41,5 +50,8 @@ fn main() { let k_d = 0x2468_ACF1_3579_BDFF_DB97_530E_CA86_420; let k_out = sub(k_d, k); assert_eq!(k, k_out); + let a = Foo { a: 1, b: 2, c: 3 }; + let b = adt_id(a); + assert_eq!(a, b); } }