Repository: ezrosent/allocators-rs (language: rust)

Issue:
Add committing back to mmap-alloc on Linux/Mac with madvise/MADV_WILLNEED?
We currently don't provide the `commit` method on mmap-alloc's `MapAlloc` on Linux and Mac because pages are always committed dynamically when they are first accessed (whereas, on Windows, accessing an uncommitted page will cause a runtime fault).

However, on Linux, we *do* allow `MapAlloc` to be configured to commit memory that it allocates by passing the `MAP_POPULATE` flag to `mmap`. Thus, perhaps it would be appropriate by to provide a `commit` method that is backed by a call to `madvise` with its equivalent flag, `MADV_WILLNEED`? It provides essentially the same guarantees as `MAP_POPULATE` (namely, it's only a hint to the kernel and it doesn't change any observable behavior other than possibly performance), which we already provide via the commit configuration.

Thoughts?


Discussion / hints:

Fix patch:
diff --git a/mmap-alloc/CHANGELOG.md b/mmap-alloc/CHANGELOG.md
index 8559599..d669908 100644
--- a/mmap-alloc/CHANGELOG.md
+++ b/mmap-alloc/CHANGELOG.md
@@ -13,9 +13,16 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+### Added
+- Added support for committing on Linux and Mac
+- Added support for configuring commit on allocation on Mac
+- Added support for alignments larger than a memory page (`large-align`
+  feature)
+
 ### Changed
 - Upgraded to new `UntypedObjectAlloc` trait that uses `NonNull<u8>` instead
   of `*mut u8`
+- Improved documentation on committed vs. uncommitted memory
 
 ## 0.2.0
 
diff --git a/mmap-alloc/Cargo.toml b/mmap-alloc/Cargo.toml
index 23425d8..ea2034d 100644
--- a/mmap-alloc/Cargo.toml
+++ b/mmap-alloc/Cargo.toml
@@ -21,6 +21,11 @@ repository = "https://github.com/ezrosent/allocators-rs/tree/master/mmap-alloc"
 
 exclude = ["appveyor.sh", "travis.sh"]
 
+[features]
+# Support alignments larger than a page size by mapping a larger region
+# and then unmapping all but a properly-aligned subset of the region.
+large-align = []
+
 [dependencies]
 errno = "0.2"
 kernel32-sys = "0.2"
diff --git a/mmap-alloc/appveyor.sh b/mmap-alloc/appveyor.sh
index 05b6379..09265cd 100755
--- a/mmap-alloc/appveyor.sh
+++ b/mmap-alloc/appveyor.sh
@@ -1,6 +1,6 @@
 #!/bin/bash
 
-# Copyright 2017 the authors. See the 'Copyright and license' section of the
+# Copyright 2017-2018 the authors. See the 'Copyright and license' section of the
 # README.md file at the top-level directory of this repository.
 #
 # Licensed under the Apache License, Version 2.0 (the LICENSE-APACHE file) or
@@ -17,4 +17,6 @@ fi
 export RUST_TEST_THREADS=1
 
 cargo build
-RUST_BACKTRACE=1 cargo test
+for feature in '' large-align; do
+  RUST_BACKTRACE=1 cargo test --features "$feature"
+done
diff --git a/mmap-alloc/src/lib.rs b/mmap-alloc/src/lib.rs
index 16ced21..e3da03b 100644
--- a/mmap-alloc/src/lib.rs
+++ b/mmap-alloc/src/lib.rs
@@ -50,6 +50,27 @@ use errno::errno;
 /// constructed using `default`, and then various other methods are used to set various
 /// configuration options.
 ///
+/// # Committed and Uncommitted Memory
+///
+/// Memory pages allocated by the kernel are [virtual memory pages][virtual memory]. Virtual
+/// memory pages can be in two states - committed or uncommitted. When in the uncommitted state,
+/// there is not necessarily any physical memory or swap space on disk associated with the page.
+/// When in the committed state, there is. When a page is allocated, it starts off in the
+/// uncommitted state (although the `commit` method can be used to configure this behavior).
+/// When an uncommitted page is committed, a physical memory page or region of swap space is
+/// allocated and associated with it. This memory has its bytes initialized to zero. When a
+/// committed page is uncommitted, the kernel is free to reclaim these physical resources, and
+/// in doing so, may wipe away the contents of the page (such that, if the page is later committed
+/// again, its contents will have been set back to zero).
+///
+/// On Linux and Mac, if an uncommitted page is accessed (read, written, or executed), it is
+/// automatically committed by the kernel. Thus, while explicitly committing memory may improve
+/// performance, it does not change the behavior of the program in any observable way.
+///
+/// On Windows, if an uncommitted page is accessed, it will cause a fault. Thus, Windows memory
+/// must be explicitly committed before use, and the uncommitted/committed distinction affects
+/// the observable behavior of the program.
+///
 /// # Memory Permissions
 ///
 /// One aspect that can be configured is the permissions of allocated memory - readable, writable,
@@ -81,6 +102,35 @@ use errno::errno;
 /// In order to avoid this scenario, it is often necessary to explicitly flush the instruction
 /// cache before executing newly-written memory. The mechanism to accomplish this differs by
 /// system.
+///
+/// # Alignment
+///
+/// Since memory is allocated by mapping pages directly from the kernel, all allocations are
+/// automatically aligned to the size of a memory page. Since there is no way to request a
+/// higher alignment from the kernel, higher alignments are not supported by `MapAlloc` by default.
+///
+/// However, this crate provides the `large-align` feature (disabled by default) which works around
+/// this limitation to allow for arbitrarily large alignments. When an alignment larger than the page
+/// size is requested, `alignment - page size` extra bytes are allocated. There is guaranteed to be
+/// a properly-aligned chunk of memory of the requested size somewhere within this larger allocation.
+/// That chunk is located and returned. The `large-align` feature also provides an `object_align`
+/// method on `MapAllocBuilder` that configures the alignment of the `UntypedObjectAlloc` implementation.
+///
+/// ## Platform-specific behavior
+///
+/// On Linux and Mac, all extra leading and trailing memory surrounding the located chunk is
+/// deallocated, leaving only the requested object allocated. On Windows, that extra memory is left
+/// allocated, but is left in the uncommitted state. This should ensure that it does not consume any
+/// physical resources, although it will still consume virtual memory space, which may be scarce,
+/// especially on 32-bit platforms.
+///
+/// ## Limitations
+///
+/// Currently, the `large-align` feature does not support large alignments in the `realloc` method,
+/// although it is fine to pass an object which was previously allocated with a large alignment
+/// as the object to be reallocated (it may have a smaller alignment after being reallocated).
+///
+/// [virtual memory]: https://en.wikipedia.org/wiki/Virtual_memory
 pub struct MapAllocBuilder {
     read: bool,
     write: bool,
@@ -90,7 +140,9 @@ pub struct MapAllocBuilder {
     // sysconf::page::pagesize might be inefficient, so store a copy of the pagesize to ensure that
     // loading it is efficient
     pagesize: usize,
+    // used for the UntypedObjectAlloc implementation
     obj_size: Option<usize>,
+    obj_align: Option<usize>,
 }
 
 impl MapAllocBuilder {
@@ -107,6 +159,19 @@ impl MapAllocBuilder {
         } else {
             self.pagesize
         };
+        let obj_align = if let Some(obj_align) = self.obj_align {
+            assert_eq!(
+                obj_size % obj_align,
+                0,
+                "object size ({}) is not a multiple of the object alignment ({})",
+                obj_size,
+                obj_align,
+            );
+            obj_align
+        } else {
+            self.pagesize
+        };
+
         MapAlloc {
             pagesize: self.pagesize,
             read: self.read,
@@ -114,7 +179,7 @@ impl MapAllocBuilder {
             exec: self.exec,
             perms: perms::get_perm(self.read, self.write, self.exec),
             commit: self.commit,
-            obj_size: obj_size,
+            obj_layout: Layout::from_size_align(obj_size, obj_align).unwrap(),
         }
     }
 
@@ -170,10 +235,8 @@ impl MapAllocBuilder {
     /// `commit` configures whether the memory returned by `alloc` is already in a committed state.
     /// The default is to have memory be returned by `alloc` uncommitted.
     ///
-    /// # Platform-specific behavior
-    ///
-    /// `commit` is only supported on Linux and Windows.
-    #[cfg(any(target_os = "linux", windows))]
+    /// See the "Committed and Uncommitted Memory" section of the `MapAllocBuilder` documentation
+    /// for more details.
     pub fn commit(mut self, commit: bool) -> MapAllocBuilder {
         self.commit = commit;
         self
@@ -188,6 +251,17 @@ impl MapAllocBuilder {
         self.obj_size = Some(obj_size);
         self
     }
+
+    /// Sets the object alignment for the `UntypedObjectAlloc` implementation.
+    ///
+    /// See the "Alignment" section of the `MapAllocBuilder` documentation for more details.
+    ///
+    /// This method is only available when the `large-align` feature is enabled.
+    #[cfg(feature = "large-align")]
+    pub fn obj_align(mut self, obj_align: usize) -> MapAllocBuilder {
+        self.obj_align = Some(obj_align);
+        self
+    }
 }
 
 impl Default for MapAllocBuilder {
@@ -199,6 +273,7 @@ impl Default for MapAllocBuilder {
             commit: false,
             pagesize: sysconf::page::pagesize(),
             obj_size: None,
+            obj_align: None,
         }
     }
 }
@@ -207,12 +282,16 @@ pub struct MapAlloc {
     // sysconf::page::pagesize might be inefficient, so store a copy of the pagesize to ensure that
     // loading it is efficient
     pagesize: usize,
-    #[cfg_attr(target_os = "linux", allow(unused))] read: bool,
-    #[cfg_attr(target_os = "linux", allow(unused))] write: bool,
-    #[cfg_attr(target_os = "linux", allow(unused))] exec: bool,
+    #[cfg_attr(target_os = "linux", allow(unused))]
+    read: bool,
+    #[cfg_attr(target_os = "linux", allow(unused))]
+    write: bool,
+    #[cfg_attr(target_os = "linux", allow(unused))]
+    exec: bool,
     perms: perms::Perm,
     commit: bool,
-    obj_size: usize,
+    // used in UntypedObjectAlloc implementation
+    obj_layout: Layout,
 }
 
 impl Default for MapAlloc {
@@ -224,26 +303,23 @@ impl Default for MapAlloc {
 impl MapAlloc {
     /// Commits an existing allocated object.
     ///
-    /// `commit` moves the given object into the committed state. After `commit` has returned, the
-    /// object can be accessed without crashing the program. If this `MapAlloc` was configured with
-    /// the "commit" option disabled (the default), then allocated objects are unusable until they
-    /// have been committed by this method.
-    ///
-    /// If `commit` is called on an already-committed object, it does nothing.
+    /// `commit` moves the given object into the committed state. If `commit` is called on an
+    /// already-committed object, it does nothing.
     ///
-    /// # Platform-specific behavior
-    ///
-    /// `commit` is only present on Windows. On Linux and Mac, uncommitted memory is automatically
-    /// committed upon the first access.
-    #[cfg(windows)]
+    /// See the "Committed and Uncommitted Memory" section of the `MapAllocBuilder` documentation
+    /// for more details.
+    #[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))]
     pub unsafe fn commit(&self, ptr: *mut u8, layout: Layout) {
         debug_assert!(layout.size() > 0, "commit: size of layout must be non-zero");
 
         // TODO: What to do about sizes that are not multiples of the page size? These are legal
-        // allocations, and so they are legal to pass to uncommit, but will VirtualFree handle them
-        // properly?
+        // allocations, and so they are legal to pass to uncommit, but will madvise/VirtualFree
+        // handle them properly?
         #[cfg(debug_assertions)]
-        self.debug_verify_ptr(ptr, layout.clone());
+        self.debug_verify_ptr(ptr, &layout);
+        #[cfg(any(target_os = "linux", target_os = "macos"))]
+        commit(ptr, layout.size());
+        #[cfg(windows)]
         commit(ptr, layout.size(), self.perms);
     }
 
@@ -252,12 +328,9 @@ impl MapAlloc {
     /// `uncommit` moves the given object into the uncommitted state. If `uncommit` is called on
     /// an already-uncommitted object, it does nothing.
     ///
-    /// # Platform-specific behavior
-    ///
-    /// On Windows, after `uncommit` has returned, the object cannot be accessed without crashing
-    /// the program. On Linux and Mac, the memory can still safely be accessed, but it may have
-    /// been zeroed.
-    #[cfg(any(target_os = "linux", target_os = "macos", windows))]
+    /// See the "Committed and Uncommitted Memory" section of the `MapAllocBuilder` documentation
+    /// for more details.
+    #[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))]
     pub unsafe fn uncommit(&self, ptr: *mut u8, layout: Layout) {
         debug_assert!(
             layout.size() > 0,
@@ -268,7 +341,7 @@ impl MapAlloc {
         // allocations, and so they are legal to pass to uncommit, but will madvise handle them
         // properly?
         #[cfg(debug_assertions)]
-        self.debug_verify_ptr(ptr, layout.clone());
+        self.debug_verify_ptr(ptr, &layout);
         uncommit(ptr, layout.size());
     }
 
@@ -276,8 +349,8 @@ impl MapAlloc {
     unsafe fn resize_in_place(
         &self,
         ptr: *mut u8,
-        layout: Layout,
-        new_layout: Layout,
+        layout: &Layout,
+        new_layout: &Layout,
     ) -> Result<(), CannotReallocInPlace> {
         // alignment less than a page is fine because page-aligned objects are also aligned to
         // any alignment less than a page
@@ -300,7 +373,7 @@ impl MapAlloc {
     }
 
     #[cfg(debug_assertions)]
-    fn debug_verify_ptr(&self, ptr: *mut u8, layout: Layout) {
+    fn debug_verify_ptr(&self, ptr: *mut u8, layout: &Layout) {
         debug_assert_eq!(
             ptr as usize % self.pagesize,
             0,
@@ -316,16 +389,49 @@ unsafe impl<'a> Alloc for &'a MapAlloc {
     unsafe fn alloc(&mut self, layout: Layout) -> Result<*mut u8, AllocErr> {
         debug_assert!(layout.size() > 0, "alloc: size of layout must be non-zero");
 
+        let size = next_multiple(layout.size(), self.pagesize);
+
         // alignment less than a page is fine because page-aligned objects are also aligned to
         // any alignment less than a page
-        if layout.align() > self.pagesize {
-            return Err(AllocErr::invalid_input(
+        if layout.align() <= self.pagesize {
+            map(size, self.perms, self.commit).ok_or(AllocErr::Exhausted { request: layout })
+        } else if cfg!(feature = "large-align") {
+            let extra = layout.align() - self.pagesize;
+            // Since we're allocating extra space, we don't commit this memory.
+            // We only commit the portion we're actually returning below.
+            let addr = map(size + extra, self.perms, false).ok_or(AllocErr::Exhausted {
+                request: layout.clone(),
+            })? as usize;
+            let aligned_addr = next_multiple(addr, layout.align());
+            let aligned_ptr = aligned_addr as *mut u8;
+            if !cfg!(windows) {
+                // We only do this on Linux and Mac because on Windows, you can only
+                // unmap entire regions allocated with VirtualAlloc. Thus, on Windows,
+                // the best we can do is to leave the unused portions of the allocation
+                // unmapped. This consumes extra virtual address space, which may be
+                // problematic on 32-bit platforms or when virtual address space is
+                // scarce for some other reason.
+                let prefix_size = aligned_addr - addr;
+                let suffix_size = extra - prefix_size;
+                if prefix_size > 0 {
+                    unmap(addr as *mut u8, prefix_size);
+                }
+                if suffix_size > 0 {
+                    unmap((aligned_addr + size) as *mut u8, suffix_size);
+                }
+            }
+            if self.commit {
+                #[cfg(any(target_os = "linux", target_os = "macos"))]
+                commit(aligned_ptr, layout.size());
+                #[cfg(windows)]
+                commit(aligned_ptr, layout.size(), self.perms);
+            }
+            Ok(aligned_ptr)
+        } else {
+            Err(AllocErr::invalid_input(
                 "cannot support alignment greater than a page",
-            ));
+            ))
         }
-
-        let size = next_multiple(layout.size(), self.pagesize);
-        map(size, self.perms, self.commit).ok_or(AllocErr::Exhausted { request: layout })
     }
 
     unsafe fn dealloc(&mut self, ptr: *mut u8, layout: Layout) {
@@ -333,6 +439,31 @@ unsafe impl<'a> Alloc for &'a MapAlloc {
             layout.size() > 0,
             "dealloc: size of layout must be non-zero"
         );
+        #[cfg(all(feature = "large-align", windows))]
+        let ptr = {
+            use core::mem::{size_of, uninitialized};
+            use kernel32::{GetLastError, VirtualQuery};
+            use winapi::winnt::MEMORY_BASIC_INFORMATION;
+
+            // Because Windows' VirtualFree function requires its argument
+            // to be a value previously returned from VirtualAlloc, we cannot
+            // simply pass ptr, as the large-align feature means ptr might
+            // not be the value we originally allocated with VirtualAlloc.
+            // We use VirtualQuery here to recover the original value
+            // returned from VirtualAlloc.
+            let mut info: MEMORY_BASIC_INFORMATION = uninitialized();
+            let info_size = size_of::<MEMORY_BASIC_INFORMATION>() as u64;
+            let ret = VirtualQuery(ptr as *mut _, &mut info, info_size);
+            assert_eq!(
+                ret,
+                info_size,
+                "Call to VirtualQuery({:?}) failed with error code {}.",
+                ptr,
+                GetLastError()
+            );
+            info.AllocationBase as *mut _
+        };
+
         unmap(ptr, layout.size());
     }
 
@@ -369,6 +500,8 @@ unsafe impl<'a> Alloc for &'a MapAlloc {
             "realloc: size of new_layout must be non-zero"
         );
 
+        // TODO: Handle large-align feature
+
         // alignment less than a page is fine because page-aligned objects are also aligned to
         // any alignment less than a page
         if new_layout.align() > self.pagesize {
@@ -405,6 +538,8 @@ unsafe impl<'a> Alloc for &'a MapAlloc {
             "realloc: size of new_layout must be non-zero"
         );
 
+        // TODO: Handle large-align feature
+
         // alignment less than a page is fine because page-aligned objects are also aligned to
         // any alignment less than a page
         if new_layout.align() > self.pagesize {
@@ -472,7 +607,7 @@ unsafe impl<'a> Alloc for &'a MapAlloc {
         debug_assert!(new_layout.size() >= layout.size());
         debug_assert_eq!(new_layout.align(), layout.align());
 
-        self.resize_in_place(ptr, layout, new_layout)
+        self.resize_in_place(ptr, &layout, &new_layout)
     }
 
     #[cfg(target_os = "linux")]
@@ -493,7 +628,7 @@ unsafe impl<'a> Alloc for &'a MapAlloc {
         debug_assert!(new_layout.size() <= layout.size());
         debug_assert_eq!(new_layout.align(), layout.align());
 
-        self.resize_in_place(ptr, layout, new_layout)
+        self.resize_in_place(ptr, &layout, &new_layout)
     }
 
     #[cfg(target_os = "macos")]
@@ -527,11 +662,7 @@ unsafe impl<'a> Alloc for &'a MapAlloc {
 
 unsafe impl<'a> UntypedObjectAlloc for &'a MapAlloc {
     fn layout(&self) -> Layout {
-        if cfg!(debug_assertions) {
-            Layout::from_size_align(self.obj_size, self.pagesize).unwrap()
-        } else {
-            unsafe { Layout::from_size_align_unchecked(self.obj_size, self.pagesize) }
-        }
+        self.obj_layout.clone()
     }
 
     unsafe fn alloc(&mut self) -> Result<NonNull<u8>, Exhausted> {
@@ -544,7 +675,7 @@ unsafe impl<'a> UntypedObjectAlloc for &'a MapAlloc {
     }
 
     unsafe fn dealloc(&mut self, ptr: NonNull<u8>) {
-        unmap(ptr.as_ptr(), self.obj_size);
+        <&MapAlloc as Alloc>::dealloc(self, ptr.as_ptr(), self.obj_layout.clone());
     }
 }
 
@@ -664,13 +795,10 @@ unsafe fn map(size: usize, perms: i32, commit: bool) -> Option<*mut u8> {
     }
 }
 
-// commit must be false
 #[cfg(target_os = "macos")]
-unsafe fn map(size: usize, perms: i32, commit: bool) -> Option<*mut u8> {
+unsafe fn map(size: usize, perms: i32, do_commit: bool) -> Option<*mut u8> {
     use libc::{ENOMEM, MAP_ANON, MAP_FAILED, MAP_PRIVATE};
 
-    debug_assert!(!commit);
-
     let ptr = libc::mmap(ptr::null_mut(), size, perms, MAP_ANON | MAP_PRIVATE, -1, 0);
 
     if ptr == MAP_FAILED {
@@ -686,6 +814,11 @@ unsafe fn map(size: usize, perms: i32, commit: bool) -> Option<*mut u8> {
         // implementation selects a value for pa, it never places a mapping at address 0, nor does
         // it replace any extant mapping."
         assert_ne!(ptr, ptr::null_mut(), "mmap returned NULL");
+        if do_commit {
+            // Mac mmap doesn't have an equivalent of the Linux MAP_POPULATE flag,
+            // so we commit memory in a separate step.
+            commit(ptr as *mut u8, size);
+        }
         Some(ptr as *mut u8)
     }
 }
@@ -801,6 +934,12 @@ unsafe fn protect(ptr: *mut u8, size: usize, perm: perms::Perm) {
     );
 }
 
+#[cfg(any(target_os = "linux", target_os = "macos"))]
+unsafe fn commit(ptr: *mut u8, size: usize) {
+    use libc::{madvise, MADV_WILLNEED};
+    madvise(ptr as *mut _, size, MADV_WILLNEED);
+}
+
 #[cfg(windows)]
 unsafe fn commit(ptr: *mut u8, size: usize, perms: u32) {
     use kernel32::VirtualAlloc;
@@ -812,17 +951,17 @@ unsafe fn commit(ptr: *mut u8, size: usize, perms: u32) {
 
 #[cfg(target_os = "linux")]
 unsafe fn uncommit(ptr: *mut u8, size: usize) {
-    use libc::MADV_DONTNEED;
+    use libc::{madvise, MADV_DONTNEED};
 
     // TODO: Other options such as MADV_FREE are available on newer versions of Linux. Is there
     // a way that we can use those when available? Is that even desirable?
-    libc::madvise(ptr as *mut _, size, MADV_DONTNEED);
+    madvise(ptr as *mut _, size, MADV_DONTNEED);
 }
 
 #[cfg(target_os = "macos")]
 unsafe fn uncommit(ptr: *mut u8, size: usize) {
-    use libc::MADV_FREE;
-    libc::madvise(ptr as *mut _, size, MADV_FREE);
+    use libc::{madvise, MADV_FREE};
+    madvise(ptr as *mut _, size, MADV_FREE);
 }
 
 #[cfg(windows)]
diff --git a/mmap-alloc/travis.sh b/mmap-alloc/travis.sh
index c038e0a..ae3dcbe 100755
--- a/mmap-alloc/travis.sh
+++ b/mmap-alloc/travis.sh
@@ -1,6 +1,6 @@
 #!/bin/bash
 
-# Copyright 2017 the authors. See the 'Copyright and license' section of the
+# Copyright 2017-2018 the authors. See the 'Copyright and license' section of the
 # README.md file at the top-level directory of this repository.
 #
 # Licensed under the Apache License, Version 2.0 (the LICENSE-APACHE file) or
@@ -17,4 +17,7 @@ travis-cargo --only nightly build
 # (e.g., see https://travis-ci.org/ezrosent/allocators-rs/jobs/291713981)
 # TODO: Remove -q and --verbose once the following issue is fixed:
 # https://github.com/huonw/travis-cargo/issues/75
-RUST_BACKTRACE=1 travis-cargo -q --only nightly test -- --verbose -- --skip test_map_panic_too_large
+for feature in '' large-align; do
+  RUST_BACKTRACE=1 travis-cargo -q --only nightly test -- --features "$feature" \
+    --verbose -- --skip test_map_panic_too_large  
+done


Test patch:
diff --git a/mmap-alloc/src/tests.rs b/mmap-alloc/src/tests.rs
index f0a1e6f..ba8fb16 100644
--- a/mmap-alloc/src/tests.rs
+++ b/mmap-alloc/src/tests.rs
@@ -1,4 +1,4 @@
-// Copyright 2017 the authors. See the 'Copyright and license' section of the
+// Copyright 2017-2018 the authors. See the 'Copyright and license' section of the
 // README.md file at the top-level directory of this repository.
 //
 // Licensed under the Apache License, Version 2.0 (the LICENSE-APACHE file) or
@@ -41,13 +41,9 @@ unsafe fn test_read(ptr: *mut u8, size: usize) {
         let got = *ptr.offset(i as isize);
         let want = (i & 0xff) as u8;
         assert_eq!(
-            got,
-            want,
+            got, want,
             "mismatch at byte {} in block {:?}: got {}, want {}",
-            i,
-            ptr,
-            got,
-            want
+            i, ptr, got, want
         );
     }
 }
@@ -145,12 +141,14 @@ fn test_realloc_small() {
         #[cfg(any(target_os = "linux", windows))]
         let perm = get_perm(read, write, exec);
 
-        let test_contents = |ptr: *mut u8, size: usize| if read && write {
-            test_write_read(ptr, size);
-        } else if read {
-            test_zero_filled(ptr, size);
-        } else if write {
-            test_write(ptr, size);
+        let test_contents = |ptr: *mut u8, size: usize| {
+            if read && write {
+                test_write_read(ptr, size);
+            } else if read {
+                test_zero_filled(ptr, size);
+            } else if write {
+                test_write(ptr, size);
+            }
         };
 
         let small = Layout::array::<u8>(1).unwrap();
@@ -235,12 +233,14 @@ fn test_realloc_large() {
         #[cfg(any(target_os = "linux", windows))]
         let perm = get_perm(read, write, exec);
 
-        let test_contents = |ptr: *mut u8, size: usize| if read && write {
-            test_write_read(ptr, size);
-        } else if read {
-            test_zero_filled(ptr, size);
-        } else if write {
-            test_write(ptr, size);
+        let test_contents = |ptr: *mut u8, size: usize| {
+            if read && write {
+                test_write_read(ptr, size);
+            } else if read {
+                test_zero_filled(ptr, size);
+            } else if write {
+                test_write(ptr, size);
+            }
         };
 
         let small = Layout::array::<u8>(alloc.pagesize).unwrap();
@@ -350,14 +350,31 @@ fn test_realloc_large() {
     }
 
     unsafe {
-        // test(false, false, false);
-        // test(true, false, false);
-        // test(false, true, false);
-        // test(false, false, true);
+        test(false, false, false);
+        test(true, false, false);
+        test(false, true, false);
+        test(false, false, true);
         test(true, true, false);
-        // test(true, false, true);
-        // test(false, true, true);
-        // test(true, true, true);
+        test(true, false, true);
+        test(false, true, true);
+        test(true, true, true);
+    }
+}
+
+#[cfg(feature = "large-align")]
+#[test]
+fn test_large_align() {
+    let mut alloc = MapAllocBuilder::default().commit(true).build();
+    // test with a large alignment so that the probability of getting this
+    // alignment by chance is low.
+    const MB64: usize = 1 << 25;
+    let layout = Layout::from_size_align(MB64, MB64).unwrap();
+    unsafe {
+        let ptr = <MapAlloc as Alloc>::alloc(&mut alloc, layout.clone()).unwrap();
+        assert_eq!(ptr as usize % MB64, 0, "ptr: {:?}", ptr);
+        test_zero_filled(ptr, MB64);
+        test_write_read(ptr, MB64);
+        <MapAlloc as Alloc>::dealloc(&mut alloc, ptr, layout.clone());
     }
 }
 
@@ -365,13 +382,12 @@ fn test_realloc_large() {
 fn test_commit() {
     unsafe {
         // Check that:
-        // - Mapping and committing a single page works (except on Mac, which doesn't support
-        //   committing)
+        // - Mapping and committing a single page works
         // - The returned pointer is non-null
         // - The returned pointer is page-aligned
         // - We can read that page, and it is zero-filled (on Unix, this test is trivial, but
         //   on Windows, it ensures that map properly committed the page)
-        let mut ptr = map(pagesize(), PROT_READ_WRITE, !cfg!(target_os = "macos")).unwrap();
+        let mut ptr = map(pagesize(), PROT_READ_WRITE, true).unwrap();
         test_valid_map_address(ptr);
         test_zero_filled(ptr, pagesize());
         unmap(ptr, pagesize());