Auto merge of #53998 - eddyb:issue-53728, r=oli-obk

rustc_codegen_llvm: don't assume offsets are always aligned.

Fixes #53728 by taking into account not just overall type alignment and the field's alignment when determining whether a field is aligned or not ("packed"), but also the field's offset within the type.

Previously, rustc assumed that the offset was always at least as aligned as `min(struct.align, field.align)`. However, there's no real reason to have that assumption, and it obviously can't always be true after we implement `#[repr(align(N), pack(K))]`. There's also a case today where that assumption is not true, involving niche discriminants in enums:

Suppose that we have the code in #53728:
```Rust
#[repr(u16)]
enum DeviceKind {
    Nil = 0,
}

#[repr(packed)]
struct DeviceInfo {
    endianness: u8,
    device_kind: DeviceKind,
}

struct Wrapper {
    device_info: DeviceInfo,
    data: u32
}
```

Observe the layout of `Option<Wrapper>`. It has an alignment of 4 because of the `u32`. `device_info.device_kind` is a good niche field to use, which means the enum ends up with this layout:
```
size = 8
align = 4
fields = [
    { offset=1, type=u16 } // discriminant, .<Some>.device_info.device_kind
]
```

And here we have an discriminant with alignment 2 (`u16`) but offset 1.

Author: bors

src/librustc_codegen_llvm/mir/place.rs

@@ -173,7 +173,10 @@ impl PlaceRef<'ll, 'tcx> {
         let cx = bx.cx;
         let field = self.layout.field(cx, ix);
         let offset = self.layout.fields.offset(ix);
-        let align = self.align.min(self.layout.align).min(field.align);
+        let effective_field_align = self.align
+            .min(self.layout.align)
+            .min(field.align)
+            .restrict_for_offset(offset);
 
         let simple = || {
             // Unions and newtypes only use an offset of 0.
@@ -195,7 +198,7 @@ impl PlaceRef<'ll, 'tcx> {
                     None
                 },
                 layout: field,
-                align,
+                align: effective_field_align,
             }
         };
 
@@ -268,7 +271,7 @@ impl PlaceRef<'ll, 'tcx> {
             llval: bx.pointercast(byte_ptr, ll_fty.ptr_to()),
             llextra: self.llextra,
             layout: field,
-            align,
+            align: effective_field_align,
         }
     }
 

src/librustc_codegen_llvm/type_of.rs

@@ -122,33 +122,37 @@ fn struct_llfields<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
 
     let mut packed = false;
     let mut offset = Size::ZERO;
-    let mut prev_align = layout.align;
+    let mut prev_effective_align = layout.align;
     let mut result: Vec<_> = Vec::with_capacity(1 + field_count * 2);
     for i in layout.fields.index_by_increasing_offset() {
-        let field = layout.field(cx, i);
-        packed |= layout.align.abi() < field.align.abi();
-
         let target_offset = layout.fields.offset(i as usize);
-        debug!("struct_llfields: {}: {:?} offset: {:?} target_offset: {:?}",
-            i, field, offset, target_offset);
+        let field = layout.field(cx, i);
+        let effective_field_align = layout.align
+            .min(field.align)
+            .restrict_for_offset(target_offset);
+        packed |= effective_field_align.abi() < field.align.abi();
+
+        debug!("struct_llfields: {}: {:?} offset: {:?} target_offset: {:?} \
+                effective_field_align: {}",
+            i, field, offset, target_offset, effective_field_align.abi());
         assert!(target_offset >= offset);
         let padding = target_offset - offset;
-        let padding_align = layout.align.min(prev_align).min(field.align);
+        let padding_align = prev_effective_align.min(effective_field_align);
         assert_eq!(offset.abi_align(padding_align) + padding, target_offset);
         result.push(Type::padding_filler(cx, padding, padding_align));
         debug!("    padding before: {:?}", padding);
 
         result.push(field.llvm_type(cx));
         offset = target_offset + field.size;
-        prev_align = field.align;
+        prev_effective_align = effective_field_align;
     }
     if !layout.is_unsized() && field_count > 0 {
         if offset > layout.size {
             bug!("layout: {:#?} stride: {:?} offset: {:?}",
                  layout, layout.size, offset);
         }
         let padding = layout.size - offset;
-        let padding_align = layout.align.min(prev_align);
+        let padding_align = prev_effective_align;
         assert_eq!(offset.abi_align(padding_align) + padding, layout.size);
         debug!("struct_llfields: pad_bytes: {:?} offset: {:?} stride: {:?}",
                padding, offset, layout.size);

src/librustc_target/abi/mod.rs

@@ -416,6 +416,24 @@ impl Align {
             pref_pow2: cmp::max(self.pref_pow2, other.pref_pow2),
         }
     }
+
+    /// Compute the best alignment possible for the given offset
+    /// (the largest power of two that the offset is a multiple of).
+    ///
+    /// NB: for an offset of `0`, this happens to return `2^64`.
+    pub fn max_for_offset(offset: Size) -> Align {
+        let pow2 = offset.bytes().trailing_zeros() as u8;
+        Align {
+            abi_pow2: pow2,
+            pref_pow2: pow2,
+        }
+    }
+
+    /// Lower the alignment, if necessary, such that the given offset
+    /// is aligned to it (the offset is a multiple of the aligment).
+    pub fn restrict_for_offset(self, offset: Size) -> Align {
+        self.min(Align::max_for_offset(offset))
+    }
 }
 
 /// Integers, also used for enum discriminants.

src/test/run-pass/issue-53728.rs

@@ -0,0 +1,26 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[repr(u16)]
+enum DeviceKind {
+    Nil = 0,
+}
+
+#[repr(packed)]
+struct DeviceInfo {
+    endianness: u8,
+    device_kind: DeviceKind,
+}
+
+fn main() {
+    let _x = None::<(DeviceInfo, u8)>;
+    let _y = None::<(DeviceInfo, u16)>;
+    let _z = None::<(DeviceInfo, u64)>;
+}