Merge pull request #32 from gnzlbg/packed

MacOSX uses #pragma pack 4 to pack all of its structs on x86_64 but mach did not. This resulted in the Rust structs of mach having a different layout than the C structs which is undefined behavior. This PR fixes this using repr(packed) for nightly Rust builds. It adds a new opt-in feature to the crate called unstable that enables the repr_packed feature and it conditionally applies repr(packed(N)) to all structs with incorrect layout.
fitzgen · Apr 20, 2018 · 8ea95a0 · 8ea95a0
2 parents eb5749a + ea395e0
commit 8ea95a0
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 36 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -5,7 +5,7 @@ matrix:
   include:
     # x86_64-apple-darwin
     - env: TARGET=x86_64-apple-darwin
-      rust: 1.11.0
+      rust: 1.13.0
       os: osx
       osx_image: xcode8.3
     - env: TARGET=x86_64-apple-darwin
@@ -31,7 +31,7 @@ matrix:
 
     # i686-apple-darwin
     - env: TARGET=i686-apple-darwin
-      rust: 1.11.0
+      rust: 1.13.0
       os: osx
       osx_image: xcode8.3
     - env: TARGET=i686-apple-darwin
@@ -57,7 +57,7 @@ matrix:
 
     # x86_64-apple-ios
     - env: TARGET=x86_64-apple-ios NORUN=1
-      rust: 1.11.0
+      rust: 1.13.0
       os: osx
       osx_image: xcode8.3
     - env: TARGET=x86_64-apple-ios NORUN=1
@@ -83,7 +83,7 @@ matrix:
 
     # i386-apple-ios
     - env: TARGET=i386-apple-ios NORUN=1
-      rust: 1.11.0
+      rust: 1.13.0
       os: osx
       osx_image: xcode8.3
     - env: TARGET=i386-apple-ios NORUN=1
@@ -109,7 +109,7 @@ matrix:
 
     # aarch64-apple-ios
     - env: TARGET=aarch64-apple-ios NORUN=1 NOCTEST=1
-      rust: 1.11.0
+      rust: 1.13.0
       os: osx
       osx_image: xcode8.3
     - env: TARGET=aarch64-apple-ios NORUN=1 NOCTEST=1
@@ -135,7 +135,7 @@ matrix:
 
     # armv7-apple-ios
     - env: TARGET=armv7-apple-ios NORUN=1 NOCTEST=1
-      rust: 1.11.0
+      rust: 1.13.0
       os: osx
       osx_image: xcode8.3
     - env: TARGET=armv7-apple-ios NORUN=1 NOCTEST=1

diff --git a/Cargo.toml b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 
 name = "mach"
-version = "0.1.2"
+version = "0.2.0"
 authors = ["Nick Fitzgerald <[email protected]>", "David Cuddeback <[email protected]>"]
 
 license = "BSD-2-Clause"
@@ -18,5 +18,7 @@ default-features = false
 [features]
 default = [ "use_std", "deprecated" ]
 use_std = [ "libc/use_std" ]
+# Enables unstable / nightly-only features
+unstable = []
 # Enables deprecated and removed APIs.
 deprecated = []
diff --git a/ci/run.sh b/ci/run.sh
@@ -45,7 +45,7 @@ fi
 
 # Runs ctest to verify mach's ABI against the system libraries:
 if [[ -z "$NOCTEST" ]]; then
-    if [[ $TRAVIS_RUST_VERSION == "beta" ]] || [[ $TRAVIS_RUST_VERSION == "nightly" ]]; then
+    if [[ $TRAVIS_RUST_VERSION == "nightly" ]]; then
         cargo test --manifest-path mach-test/Cargo.toml --target $TARGET --verbose
         cargo test --no-default-features --manifest-path mach-test/Cargo.toml --target $TARGET --verbose
     fi

diff --git a/mach-test/Cargo.toml b/mach-test/Cargo.toml
@@ -5,7 +5,7 @@ authors = ["gnzlbg <[email protected]>"]
 build = "build.rs"
 
 [dependencies]
-mach = { path = ".." }
+mach = { path = "..", features = ["unstable"] }
 libc = "0.2"
 
 [build-dependencies]

diff --git a/mach-test/build.rs b/mach-test/build.rs
@@ -141,15 +141,7 @@ fn main() {
             // FIXME: this type is not exposed in /usr/include/mach
             // but seems to be exposed in
             // SDKs/MacOSX.sdk/System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach
-            "ipc_port" |
-
-            // FIXME: should use repr(packed(4))
-            "task_dyld_info" |
-            "vm_region_basic_info_64" |
-            "vm_region_submap_info_64" |
-            "vm_region_submap_short_info_64" |
-            "mach_vm_read_entry"
-                => true,
+            "ipc_port" => true,
 
             // These are not available in previous MacOSX versions:
             "dyld_kernel_image_info" |
@@ -166,14 +158,7 @@ fn main() {
             // FIXME: this type is not exposed in /usr/include/mach
             // but seems to be exposed in
             // SDKs/MacOSX.sdk/System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach
-            "ipc_port_t" |
-
-            // FIXME: corresponding struct should use repr(packed(4))
-            "vm_region_basic_info_data_64_t" |
-            "vm_region_submap_info_data_64_t"|
-            "vm_region_submap_short_info_data_64_t" |
-            "mach_vm_read_entry_t"
-                => true,
+            "ipc_port_t" => true,
 
             // These are not available in previous MacOSX versions
             "dyld_kernel_image_info_t" |

diff --git a/src/lib.rs b/src/lib.rs
@@ -2,6 +2,7 @@
 #![allow(non_upper_case_globals)]
 
 #![cfg_attr(not(feature = "use_std"), no_std)]
+#![cfg_attr(feature = "unstable", feature(repr_packed))]
 
 #[cfg(feature = "use_std")]
 extern crate core;

diff --git a/src/task_info.rs b/src/task_info.rs
@@ -35,8 +35,7 @@ pub type task_flavor_t = natural_t;
 pub type task_info_t = *mut integer_t;
 
 #[repr(C)]
-// Undefined behavior: should be #[repr(packed(4))] once
-// that is stable: https://github.com/rust-lang/rust/issues/33158
+#[cfg_attr(feature = "unstable", repr(packed(4)))]
 pub struct task_dyld_info {
     pub all_image_info_addr: mach_vm_address_t,
     pub all_image_info_size: mach_vm_size_t,

diff --git a/src/vm_region.rs b/src/vm_region.rs
@@ -57,9 +57,8 @@ pub const SM_PRIVATE_ALIASED: ::libc::c_uchar = 6;
 pub const SM_SHARED_ALIASED: ::libc::c_uchar  = 7;
 
 #[repr(C)]
+#[cfg_attr(feature = "unstable", repr(packed(4)))]
 #[derive(Copy, Clone, Debug)]
-// Undefined behavior: should be #[repr(packed(4))] once
-// that is stable: https://github.com/rust-lang/rust/issues/33158
 pub struct vm_region_basic_info_64 {
     pub protection: vm_prot_t,
     pub max_protection: vm_prot_t,
@@ -163,9 +162,8 @@ impl vm_region_submap_info {
 }
 
 #[repr(C)]
+#[cfg_attr(feature = "unstable", repr(packed(4)))]
 #[derive(Copy, Clone, Debug)]
-// Undefined behavior: should be #[repr(packed(4))] once
-// that is stable: https://github.com/rust-lang/rust/issues/33158
 pub struct vm_region_submap_info_64 {
     pub protection: vm_prot_t,
     pub max_protection: vm_prot_t,
@@ -194,9 +192,8 @@ impl vm_region_submap_info_64 {
 }
 
 #[repr(C)]
+#[cfg_attr(feature = "unstable", repr(packed(4)))]
 #[derive(Copy, Clone, Debug)]
-// Undefined behavior: should be #[repr(packed(4))] once
-// that is stable: https://github.com/rust-lang/rust/issues/33158
 pub struct vm_region_submap_short_info_64 {
     pub protection: vm_prot_t,
     pub max_protection: vm_prot_t,
@@ -237,9 +234,8 @@ impl vm_page_info_basic {
 }
 
 #[repr(C)]
+#[cfg_attr(feature = "unstable", repr(packed(4)))]
 #[derive(Copy, Clone, Debug)]
-// Undefined behavior: should be #[repr(packed(4))] once
-// that is stable: https://github.com/rust-lang/rust/issues/33158
 pub struct mach_vm_read_entry {
     pub address: mach_vm_address_t,
     pub size: mach_vm_size_t,