Skip to content

Commit

Permalink
Support #[repr(packed(N))] on Rust 1.33+
Browse files Browse the repository at this point in the history
  • Loading branch information
LegNeato committed Jan 8, 2019
1 parent 28c0eb4 commit 3994a9a
Show file tree
Hide file tree
Showing 9 changed files with 222 additions and 21 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,14 @@ Released YYYY/MM/DD

## Changed

- `#pragma pack(n)` is now translated to `#[repr(C, packed(n))]` when targeting Rust 1.33+. [#537][]

[#537]: https://github.com/rust-lang-nursery/rust-bindgen/issues/537

* Bitfield enums now use `#[repr(transparent)]` instead of `#[repr(C)]` when targeting Rust 1.28+. [#1474][]

[#1474]: https://github.com/rust-lang-nursery/rust-bindgen/issues/1474


## Deprecated

* TODO (or remove section if none)
Expand Down
3 changes: 2 additions & 1 deletion src/codegen/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use quote::TokenStreamExt;

pub mod attributes {
use proc_macro2::{Ident, Span, TokenStream};
use std::str::FromStr;

pub fn repr(which: &str) -> TokenStream {
let which = Ident::new(which, Span::call_site());
Expand All @@ -16,7 +17,7 @@ pub mod attributes {
}

pub fn repr_list(which_ones: &[&str]) -> TokenStream {
let which_ones = which_ones.iter().cloned().map(|one| Ident::new(one, Span::call_site()));
let which_ones = which_ones.iter().cloned().map(|one| TokenStream::from_str(one).expect("repr to be valid"));
quote! {
#[repr( #( #which_ones ),* )]
}
Expand Down
5 changes: 4 additions & 1 deletion src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1673,7 +1673,10 @@ impl CodeGenerator for CompInfo {
attributes.push(attributes::doc(comment));
}
if packed && !is_opaque {
attributes.push(attributes::repr_list(&["C", "packed"]));
let n = layout.map_or(1, |l| l.align);
assert!(ctx.options().rust_features().repr_packed_n || n == 1);
let packed_repr = if n == 1 { "packed".to_string() } else { format!("packed({})", n) };
attributes.push(attributes::repr_list(&["C", &packed_repr]));
} else {
attributes.push(attributes::repr("C"));
}
Expand Down
6 changes: 6 additions & 0 deletions src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ macro_rules! rust_target_base {
=> Stable_1_27 => 1.27;
/// Rust stable 1.28
=> Stable_1_28 => 1.28;
/// Rust stable 1.33
=> Stable_1_33 => 1.33;
/// Nightly rust
=> Nightly => nightly;
);
Expand Down Expand Up @@ -190,6 +192,10 @@ rust_feature_def!(
/// repr(transparent) ([PR](https://github.com/rust-lang/rust/pull/51562))
=> repr_transparent;
}
Stable_1_33 {
/// repr(packed(N)) ([PR](https://github.com/rust-lang/rust/pull/57049))
=> repr_packed_n;
}
Nightly {
/// `thiscall` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/42202))
=> thiscall_abi;
Expand Down
23 changes: 13 additions & 10 deletions src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1649,16 +1649,19 @@ impl IsOpaque for CompInfo {
return true;
}

// We don't have `#[repr(packed = "N")]` in Rust yet, so the best we can
// do is make this struct opaque.
//
// See https://github.com/rust-lang-nursery/rust-bindgen/issues/537 and
// https://github.com/rust-lang/rust/issues/33158
if self.is_packed(ctx, layout) && layout.map_or(false, |l| l.align > 1) {
warn!("Found a type that is both packed and aligned to greater than \
1; Rust doesn't have `#[repr(packed = \"N\")]` yet, so we \
are treating it as opaque");
return true;
if !ctx.options().rust_features().repr_packed_n {
// If we don't have `#[repr(packed(N)]`, the best we can
// do is make this struct opaque.
//
// See https://github.com/rust-lang-nursery/rust-bindgen/issues/537 and
// https://github.com/rust-lang/rust/issues/33158
if self.is_packed(ctx, layout) && layout.map_or(false, |l| l.align > 1) {
warn!("Found a type that is both packed and aligned to greater than \
1; Rust before version 1.33 doesn't have `#[repr(packed(N))]`, so we \
are treating it as opaque. You may wish to set bindgen's rust target \
version to 1.33 or later to enable `#[repr(packed(N))]` support.");
return true;
}
}

false
Expand Down
151 changes: 151 additions & 0 deletions tests/expectations/tests/issue-537-repr-packed-n.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/* automatically generated by rust-bindgen */

#![allow(
dead_code,
non_snake_case,
non_camel_case_types,
non_upper_case_globals
)]
#![cfg(feature = "nightly")]

/// This should not be opaque; we can see the attributes and can pack the
/// struct.
#[repr(C, packed)]
#[derive(Debug, Default, Copy, Clone)]
pub struct AlignedToOne {
pub i: ::std::os::raw::c_int,
}
#[test]
fn bindgen_test_layout_AlignedToOne() {
assert_eq!(
::std::mem::size_of::<AlignedToOne>(),
4usize,
concat!("Size of: ", stringify!(AlignedToOne))
);
assert_eq!(
::std::mem::align_of::<AlignedToOne>(),
1usize,
concat!("Alignment of ", stringify!(AlignedToOne))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<AlignedToOne>())).i as *const _ as usize },
0usize,
concat!(
"Offset of field: ",
stringify!(AlignedToOne),
"::",
stringify!(i)
)
);
}
/// This should be be packed because Rust 1.33 has `#[repr(packed(N))]`.
#[repr(C, packed(2))]
#[derive(Debug, Default, Copy, Clone)]
pub struct AlignedToTwo {
pub i: ::std::os::raw::c_int,
}
#[test]
fn bindgen_test_layout_AlignedToTwo() {
assert_eq!(
::std::mem::size_of::<AlignedToTwo>(),
4usize,
concat!("Size of: ", stringify!(AlignedToTwo))
);
assert_eq!(
::std::mem::align_of::<AlignedToTwo>(),
2usize,
concat!("Alignment of ", stringify!(AlignedToTwo))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<AlignedToTwo>())).i as *const _ as usize },
0usize,
concat!(
"Offset of field: ",
stringify!(AlignedToTwo),
"::",
stringify!(i)
)
);
}
/// This should not be opaque because although `libclang` doesn't give us the
/// `#pragma pack(1)`, we can detect that alignment is 1 and add
/// `#[repr(packed)]` to the struct ourselves.
#[repr(C, packed)]
#[derive(Debug, Default, Copy, Clone)]
pub struct PackedToOne {
pub x: ::std::os::raw::c_int,
pub y: ::std::os::raw::c_int,
}
#[test]
fn bindgen_test_layout_PackedToOne() {
assert_eq!(
::std::mem::size_of::<PackedToOne>(),
8usize,
concat!("Size of: ", stringify!(PackedToOne))
);
assert_eq!(
::std::mem::align_of::<PackedToOne>(),
1usize,
concat!("Alignment of ", stringify!(PackedToOne))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<PackedToOne>())).x as *const _ as usize },
0usize,
concat!(
"Offset of field: ",
stringify!(PackedToOne),
"::",
stringify!(x)
)
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<PackedToOne>())).y as *const _ as usize },
4usize,
concat!(
"Offset of field: ",
stringify!(PackedToOne),
"::",
stringify!(y)
)
);
}
/// This should be be packed because Rust 1.33 has `#[repr(packed(N))]`.
#[repr(C, packed(2))]
#[derive(Debug, Default, Copy, Clone)]
pub struct PackedToTwo {
pub x: ::std::os::raw::c_int,
pub y: ::std::os::raw::c_int,
}
#[test]
fn bindgen_test_layout_PackedToTwo() {
assert_eq!(
::std::mem::size_of::<PackedToTwo>(),
8usize,
concat!("Size of: ", stringify!(PackedToTwo))
);
assert_eq!(
::std::mem::align_of::<PackedToTwo>(),
2usize,
concat!("Alignment of ", stringify!(PackedToTwo))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<PackedToTwo>())).x as *const _ as usize },
0usize,
concat!(
"Offset of field: ",
stringify!(PackedToTwo),
"::",
stringify!(x)
)
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<PackedToTwo>())).y as *const _ as usize },
4usize,
concat!(
"Offset of field: ",
stringify!(PackedToTwo),
"::",
stringify!(y)
)
);
}
8 changes: 4 additions & 4 deletions tests/expectations/tests/issue-537.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ fn bindgen_test_layout_AlignedToOne() {
)
);
}
/// This should be opaque because although we can see the attributes, Rust
/// doesn't have `#[repr(packed = "N")]` yet.
/// This should be opaque because although we can see the attributes, Rust before
/// 1.33 doesn't have `#[repr(packed(N))]`.
#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct AlignedToTwo {
Expand Down Expand Up @@ -100,8 +100,8 @@ fn bindgen_test_layout_PackedToOne() {
);
}
/// In this case, even if we can detect the weird alignment triggered by
/// `#pragma pack(2)`, we can't do anything about it because Rust doesn't have
/// `#[repr(packed = "N")]`. Therefore, we must make it opaque.
/// `#pragma pack(2)`, we can't do anything about it because Rust before 1.33
/// doesn't have `#[repr(packed(N))]`. Therefore, we must make it opaque.
#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct PackedToTwo {
Expand Down
34 changes: 34 additions & 0 deletions tests/headers/issue-537-repr-packed-n.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// bindgen-flags: --raw-line '#![cfg(feature = "nightly")]' --rust-target 1.33

/// This should not be opaque; we can see the attributes and can pack the
/// struct.
struct AlignedToOne {
int i;
} __attribute__ ((packed,aligned(1)));

/// This should be be packed because Rust 1.33 has `#[repr(packed(N))]`.
struct AlignedToTwo {
int i;
} __attribute__ ((packed,aligned(2)));

#pragma pack(1)

/// This should not be opaque because although `libclang` doesn't give us the
/// `#pragma pack(1)`, we can detect that alignment is 1 and add
/// `#[repr(packed)]` to the struct ourselves.
struct PackedToOne {
int x;
int y;
};

#pragma pack()

#pragma pack(2)

/// This should be be packed because Rust 1.33 has `#[repr(packed(N))]`.
struct PackedToTwo {
int x;
int y;
};

#pragma pack()
8 changes: 4 additions & 4 deletions tests/headers/issue-537.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ struct AlignedToOne {
int i;
} __attribute__ ((packed,aligned(1)));

/// This should be opaque because although we can see the attributes, Rust
/// doesn't have `#[repr(packed = "N")]` yet.
/// This should be opaque because although we can see the attributes, Rust before
/// 1.33 doesn't have `#[repr(packed(N))]`.
struct AlignedToTwo {
int i;
} __attribute__ ((packed,aligned(2)));
Expand All @@ -25,8 +25,8 @@ struct PackedToOne {
#pragma pack(2)

/// In this case, even if we can detect the weird alignment triggered by
/// `#pragma pack(2)`, we can't do anything about it because Rust doesn't have
/// `#[repr(packed = "N")]`. Therefore, we must make it opaque.
/// `#pragma pack(2)`, we can't do anything about it because Rust before 1.33
/// doesn't have `#[repr(packed(N))]`. Therefore, we must make it opaque.
struct PackedToTwo {
int x;
int y;
Expand Down

0 comments on commit 3994a9a

Please sign in to comment.