From 8624d5b186c1fba01f3e3cf38403db5ad93b87ed Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Fri, 22 Nov 2013 01:16:17 -0800 Subject: [PATCH 1/2] Represent C-like enums with a plain LLVM integer, not a struct. This is needed so that the FFI works as expected on platforms that don't flatten aggregates the way the AMD64 ABI does, especially for `#[repr(C)]`. This moves more of `type_of` into `trans::adt`, because the type might or might not be an LLVM struct. --- src/librustc/middle/trans/adt.rs | 66 +++++++++++++++------- src/librustc/middle/trans/type_of.rs | 34 +++++------ src/test/run-pass/enum-clike-ffi-as-int.rs | 39 +++++++++++++ 3 files changed, 99 insertions(+), 40 deletions(-) create mode 100644 src/test/run-pass/enum-clike-ffi-as-int.rs diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index 997bcbca9ce0b..d75fa9341bf57 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -366,22 +366,41 @@ pub fn ty_of_inttype(ity: IntType) -> ty::t { /** - * Returns the fields of a struct for the given representation. - * All nominal types are LLVM structs, in order to be able to use - * forward-declared opaque types to prevent circularity in `type_of`. + * LLVM-level types are a little complicated. + * + * C-like enums need to be actual ints, not wrapped in a struct, + * because that changes the ABI on some platforms (see issue #10308). + * + * For nominal types, in some cases, we need to use LLVM named structs + * and fill in the actual contents in a second pass to prevent + * unbounded recursion; see also the comments in `trans::type_of`. */ -pub fn fields_of(cx: &mut CrateContext, r: &Repr) -> ~[Type] { - generic_fields_of(cx, r, false) +pub fn type_of(cx: &mut CrateContext, r: &Repr) -> Type { + generic_type_of(cx, r, None, false) +} +pub fn sizing_type_of(cx: &mut CrateContext, r: &Repr) -> Type { + generic_type_of(cx, r, None, true) } -/// Like `fields_of`, but for `type_of::sizing_type_of` (q.v.). -pub fn sizing_fields_of(cx: &mut CrateContext, r: &Repr) -> ~[Type] { - generic_fields_of(cx, r, true) +pub fn incomplete_type_of(cx: &mut CrateContext, r: &Repr, name: &str) -> Type { + generic_type_of(cx, r, Some(name), false) +} +pub fn finish_type_of(cx: &mut CrateContext, r: &Repr, llty: &mut Type) { + match *r { + CEnum(*) | General(*) => { } + Univariant(ref st, _) | NullablePointer{ nonnull: ref st, _ } => + llty.set_struct_body(struct_llfields(cx, st, false), st.packed) + } } -fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] { + +fn generic_type_of(cx: &mut CrateContext, r: &Repr, name: Option<&str>, sizing: bool) -> Type { match *r { - CEnum(ity, _, _) => ~[ll_inttype(cx, ity)], - Univariant(ref st, _dtor) => struct_llfields(cx, st, sizing), - NullablePointer{ nonnull: ref st, _ } => struct_llfields(cx, st, sizing), + CEnum(ity, _, _) => ll_inttype(cx, ity), + Univariant(ref st, _) | NullablePointer{ nonnull: ref st, _ } => { + match name { + None => Type::struct_(struct_llfields(cx, st, sizing), st.packed), + Some(name) => { assert_eq!(sizing, false); Type::named_struct(name) } + } + } General(ity, ref sts) => { // We need a representation that has: // * The alignment of the most-aligned field @@ -394,8 +413,7 @@ fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] { // more of its own type, then use alignment-sized ints to get the rest // of the size. // - // Note: if/when we start exposing SIMD vector types (or f80, on some - // platforms that have it), this will need some adjustment. + // FIXME #10604: this breaks when vector types are present. let size = sts.iter().map(|st| st.size).max().unwrap(); let most_aligned = sts.iter().max_by(|st| st.align).unwrap(); let align = most_aligned.align; @@ -411,9 +429,17 @@ fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] { assert_eq!(machine::llalign_of_min(cx, pad_ty) as u64, align); let align_units = (size + align - 1) / align; assert_eq!(align % discr_size, 0); - ~[discr_ty, + let fields = ~[discr_ty, Type::array(&discr_ty, align / discr_size - 1), - Type::array(&pad_ty, align_units - 1)] + Type::array(&pad_ty, align_units - 1)]; + match name { + None => Type::struct_(fields, false), + Some(name) => { + let mut llty = Type::named_struct(name); + llty.set_struct_body(fields, false); + llty + } + } } } } @@ -460,7 +486,8 @@ pub fn trans_get_discr(bcx: @mut Block, r: &Repr, scrutinee: ValueRef, cast_to: signed = ity.is_signed(); } General(ity, ref cases) => { - val = load_discr(bcx, ity, scrutinee, 0, (cases.len() - 1) as Disr); + let ptr = GEPi(bcx, scrutinee, [0, 0]); + val = load_discr(bcx, ity, ptr, 0, (cases.len() - 1) as Disr); signed = ity.is_signed(); } Univariant(*) => { @@ -487,9 +514,8 @@ fn nullable_bitdiscr(bcx: @mut Block, nonnull: &Struct, nndiscr: Disr, ptrfield: } /// Helper for cases where the discriminant is simply loaded. -fn load_discr(bcx: @mut Block, ity: IntType, scrutinee: ValueRef, min: Disr, max: Disr) +fn load_discr(bcx: @mut Block, ity: IntType, ptr: ValueRef, min: Disr, max: Disr) -> ValueRef { - let ptr = GEPi(bcx, scrutinee, [0, 0]); let llty = ll_inttype(bcx.ccx(), ity); assert_eq!(val_ty(ptr), llty.ptr_to()); let bits = machine::llbitsize_of_real(bcx.ccx(), llty); @@ -546,7 +572,7 @@ pub fn trans_start_init(bcx: @mut Block, r: &Repr, val: ValueRef, discr: Disr) { CEnum(ity, min, max) => { assert_discr_in_range(ity, min, max, discr); Store(bcx, C_integral(ll_inttype(bcx.ccx(), ity), discr as u64, true), - GEPi(bcx, val, [0, 0])) + val) } General(ity, _) => { Store(bcx, C_integral(ll_inttype(bcx.ccx(), ity), discr as u64, true), diff --git a/src/librustc/middle/trans/type_of.rs b/src/librustc/middle/trans/type_of.rs index 36af13d34e656..0b51105cdde39 100644 --- a/src/librustc/middle/trans/type_of.rs +++ b/src/librustc/middle/trans/type_of.rs @@ -147,18 +147,17 @@ pub fn sizing_type_of(cx: &mut CrateContext, t: ty::t) -> Type { ty::ty_tup(*) | ty::ty_enum(*) => { let repr = adt::represent_type(cx, t); - Type::struct_(adt::sizing_fields_of(cx, repr), false) + adt::sizing_type_of(cx, repr) } - ty::ty_struct(did, _) => { + ty::ty_struct(*) => { if ty::type_is_simd(cx.tcx, t) { let et = ty::simd_type(cx.tcx, t); let n = ty::simd_size(cx.tcx, t); Type::vector(&type_of(cx, et), n as u64) } else { let repr = adt::represent_type(cx, t); - let packed = ty::lookup_packed(cx.tcx, did); - Type::struct_(adt::sizing_fields_of(cx, repr), packed) + adt::sizing_type_of(cx, repr) } } @@ -217,8 +216,9 @@ pub fn type_of(cx: &mut CrateContext, t: ty::t) -> Type { // fill it in *after* placing it into the type cache. This // avoids creating more than one copy of the enum when one // of the enum's variants refers to the enum itself. - - Type::named_struct(llvm_type_name(cx, an_enum, did, substs.tps)) + let repr = adt::represent_type(cx, t); + let name = llvm_type_name(cx, an_enum, did, substs.tps); + adt::incomplete_type_of(cx, repr, name) } ty::ty_estr(ty::vstore_box) => { Type::box(cx, &Type::vec(cx.sess.targ_cfg.arch, &Type::i8())).ptr_to() @@ -287,7 +287,7 @@ pub fn type_of(cx: &mut CrateContext, t: ty::t) -> Type { ty::ty_type => cx.tydesc_type.ptr_to(), ty::ty_tup(*) => { let repr = adt::represent_type(cx, t); - Type::struct_(adt::fields_of(cx, repr), false) + adt::type_of(cx, repr) } ty::ty_opaque_closure_ptr(_) => Type::opaque_box(cx).ptr_to(), ty::ty_struct(did, ref substs) => { @@ -299,7 +299,9 @@ pub fn type_of(cx: &mut CrateContext, t: ty::t) -> Type { // Only create the named struct, but don't fill it in. We fill it // in *after* placing it into the type cache. This prevents // infinite recursion with recursive struct types. - Type::named_struct(llvm_type_name(cx, a_struct, did, substs.tps)) + let repr = adt::represent_type(cx, t); + let name = llvm_type_name(cx, a_struct, did, substs.tps); + adt::incomplete_type_of(cx, repr, name) } } ty::ty_self(*) => cx.tcx.sess.unimpl("type_of: ty_self"), @@ -316,19 +318,11 @@ pub fn type_of(cx: &mut CrateContext, t: ty::t) -> Type { // If this was an enum or struct, fill in the type now. match ty::get(t).sty { - ty::ty_enum(*) => { - let repr = adt::represent_type(cx, t); - llty.set_struct_body(adt::fields_of(cx, repr), false); - } - - ty::ty_struct(did, _) => { - if !ty::type_is_simd(cx.tcx, t) { - let repr = adt::represent_type(cx, t); - let packed = ty::lookup_packed(cx.tcx, did); - llty.set_struct_body(adt::fields_of(cx, repr), packed); + ty::ty_enum(*) | ty::ty_struct(*) if !ty::type_is_simd(cx.tcx, t) => { + let repr = adt::represent_type(cx, t); + adt::finish_type_of(cx, repr, &mut llty); } - } - _ => () + _ => () } return llty; diff --git a/src/test/run-pass/enum-clike-ffi-as-int.rs b/src/test/run-pass/enum-clike-ffi-as-int.rs new file mode 100644 index 0000000000000..6216f4884e387 --- /dev/null +++ b/src/test/run-pass/enum-clike-ffi-as-int.rs @@ -0,0 +1,39 @@ +// Copyright 2013 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +/*! + * C-like enums have to be represented as LLVM ints, not wrapped in a + * struct, because it's important for the FFI that they interoperate + * with C integers/enums, and the ABI can treat structs differently. + * For example, on i686-linux-gnu, a struct return value is passed by + * storing to a hidden out parameter, whereas an integer would be + * returned in a register. + * + * This test just checks that the ABIs for the enum and the plain + * integer are compatible, rather than actually calling C code. + * The unused parameter to `foo` is to increase the likelihood of + * crashing if something goes wrong here. + */ + +#[repr(u32)] +enum Foo { + A = 0, + B = 23 +} + +#[inline(never)] +extern "C" fn foo(_x: uint) -> Foo { B } + +pub fn main() { + unsafe { + let f: extern "C" fn(uint) -> u32 = std::cast::transmute(foo); + assert_eq!(f(0xDEADBEEF), B as u32); + } +} From 0c04a26b3f8ba70ea686e1a365729835c62c26eb Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Mon, 25 Nov 2013 19:42:57 -0800 Subject: [PATCH 2/2] Fix the usual check-fast scoping mistake. --- src/test/run-pass/enum-clike-ffi-as-int.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/run-pass/enum-clike-ffi-as-int.rs b/src/test/run-pass/enum-clike-ffi-as-int.rs index 6216f4884e387..0c897b959a5a3 100644 --- a/src/test/run-pass/enum-clike-ffi-as-int.rs +++ b/src/test/run-pass/enum-clike-ffi-as-int.rs @@ -33,7 +33,7 @@ extern "C" fn foo(_x: uint) -> Foo { B } pub fn main() { unsafe { - let f: extern "C" fn(uint) -> u32 = std::cast::transmute(foo); + let f: extern "C" fn(uint) -> u32 = ::std::cast::transmute(foo); assert_eq!(f(0xDEADBEEF), B as u32); } }