From 5fb93258488c1b2083ea1042ee9674b842c29548 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 17 Jun 2011 16:49:04 -0700 Subject: [PATCH] rustc: Fix a bunch of memory management bugs relating to generic interior vectors. Uncomment all tests in lib-ivec. --- src/comp/middle/trans.rs | 101 +++++++++++++++++++++++++++++++--- src/rt/rust_builtin.cpp | 2 +- src/test/run-pass/lib-ivec.rs | 19 +++++-- 3 files changed, 110 insertions(+), 12 deletions(-) diff --git a/src/comp/middle/trans.rs b/src/comp/middle/trans.rs index 57880dd2815..da3ff2ba412 100644 --- a/src/comp/middle/trans.rs +++ b/src/comp/middle/trans.rs @@ -1327,7 +1327,8 @@ fn dynamic_size_of(&@block_ctxt cx, ty::t t) -> result { auto bcx = rslt.bcx; auto llunitszptr = rslt.val; auto llunitsz = bcx.build.Load(llunitszptr); - auto llsz = bcx.build.Add(llsize_of(T_opaque_ivec()), llunitsz); + auto llsz = bcx.build.Add(llsize_of(T_opaque_ivec()), + bcx.build.Mul(llunitsz, C_uint(abi::ivec_default_length))); ret res(bcx, llsz); } } @@ -2969,12 +2970,23 @@ fn memmove_ty(&@block_ctxt cx, ValueRef dst, ValueRef src, &ty::t t) -> } else { ret res(cx, cx.build.Store(cx.build.Load(src), dst)); } } +// Duplicates the heap-owned memory owned by a value of the given type. +fn duplicate_heap_parts(&@block_ctxt cx, ValueRef vptr, ty::t typ) -> result { + alt (ty::struct(cx.fcx.lcx.ccx.tcx, typ)) { + case (ty::ty_ivec(?tm)) { + ret ivec::duplicate_heap_part(cx, vptr, tm.ty); + } + case (ty::ty_str) { + ret ivec::duplicate_heap_part(cx, vptr, + ty::mk_mach(cx.fcx.lcx.ccx.tcx, common::ty_u8)); + } + } +} + tag copy_action { INIT; DROP_EXISTING; } -// FIXME: This should copy the contents of the heap part for ivecs. fn copy_val(&@block_ctxt cx, copy_action action, ValueRef dst, ValueRef src, &ty::t t) -> result { - if (ty::type_is_scalar(cx.fcx.lcx.ccx.tcx, t) || ty::type_is_native(cx.fcx.lcx.ccx.tcx, t)) { ret res(cx, cx.build.Store(src, dst)); @@ -2991,7 +3003,11 @@ fn copy_val(&@block_ctxt cx, copy_action action, ValueRef dst, ValueRef src, ty::type_has_dynamic_size(cx.fcx.lcx.ccx.tcx, t)) { auto r = take_ty(cx, src, t); if (action == DROP_EXISTING) { r = drop_ty(r.bcx, dst, t); } - ret memmove_ty(r.bcx, dst, src, t); + r = memmove_ty(r.bcx, dst, src, t); + if (ty::type_owns_heap_mem(cx.fcx.lcx.ccx.tcx, t)) { + r = duplicate_heap_parts(cx, dst, t); + } + ret r; } cx.fcx.lcx.ccx.sess.bug("unexpected type in trans::copy_val: " + ty_to_str(cx.fcx.lcx.ccx.tcx, t)); @@ -3500,9 +3516,13 @@ mod ivec { copy_loop_header_cx.build.CondBr(not_yet_at_end, copy_loop_body_cx.llbb, next_cx.llbb); + auto copy_src_ptr = copy_loop_body_cx.build.Load(src_ptr); - rslt = - copy_val(copy_loop_body_cx, INIT, copy_dest_ptr, copy_src_ptr, t); + auto copy_src = load_if_immediate(copy_loop_body_cx, copy_src_ptr, + unit_ty); + + rslt = copy_val(copy_loop_body_cx, INIT, copy_dest_ptr, copy_src, + unit_ty); auto post_copy_cx = rslt.bcx; // Increment both pointers. @@ -3575,6 +3595,9 @@ mod ivec { auto unit_sz = ares.llunitsz; auto llalen = ares.llalen; + find_scope_cx(bcx).cleanups += + [clean(bind drop_ty(_, llvecptr, vec_ty))]; + auto llunitty = type_of_or_i8(bcx, unit_ty); auto llheappartty = T_ivec_heap_part(llunitty); auto lhs_len_and_data = get_len_and_data(bcx, lhs, unit_ty); @@ -3740,6 +3763,66 @@ mod ivec { ret res(next_cx, llvecptr); } + + // NB: This does *not* adjust reference counts. The caller must have done + // this via take_ty() beforehand. + fn duplicate_heap_part(&@block_ctxt cx, ValueRef orig_vptr, + ty::t unit_ty) -> result { + // Cast to an opaque interior vector if we can't trust the pointer + // type. + auto vptr; + if (ty::type_has_dynamic_size(cx.fcx.lcx.ccx.tcx, unit_ty)) { + vptr = cx.build.PointerCast(orig_vptr, T_ptr(T_opaque_ivec())); + } else { + vptr = orig_vptr; + } + + auto llunitty = type_of_or_i8(cx, unit_ty); + auto llheappartty = T_ivec_heap_part(llunitty); + + // Check to see if the vector is heapified. + auto stack_len_ptr = cx.build.InBoundsGEP(vptr, [C_int(0), + C_uint(abi::ivec_elt_len)]); + auto stack_len = cx.build.Load(stack_len_ptr); + auto stack_len_is_zero = cx.build.ICmp(lib::llvm::LLVMIntEQ, + stack_len, C_int(0)); + auto maybe_on_heap_cx = new_sub_block_ctxt(cx, "maybe_on_heap"); + auto next_cx = new_sub_block_ctxt(cx, "next"); + cx.build.CondBr(stack_len_is_zero, maybe_on_heap_cx.llbb, + next_cx.llbb); + + auto stub_ptr = maybe_on_heap_cx.build.PointerCast(vptr, + T_ptr(T_ivec_heap(llunitty))); + auto heap_ptr_ptr = maybe_on_heap_cx.build.InBoundsGEP(stub_ptr, + [C_int(0), C_uint(abi::ivec_heap_stub_elt_ptr)]); + auto heap_ptr = maybe_on_heap_cx.build.Load(heap_ptr_ptr); + auto heap_ptr_is_nonnull = maybe_on_heap_cx.build.ICmp( + lib::llvm::LLVMIntNE, heap_ptr, C_null(T_ptr(llheappartty))); + auto on_heap_cx = new_sub_block_ctxt(cx, "on_heap"); + maybe_on_heap_cx.build.CondBr(heap_ptr_is_nonnull, on_heap_cx.llbb, + next_cx.llbb); + + // Ok, the vector is on the heap. Copy the heap part. + auto alen_ptr = on_heap_cx.build.InBoundsGEP(stub_ptr, + [C_int(0), C_uint(abi::ivec_heap_stub_elt_alen)]); + auto alen = on_heap_cx.build.Load(alen_ptr); + + auto heap_part_sz = on_heap_cx.build.Add(alen, + llsize_of(T_opaque_ivec_heap_part())); + auto rslt = trans_raw_malloc(on_heap_cx, T_ptr(llheappartty), + heap_part_sz); + on_heap_cx = rslt.bcx; + auto new_heap_ptr = rslt.val; + + rslt = call_memmove(on_heap_cx, new_heap_ptr, heap_ptr, heap_part_sz, + C_int(4)); // FIXME: align + on_heap_cx = rslt.bcx; + + on_heap_cx.build.Store(new_heap_ptr, heap_ptr_ptr); + on_heap_cx.build.Br(next_cx.llbb); + + ret res(next_cx, C_nil()); + } } fn trans_vec_add(&@block_ctxt cx, &ty::t t, ValueRef lhs, ValueRef rhs) -> @@ -5431,6 +5514,8 @@ fn trans_ivec(@block_ctxt bcx, &vec[@ast::expr] args, &ast::ann ann) -> auto unit_sz = ares.llunitsz; auto llalen = ares.llalen; + find_scope_cx(bcx).cleanups += [clean(bind drop_ty(_, llvecptr, typ))]; + auto lllen = bcx.build.Mul(C_uint(vec::len(args)), unit_sz); // Allocate the vector pieces and store length and allocated length. @@ -5459,15 +5544,17 @@ fn trans_ivec(@block_ctxt bcx, &vec[@ast::expr] args, &ast::ann ann) -> auto llstubty = T_ivec_heap(llunitty); auto llstubptr = bcx.build.PointerCast(llvecptr, T_ptr(llstubty)); bcx.build.Store(C_int(0), bcx.build.InBoundsGEP(llstubptr, stub_z)); - bcx.build.Store(lllen, bcx.build.InBoundsGEP(llstubptr, stub_a)); auto llheapty = T_ivec_heap_part(llunitty); if (vec::len(args) == 0u) { // Null heap pointer indicates a zero-length vector. + bcx.build.Store(llalen, bcx.build.InBoundsGEP(llstubptr, stub_a)); bcx.build.Store(C_null(T_ptr(llheapty)), bcx.build.InBoundsGEP(llstubptr, stub_p)); llfirsteltptr = C_null(T_ptr(llunitty)); } else { + bcx.build.Store(lllen, bcx.build.InBoundsGEP(llstubptr, stub_a)); + auto llheapsz = bcx.build.Add(llsize_of(llheapty), lllen); auto rslt = trans_raw_malloc(bcx, T_ptr(llheapty), llheapsz); bcx = rslt.bcx; diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp index afa022fe533..ca8f24a4227 100644 --- a/src/rt/rust_builtin.cpp +++ b/src/rt/rust_builtin.cpp @@ -682,7 +682,7 @@ ivec_copy_from_buf(rust_task *task, type_desc *ty, rust_ivec *v, void *ptr, ivec_reserve(task, ty, v, count); size_t new_size = count * ty->size; - if (v->fill) { + if (v->fill || !v->payload.ptr) { // On stack. memmove(v->payload.data, ptr, new_size); v->fill = new_size; diff --git a/src/test/run-pass/lib-ivec.rs b/src/test/run-pass/lib-ivec.rs index f7e202a64d4..43031f7f052 100644 --- a/src/test/run-pass/lib-ivec.rs +++ b/src/test/run-pass/lib-ivec.rs @@ -36,16 +36,27 @@ fn test_unsafe_ptrs() { fn test_init_fn() { fn square(uint n) -> uint { ret n * n; } + + // Test on-stack init-fn. auto v = ivec::init_fn(square, 3u); assert (ivec::len(v) == 3u); - assert (v.(0) == 1u); - assert (v.(1) == 4u); - assert (v.(2) == 9u); + assert (v.(0) == 0u); + assert (v.(1) == 1u); + assert (v.(2) == 4u); + + // Test on-heap init-fn. + v = ivec::init_fn(square, 5u); + assert (ivec::len(v) == 5u); + assert (v.(0) == 0u); + assert (v.(1) == 1u); + assert (v.(2) == 4u); + assert (v.(3) == 9u); + assert (v.(4) == 16u); } fn main() { test_reserve_and_on_heap(); test_unsafe_ptrs(); - //test_init_fn(); + test_init_fn(); }