Skip to content

Commit fd625dd

Browse files
committed
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
2 parents 2be738a + 7fefc1c commit fd625dd

File tree

2 files changed

+104
-12
lines changed

2 files changed

+104
-12
lines changed

src/librustc/middle/trans/adt.rs

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,26 @@ pub fn trans_const(ccx: &CrateContext, r: &Repr, discr: Disr,
779779
}
780780
}
781781

782+
/**
783+
* Compute struct field offsets relative to struct begin.
784+
*/
785+
fn compute_struct_field_offsets(ccx: &CrateContext, st: &Struct) -> Vec<u64> {
786+
let mut offsets = vec!();
787+
788+
let mut offset = 0;
789+
for &ty in st.fields.iter() {
790+
let llty = type_of::sizing_type_of(ccx, ty);
791+
if !st.packed {
792+
let type_align = machine::llalign_of_min(ccx, llty) as u64;
793+
offset = roundup(offset, type_align);
794+
}
795+
offsets.push(offset);
796+
offset += machine::llsize_of_alloc(ccx, llty) as u64;
797+
}
798+
assert_eq!(st.fields.len(), offsets.len());
799+
offsets
800+
}
801+
782802
/**
783803
* Building structs is a little complicated, because we might need to
784804
* 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])
793813
-> Vec<ValueRef> {
794814
assert_eq!(vals.len(), st.fields.len());
795815

816+
let target_offsets = compute_struct_field_offsets(ccx, st);
817+
818+
// offset of current value
796819
let mut offset = 0;
797820
let mut cfields = Vec::new();
798-
for (i, &ty) in st.fields.iter().enumerate() {
799-
let llty = type_of::sizing_type_of(ccx, ty);
800-
let type_align = machine::llalign_of_min(ccx, llty)
801-
/*bad*/as u64;
802-
let val_align = machine::llalign_of_min(ccx, val_ty(vals[i]))
803-
/*bad*/as u64;
804-
let target_offset = roundup(offset, type_align);
805-
offset = roundup(offset, val_align);
821+
for (&val, &target_offset) in vals.iter().zip(target_offsets.iter()) {
822+
if !st.packed {
823+
let val_align = machine::llalign_of_min(ccx, val_ty(val))
824+
/*bad*/as u64;
825+
offset = roundup(offset, val_align);
826+
}
806827
if offset != target_offset {
807828
cfields.push(padding(ccx, target_offset - offset));
808829
offset = target_offset;
809830
}
810-
assert!(!is_undef(vals[i]));
811-
cfields.push(vals[i]);
812-
offset += machine::llsize_of_alloc(ccx, llty) as u64
831+
assert!(!is_undef(val));
832+
cfields.push(val);
833+
offset += machine::llsize_of_alloc(ccx, val_ty(val)) as u64;
834+
}
835+
836+
assert!(offset <= st.size);
837+
if offset != st.size {
838+
cfields.push(padding(ccx, st.size - offset));
813839
}
814840

815-
return cfields;
841+
cfields
816842
}
817843

818844
fn padding(ccx: &CrateContext, size: u64) -> ValueRef {
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
12+
// Issue #13186
13+
14+
// For simplicity of explanations assuming code is compiled for x86_64
15+
// Linux ABI.
16+
17+
// Size of TestOption<u64> is 16, and alignment of TestOption<u64> is 8.
18+
// Size of u8 is 1, and alignment of u8 is 1.
19+
// So size of Request is 24, and alignment of Request must be 8:
20+
// the maximum alignment of its fields.
21+
// Last 7 bytes of Request struct are not occupied by any fields.
22+
23+
24+
enum TestOption<T> {
25+
TestNone,
26+
TestSome(T),
27+
}
28+
29+
pub struct Request {
30+
foo: TestOption<u64>,
31+
bar: u8,
32+
}
33+
34+
fn default_instance() -> &'static Request {
35+
static instance: Request = Request {
36+
// LLVM does not allow to specify alignment of expressions, thus
37+
// alignment of `foo` in constant is 1, not 8.
38+
foo: TestNone,
39+
bar: 17,
40+
// Space after last field is not occupied by any data, but it is
41+
// reserved to make struct aligned properly. If compiler does
42+
// not insert padding after last field when emitting constant,
43+
// size of struct may be not equal to size of struct, and
44+
// compiler crashes in internal assertion check.
45+
};
46+
&'static instance
47+
}
48+
49+
fn non_default_instance() -> &'static Request {
50+
static instance: Request = Request {
51+
foo: TestSome(0x1020304050607080),
52+
bar: 19,
53+
};
54+
&'static instance
55+
}
56+
57+
pub fn main() {
58+
match default_instance() {
59+
&Request { foo: TestNone, bar: 17 } => {},
60+
_ => fail!(),
61+
};
62+
match non_default_instance() {
63+
&Request { foo: TestSome(0x1020304050607080), bar: 19 } => {},
64+
_ => fail!(),
65+
};
66+
}

0 commit comments

Comments
 (0)