about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2014-05-05 10:06:39 -0700
committerbors <bors@rust-lang.org>2014-05-05 10:06:39 -0700
commitfd625dda9a25c054d622e20822f2889f17b15aa6 (patch)
treed2c6aed0d2214322f5ee55392c609e7d46ff2da6
parent2be738ae36600e562fcfc9ed938e183875cd72ad (diff)
parent7fefc1c7f417a8445da85e57a9523508a2561ef3 (diff)
downloadrust-fd625dda9a25c054d622e20822f2889f17b15aa6.tar.gz
rust-fd625dda9a25c054d622e20822f2889f17b15aa6.zip
auto merge of #13271 : stepancheg/rust/align, r=pcwalton
This patch fixes issue #13186.

When generating constant expression for enum, it is possible that
alignment of expression may be not equal to alignment of type.  In that
case space after last struct field must be padded to match size of value
and size of struct. This commit adds that padding.

See detailed explanation in src/test/run-pass/trans-tag-static-padding.rs
-rw-r--r--src/librustc/middle/trans/adt.rs50
-rw-r--r--src/test/run-pass/trans-tag-static-padding.rs66
2 files changed, 104 insertions, 12 deletions
diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs
index 5046f636e2c..ca183e14012 100644
--- a/src/librustc/middle/trans/adt.rs
+++ b/src/librustc/middle/trans/adt.rs
@@ -780,6 +780,26 @@ pub fn trans_const(ccx: &CrateContext, r: &Repr, discr: Disr,
 }
 
 /**
+ * Compute struct field offsets relative to struct begin.
+ */
+fn compute_struct_field_offsets(ccx: &CrateContext, st: &Struct) -> Vec<u64> {
+    let mut offsets = vec!();
+
+    let mut offset = 0;
+    for &ty in st.fields.iter() {
+        let llty = type_of::sizing_type_of(ccx, ty);
+        if !st.packed {
+            let type_align = machine::llalign_of_min(ccx, llty) as u64;
+            offset = roundup(offset, type_align);
+        }
+        offsets.push(offset);
+        offset += machine::llsize_of_alloc(ccx, llty) as u64;
+    }
+    assert_eq!(st.fields.len(), offsets.len());
+    offsets
+}
+
+/**
  * Building structs is a little complicated, because we might need to
  * insert padding if a field's value is less aligned than its type.
  *
@@ -793,26 +813,32 @@ fn build_const_struct(ccx: &CrateContext, st: &Struct, vals: &[ValueRef])
     -> Vec<ValueRef> {
     assert_eq!(vals.len(), st.fields.len());
 
+    let target_offsets = compute_struct_field_offsets(ccx, st);
+
+    // offset of current value
     let mut offset = 0;
     let mut cfields = Vec::new();
-    for (i, &ty) in st.fields.iter().enumerate() {
-        let llty = type_of::sizing_type_of(ccx, ty);
-        let type_align = machine::llalign_of_min(ccx, llty)
-            /*bad*/as u64;
-        let val_align = machine::llalign_of_min(ccx, val_ty(vals[i]))
-            /*bad*/as u64;
-        let target_offset = roundup(offset, type_align);
-        offset = roundup(offset, val_align);
+    for (&val, &target_offset) in vals.iter().zip(target_offsets.iter()) {
+        if !st.packed {
+            let val_align = machine::llalign_of_min(ccx, val_ty(val))
+                /*bad*/as u64;
+            offset = roundup(offset, val_align);
+        }
         if offset != target_offset {
             cfields.push(padding(ccx, target_offset - offset));
             offset = target_offset;
         }
-        assert!(!is_undef(vals[i]));
-        cfields.push(vals[i]);
-        offset += machine::llsize_of_alloc(ccx, llty) as u64
+        assert!(!is_undef(val));
+        cfields.push(val);
+        offset += machine::llsize_of_alloc(ccx, val_ty(val)) as u64;
+    }
+
+    assert!(offset <= st.size);
+    if offset != st.size {
+        cfields.push(padding(ccx, st.size - offset));
     }
 
-    return cfields;
+    cfields
 }
 
 fn padding(ccx: &CrateContext, size: u64) -> ValueRef {
diff --git a/src/test/run-pass/trans-tag-static-padding.rs b/src/test/run-pass/trans-tag-static-padding.rs
new file mode 100644
index 00000000000..c6a3a0b0409
--- /dev/null
+++ b/src/test/run-pass/trans-tag-static-padding.rs
@@ -0,0 +1,66 @@
+// Copyright 2014 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.
+
+
+// Issue #13186
+
+// For simplicity of explanations assuming code is compiled for x86_64
+// Linux ABI.
+
+// Size of TestOption<u64> is 16, and alignment of TestOption<u64> is 8.
+// Size of u8 is 1, and alignment of u8 is 1.
+// So size of Request is 24, and alignment of Request must be 8:
+// the maximum alignment of its fields.
+// Last 7 bytes of Request struct are not occupied by any fields.
+
+
+enum TestOption<T> {
+    TestNone,
+    TestSome(T),
+}
+
+pub struct Request {
+    foo: TestOption<u64>,
+    bar: u8,
+}
+
+fn default_instance() -> &'static Request {
+    static instance: Request = Request {
+        // LLVM does not allow to specify alignment of expressions, thus
+        // alignment of `foo` in constant is 1, not 8.
+        foo: TestNone,
+        bar: 17,
+        // Space after last field is not occupied by any data, but it is
+        // reserved to make struct aligned properly. If compiler does
+        // not insert padding after last field when emitting constant,
+        // size of struct may be not equal to size of struct, and
+        // compiler crashes in internal assertion check.
+    };
+    &'static instance
+}
+
+fn non_default_instance() -> &'static Request {
+    static instance: Request = Request {
+        foo: TestSome(0x1020304050607080),
+        bar: 19,
+    };
+    &'static instance
+}
+
+pub fn main() {
+    match default_instance() {
+        &Request { foo: TestNone, bar: 17 } => {},
+        _ => fail!(),
+    };
+    match non_default_instance() {
+        &Request { foo: TestSome(0x1020304050607080), bar: 19 } => {},
+        _ => fail!(),
+    };
+}