rustc: Fix a bunch of memory management bugs relating to generic interior vectors. Uncomment all tests in lib-ivec.

Author: pcwalton

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;

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;

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();
 }