Emit correct alignment information for loads/store of small aggregates

Loading from and storing to small aggregates happens by casting the
aggregate pointer to an appropriately sized integer pointer to avoid
the usage of first class aggregates which would lead to less optimized
code.

But this means that, for example, a tuple of type (i16, i16) will be
loading through an i32 pointer and because we currently don't provide
alignment information LLVM assumes that the load should use the ABI
alignment for i32 which would usually be 4 byte alignment. But the
alignment requirement for the (i16, i16) tuple will usually be just 2
bytes, so we're overestimating alignment, which invokes undefined
behaviour.

Therefore we must emit appropriate alignment information for
stores/loads through such casted pointers.

Fixes #23431

diff --git a/src/librustc_trans/trans/adt.rs b/src/librustc_trans/trans/adt.rs
index f574b4ed8db90ba60dafba987b94f7056742f2b4..ffa068a2ae44b80f39001abe1370605964416ec7 100644 (file)
--- a/src/librustc_trans/trans/adt.rs
+++ b/src/librustc_trans/trans/adt.rs
@@ -880,7 +880,7 @@ pub fn trans_set_discr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, r: &Repr<'tcx>,
         CEnum(ity, min, max) => {
             assert_discr_in_range(ity, min, max, discr);
             Store(bcx, C_integral(ll_inttype(bcx.ccx(), ity), discr as u64, true),
-                  val)
+                  val);
         }
         General(ity, ref cases, dtor) => {
             if dtor_active(dtor) {
@@ -889,7 +889,7 @@ pub fn trans_set_discr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, r: &Repr<'tcx>,
                 Store(bcx, C_u8(bcx.ccx(), DTOR_NEEDED as usize), ptr);
             }
             Store(bcx, C_integral(ll_inttype(bcx.ccx(), ity), discr as u64, true),
-                  GEPi(bcx, val, &[0, 0]))
+                  GEPi(bcx, val, &[0, 0]));
         }
         Univariant(ref st, dtor) => {
             assert_eq!(discr, 0);
@@ -901,14 +901,14 @@ pub fn trans_set_discr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, r: &Repr<'tcx>,
         RawNullablePointer { nndiscr, nnty, ..} => {
             if discr != nndiscr {
                 let llptrty = type_of::sizing_type_of(bcx.ccx(), nnty);
-                Store(bcx, C_null(llptrty), val)
+                Store(bcx, C_null(llptrty), val);
             }
         }
         StructWrappedNullablePointer { nndiscr, ref discrfield, .. } => {
             if discr != nndiscr {
                 let llptrptr = GEPi(bcx, val, &discrfield[..]);
                 let llptrty = val_ty(llptrptr).element_type();
-                Store(bcx, C_null(llptrty), llptrptr)
+                Store(bcx, C_null(llptrty), llptrptr);
             }
         }
     }

diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs
index 023f9e0bda1865ea41e65bee0527ad3a5269c447..59f3ff72602616f0dfe246234a55fd94f501b385 100644 (file)
--- a/src/librustc_trans/trans/base.rs
+++ b/src/librustc_trans/trans/base.rs
@@ -765,9 +765,14 @@ pub fn load_ty<'blk, 'tcx>(cx: Block<'blk, 'tcx>,
     }
 
     let ptr = to_arg_ty_ptr(cx, ptr, t);
+    let align = type_of::align_of(cx.ccx(), t);
 
     if type_is_immediate(cx.ccx(), t) && type_of::type_of(cx.ccx(), t).is_aggregate() {
-        return Load(cx, ptr);
+        let load = Load(cx, ptr);
+        unsafe {
+            llvm::LLVMSetAlignment(load, align);
+        }
+        return load;
     }
 
     unsafe {
@@ -793,13 +798,24 @@ pub fn load_ty<'blk, 'tcx>(cx: Block<'blk, 'tcx>,
         Load(cx, ptr)
     };
 
+    unsafe {
+        llvm::LLVMSetAlignment(val, align);
+    }
+
     from_arg_ty(cx, val, t)
 }
 
 /// 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<'blk, 'tcx>(cx: Block<'blk, 'tcx>, v: ValueRef, dst: ValueRef, t: Ty<'tcx>) {
-    Store(cx, to_arg_ty(cx, v, t), to_arg_ty_ptr(cx, dst, t));
+    if cx.unreachable.get() {
+        return;
+    }
+
+    let store = Store(cx, to_arg_ty(cx, v, t), to_arg_ty_ptr(cx, dst, t));
+    unsafe {
+        llvm::LLVMSetAlignment(store, type_of::align_of(cx.ccx(), t));
+    }
 }
 
 pub fn to_arg_ty(bcx: Block, val: ValueRef, ty: Ty) -> ValueRef {

diff --git a/src/librustc_trans/trans/build.rs b/src/librustc_trans/trans/build.rs
index a16c4d6c2c4a78f1e1bb42a1095df124622a7167..32d73e50e6b6bf1b3cef7a46f21c907be169a5fc 100644 (file)
--- a/src/librustc_trans/trans/build.rs
+++ b/src/librustc_trans/trans/build.rs
@@ -646,13 +646,13 @@ pub fn LoadNonNull(cx: Block, ptr: ValueRef) -> ValueRef {
     }
 }
 
-pub fn Store(cx: Block, val: ValueRef, ptr: ValueRef) {
-    if cx.unreachable.get() { return; }
+pub fn Store(cx: Block, val: ValueRef, ptr: ValueRef) -> ValueRef {
+    if cx.unreachable.get() { return C_nil(cx.ccx()); }
     B(cx).store(val, ptr)
 }
 
-pub fn VolatileStore(cx: Block, val: ValueRef, ptr: ValueRef) {
-    if cx.unreachable.get() { return; }
+pub fn VolatileStore(cx: Block, val: ValueRef, ptr: ValueRef) -> ValueRef {
+    if cx.unreachable.get() { return C_nil(cx.ccx()); }
     B(cx).volatile_store(val, ptr)
 }
 

diff --git a/src/librustc_trans/trans/builder.rs b/src/librustc_trans/trans/builder.rs
index 92bc20bafcfbe59412f82290db5173b71a044e6c..3febd41bdce2eccbdccf71ff79947fa68e48639c 100644 (file)
--- a/src/librustc_trans/trans/builder.rs
+++ b/src/librustc_trans/trans/builder.rs
@@ -509,18 +509,18 @@ pub fn load_nonnull(&self, ptr: ValueRef) -> ValueRef {
         value
     }
 
-    pub fn store(&self, val: ValueRef, ptr: ValueRef) {
+    pub fn store(&self, val: ValueRef, ptr: ValueRef) -> ValueRef {
         debug!("Store {} -> {}",
                self.ccx.tn().val_to_string(val),
                self.ccx.tn().val_to_string(ptr));
         assert!(!self.llbuilder.is_null());
         self.count_insn("store");
         unsafe {
-            llvm::LLVMBuildStore(self.llbuilder, val, ptr);
+            llvm::LLVMBuildStore(self.llbuilder, val, ptr)
         }
     }
 
-    pub fn volatile_store(&self, val: ValueRef, ptr: ValueRef) {
+    pub fn volatile_store(&self, val: ValueRef, ptr: ValueRef) -> ValueRef {
         debug!("Store {} -> {}",
                self.ccx.tn().val_to_string(val),
                self.ccx.tn().val_to_string(ptr));
@@ -529,6 +529,7 @@ pub fn volatile_store(&self, val: ValueRef, ptr: ValueRef) {
         unsafe {
             let insn = llvm::LLVMBuildStore(self.llbuilder, val, ptr);
             llvm::LLVMSetVolatile(insn, llvm::True);
+            insn
         }
     }
 

diff --git a/src/librustc_trans/trans/foreign.rs b/src/librustc_trans/trans/foreign.rs
index 8f3a51a5007096984835901d54f13b8f07b21150..c025be2ee98772eb7241dd565501c90a77bbe757 100644 (file)
--- a/src/librustc_trans/trans/foreign.rs
+++ b/src/librustc_trans/trans/foreign.rs
@@ -802,7 +802,9 @@ unsafe fn build_wrap_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
                     // appropriately sized integer and we have to convert it
                     let tmp = builder.bitcast(llforeign_arg,
                                               type_of::arg_type_of(ccx, rust_ty).ptr_to());
-                    builder.load(tmp)
+                    let load = builder.load(tmp);
+                    llvm::LLVMSetAlignment(load, type_of::align_of(ccx, rust_ty));
+                    load
                 } else {
                     builder.load(llforeign_arg)
                 }

diff --git a/src/librustc_trans/trans/intrinsic.rs b/src/librustc_trans/trans/intrinsic.rs
index fc3c0841dd84e9a3c6f837dfe7c64cc84baaf862..1e67212871a1e10e3b765e417f51f936afe29a3d 100644 (file)
--- a/src/librustc_trans/trans/intrinsic.rs
+++ b/src/librustc_trans/trans/intrinsic.rs
@@ -456,13 +456,20 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
         (_, "volatile_load") => {
             let tp_ty = *substs.types.get(FnSpace, 0);
             let ptr = to_arg_ty_ptr(bcx, llargs[0], tp_ty);
-            from_arg_ty(bcx, VolatileLoad(bcx, ptr), tp_ty)
+            let load = VolatileLoad(bcx, ptr);
+            unsafe {
+                llvm::LLVMSetAlignment(load, type_of::align_of(ccx, tp_ty));
+            }
+            from_arg_ty(bcx, load, tp_ty)
         },
         (_, "volatile_store") => {
             let tp_ty = *substs.types.get(FnSpace, 0);
             let ptr = to_arg_ty_ptr(bcx, llargs[0], tp_ty);
             let val = to_arg_ty(bcx, llargs[1], tp_ty);
-            VolatileStore(bcx, val, ptr);
+            let store = VolatileStore(bcx, val, ptr);
+            unsafe {
+                llvm::LLVMSetAlignment(store, type_of::align_of(ccx, tp_ty));
+            }
             C_nil(ccx)
         },