From 03d4d5f80ed1d9e168869bdb244e4fef67b7d3d0 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Mon, 20 Apr 2015 10:52:26 +1200 Subject: [PATCH] Fix a bunch of bugs * segfault due to not copying drop flag when coercing * fat pointer casts * segfault due to not checking drop flag properly * debuginfo for DST smart pointers * unreachable code in drop glue --- src/liballoc/rc.rs | 6 ++-- src/librustc/middle/ty.rs | 3 +- src/librustc_trans/trans/adt.rs | 14 +++++--- src/librustc_trans/trans/expr.rs | 49 +++++++++++++++++---------- src/librustc_trans/trans/glue.rs | 13 +++---- src/librustc_trans/trans/machine.rs | 3 +- src/librustc_typeck/check/cast.rs | 17 +++++----- src/test/compile-fail/fat-ptr-cast.rs | 12 +++---- src/test/compile-fail/issue-22289.rs | 2 +- src/test/run-pass/fat-ptr-cast.rs | 49 +++++++++++++++++++++++++++ 10 files changed, 114 insertions(+), 54 deletions(-) create mode 100644 src/test/run-pass/fat-ptr-cast.rs diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 4e8a7e8bfc9..837cf590024 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -544,7 +544,8 @@ impl Drop for Rc { fn drop(&mut self) { unsafe { let ptr = *self._ptr; - if !(*(&ptr as *const _ as *const *const ())).is_null() { + if !(*(&ptr as *const _ as *const *const ())).is_null() && + ptr as usize != mem::POST_DROP_USIZE { self.dec_strong(); if self.strong() == 0 { // destroy the contained object @@ -1010,7 +1011,8 @@ impl Drop for Weak { fn drop(&mut self) { unsafe { let ptr = *self._ptr; - if !(*(&ptr as *const _ as *const *const ())).is_null() { + if !(*(&ptr as *const _ as *const *const ())).is_null() && + ptr as usize != mem::POST_DROP_USIZE { self.dec_weak(); // the weak count starts at 1, and will only go to zero if all // the strong pointers have disappeared. diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 961f8a79324..5a84edd3cdf 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -5606,8 +5606,7 @@ impl DtorKind { } } -/* If struct_id names a struct with a dtor, return Some(the dtor's id). - Otherwise return none. */ +/* If struct_id names a struct with a dtor. */ pub fn ty_dtor(cx: &ctxt, struct_id: DefId) -> DtorKind { match cx.destructor_for_type.borrow().get(&struct_id) { Some(&method_def_id) => { diff --git a/src/librustc_trans/trans/adt.rs b/src/librustc_trans/trans/adt.rs index 001de615fb1..17c9fa24818 100644 --- a/src/librustc_trans/trans/adt.rs +++ b/src/librustc_trans/trans/adt.rs @@ -141,7 +141,8 @@ pub fn represent_node<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, /// Decides how to represent a given type. pub fn represent_type<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, - t: Ty<'tcx>) -> Rc> { + t: Ty<'tcx>) + -> Rc> { debug!("Representing: {}", ty_to_string(cx.tcx(), t)); match cx.adt_reprs().borrow().get(&t) { Some(repr) => return repr.clone(), @@ -216,7 +217,9 @@ fn represent_type_uncached<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, }).collect::>(); let packed = ty::lookup_packed(cx.tcx(), def_id); let dtor = ty::ty_dtor(cx.tcx(), def_id).has_drop_flag(); - if dtor { ftys.push(cx.tcx().dtor_type()); } + if dtor { + ftys.push(cx.tcx().dtor_type()); + } Univariant(mk_struct(cx, &ftys[..], packed, t), dtor_to_init_u8(dtor)) } @@ -517,8 +520,7 @@ fn mk_struct<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, -> Struct<'tcx> { let sized = tys.iter().all(|&ty| type_is_sized(cx.tcx(), ty)); let lltys : Vec = if sized { - tys.iter() - .map(|&ty| type_of::sizing_type_of(cx, ty)).collect() + tys.iter().map(|&ty| type_of::sizing_type_of(cx, ty)).collect() } else { tys.iter().filter(|&ty| type_is_sized(cx.tcx(), *ty)) .map(|&ty| type_of::sizing_type_of(cx, ty)).collect() @@ -1060,7 +1062,9 @@ pub fn fold_variants<'blk, 'tcx, F>(bcx: Block<'blk, 'tcx>, } /// Access the struct drop flag, if present. -pub fn trans_drop_flag_ptr<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, r: &Repr<'tcx>, val: ValueRef) +pub fn trans_drop_flag_ptr<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, + r: &Repr<'tcx>, + val: ValueRef) -> datum::DatumBlock<'blk, 'tcx, datum::Expr> { let tcx = bcx.tcx(); diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 0343571562f..4018df0da47 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/src/librustc_trans/trans/expr.rs @@ -58,7 +58,7 @@ use middle::check_const; use middle::def; use middle::lang_items::CoerceUnsizedTraitLangItem; use middle::mem_categorization::Typer; -use middle::subst::{Subst, Substs, VecPerParamSpace}; +use middle::subst::{Substs, VecPerParamSpace}; use middle::traits; use trans::{_match, adt, asm, base, callee, closure, consts, controlflow}; use trans::base::*; @@ -476,8 +476,8 @@ fn coerce_unsized<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, } // This can be extended to enums and tuples in the future. - // (&ty::ty_enum(def_id_a, substs_a), &ty::ty_enum(def_id_b, substs_b)) | - (&ty::ty_struct(def_id_a, substs_a), &ty::ty_struct(def_id_b, substs_b)) => { + // (&ty::ty_enum(def_id_a, _), &ty::ty_enum(def_id_b, _)) | + (&ty::ty_struct(def_id_a, _), &ty::ty_struct(def_id_b, _)) => { assert_eq!(def_id_a, def_id_b); // The target is already by-ref because it's to be written to. @@ -504,35 +504,41 @@ fn coerce_unsized<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, }; let repr_source = adt::represent_type(bcx.ccx(), source.ty); + let src_fields = match &*repr_source { + &adt::Repr::Univariant(ref s, _) => &s.fields, + _ => bcx.sess().span_bug(span, + &format!("Non univariant struct? (repr_source: {:?})", + repr_source)), + }; let repr_target = adt::represent_type(bcx.ccx(), target.ty); - let fields = ty::lookup_struct_fields(bcx.tcx(), def_id_a); + let target_fields = match &*repr_target { + &adt::Repr::Univariant(ref s, _) => &s.fields, + _ => bcx.sess().span_bug(span, + &format!("Non univariant struct? (repr_target: {:?})", + repr_target)), + }; let coerce_index = match kind { ty::CustomCoerceUnsized::Struct(i) => i }; - assert!(coerce_index < fields.len()); + assert!(coerce_index < src_fields.len() && src_fields.len() == target_fields.len()); - for (i, field) in fields.iter().enumerate() { + let iter = src_fields.iter().zip(target_fields.iter()).enumerate(); + for (i, (src_ty, target_ty)) in iter { let ll_source = adt::trans_field_ptr(bcx, &repr_source, source.val, 0, i); let ll_target = adt::trans_field_ptr(bcx, &repr_target, target.val, 0, i); - let ty = ty::lookup_field_type_unsubstituted(bcx.tcx(), - def_id_a, - field.id); - let field_source = ty.subst(bcx.tcx(), substs_a); - let field_target = ty.subst(bcx.tcx(), substs_b); - // If this is the field we need to coerce, recurse on it. if i == coerce_index { coerce_unsized(bcx, span, - Datum::new(ll_source, field_source, + Datum::new(ll_source, src_ty, Rvalue::new(ByRef)), - Datum::new(ll_target, field_target, + Datum::new(ll_target, target_ty, Rvalue::new(ByRef))); } else { // Otherwise, simply copy the data from the source. - assert_eq!(field_source, field_target); - memcpy_ty(bcx, ll_target, ll_source, field_source); + assert_eq!(src_ty, target_ty); + memcpy_ty(bcx, ll_target, ll_source, src_ty); } } } @@ -2013,6 +2019,7 @@ fn float_cast(bcx: Block, #[derive(Copy, Clone, PartialEq, Debug)] pub enum cast_kind { cast_pointer, + cast_fat_ptr, cast_integral, cast_float, cast_enum, @@ -2027,7 +2034,7 @@ pub fn cast_type_kind<'tcx>(tcx: &ty::ctxt<'tcx>, t: Ty<'tcx>) -> cast_kind { if type_is_sized(tcx, mt.ty) { cast_pointer } else { - cast_other + cast_fat_ptr } } ty::ty_bare_fn(..) => cast_pointer, @@ -2103,10 +2110,18 @@ fn trans_imm_cast<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let llexpr = datum.to_llscalarish(bcx); PtrToInt(bcx, llexpr, ll_t_out) } + (cast_fat_ptr, cast_integral) => { + let data_ptr = Load(bcx, get_dataptr(bcx, datum.val)); + PtrToInt(bcx, data_ptr, ll_t_out) + } (cast_pointer, cast_pointer) => { let llexpr = datum.to_llscalarish(bcx); PointerCast(bcx, llexpr, ll_t_out) } + (cast_fat_ptr, cast_pointer) => { + let data_ptr = Load(bcx, get_dataptr(bcx, datum.val)); + PointerCast(bcx, data_ptr, ll_t_out) + } (cast_enum, cast_integral) | (cast_enum, cast_float) => { let mut bcx = bcx; diff --git a/src/librustc_trans/trans/glue.rs b/src/librustc_trans/trans/glue.rs index fd1f22e1d9d..264957d3651 100644 --- a/src/librustc_trans/trans/glue.rs +++ b/src/librustc_trans/trans/glue.rs @@ -278,18 +278,14 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, fn trans_struct_drop_flag<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, t: Ty<'tcx>, - v0: ValueRef, + struct_data: ValueRef, dtor_did: ast::DefId, class_did: ast::DefId, substs: &subst::Substs<'tcx>) -> Block<'blk, 'tcx> { + assert!(type_is_sized(bcx.tcx(), t), "Precondition: caller must ensure t is sized"); + let repr = adt::represent_type(bcx.ccx(), t); - let struct_data = if type_is_sized(bcx.tcx(), t) { - v0 - } else { - let llval = GEPi(bcx, v0, &[0, abi::FAT_PTR_ADDR]); - Load(bcx, llval) - }; let drop_flag = unpack_datum!(bcx, adt::trans_drop_flag_ptr(bcx, &*repr, struct_data)); let loaded = load_ty(bcx, drop_flag.val, bcx.tcx().dtor_type()); let drop_flag_llty = type_of(bcx.fcx.ccx, bcx.tcx().dtor_type()); @@ -313,9 +309,8 @@ fn trans_struct_drop_flag<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, let drop_flag_dtor_needed = ICmp(bcx, llvm::IntEQ, loaded, init_val, DebugLoc::None); with_cond(bcx, drop_flag_dtor_needed, |cx| { - trans_struct_drop(cx, t, v0, dtor_did, class_did, substs) + trans_struct_drop(cx, t, struct_data, dtor_did, class_did, substs) }) - } pub fn get_res_dtor<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, diff --git a/src/librustc_trans/trans/machine.rs b/src/librustc_trans/trans/machine.rs index ce37d38dc89..39bc547a1a7 100644 --- a/src/librustc_trans/trans/machine.rs +++ b/src/librustc_trans/trans/machine.rs @@ -101,7 +101,8 @@ pub fn llalign_of_min(cx: &CrateContext, ty: Type) -> llalign { pub fn llelement_offset(cx: &CrateContext, struct_ty: Type, element: usize) -> u64 { unsafe { - return llvm::LLVMOffsetOfElement(cx.td().lltd, struct_ty.to_ref(), + return llvm::LLVMOffsetOfElement(cx.td().lltd, + struct_ty.to_ref(), element as u32); } } diff --git a/src/librustc_typeck/check/cast.rs b/src/librustc_typeck/check/cast.rs index f0495436bc1..bfd159720c2 100644 --- a/src/librustc_typeck/check/cast.rs +++ b/src/librustc_typeck/check/cast.rs @@ -99,6 +99,7 @@ pub fn check_cast<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, cast: &CastCheck<'tcx>) { let t_1_is_bare_fn = ty::type_is_bare_fn(t_1); let t_1_is_float = ty::type_is_floating_point(t_1); let t_1_is_c_enum = ty::type_is_c_like_enum(fcx.tcx(), t_1); + let t1_is_fat_ptr = fcx.type_is_fat_ptr(t_1, span); // casts to scalars other than `char` and `bare fn` are trivial let t_1_is_trivial = t_1_is_scalar && !t_1_is_char && !t_1_is_bare_fn; @@ -170,18 +171,16 @@ pub fn check_cast<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, cast: &CastCheck<'tcx>) { demand::coerce(fcx, e.span, t_1, &e); } } - } else if fcx.type_is_fat_ptr(t_e, span) != fcx.type_is_fat_ptr(t_1, span) { + } else if t1_is_fat_ptr { + // FIXME This should be allowed where the lefthandside is also a fat + // pointer and is the same kind of fat pointer, i.e., array to array, + // trait object to trait object. fcx.type_error_message(span, |actual| { - format!("illegal cast; cast to or from fat pointer: `{}` as `{}` \ - involving incompatible type.", - actual, fcx.infcx().ty_to_string(t_1)) + format!("cast to fat pointer: `{}` as `{}`", + actual, + fcx.infcx().ty_to_string(t_1)) }, t_e, None); } else if !(t_e_is_scalar && t_1_is_trivial) { - /* - If more type combinations should be supported than are - supported here, then file an enhancement issue and - record the issue number in this comment. - */ fcx.type_error_message(span, |actual| { format!("non-scalar cast: `{}` as `{}`", actual, diff --git a/src/test/compile-fail/fat-ptr-cast.rs b/src/test/compile-fail/fat-ptr-cast.rs index 381dff36b7d..415785a1174 100644 --- a/src/test/compile-fail/fat-ptr-cast.rs +++ b/src/test/compile-fail/fat-ptr-cast.rs @@ -15,17 +15,13 @@ pub trait Trait {} fn main() { let a: &[i32] = &[1, 2, 3]; let b: Box<[i32]> = Box::new([1, 2, 3]); - let p = a as *const [i32]; - let q = a.as_ptr(); a as usize; //~ ERROR illegal cast b as usize; //~ ERROR illegal cast - p as usize; //~ ERROR illegal cast - // #22955 - q as *const [i32]; //~ ERROR illegal cast + let a: usize = 42; + a as *const [i32]; //~ ERROR cast to fat pointer: `usize` as `*const [i32]` - // #21397 - let t: *mut (Trait + 'static) = 0 as *mut _; //~ ERROR illegal cast - let mut fail: *const str = 0 as *const str; //~ ERROR illegal cast + let a: *const u8 = &42; + a as *const [u8]; //~ ERROR cast to fat pointer: `*const u8` as `*const [u8]` } diff --git a/src/test/compile-fail/issue-22289.rs b/src/test/compile-fail/issue-22289.rs index 5adea183bc9..1fdc8735714 100644 --- a/src/test/compile-fail/issue-22289.rs +++ b/src/test/compile-fail/issue-22289.rs @@ -9,5 +9,5 @@ // except according to those terms. fn main() { - 0 as &std::any::Any; //~ ERROR illegal cast + 0 as &std::any::Any; //~ ERROR cast to fat pointer: `i32` as `&core::any::Any` } diff --git a/src/test/run-pass/fat-ptr-cast.rs b/src/test/run-pass/fat-ptr-cast.rs new file mode 100644 index 00000000000..b7513da99c8 --- /dev/null +++ b/src/test/run-pass/fat-ptr-cast.rs @@ -0,0 +1,49 @@ +// Copyright 2015 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. + +#![feature(core)] + +use std::mem; +use std::raw; + +trait Foo { + fn foo(&self) {} +} + +struct Bar; + +impl Foo for Bar {} + +fn main() { + // Test we can turn a fat pointer to array back into a thin pointer. + let a: *const [i32] = &[1, 2, 3]; + let b = a as *const [i32; 2]; + unsafe { + assert!(*b == [1, 2]); + } + + // Test conversion to an address (usize). + let a: *const [i32; 3] = &[1, 2, 3]; + let b: *const [i32] = a; + assert!(a as usize == b as usize); + + // And conversion to a void pointer/address for trait objects too. + let a: *mut Foo = &mut Bar; + let b = a as *mut (); + let c = a as usize; + + let d = unsafe { + let r: raw::TraitObject = mem::transmute(a); + r.data + }; + + assert!(b == d); + assert!(c == d as usize); +}