Skip to content

Commit

Permalink
Auto merge of #1044 - faern:modern-alignment, r=alexcrichton
Browse files Browse the repository at this point in the history
Add alignment feature and use #[repr(align(x))]

Trying to solve #1042.

Here I introduce the discussed feature that will allow going from struct alignment with a private `__align` field to using `#[repr(align(x))]`. However, I have not implemented it for all structs that require alignment yet, only `in6_addr`. This because I did not want to spend too much time before we have discussed and solved the remaining questions regarding this.

One thing to discuss is testing. I have so far not done anything to the CI scripts. So currently they will still test the crate only with the `align` feature disabled. Thus they will make sure the `__align` fields are still correct. But no automatic tests make sure everything is correct when the `align` feature is turned on. What do we want to do about that? Can we insert another `cargo test` with `--features align` to make all the Travis jobs run the test suite twice, or will that slow things down too much?

I have tried using this version of libc in rustc and the standard library. And successfully changed `Ipv6Addr::new` to not use any `unsafe` and to become a `const fn`. Whether or not we want that is out of scope for this PR, but my point was that the changes introduced with this PR allow much more flexible usage of the libc structs that have alignment.
  • Loading branch information
bors committed Jul 30, 2018
2 parents 7d262c0 + e167a73 commit 8565755
Show file tree
Hide file tree
Showing 26 changed files with 886 additions and 515 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ appveyor = { repository = "rust-lang/libc", project_name = "rust-lang-libs/libc"
[features]
default = ["use_std"]
use_std = []
align = []

[workspace]
members = ["libc-test"]
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ this via:
libc = { version = "0.2", default-features = false }
```

By default libc uses private fields in structs in order to enforce a certain
memory alignment on them. These structs can be hard to instantiate outside of
libc. To make libc use `#[repr(align(x))]`, instead of the private fields,
activate the *align* feature. This requires Rust 1.25 or newer:

```toml
[dependencies]
libc = { version = "0.2", features = ["align"] }
```

## What is libc?

The primary purpose of this crate is to provide all of the definitions necessary
Expand Down
6 changes: 5 additions & 1 deletion ci/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ if [ "$QEMU" != "" ]; then
exec grep "^PASSED .* tests" $CARGO_TARGET_DIR/out.log
fi

# FIXME: x86_64-unknown-linux-gnux32 fail to compile wihout --release
# FIXME: x86_64-unknown-linux-gnux32 fail to compile without --release
# See https://github.com/rust-lang/rust/issues/45417
opt=
if [ "$TARGET" = "x86_64-unknown-linux-gnux32" ]; then
Expand All @@ -91,4 +91,8 @@ fi
if [ "$TARGET" != "x86_64-rumprun-netbsd" ]; then
cargo test $opt --no-default-features --manifest-path libc-test/Cargo.toml --target $TARGET
fi
# Test the #[repr(align(x))] feature if this is building on Rust >= 1.25
if [ $(rustc --version | sed -E 's/^rustc 1\.([0-9]*)\..*/\1/') -ge 25 ]; then
cargo test $opt --features align --manifest-path libc-test/Cargo.toml --target $TARGET
fi
exec cargo test $opt --manifest-path libc-test/Cargo.toml --target $TARGET
1 change: 1 addition & 0 deletions libc-test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ctest = { git = "https://github.com/alexcrichton/ctest" }
[features]
default = [ "use_std" ]
use_std = [ "libc/use_std" ]
align = [ "libc/align" ]

[[test]]
name = "main"
Expand Down
148 changes: 110 additions & 38 deletions src/fuchsia/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,10 @@ s! {
pub s_addr: in_addr_t,
}

#[cfg_attr(feature = "align", repr(align(4)))]
pub struct in6_addr {
pub s6_addr: [u8; 16],
#[cfg(not(feature = "align"))]
__align: [u32; 0],
}

Expand Down Expand Up @@ -518,14 +520,30 @@ s! {
pub ifa_data: *mut ::c_void
}

#[cfg_attr(all(feature = "align",
target_pointer_width = "32",
any(target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
target_arch = "x86_64")),
repr(align(4)))]
#[cfg_attr(all(feature = "align",
any(target_pointer_width = "64",
not(any(target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
target_arch = "x86_64")))),
repr(align(8)))]
pub struct pthread_mutex_t {
#[cfg(any(target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
all(target_arch = "x86_64",
target_pointer_width = "32")))]
#[cfg(all(not(feature = "align"),
any(target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
all(target_arch = "x86_64",
target_pointer_width = "32"))))]
__align: [::c_long; 0],
#[cfg(not(any(target_arch = "mips",
#[cfg(not(any(feature = "align",
target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
all(target_arch = "x86_64",
Expand All @@ -534,14 +552,30 @@ s! {
size: [u8; __SIZEOF_PTHREAD_MUTEX_T],
}

#[cfg_attr(all(feature = "align",
target_pointer_width = "32",
any(target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
target_arch = "x86_64")),
repr(align(4)))]
#[cfg_attr(all(feature = "align",
any(target_pointer_width = "64",
not(any(target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
target_arch = "x86_64")))),
repr(align(8)))]
pub struct pthread_rwlock_t {
#[cfg(any(target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
all(target_arch = "x86_64",
target_pointer_width = "32")))]
#[cfg(all(not(feature = "align"),
any(target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
all(target_arch = "x86_64",
target_pointer_width = "32"))))]
__align: [::c_long; 0],
#[cfg(not(any(target_arch = "mips",
#[cfg(not(any(feature = "align",
target_arch = "mips",
target_arch = "arm",
target_arch = "powerpc",
all(target_arch = "x86_64",
Expand All @@ -550,39 +584,78 @@ s! {
size: [u8; __SIZEOF_PTHREAD_RWLOCK_T],
}

#[cfg_attr(all(feature = "align",
any(target_pointer_width = "32",
target_arch = "x86_64", target_arch = "powerpc64",
target_arch = "mips64", target_arch = "s390x",
target_arch = "sparc64",
all(target_arch = "aarch64", target_env = "musl"))),
repr(align(4)))]
#[cfg_attr(all(feature = "align",
not(any(target_pointer_width = "32",
target_arch = "x86_64", target_arch = "powerpc64",
target_arch = "mips64", target_arch = "s390x",
target_arch = "sparc64",
all(target_arch = "aarch64", target_env = "musl")))),
repr(align(8)))]
pub struct pthread_mutexattr_t {
#[cfg(any(target_arch = "x86_64", target_arch = "powerpc64",
target_arch = "mips64", target_arch = "s390x",
target_arch = "sparc64"))]
__align: [::c_int; 0],
#[cfg(not(any(target_arch = "x86_64", target_arch = "powerpc64",
#[cfg(all(not(features = "align"),
any(target_arch = "x86_64", target_arch = "powerpc64",
target_arch = "mips64", target_arch = "s390x",
target_arch = "sparc64", target_arch = "aarch64")))]
__align: [::c_long; 0],
#[cfg(all(target_arch = "aarch64", target_env = "gnu"))]
__align: [::c_long; 0],
#[cfg(all(target_arch = "aarch64", target_env = "musl"))]
target_arch = "sparc64",
all(target_arch = "aarch64", target_env = "musl"))))]
__align: [::c_int; 0],
#[cfg(all(not(features = "align"),
not(any(target_arch = "x86_64", target_arch = "powerpc64",
target_arch = "mips64", target_arch = "s390x",
target_arch = "sparc64",
all(target_arch = "aarch64", target_env = "musl")))))]
__align: [::c_long; 0],
size: [u8; __SIZEOF_PTHREAD_MUTEXATTR_T],
}

#[cfg_attr(all(feature = "align",
any(target_env = "musl", target_pointer_width = "32")),
repr(align(4)))]
#[cfg_attr(all(feature = "align",
not(target_env = "musl"),
target_pointer_width = "64"),
repr(align(8)))]
pub struct pthread_rwlockattr_t {
#[cfg(any(target_env = "musl"))]
#[cfg(all(not(feature = "align"), target_env = "musl"))]
__align: [::c_int; 0],
#[cfg(not(any(target_env = "musl")))]
#[cfg(all(not(feature = "align"), not(target_env = "musl")))]
__align: [::c_long; 0],
size: [u8; __SIZEOF_PTHREAD_RWLOCKATTR_T],
}

#[cfg_attr(all(feature = "align",
target_env = "musl",
target_pointer_width = "32"),
repr(align(4)))]
#[cfg_attr(all(feature = "align",
target_env = "musl",
target_pointer_width = "64"),
repr(align(8)))]
#[cfg_attr(all(feature = "align",
not(target_env = "musl"),
target_arch = "x86"),
repr(align(4)))]
#[cfg_attr(all(feature = "align",
not(target_env = "musl"),
not(target_arch = "x86")),
repr(align(8)))]
pub struct pthread_cond_t {
#[cfg(any(target_env = "musl"))]
#[cfg(all(not(feature = "align"), target_env = "musl"))]
__align: [*const ::c_void; 0],
#[cfg(not(any(target_env = "musl")))]
#[cfg(not(any(feature = "align", target_env = "musl")))]
__align: [::c_longlong; 0],
size: [u8; __SIZEOF_PTHREAD_COND_T],
}

#[cfg_attr(feature = "align", repr(align(4)))]
pub struct pthread_condattr_t {
#[cfg(not(feature = "align"))]
__align: [::c_int; 0],
size: [u8; __SIZEOF_PTHREAD_CONDATTR_T],
}
Expand Down Expand Up @@ -2004,18 +2077,17 @@ pub const RTLD_NOW: ::c_int = 0x2;

pub const TCP_MD5SIG: ::c_int = 14;

pub const PTHREAD_MUTEX_INITIALIZER: pthread_mutex_t = pthread_mutex_t {
__align: [],
size: [0; __SIZEOF_PTHREAD_MUTEX_T],
};
pub const PTHREAD_COND_INITIALIZER: pthread_cond_t = pthread_cond_t {
__align: [],
size: [0; __SIZEOF_PTHREAD_COND_T],
};
pub const PTHREAD_RWLOCK_INITIALIZER: pthread_rwlock_t = pthread_rwlock_t {
__align: [],
size: [0; __SIZEOF_PTHREAD_RWLOCK_T],
};
align_const! {
pub const PTHREAD_MUTEX_INITIALIZER: pthread_mutex_t = pthread_mutex_t {
size: [0; __SIZEOF_PTHREAD_MUTEX_T],
};
pub const PTHREAD_COND_INITIALIZER: pthread_cond_t = pthread_cond_t {
size: [0; __SIZEOF_PTHREAD_COND_T],
};
pub const PTHREAD_RWLOCK_INITIALIZER: pthread_rwlock_t = pthread_rwlock_t {
size: [0; __SIZEOF_PTHREAD_RWLOCK_T],
};
}
pub const PTHREAD_MUTEX_NORMAL: ::c_int = 0;
pub const PTHREAD_MUTEX_RECURSIVE: ::c_int = 1;
pub const PTHREAD_MUTEX_ERRORCHECK: ::c_int = 2;
Expand Down
17 changes: 17 additions & 0 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,20 @@ macro_rules! f {
macro_rules! __item {
($i:item) => ($i)
}

#[allow(unused_macros)]
macro_rules! align_const {
($($(#[$attr:meta])* pub const $name:ident : $t1:ty = $t2:ident { $($field:tt)* };)*) => ($(
#[cfg(feature = "align")]
$(#[$attr])*
pub const $name : $t1 = $t2 {
$($field)*
};
#[cfg(not(feature = "align"))]
$(#[$attr])*
pub const $name : $t1 = $t2 {
$($field)*
__align: [],
};
)*)
}
2 changes: 2 additions & 0 deletions src/redox/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ s! {
pub s_addr: in_addr_t,
}

#[cfg_attr(feature = "align", repr(align(4)))]
pub struct in6_addr {
pub s6_addr: [u8; 16],
#[cfg(not(feature = "align"))]
__align: [u32; 0],
}

Expand Down
2 changes: 2 additions & 0 deletions src/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,10 @@ s! {
pub s_addr: in_addr_t,
}

#[cfg_attr(feature = "align", repr(align(4)))]
pub struct in6_addr {
pub s6_addr: [u8; 16],
#[cfg(not(feature = "align"))]
__align: [u32; 0],
}

Expand Down
Loading

0 comments on commit 8565755

Please sign in to comment.