Skip to content

Commit

Permalink
librustc: handle repr on structs, require it for ffi, unify with packed
Browse files Browse the repository at this point in the history
As of RFC 18, struct layout is undefined. Opting into a C-compatible struct
layout is now down with #[repr(C)]. For consistency, specifying a packed
layout is now also down with #[repr(packed)]. Both can be specified.

To fix errors caused by this, just add #[repr(C)] to the structs, and change
 #[packed] to #[repr(packed)]

Closes #14309

[breaking-change]
  • Loading branch information
emberian committed Aug 21, 2014
1 parent 54bd9e6 commit 6e8ff99
Show file tree
Hide file tree
Showing 27 changed files with 210 additions and 118 deletions.
1 change: 1 addition & 0 deletions src/liblibc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ pub mod types {
__variant1,
__variant2,
}

pub enum FILE {}
pub enum fpos_t {}
}
Expand Down
18 changes: 12 additions & 6 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

use metadata::csearch;
use middle::def::*;
use middle::trans::adt; // for `adt::is_ffi_safe`
use middle::typeck::astconv::ast_ty_to_ty;
use middle::typeck::infer;
use middle::{typeck, ty, def, pat_util, stability};
Expand Down Expand Up @@ -362,8 +361,13 @@ impl LintPass for CTypes {
"found rust type `uint` in foreign module, while \
libc::c_uint or libc::c_ulong should be used");
}
def::DefTy(def_id) => {
if !adt::is_ffi_safe(cx.tcx, def_id) {
def::DefTy(..) => {
let tty = match cx.tcx.ast_ty_to_ty_cache.borrow().find(&ty.id) {
Some(&ty::atttce_resolved(t)) => t,
_ => fail!("ast_ty_to_ty_cache was incomplete after typeck!")
};

if !ty::is_ffi_safe(cx.tcx, tty) {
cx.span_lint(CTYPES, ty.span,
"found enum type without foreign-function-safe
representation annotation in foreign module, consider \
Expand Down Expand Up @@ -770,9 +774,10 @@ impl LintPass for NonCamelCaseTypes {
}
}

let has_extern_repr = it.attrs.iter().fold(attr::ReprAny, |acc, attr| {
attr::find_repr_attr(cx.tcx.sess.diagnostic(), attr, acc)
}) == attr::ReprExtern;
let has_extern_repr = it.attrs.iter().map(|attr| {
attr::find_repr_attrs(cx.tcx.sess.diagnostic(), attr).iter()
.any(|r| r == &attr::ReprExtern)
}).any(|x| x);
if has_extern_repr { return }

match it.node {
Expand All @@ -783,6 +788,7 @@ impl LintPass for NonCamelCaseTypes {
check_case(cx, "trait", it.ident, it.span)
}
ast::ItemEnum(ref enum_definition, _) => {
if has_extern_repr { return }
check_case(cx, "type", it.ident, it.span);
for variant in enum_definition.variants.iter() {
check_case(cx, "variant", variant.node.name, variant.span);
Expand Down
7 changes: 4 additions & 3 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ impl<'a> MarkSymbolVisitor<'a> {
ast_map::NodeItem(item) => {
match item.node {
ast::ItemStruct(..) => {
let has_extern_repr = item.attrs.iter().fold(attr::ReprAny, |acc, attr| {
attr::find_repr_attr(self.tcx.sess.diagnostic(), attr, acc)
}) == attr::ReprExtern;
let has_extern_repr = item.attrs.iter().fold(false, |acc, attr| {
acc || attr::find_repr_attrs(self.tcx.sess.diagnostic(), attr)
.iter().any(|&x| x == attr::ReprExtern)
});

visit::walk_item(self, &*item, MarkSymbolVisitorContext {
struct_has_extern_repr: has_extern_repr,
Expand Down
38 changes: 7 additions & 31 deletions src/librustc/middle/trans/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
#![allow(unsigned_negate)]

use libc::c_ulonglong;
use std::collections::Map;
use std::num::Int;
use std::rc::Rc;

use llvm::{ValueRef, True, IntEQ, IntNE};
Expand Down Expand Up @@ -178,7 +180,8 @@ fn represent_type_uncached(cx: &CrateContext, t: ty::t) -> Repr {
}
ty::ty_enum(def_id, ref substs) => {
let cases = get_cases(cx.tcx(), def_id, substs);
let hint = ty::lookup_repr_hint(cx.tcx(), def_id);
let hint = *ty::lookup_repr_hints(cx.tcx(), def_id).as_slice().get(0)
.unwrap_or(&attr::ReprAny);

let dtor = ty::ty_dtor(cx.tcx(), def_id).has_drop_flag();

Expand Down Expand Up @@ -266,36 +269,6 @@ fn represent_type_uncached(cx: &CrateContext, t: ty::t) -> Repr {
}
}

/// Determine, without doing translation, whether an ADT must be FFI-safe.
/// For use in lint or similar, where being sound but slightly incomplete is acceptable.
pub fn is_ffi_safe(tcx: &ty::ctxt, def_id: ast::DefId) -> bool {
match ty::get(ty::lookup_item_type(tcx, def_id).ty).sty {
ty::ty_enum(def_id, _) => {
let variants = ty::enum_variants(tcx, def_id);
// Univariant => like struct/tuple.
if variants.len() <= 1 {
return true;
}
let hint = ty::lookup_repr_hint(tcx, def_id);
// Appropriate representation explicitly selected?
if hint.is_ffi_safe() {
return true;
}
// Option<Box<T>> and similar are used in FFI. Rather than try to
// resolve type parameters and recognize this case exactly, this
// overapproximates -- assuming that if a non-C-like enum is being
// used in FFI then the user knows what they're doing.
if variants.iter().any(|vi| !vi.args.is_empty()) {
return true;
}
false
}
// struct, tuple, etc.
// (is this right in the present of typedefs?)
_ => true
}
}

// this should probably all be in ty
struct Case {
discr: Disr,
Expand Down Expand Up @@ -427,6 +400,9 @@ fn range_to_inttype(cx: &CrateContext, hint: Hint, bounds: &IntBounds) -> IntTyp
}
attr::ReprAny => {
attempts = choose_shortest;
},
attr::ReprPacked => {
cx.tcx.sess.bug("range_to_inttype: found ReprPacked on an enum");
}
}
for &ity in attempts.iter() {
Expand Down
88 changes: 73 additions & 15 deletions src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ pub struct t { inner: *const t_opaque }

impl fmt::Show for t {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
"*t_opaque".fmt(f)
write!(f, "{}", get(*self))
}
}

Expand Down Expand Up @@ -1962,7 +1962,8 @@ def_type_content_sets!(
// ReachesManaged /* see [1] below */ = 0b0000_0100__0000_0000__0000,
ReachesMutable = 0b0000_1000__0000_0000__0000,
ReachesNoSync = 0b0001_0000__0000_0000__0000,
ReachesAll = 0b0001_1111__0000_0000__0000,
ReachesFfiUnsafe = 0b0010_0000__0000_0000__0000,
ReachesAll = 0b0011_1111__0000_0000__0000,

// Things that cause values to *move* rather than *copy*
Moves = 0b0000_0000__0000_1011__0000,
Expand Down Expand Up @@ -2199,38 +2200,44 @@ pub fn type_contents(cx: &ctxt, ty: t) -> TypeContents {
cache.insert(ty_id, TC::None);

let result = match get(ty).sty {
// uint and int are ffi-unsafe
ty_uint(ast::TyU) | ty_int(ast::TyI) => {
TC::ReachesFfiUnsafe
}

// Scalar and unique types are sendable, and durable
ty_nil | ty_bot | ty_bool | ty_int(_) | ty_uint(_) | ty_float(_) |
ty_bare_fn(_) | ty::ty_char | ty_str => {
TC::None
}

ty_closure(ref c) => {
closure_contents(cx, &**c)
closure_contents(cx, &**c) | TC::ReachesFfiUnsafe
}

ty_box(typ) => {
tc_ty(cx, typ, cache).managed_pointer()
tc_ty(cx, typ, cache).managed_pointer() | TC::ReachesFfiUnsafe
}

ty_uniq(typ) => {
match get(typ).sty {
TC::ReachesFfiUnsafe | match get(typ).sty {
ty_str => TC::OwnsOwned,
_ => tc_ty(cx, typ, cache).owned_pointer(),
}
}

ty_trait(box ty::TyTrait { bounds, .. }) => {
object_contents(cx, bounds)
object_contents(cx, bounds) | TC::ReachesFfiUnsafe
}

ty_ptr(ref mt) => {
tc_ty(cx, mt.ty, cache).unsafe_pointer()
}

ty_rptr(r, ref mt) => {
match get(mt.ty).sty {
TC::ReachesFfiUnsafe | match get(mt.ty).sty {
ty_str => borrowed_contents(r, ast::MutImmutable),
ty_vec(..) => tc_ty(cx, mt.ty, cache).reference(borrowed_contents(r, mt.mutbl)),
_ => tc_ty(cx, mt.ty, cache).reference(borrowed_contents(r, mt.mutbl)),
}
}
Expand All @@ -2244,6 +2251,11 @@ pub fn type_contents(cx: &ctxt, ty: t) -> TypeContents {
let mut res =
TypeContents::union(flds.as_slice(),
|f| tc_mt(cx, f.mt, cache));

if !lookup_repr_hints(cx, did).contains(&attr::ReprExtern) {
res = res | TC::ReachesFfiUnsafe;
}

if ty::has_dtor(cx, did) {
res = res | TC::OwnsDtor;
}
Expand Down Expand Up @@ -2273,9 +2285,49 @@ pub fn type_contents(cx: &ctxt, ty: t) -> TypeContents {
tc_ty(cx, *arg_ty, cache)
})
});

if ty::has_dtor(cx, did) {
res = res | TC::OwnsDtor;
}

if variants.len() != 0 {
let repr_hints = lookup_repr_hints(cx, did);
if repr_hints.len() > 1 {
// this is an error later on, but this type isn't safe
res = res | TC::ReachesFfiUnsafe;
}

match repr_hints.as_slice().get(0) {
Some(h) => if !h.is_ffi_safe() {
res = res | TC::ReachesFfiUnsafe;
},
// ReprAny
None => {
res = res | TC::ReachesFfiUnsafe;

// We allow ReprAny enums if they are eligible for
// the nullable pointer optimization and the
// contained type is an `extern fn`

if variants.len() == 2 {
let mut data_idx = 0;

if variants.get(0).args.len() == 0 {
data_idx = 1;
}

if variants.get(data_idx).args.len() == 1 {
match get(*variants.get(data_idx).args.get(0)).sty {
ty_bare_fn(..) => { res = res - TC::ReachesFfiUnsafe; }
_ => { }
}
}
}
}
}
}


apply_lang_items(cx, did, res)
}

Expand Down Expand Up @@ -2427,6 +2479,10 @@ pub fn type_moves_by_default(cx: &ctxt, ty: t) -> bool {
type_contents(cx, ty).moves_by_default(cx)
}

pub fn is_ffi_safe(cx: &ctxt, ty: t) -> bool {
!type_contents(cx, ty).intersects(TC::ReachesFfiUnsafe)
}

// True if instantiating an instance of `r_ty` requires an instance of `r_ty`.
pub fn is_instantiable(cx: &ctxt, r_ty: t) -> bool {
fn type_requires(cx: &ctxt, seen: &mut Vec<DefId>,
Expand Down Expand Up @@ -3945,7 +4001,7 @@ pub fn substd_enum_variants(cx: &ctxt,
-> Vec<Rc<VariantInfo>> {
enum_variants(cx, id).iter().map(|variant_info| {
let substd_args = variant_info.args.iter()
.map(|aty| aty.subst(cx, substs)).collect();
.map(|aty| aty.subst(cx, substs)).collect::<Vec<_>>();

let substd_ctor_ty = variant_info.ctor_ty.subst(cx, substs);

Expand Down Expand Up @@ -4168,24 +4224,26 @@ pub fn has_attr(tcx: &ctxt, did: DefId, attr: &str) -> bool {
found
}

/// Determine whether an item is annotated with `#[packed]`
/// Determine whether an item is annotated with `#[repr(packed)]`
pub fn lookup_packed(tcx: &ctxt, did: DefId) -> bool {
has_attr(tcx, did, "packed")
lookup_repr_hints(tcx, did).contains(&attr::ReprPacked)
}

/// Determine whether an item is annotated with `#[simd]`
pub fn lookup_simd(tcx: &ctxt, did: DefId) -> bool {
has_attr(tcx, did, "simd")
}

// Obtain the representation annotation for a definition.
pub fn lookup_repr_hint(tcx: &ctxt, did: DefId) -> attr::ReprAttr {
let mut acc = attr::ReprAny;
/// Obtain the representation annotation for a struct definition.
pub fn lookup_repr_hints(tcx: &ctxt, did: DefId) -> Vec<attr::ReprAttr> {
let mut acc = Vec::new();

ty::each_attr(tcx, did, |meta| {
acc = attr::find_repr_attr(tcx.sess.diagnostic(), meta, acc);
acc.extend(attr::find_repr_attrs(tcx.sess.diagnostic(), meta).move_iter());
true
});
return acc;

acc
}

// Look up a field ID, whether or not it's local
Expand Down
11 changes: 8 additions & 3 deletions src/librustc/middle/typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4180,13 +4180,13 @@ pub fn check_enum_variants(ccx: &CrateCtxt,
let inh = blank_inherited_fields(ccx);
let fcx = blank_fn_ctxt(ccx, &inh, rty, e.id);
let declty = match hint {
attr::ReprAny | attr::ReprExtern => ty::mk_int(),
attr::ReprAny | attr::ReprPacked | attr::ReprExtern => ty::mk_int(),
attr::ReprInt(_, attr::SignedInt(ity)) => {
ty::mk_mach_int(ity)
}
attr::ReprInt(_, attr::UnsignedInt(ity)) => {
ty::mk_mach_uint(ity)
}
},
};
check_const_with_ty(&fcx, e.span, &*e, declty);
// check_expr (from check_const pass) doesn't guarantee
Expand Down Expand Up @@ -4225,6 +4225,9 @@ pub fn check_enum_variants(ccx: &CrateCtxt,
"discriminant type specified here");
}
}
attr::ReprPacked => {
ccx.tcx.sess.bug("range_to_inttype: found ReprPacked on an enum");
}
}
disr_vals.push(current_disr_val);

Expand All @@ -4238,7 +4241,9 @@ pub fn check_enum_variants(ccx: &CrateCtxt,
return variants;
}

let hint = ty::lookup_repr_hint(ccx.tcx, ast::DefId { krate: ast::LOCAL_CRATE, node: id });
let hint = *ty::lookup_repr_hints(ccx.tcx, ast::DefId { krate: ast::LOCAL_CRATE, node: id })
.as_slice().get(0).unwrap_or(&attr::ReprAny);

if hint != attr::ReprAny && vs.len() <= 1 {
if vs.len() == 1 {
span_err!(ccx.tcx.sess, sp, E0083,
Expand Down
1 change: 1 addition & 0 deletions src/librustrt/libunwind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub static unwinder_private_data_size: uint = 5;
#[cfg(target_arch = "mipsel")]
pub static unwinder_private_data_size: uint = 2;

#[repr(C)]
pub struct _Unwind_Exception {
pub exception_class: _Unwind_Exception_Class,
pub exception_cleanup: _Unwind_Exception_Cleanup_Fn,
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/rt/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ mod imp {
static IMAGE_FILE_MACHINE_IA64: libc::DWORD = 0x0200;
static IMAGE_FILE_MACHINE_AMD64: libc::DWORD = 0x8664;

#[packed]
#[repr(packed)]
struct SYMBOL_INFO {
SizeOfStruct: libc::c_ulong,
TypeIndex: libc::c_ulong,
Expand Down
Loading

0 comments on commit 6e8ff99

Please sign in to comment.