Auto merge of #39586 - arielb1:packed-values, r=eddyb

emit "align 1" metadata on loads/stores of packed structs

According to the LLVM reference:
> A value of 0 or an omitted align argument means that the operation has
the ABI alignment for the target.

So loads/stores of fields of packed structs need to have their align set
to 1. Implement that by tracking the alignment of `LvalueRef`s.

Fixes #39376.

r? @eddyb

Author: bors

src/librustc_trans/adt.rs

@@ -56,6 +56,8 @@ use monomorphize;
 use type_::Type;
 use type_of;
 
+use mir::lvalue::Alignment;
+
 /// Given an enum, struct, closure, or tuple, extracts fields.
 /// Treats closures as a struct with one variant.
 /// `empty_if_no_variants` is a switch to deal with empty enums.
@@ -279,6 +281,7 @@ pub fn trans_get_discr<'a, 'tcx>(
     bcx: &Builder<'a, 'tcx>,
     t: Ty<'tcx>,
     scrutinee: ValueRef,
+    alignment: Alignment,
     cast_to: Option<Type>,
     range_assert: bool
 ) -> ValueRef {
@@ -292,11 +295,12 @@ pub fn trans_get_discr<'a, 'tcx>(
 
     let val = match *l {
         layout::CEnum { discr, min, max, .. } => {
-            load_discr(bcx, discr, scrutinee, min, max, range_assert)
+            load_discr(bcx, discr, scrutinee, alignment, min, max, range_assert)
         }
         layout::General { discr, .. } => {
             let ptr = bcx.struct_gep(scrutinee, 0);
-            load_discr(bcx, discr, ptr, 0, def.variants.len() as u64 - 1,
+            load_discr(bcx, discr, ptr, alignment,
+                       0, def.variants.len() as u64 - 1,
                        range_assert)
         }
         layout::Univariant { .. } | layout::UntaggedUnion { .. } => C_u8(bcx.ccx, 0),
@@ -305,10 +309,10 @@ pub fn trans_get_discr<'a, 'tcx>(
             let llptrty = type_of::sizing_type_of(bcx.ccx,
                 monomorphize::field_ty(bcx.tcx(), substs,
                 &def.variants[nndiscr as usize].fields[0]));
-            bcx.icmp(cmp, bcx.load(scrutinee), C_null(llptrty))
+            bcx.icmp(cmp, bcx.load(scrutinee, alignment.to_align()), C_null(llptrty))
         }
         layout::StructWrappedNullablePointer { nndiscr, ref discrfield, .. } => {
-            struct_wrapped_nullable_bitdiscr(bcx, nndiscr, discrfield, scrutinee)
+            struct_wrapped_nullable_bitdiscr(bcx, nndiscr, discrfield, scrutinee, alignment)
         },
         _ => bug!("{} is not an enum", t)
     };
@@ -322,17 +326,19 @@ fn struct_wrapped_nullable_bitdiscr(
     bcx: &Builder,
     nndiscr: u64,
     discrfield: &layout::FieldPath,
-    scrutinee: ValueRef
+    scrutinee: ValueRef,
+    alignment: Alignment,
 ) -> ValueRef {
     let llptrptr = bcx.gepi(scrutinee,
         &discrfield.iter().map(|f| *f as usize).collect::<Vec<_>>()[..]);
-    let llptr = bcx.load(llptrptr);
+    let llptr = bcx.load(llptrptr, alignment.to_align());
     let cmp = if nndiscr == 0 { IntEQ } else { IntNE };
     bcx.icmp(cmp, llptr, C_null(val_ty(llptr)))
 }
 
 /// Helper for cases where the discriminant is simply loaded.
-fn load_discr(bcx: &Builder, ity: layout::Integer, ptr: ValueRef, min: u64, max: u64,
+fn load_discr(bcx: &Builder, ity: layout::Integer, ptr: ValueRef,
+              alignment: Alignment, min: u64, max: u64,
               range_assert: bool)
     -> ValueRef {
     let llty = Type::from_integer(bcx.ccx, ity);
@@ -348,11 +354,12 @@ fn load_discr(bcx: &Builder, ity: layout::Integer, ptr: ValueRef, min: u64, max:
         // rejected by the LLVM verifier (it would mean either an
         // empty set, which is impossible, or the entire range of the
         // type, which is pointless).
-        bcx.load(ptr)
+        bcx.load(ptr, alignment.to_align())
     } else {
         // llvm::ConstantRange can deal with ranges that wrap around,
         // so an overflow on (max + 1) is fine.
-        bcx.load_range_assert(ptr, min, max.wrapping_add(1), /* signed: */ True)
+        bcx.load_range_assert(ptr, min, max.wrapping_add(1), /* signed: */ True,
+                              alignment.to_align())
     }
 }
 

src/librustc_trans/asm.rs

@@ -20,6 +20,8 @@ use builder::Builder;
 use rustc::hir;
 use rustc::ty::Ty;
 
+use mir::lvalue::Alignment;
+
 use std::ffi::CString;
 use syntax::ast::AsmDialect;
 use libc::{c_uint, c_char};
@@ -38,7 +40,7 @@ pub fn trans_inline_asm<'a, 'tcx>(
     let mut indirect_outputs = vec![];
     for (i, (out, &(val, ty))) in ia.outputs.iter().zip(&outputs).enumerate() {
         let val = if out.is_rw || out.is_indirect {
-            Some(base::load_ty(bcx, val, ty))
+            Some(base::load_ty(bcx, val, Alignment::Packed, ty))
         } else {
             None
         };

src/librustc_trans/base.rs

@@ -90,6 +90,8 @@ use rustc::hir;
 use rustc::ty::layout::{self, Layout};
 use syntax::ast;
 
+use mir::lvalue::Alignment;
+
 pub struct StatRecorder<'a, 'tcx: 'a> {
     ccx: &'a CrateContext<'a, 'tcx>,
     name: Option<String>,
@@ -250,25 +252,25 @@ pub fn unsize_thin_ptr<'a, 'tcx>(
 /// Coerce `src`, which is a reference to a value of type `src_ty`,
 /// to a value of type `dst_ty` and store the result in `dst`
 pub fn coerce_unsized_into<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
-                                     src: ValueRef,
-                                     src_ty: Ty<'tcx>,
-                                     dst: ValueRef,
-                                     dst_ty: Ty<'tcx>) {
+                                     src: &LvalueRef<'tcx>,
+                                     dst: &LvalueRef<'tcx>) {
+    let src_ty = src.ty.to_ty(bcx.tcx());
+    let dst_ty = dst.ty.to_ty(bcx.tcx());
     let coerce_ptr = || {
         let (base, info) = if common::type_is_fat_ptr(bcx.ccx, src_ty) {
             // fat-ptr to fat-ptr unsize preserves the vtable
             // i.e. &'a fmt::Debug+Send => &'a fmt::Debug
             // So we need to pointercast the base to ensure
             // the types match up.
-            let (base, info) = load_fat_ptr(bcx, src, src_ty);
+            let (base, info) = load_fat_ptr(bcx, src.llval, src.alignment, src_ty);
             let llcast_ty = type_of::fat_ptr_base_ty(bcx.ccx, dst_ty);
             let base = bcx.pointercast(base, llcast_ty);
             (base, info)
         } else {
-            let base = load_ty(bcx, src, src_ty);
+            let base = load_ty(bcx, src.llval, src.alignment, src_ty);
             unsize_thin_ptr(bcx, base, src_ty, dst_ty)
         };
-        store_fat_ptr(bcx, base, info, dst, dst_ty);
+        store_fat_ptr(bcx, base, info, dst.llval, dst.alignment, dst_ty);
     };
     match (&src_ty.sty, &dst_ty.sty) {
         (&ty::TyRef(..), &ty::TyRef(..)) |
@@ -290,21 +292,22 @@ pub fn coerce_unsized_into<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
                 monomorphize::field_ty(bcx.tcx(), substs_b, f)
             });
 
-            let src = LvalueRef::new_sized_ty(src, src_ty);
-            let dst = LvalueRef::new_sized_ty(dst, dst_ty);
-
             let iter = src_fields.zip(dst_fields).enumerate();
             for (i, (src_fty, dst_fty)) in iter {
                 if type_is_zero_size(bcx.ccx, dst_fty) {
                     continue;
                 }
 
-                let src_f = src.trans_field_ptr(bcx, i);
-                let dst_f = dst.trans_field_ptr(bcx, i);
+                let (src_f, src_f_align) = src.trans_field_ptr(bcx, i);
+                let (dst_f, dst_f_align) = dst.trans_field_ptr(bcx, i);
                 if src_fty == dst_fty {
                     memcpy_ty(bcx, dst_f, src_f, src_fty, None);
                 } else {
-                    coerce_unsized_into(bcx, src_f, src_fty, dst_f, dst_fty);
+                    coerce_unsized_into(
+                        bcx,
+                        &LvalueRef::new_sized_ty(src_f, src_fty, src_f_align),
+                        &LvalueRef::new_sized_ty(dst_f, dst_fty, dst_f_align)
+                    );
                 }
             }
         }
@@ -399,7 +402,8 @@ pub fn call_assume<'a, 'tcx>(b: &Builder<'a, 'tcx>, val: ValueRef) {
 /// Helper for loading values from memory. Does the necessary conversion if the in-memory type
 /// differs from the type used for SSA values. Also handles various special cases where the type
 /// gives us better information about what we are loading.
-pub fn load_ty<'a, 'tcx>(b: &Builder<'a, 'tcx>, ptr: ValueRef, t: Ty<'tcx>) -> ValueRef {
+pub fn load_ty<'a, 'tcx>(b: &Builder<'a, 'tcx>, ptr: ValueRef,
+                         alignment: Alignment, t: Ty<'tcx>) -> ValueRef {
     let ccx = b.ccx;
     if type_is_zero_size(ccx, t) {
         return C_undef(type_of::type_of(ccx, t));
@@ -419,54 +423,57 @@ pub fn load_ty<'a, 'tcx>(b: &Builder<'a, 'tcx>, ptr: ValueRef, t: Ty<'tcx>) -> V
     }
 
     if t.is_bool() {
-        b.trunc(b.load_range_assert(ptr, 0, 2, llvm::False), Type::i1(ccx))
+        b.trunc(b.load_range_assert(ptr, 0, 2, llvm::False, alignment.to_align()),
+                Type::i1(ccx))
     } else if t.is_char() {
         // a char is a Unicode codepoint, and so takes values from 0
         // to 0x10FFFF inclusive only.
-        b.load_range_assert(ptr, 0, 0x10FFFF + 1, llvm::False)
+        b.load_range_assert(ptr, 0, 0x10FFFF + 1, llvm::False, alignment.to_align())
     } else if (t.is_region_ptr() || t.is_box()) && !common::type_is_fat_ptr(ccx, t) {
-        b.load_nonnull(ptr)
+        b.load_nonnull(ptr, alignment.to_align())
     } else {
-        b.load(ptr)
+        b.load(ptr, alignment.to_align())
     }
 }
 
 /// Helper for storing values in memory. Does the necessary conversion if the in-memory type
 /// differs from the type used for SSA values.
-pub fn store_ty<'a, 'tcx>(cx: &Builder<'a, 'tcx>, v: ValueRef, dst: ValueRef, t: Ty<'tcx>) {
+pub fn store_ty<'a, 'tcx>(cx: &Builder<'a, 'tcx>, v: ValueRef, dst: ValueRef,
+                          dst_align: Alignment, t: Ty<'tcx>) {
     debug!("store_ty: {:?} : {:?} <- {:?}", Value(dst), t, Value(v));
 
     if common::type_is_fat_ptr(cx.ccx, t) {
         let lladdr = cx.extract_value(v, abi::FAT_PTR_ADDR);
         let llextra = cx.extract_value(v, abi::FAT_PTR_EXTRA);
-        store_fat_ptr(cx, lladdr, llextra, dst, t);
+        store_fat_ptr(cx, lladdr, llextra, dst, dst_align, t);
     } else {
-        cx.store(from_immediate(cx, v), dst, None);
+        cx.store(from_immediate(cx, v), dst, dst_align.to_align());
     }
 }
 
 pub fn store_fat_ptr<'a, 'tcx>(cx: &Builder<'a, 'tcx>,
                                data: ValueRef,
                                extra: ValueRef,
                                dst: ValueRef,
+                               dst_align: Alignment,
                                _ty: Ty<'tcx>) {
     // FIXME: emit metadata
-    cx.store(data, get_dataptr(cx, dst), None);
-    cx.store(extra, get_meta(cx, dst), None);
+    cx.store(data, get_dataptr(cx, dst), dst_align.to_align());
+    cx.store(extra, get_meta(cx, dst), dst_align.to_align());
 }
 
 pub fn load_fat_ptr<'a, 'tcx>(
-    b: &Builder<'a, 'tcx>, src: ValueRef, t: Ty<'tcx>
+    b: &Builder<'a, 'tcx>, src: ValueRef, alignment: Alignment, t: Ty<'tcx>
 ) -> (ValueRef, ValueRef) {
     let ptr = get_dataptr(b, src);
     let ptr = if t.is_region_ptr() || t.is_box() {
-        b.load_nonnull(ptr)
+        b.load_nonnull(ptr, alignment.to_align())
     } else {
-        b.load(ptr)
+        b.load(ptr, alignment.to_align())
     };
 
     // FIXME: emit metadata on `meta`.
-    let meta = b.load(get_meta(b, src));
+    let meta = b.load(get_meta(b, src), alignment.to_align());
 
     (ptr, meta)
 }
@@ -633,7 +640,7 @@ pub fn trans_ctor_shim<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
             bcx.alloca(fn_ty.ret.memory_ty(ccx), "sret_slot")
         };
         // Can return unsized value
-        let mut dest_val = LvalueRef::new_sized_ty(dest, sig.output());
+        let mut dest_val = LvalueRef::new_sized_ty(dest, sig.output(), Alignment::AbiAligned);
         dest_val.ty = LvalueTy::Downcast {
             adt_def: sig.output().ty_adt_def().unwrap(),
             substs: substs,
@@ -642,7 +649,7 @@ pub fn trans_ctor_shim<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
         let mut llarg_idx = fn_ty.ret.is_indirect() as usize;
         let mut arg_idx = 0;
         for (i, arg_ty) in sig.inputs().iter().enumerate() {
-            let lldestptr = dest_val.trans_field_ptr(&bcx, i);
+            let (lldestptr, _) = dest_val.trans_field_ptr(&bcx, i);
             let arg = &fn_ty.args[arg_idx];
             arg_idx += 1;
             if common::type_is_fat_ptr(bcx.ccx, arg_ty) {
@@ -662,14 +669,12 @@ pub fn trans_ctor_shim<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
         }
 
         if let Some(cast_ty) = fn_ty.ret.cast {
-            let load = bcx.load(bcx.pointercast(dest, cast_ty.ptr_to()));
-            let llalign = llalign_of_min(ccx, fn_ty.ret.ty);
-            unsafe {
-                llvm::LLVMSetAlignment(load, llalign);
-            }
-            bcx.ret(load)
+            bcx.ret(bcx.load(
+                bcx.pointercast(dest, cast_ty.ptr_to()),
+                Some(llalign_of_min(ccx, fn_ty.ret.ty))
+            ));
         } else {
-            bcx.ret(bcx.load(dest))
+            bcx.ret(bcx.load(dest, None))
         }
     } else {
         bcx.ret_void();

src/librustc_trans/builder.rs

@@ -19,9 +19,8 @@ use machine::llalign_of_pref;
 use type_::Type;
 use value::Value;
 use libc::{c_uint, c_char};
-use rustc::ty::{Ty, TyCtxt, TypeFoldable};
+use rustc::ty::TyCtxt;
 use rustc::session::Session;
-use type_of;
 
 use std::borrow::Cow;
 use std::ffi::CString;
@@ -486,11 +485,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         builder.dynamic_alloca(ty, name)
     }
 
-    pub fn alloca_ty(&self, ty: Ty<'tcx>, name: &str) -> ValueRef {
-        assert!(!ty.has_param_types());
-        self.alloca(type_of::type_of(self.ccx, ty), name)
-    }
-
     pub fn dynamic_alloca(&self, ty: Type, name: &str) -> ValueRef {
         self.count_insn("alloca");
         unsafe {
@@ -511,10 +505,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
     }
 
-    pub fn load(&self, ptr: ValueRef) -> ValueRef {
+    pub fn load(&self, ptr: ValueRef, align: Option<u32>) -> ValueRef {
         self.count_insn("load");
         unsafe {
-            llvm::LLVMBuildLoad(self.llbuilder, ptr, noname())
+            let load = llvm::LLVMBuildLoad(self.llbuilder, ptr, noname());
+            if let Some(align) = align {
+                llvm::LLVMSetAlignment(load, align as c_uint);
+            }
+            load
         }
     }
 
@@ -539,8 +537,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
 
     pub fn load_range_assert(&self, ptr: ValueRef, lo: u64,
-                             hi: u64, signed: llvm::Bool) -> ValueRef {
-        let value = self.load(ptr);
+                             hi: u64, signed: llvm::Bool,
+                             align: Option<u32>) -> ValueRef {
+        let value = self.load(ptr, align);
 
         unsafe {
             let t = llvm::LLVMGetElementType(llvm::LLVMTypeOf(ptr));
@@ -558,8 +557,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         value
     }
 
-    pub fn load_nonnull(&self, ptr: ValueRef) -> ValueRef {
-        let value = self.load(ptr);
+    pub fn load_nonnull(&self, ptr: ValueRef, align: Option<u32>) -> ValueRef {
+        let value = self.load(ptr, align);
         unsafe {
             llvm::LLVMSetMetadata(value, llvm::MD_nonnull as c_uint,
                                   llvm::LLVMMDNodeInContext(self.ccx.llcx(), ptr::null(), 0));