Skip to content

Support #[repr(packed(N))] on Rust 1.33+ #1485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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