Skip to content

Commit

Permalink
Fix unsized structs with destructors
Browse files Browse the repository at this point in the history
The presence of the drop flag caused the offset calculation to be
incorrect, leading to the pointer being incorrect. This has been fixed
by calculating the offset based on the field index (and not assuming
that the field is always the last one).

However, I've also stopped the drop flag from being added to the end of
unsized structs to begin with. Since it's not actually accessed for
unsized structs, and isn't actually where we would say it is, this made
more sense.
  • Loading branch information
Aatch committed Dec 8, 2015
1 parent a2557d4 commit d6eb063
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
22 changes: 16 additions & 6 deletions src/librustc_trans/trans/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,11 @@ fn represent_type_uncached<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
monomorphize::field_ty(cx.tcx(), substs, field)
}).collect::<Vec<_>>();
let packed = cx.tcx().lookup_packed(def.did);
let dtor = def.dtor_kind().has_drop_flag();
// FIXME(16758) don't add a drop flag to unsized structs, as it
// won't actually be in the location we say it is because it'll be after
// the unsized field. Several other pieces of code assume that the unsized
// field is definitely the last one.
let dtor = def.dtor_kind().has_drop_flag() && type_is_sized(cx.tcx(), t);
if dtor {
ftys.push(cx.tcx().dtor_type());
}
Expand Down Expand Up @@ -1105,8 +1109,8 @@ pub fn trans_field_ptr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, r: &Repr<'tcx>,

pub fn struct_field_ptr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, st: &Struct<'tcx>, val: MaybeSizedValue,
ix: usize, needs_cast: bool) -> ValueRef {
let ccx = bcx.ccx();
let ptr_val = if needs_cast {
let ccx = bcx.ccx();
let fields = st.fields.iter().map(|&ty| {
type_of::in_memory_type_of(ccx, ty)
}).collect::<Vec<_>>();
Expand Down Expand Up @@ -1147,7 +1151,7 @@ pub fn struct_field_ptr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, st: &Struct<'tcx>, v
// We need to get the pointer manually now.
// We do this by casting to a *i8, then offsetting it by the appropriate amount.
// We do this instead of, say, simply adjusting the pointer from the result of a GEP
// because the the field may have an arbitrary alignment in the LLVM representation
// because the field may have an arbitrary alignment in the LLVM representation
// anyway.
//
// To demonstrate:
Expand All @@ -1161,9 +1165,15 @@ pub fn struct_field_ptr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, st: &Struct<'tcx>, v

let meta = val.meta;

// st.size is the size of the sized portion of the struct. So the position
// exactly after it is the offset for unaligned data.
let unaligned_offset = C_uint(bcx.ccx(), st.size);
// Calculate the unaligned offset of the the unsized field.
let mut offset = 0;
for &ty in &st.fields[0..ix] {
let llty = type_of::sizing_type_of(ccx, ty);
let type_align = type_of::align_of(ccx, ty);
offset = roundup(offset, type_align);
offset += machine::llsize_of_alloc(ccx, llty);
}
let unaligned_offset = C_uint(bcx.ccx(), offset);

// Get the alignment of the field
let (_, align) = glue::size_and_align_of_dst(bcx, fty, meta);
Expand Down
18 changes: 17 additions & 1 deletion src/test/run-pass/dst-field-align.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// except according to those terms.

struct Foo<T: ?Sized> {
a: u8,
a: u16,
b: T
}

Expand All @@ -31,6 +31,11 @@ struct Packed<T: ?Sized> {
b: T
}

struct HasDrop<T: ?Sized> {
ptr: Box<usize>,
data: T
}

fn main() {
// Test that zero-offset works properly
let b : Baz<usize> = Baz { a: 7 };
Expand Down Expand Up @@ -68,4 +73,15 @@ fn main() {
let f : &Foo<Bar> = &f;
let &Foo { a: _, b: ref bar } = f;
assert_eq!(bar.get(), 11);

// Make sure that drop flags don't screw things up

let d : HasDrop<Baz<[i32; 4]>> = HasDrop {
ptr: Box::new(0),
data: Baz { a: [1,2,3,4] }
};
assert_eq!([1,2,3,4], d.data.a);

let d : &HasDrop<Baz<[i32]>> = &d;
assert_eq!(&[1,2,3,4], &d.data.a);
}

0 comments on commit d6eb063

Please sign in to comment.