diff --git a/CHANGELOG.md b/CHANGELOG.md index d4f7c57f62..01be62f2fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/src/codegen/helpers.rs b/src/codegen/helpers.rs index d6377194b0..457979b610 100644 --- a/src/codegen/helpers.rs +++ b/src/codegen/helpers.rs @@ -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()); @@ -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 ),* )] } diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 6fd7e7d35f..d3395edff5 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -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")); } diff --git a/src/features.rs b/src/features.rs index 1f70003931..b5a6340246 100644 --- a/src/features.rs +++ b/src/features.rs @@ -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; ); @@ -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; diff --git a/src/ir/comp.rs b/src/ir/comp.rs index fd4c827438..12a8fa323f 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -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 diff --git a/tests/expectations/tests/issue-537-repr-packed-n.rs b/tests/expectations/tests/issue-537-repr-packed-n.rs new file mode 100644 index 0000000000..1c930a8b98 --- /dev/null +++ b/tests/expectations/tests/issue-537-repr-packed-n.rs @@ -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::(), + 4usize, + concat!("Size of: ", stringify!(AlignedToOne)) + ); + assert_eq!( + ::std::mem::align_of::(), + 1usize, + concat!("Alignment of ", stringify!(AlignedToOne)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).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::(), + 4usize, + concat!("Size of: ", stringify!(AlignedToTwo)) + ); + assert_eq!( + ::std::mem::align_of::(), + 2usize, + concat!("Alignment of ", stringify!(AlignedToTwo)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).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::(), + 8usize, + concat!("Size of: ", stringify!(PackedToOne)) + ); + assert_eq!( + ::std::mem::align_of::(), + 1usize, + concat!("Alignment of ", stringify!(PackedToOne)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).x as *const _ as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(PackedToOne), + "::", + stringify!(x) + ) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).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::(), + 8usize, + concat!("Size of: ", stringify!(PackedToTwo)) + ); + assert_eq!( + ::std::mem::align_of::(), + 2usize, + concat!("Alignment of ", stringify!(PackedToTwo)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).x as *const _ as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(PackedToTwo), + "::", + stringify!(x) + ) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).y as *const _ as usize }, + 4usize, + concat!( + "Offset of field: ", + stringify!(PackedToTwo), + "::", + stringify!(y) + ) + ); +} diff --git a/tests/expectations/tests/issue-537.rs b/tests/expectations/tests/issue-537.rs index 8913db3617..94c6dd3b3f 100644 --- a/tests/expectations/tests/issue-537.rs +++ b/tests/expectations/tests/issue-537.rs @@ -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 { @@ -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 { diff --git a/tests/headers/issue-537-repr-packed-n.h b/tests/headers/issue-537-repr-packed-n.h new file mode 100644 index 0000000000..f4c0070a4a --- /dev/null +++ b/tests/headers/issue-537-repr-packed-n.h @@ -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() diff --git a/tests/headers/issue-537.h b/tests/headers/issue-537.h index 9a1072a7b6..a773199f09 100644 --- a/tests/headers/issue-537.h +++ b/tests/headers/issue-537.h @@ -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))); @@ -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;