Issue 30530: initialize allocas for Datum::to_lvalue_datum_in_scope.

In particular, bring back the `zero` flag for `lvalue_scratch_datum`,
which controls whether the alloca's created immediately at function
start are uninitialized at that point or have their embedded
drop-flags initialized to "dropped".

Then made `to_lvalue_datum_in_scope` pass "dropped" as `zero` flag.

Author: pnkfelix

src/librustc_trans/trans/adt.rs

@@ -55,6 +55,7 @@ use syntax::ast;
 use syntax::attr;
 use syntax::attr::IntType;
 use trans::_match;
+use trans::base::InitAlloca;
 use trans::build::*;
 use trans::cleanup;
 use trans::cleanup::CleanupMethods;
@@ -1279,6 +1280,7 @@ pub fn trans_drop_flag_ptr<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
             let custom_cleanup_scope = fcx.push_custom_cleanup_scope();
             let scratch = unpack_datum!(bcx, datum::lvalue_scratch_datum(
                 bcx, tcx.dtor_type(), "drop_flag",
+                InitAlloca::Uninit("drop flag itself has no dtor"),
                 cleanup::CustomScope(custom_cleanup_scope), (), |_, bcx, _| bcx
             ));
             bcx = fold_variants(bcx, r, val, |variant_cx, st, value| {

src/librustc_trans/trans/base.rs

@@ -1285,12 +1285,62 @@ fn memfill<'a, 'tcx>(b: &Builder<'a, 'tcx>, llptr: ValueRef, ty: Ty<'tcx>, byte:
            None);
 }
 
-pub fn alloc_ty<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, t: Ty<'tcx>, name: &str) -> ValueRef {
+/// In general, when we create an scratch value in an alloca, the
+/// creator may not know if the block (that initializes the scratch
+/// with the desired value) actually dominates the cleanup associated
+/// with the scratch value.
+///
+/// To deal with this, when we do an alloca (at the *start* of whole
+/// function body), we optionally can also set the associated
+/// dropped-flag state of the alloca to "dropped."
+#[derive(Copy, Clone, Debug)]
+pub enum InitAlloca {
+    /// Indicates that the state should have its associated drop flag
+    /// set to "dropped" at the point of allocation.
+    Dropped,
+    /// Indicates the value of the associated drop flag is irrelevant.
+    /// The embedded string literal is a programmer provided argument
+    /// for why. This is a safeguard forcing compiler devs to
+    /// document; it might be a good idea to also emit this as a
+    /// comment with the alloca itself when emitting LLVM output.ll.
+    Uninit(&'static str),
+}
+
+
+pub fn alloc_ty<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
+                            t: Ty<'tcx>,
+                            name: &str) -> ValueRef {
+    // pnkfelix: I do not know why alloc_ty meets the assumptions for
+    // passing Uninit, but it was never needed (even back when we had
+    // the original boolean `zero` flag on `lvalue_scratch_datum`).
+    alloc_ty_init(bcx, t, InitAlloca::Uninit("all alloc_ty are uninit"), name)
+}
+
+pub fn alloc_ty_init<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
+                             t: Ty<'tcx>,
+                             init: InitAlloca,
+                             name: &str) -> ValueRef {
     let _icx = push_ctxt("alloc_ty");
     let ccx = bcx.ccx();
     let ty = type_of::type_of(ccx, t);
     assert!(!t.has_param_types());
-    alloca(bcx, ty, name)
+    match init {
+        InitAlloca::Dropped => alloca_dropped(bcx, t, name),
+        InitAlloca::Uninit(_) => alloca(bcx, ty, name),
+    }
+}
+
+pub fn alloca_dropped<'blk, 'tcx>(cx: Block<'blk, 'tcx>, ty: Ty<'tcx>, name: &str) -> ValueRef {
+    let _icx = push_ctxt("alloca_dropped");
+    let llty = type_of::type_of(cx.ccx(), ty);
+    if cx.unreachable.get() {
+        unsafe { return llvm::LLVMGetUndef(llty.ptr_to().to_ref()); }
+    }
+    let p = alloca(cx, llty, name);
+    let b = cx.fcx.ccx.builder();
+    b.position_before(cx.fcx.alloca_insert_pt.get().unwrap());
+    memfill(&b, p, ty, adt::DTOR_DONE);
+    p
 }
 
 pub fn alloca(cx: Block, ty: Type, name: &str) -> ValueRef {
@@ -1650,6 +1700,7 @@ pub fn create_datums_for_fn_args<'a, 'tcx>(mut bcx: Block<'a, 'tcx>,
     // This alloca should be optimized away by LLVM's mem-to-reg pass in
     // the event it's not truly needed.
     let mut idx = fcx.arg_offset() as c_uint;
+    let uninit_reason = InitAlloca::Uninit("fn_arg populate dominates dtor");
     for (i, &arg_ty) in arg_tys.iter().enumerate() {
         let arg_datum = if !has_tupled_arg || i < arg_tys.len() - 1 {
             if type_of::arg_is_indirect(bcx.ccx(), arg_ty) &&
@@ -1669,7 +1720,7 @@ pub fn create_datums_for_fn_args<'a, 'tcx>(mut bcx: Block<'a, 'tcx>,
                 let data = get_param(fcx.llfn, idx);
                 let extra = get_param(fcx.llfn, idx + 1);
                 idx += 2;
-                unpack_datum!(bcx, datum::lvalue_scratch_datum(bcx, arg_ty, "",
+                unpack_datum!(bcx, datum::lvalue_scratch_datum(bcx, arg_ty, "", uninit_reason,
                                                         arg_scope_id, (data, extra),
                                                         |(data, extra), bcx, dst| {
                     Store(bcx, data, expr::get_dataptr(bcx, dst));
@@ -1684,6 +1735,7 @@ pub fn create_datums_for_fn_args<'a, 'tcx>(mut bcx: Block<'a, 'tcx>,
                               datum::lvalue_scratch_datum(bcx,
                                                           arg_ty,
                                                           "",
+                                                          uninit_reason,
                                                           arg_scope_id,
                                                           tmp,
                                                           |tmp, bcx, dst| tmp.store_to(bcx, dst)))
@@ -1696,6 +1748,7 @@ pub fn create_datums_for_fn_args<'a, 'tcx>(mut bcx: Block<'a, 'tcx>,
                                   datum::lvalue_scratch_datum(bcx,
                                                               arg_ty,
                                                               "tupled_args",
+                                                              uninit_reason,
                                                               arg_scope_id,
                                                               (),
                                                               |(),

src/librustc_trans/trans/datum.rs

@@ -288,20 +288,29 @@ pub fn immediate_rvalue_bcx<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     return DatumBlock::new(bcx, immediate_rvalue(val, ty))
 }
 
-
 /// Allocates temporary space on the stack using alloca() and returns a by-ref Datum pointing to
 /// it. The memory will be dropped upon exit from `scope`. The callback `populate` should
 /// initialize the memory.
+///
+/// The flag `zero` indicates how the temporary space itself should be
+/// initialized at the outset of the function; the only time that
+/// `InitAlloca::Uninit` is a valid value for `zero` is when the
+/// caller can prove that either (1.) the code injected by `populate`
+/// onto `bcx` always dominates the end of `scope`, or (2.) the data
+/// being allocated has no associated destructor.
 pub fn lvalue_scratch_datum<'blk, 'tcx, A, F>(bcx: Block<'blk, 'tcx>,
                                               ty: Ty<'tcx>,
                                               name: &str,
+                                              zero: InitAlloca,
                                               scope: cleanup::ScopeId,
                                               arg: A,
                                               populate: F)
                                               -> DatumBlock<'blk, 'tcx, Lvalue> where
     F: FnOnce(A, Block<'blk, 'tcx>, ValueRef) -> Block<'blk, 'tcx>,
 {
-    let scratch = alloc_ty(bcx, ty, name);
+    // Very subtle: potentially initialize the scratch memory at point where it is alloca'ed.
+    // (See discussion at Issue 30530.)
+    let scratch = alloc_ty_init(bcx, ty, zero, name);
 
     // Subtle. Populate the scratch memory *before* scheduling cleanup.
     let bcx = populate(arg, bcx, scratch);
@@ -496,7 +505,7 @@ impl<'tcx> Datum<'tcx, Rvalue> {
 
             ByValue => {
                 lvalue_scratch_datum(
-                    bcx, self.ty, name, scope, self,
+                    bcx, self.ty, name, InitAlloca::Dropped, scope, self,
                     |this, bcx, llval| {
                         call_lifetime_start(bcx, llval);
                         let bcx = this.store_to(bcx, llval);