about summary refs log tree commit diff
path: root/src/librustc_codegen_llvm
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2018-09-09 08:54:29 +0000
committerbors <bors@rust-lang.org>2018-09-09 08:54:29 +0000
commitdf6ba0c4acceb5f63090bb20bd23f29c4f439376 (patch)
treed860adafcbc8edaa2d572daf3133b8690e3381d9 /src/librustc_codegen_llvm
parent3d2fc456a91e898f735f97f1a9ea79432da4e1a5 (diff)
parentb9e7574bf2d760fc011cc90890f3280c474746e4 (diff)
downloadrust-df6ba0c4acceb5f63090bb20bd23f29c4f439376.tar.gz
rust-df6ba0c4acceb5f63090bb20bd23f29c4f439376.zip
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.
Diffstat (limited to 'src/librustc_codegen_llvm')
-rw-r--r--src/librustc_codegen_llvm/mir/place.rs9
-rw-r--r--src/librustc_codegen_llvm/type_of.rs22
2 files changed, 19 insertions, 12 deletions
diff --git a/src/librustc_codegen_llvm/mir/place.rs b/src/librustc_codegen_llvm/mir/place.rs
index 4baab1763c3..70ace15e523 100644
--- a/src/librustc_codegen_llvm/mir/place.rs
+++ b/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,
         }
     }
 
diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs
index e6907030ae6..6eec0b0b68c 100644
--- a/src/librustc_codegen_llvm/type_of.rs
+++ b/src/librustc_codegen_llvm/type_of.rs
@@ -122,25 +122,29 @@ 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 {
@@ -148,7 +152,7 @@ fn struct_llfields<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                  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);