From 294a43b9d0a70f4d086c7e76d9551c5248286758 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Sat, 2 Oct 2021 11:37:52 -0700 Subject: [PATCH 1/7] Avoid passing `null_mut()` to `ptr::offset`. Split `Chunk::mem_offset` out of `Chunk::to_mem`, and use that to avoid passing a null pointer to `Chunk::to_mem`, which calls `ptr::offset`, which Miri diagnoses as Undefined Behavior. Fixes #19. --- src/dlmalloc.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/dlmalloc.rs b/src/dlmalloc.rs index d16f523..a4b4b71 100644 --- a/src/dlmalloc.rs +++ b/src/dlmalloc.rs @@ -193,11 +193,15 @@ impl Dlmalloc { } fn align_offset(&self, addr: *mut u8) -> usize { - align_up(addr as usize, self.malloc_alignment()) - (addr as usize) + self.align_offset_usize(addr as usize) + } + + fn align_offset_usize(&self, addr: usize) -> usize { + align_up(addr, self.malloc_alignment()) - (addr as usize) } fn top_foot_size(&self) -> usize { - self.align_offset(unsafe { Chunk::to_mem(ptr::null_mut()) }) + self.align_offset_usize(Chunk::mem_offset() as usize) + self.pad_request(mem::size_of::()) + self.min_chunk_size() } @@ -1723,7 +1727,11 @@ impl Chunk { } unsafe fn to_mem(me: *mut Chunk) -> *mut u8 { - (me as *mut u8).offset(2 * (mem::size_of::() as isize)) + (me as *mut u8).offset(Chunk::mem_offset()) + } + + fn mem_offset() -> isize { + 2 * (mem::size_of::() as isize) } unsafe fn from_mem(mem: *mut u8) -> *mut Chunk { From 3bc9ca2d9ae0f5d356f96b7c0aa4e80bca161d27 Mon Sep 17 00:00:00 2001 From: Niv Kaminer Date: Sun, 24 Oct 2021 18:47:23 +0300 Subject: [PATCH 2/7] add feature to make allocations safe in fork --- Cargo.toml | 4 ++++ src/linux.rs | 19 ++++++++++++++++++- src/macos.rs | 19 ++++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 56fe4a8..d322f82 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ libc = { version = "0.2", default-features = false } # `src/tools/rustc-std-workspace` folder core = { version = '1.0.0', optional = true, package = 'rustc-std-workspace-core' } compiler_builtins = { version = '0.1.0', optional = true } +once_cell = { version = '1.8', default-features= false, optional = true } [dev-dependencies] rand = "0.3" @@ -40,4 +41,7 @@ global = [] # Enable very expensive debug checks in this crate debug = [] +# makes allocations safe for use during `fork(2)` +fork-safe = ["once_cell"] + rustc-dep-of-std = ['core', 'compiler_builtins/rustc-dep-of-std'] diff --git a/src/linux.rs b/src/linux.rs index 42e6549..c61d51d 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -75,7 +75,24 @@ unsafe impl Allocator for System { #[cfg(feature = "global")] pub fn acquire_global_lock() { - unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } + fn _acquire_global_lock() { + unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } + } + + #[cfg(feature = "fork-safe")] + unsafe { + static EXEC_ONCE: once_cell::sync::OnceCell(); + + EXEC_ONCE.get_or_init(|| { + libc::pthread_atfork( + _acquire_global_lock, + release_global_lock, + release_global_lock, + ) + }); + } + + _acquire_global_lock(); } #[cfg(feature = "global")] diff --git a/src/macos.rs b/src/macos.rs index d131271..7f0f605 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -63,7 +63,24 @@ unsafe impl Allocator for System { #[cfg(feature = "global")] pub fn acquire_global_lock() { - unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } + fn _acquire_global_lock() { + unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } + } + + #[cfg(feature = "fork-safe")] + unsafe { + static EXEC_ONCE: once_cell::sync::OnceCell(); + + EXEC_ONCE.get_or_init(|| { + libc::pthread_atfork( + _acquire_global_lock, + release_global_lock, + release_global_lock, + ) + }); + } + + _acquire_global_lock(); } #[cfg(feature = "global")] From 4c7334aec6436816e7cd4c5aa275e0ac549e67d1 Mon Sep 17 00:00:00 2001 From: Niv Kaminer Date: Mon, 25 Oct 2021 23:42:06 +0300 Subject: [PATCH 3/7] use existing global lock to protect against fork --- Cargo.toml | 4 ---- src/linux.rs | 33 ++++++++++++++++++++------------- src/macos.rs | 33 ++++++++++++++++++++------------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d322f82..56fe4a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,6 @@ libc = { version = "0.2", default-features = false } # `src/tools/rustc-std-workspace` folder core = { version = '1.0.0', optional = true, package = 'rustc-std-workspace-core' } compiler_builtins = { version = '0.1.0', optional = true } -once_cell = { version = '1.8', default-features= false, optional = true } [dev-dependencies] rand = "0.3" @@ -41,7 +40,4 @@ global = [] # Enable very expensive debug checks in this crate debug = [] -# makes allocations safe for use during `fork(2)` -fork-safe = ["once_cell"] - rustc-dep-of-std = ['core', 'compiler_builtins/rustc-dep-of-std'] diff --git a/src/linux.rs b/src/linux.rs index c61d51d..27ba688 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -75,24 +75,31 @@ unsafe impl Allocator for System { #[cfg(feature = "global")] pub fn acquire_global_lock() { - fn _acquire_global_lock() { - unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } - } + static mut FORK_PROTECTED: bool = false; - #[cfg(feature = "fork-safe")] unsafe { - static EXEC_ONCE: once_cell::sync::OnceCell(); - - EXEC_ONCE.get_or_init(|| { + assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0); + if !FORK_PROTECTED { + // ensures that if a process forks, + // it will acquire the lock before any other thread, + // protecting it from deadlock, + // when the child is created with only that thread. libc::pthread_atfork( - _acquire_global_lock, - release_global_lock, - release_global_lock, - ) - }); + Some(_acquire_global_lock), + Some(_release_global_lock), + Some(_release_global_lock), + ); + FORK_PROTECTED = true; + } } - _acquire_global_lock(); + unsafe extern "C" fn _acquire_global_lock() { + assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) + } + + unsafe extern "C" fn _release_global_lock() { + release_global_lock() + } } #[cfg(feature = "global")] diff --git a/src/macos.rs b/src/macos.rs index 7f0f605..40b90ce 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -63,24 +63,31 @@ unsafe impl Allocator for System { #[cfg(feature = "global")] pub fn acquire_global_lock() { - fn _acquire_global_lock() { - unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } - } + static mut FORK_PROTECTED: bool = false; - #[cfg(feature = "fork-safe")] unsafe { - static EXEC_ONCE: once_cell::sync::OnceCell(); - - EXEC_ONCE.get_or_init(|| { + assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0); + if !FORK_PROTECTED { + // ensures that if a process forks, + // it will acquire the lock before any other thread, + // protecting it from deadlock, + // when the child is created with only that thread. libc::pthread_atfork( - _acquire_global_lock, - release_global_lock, - release_global_lock, - ) - }); + Some(_acquire_global_lock), + Some(_release_global_lock), + Some(_release_global_lock), + ); + FORK_PROTECTED = true; + } } - _acquire_global_lock(); + unsafe extern "C" fn _acquire_global_lock() { + assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) + } + + unsafe extern "C" fn _release_global_lock() { + release_global_lock() + } } #[cfg(feature = "global")] From 96c59c68fb0cf8a54e5f51c01f7996eab9101509 Mon Sep 17 00:00:00 2001 From: Niv Kaminer Date: Tue, 26 Oct 2021 21:25:03 +0300 Subject: [PATCH 4/7] use a dedicated function for registering atfork --- src/global.rs | 2 ++ src/lib.rs | 2 +- src/linux.rs | 55 +++++++++++++++++++++++++++++++-------------------- src/macos.rs | 55 +++++++++++++++++++++++++++++++-------------------- src/wasm.rs | 5 +++++ 5 files changed, 76 insertions(+), 43 deletions(-) diff --git a/src/global.rs b/src/global.rs index fbac315..c761fde 100644 --- a/src/global.rs +++ b/src/global.rs @@ -3,6 +3,8 @@ use core::ops::{Deref, DerefMut}; use Dlmalloc; +pub use sys::enable_alloc_after_fork; + /// An instance of a "global allocator" backed by `Dlmalloc` /// /// This API requires the `global` feature is activated, and this type diff --git a/src/lib.rs b/src/lib.rs index 22a43c6..efd6971 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,7 +20,7 @@ use core::ptr; use sys::System; #[cfg(feature = "global")] -pub use self::global::GlobalDlmalloc; +pub use self::global::{enable_alloc_after_fork, GlobalDlmalloc}; mod dlmalloc; #[cfg(feature = "global")] diff --git a/src/linux.rs b/src/linux.rs index 27ba688..cd61ac4 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -75,34 +75,47 @@ unsafe impl Allocator for System { #[cfg(feature = "global")] pub fn acquire_global_lock() { - static mut FORK_PROTECTED: bool = false; + unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } +} - unsafe { - assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0); - if !FORK_PROTECTED { - // ensures that if a process forks, - // it will acquire the lock before any other thread, - // protecting it from deadlock, - // when the child is created with only that thread. - libc::pthread_atfork( - Some(_acquire_global_lock), - Some(_release_global_lock), - Some(_release_global_lock), - ); - FORK_PROTECTED = true; - } - } +#[cfg(feature = "global")] +pub fn release_global_lock() { + unsafe { assert_eq!(libc::pthread_mutex_unlock(&mut LOCK), 0) } +} + +#[cfg(feature = "global")] +/// allows the allocator to remain unsable in the child process, +/// after a call to `fork(2)` +/// +/// #Safety +/// +/// if used, this function must be called, +/// before any allocations are made with the global allocator. +pub unsafe fn enable_alloc_after_fork() { + // atfork must only be called once, to avoid a deadlock, + // where the handler attempts to acquire the global lock twice + static mut FORK_PROTECTED: bool = false; unsafe extern "C" fn _acquire_global_lock() { - assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) + acquire_global_lock() } unsafe extern "C" fn _release_global_lock() { release_global_lock() } -} -#[cfg(feature = "global")] -pub fn release_global_lock() { - unsafe { assert_eq!(libc::pthread_mutex_unlock(&mut LOCK), 0) } + acquire_global_lock(); + // if a process forks, + // it will acquire the lock before any other thread, + // protecting it from deadlock, + // due to the child being created with only the calling thread. + if !FORK_PROTECTED { + libc::pthread_atfork( + Some(_acquire_global_lock), + Some(_release_global_lock), + Some(_release_global_lock), + ); + FORK_PROTECTED = true; + } + release_global_lock(); } diff --git a/src/macos.rs b/src/macos.rs index 40b90ce..51250dc 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -63,34 +63,47 @@ unsafe impl Allocator for System { #[cfg(feature = "global")] pub fn acquire_global_lock() { - static mut FORK_PROTECTED: bool = false; + unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } +} - unsafe { - assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0); - if !FORK_PROTECTED { - // ensures that if a process forks, - // it will acquire the lock before any other thread, - // protecting it from deadlock, - // when the child is created with only that thread. - libc::pthread_atfork( - Some(_acquire_global_lock), - Some(_release_global_lock), - Some(_release_global_lock), - ); - FORK_PROTECTED = true; - } - } +#[cfg(feature = "global")] +pub fn release_global_lock() { + unsafe { assert_eq!(libc::pthread_mutex_unlock(&mut LOCK), 0) } +} + +#[cfg(feature = "global")] +/// allows the allocator to remain unsable in the child process, +/// after a call to `fork(2)` +/// +/// #Safety +/// +/// if used, this function must be called, +/// before any allocations are made with the global allocator. +pub unsafe fn enable_alloc_after_fork() { + // atfork must only be called once, to avoid a deadlock, + // where the handler attempts to acquire the global lock twice + static mut FORK_PROTECTED: bool = false; unsafe extern "C" fn _acquire_global_lock() { - assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) + acquire_global_lock() } unsafe extern "C" fn _release_global_lock() { release_global_lock() } -} -#[cfg(feature = "global")] -pub fn release_global_lock() { - unsafe { assert_eq!(libc::pthread_mutex_unlock(&mut LOCK), 0) } + acquire_global_lock(); + // if a process forks, + // it will acquire the lock before any other thread, + // protecting it from deadlock, + // due to the child being created with only the calling thread. + if !FORK_PROTECTED { + libc::pthread_atfork( + Some(_acquire_global_lock), + Some(_release_global_lock), + Some(_release_global_lock), + ); + FORK_PROTECTED = true; + } + release_global_lock(); } diff --git a/src/wasm.rs b/src/wasm.rs index 381fb02..e47d19a 100644 --- a/src/wasm.rs +++ b/src/wasm.rs @@ -62,3 +62,8 @@ pub fn acquire_global_lock() { pub fn release_global_lock() { // single threaded, no need! } + +#[cfg(feature = "global")] +pub unsafe fn enable_alloc_after_fork() { + // single threaded, no need! +} From 8dc1a86ce0f250900abffd65936ade62069f000b Mon Sep 17 00:00:00 2001 From: Niv Kaminer Date: Wed, 27 Oct 2021 01:03:43 +0300 Subject: [PATCH 5/7] unify linux and macos into a single module --- src/lib.rs | 8 +-- src/macos.rs | 109 -------------------------------------- src/{linux.rs => unix.rs} | 14 ++++- 3 files changed, 15 insertions(+), 116 deletions(-) delete mode 100644 src/macos.rs rename src/{linux.rs => unix.rs} (86%) diff --git a/src/lib.rs b/src/lib.rs index efd6971..ce59652 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -76,12 +76,8 @@ pub struct Dlmalloc(dlmalloc::Dlmalloc); #[path = "wasm.rs"] mod sys; -#[cfg(target_os = "macos")] -#[path = "macos.rs"] -mod sys; - -#[cfg(target_os = "linux")] -#[path = "linux.rs"] +#[cfg(any(target_os = "linux", target_os = "macos"))] +#[path = "unix.rs"] mod sys; #[cfg(not(any(target_os = "linux", target_os = "macos", target_arch = "wasm32")))] diff --git a/src/macos.rs b/src/macos.rs deleted file mode 100644 index 51250dc..0000000 --- a/src/macos.rs +++ /dev/null @@ -1,109 +0,0 @@ -extern crate libc; - -use core::ptr; -use Allocator; - -/// System setting for MacOS -pub struct System { - _priv: (), -} - -impl System { - pub const fn new() -> System { - System { _priv: () } - } -} - -#[cfg(feature = "global")] -static mut LOCK: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER; - -unsafe impl Allocator for System { - fn alloc(&self, size: usize) -> (*mut u8, usize, u32) { - let addr = unsafe { - libc::mmap( - 0 as *mut _, - size, - libc::PROT_WRITE | libc::PROT_READ, - libc::MAP_ANON | libc::MAP_PRIVATE, - -1, - 0, - ) - }; - if addr == libc::MAP_FAILED { - (ptr::null_mut(), 0, 0) - } else { - (addr as *mut u8, size, 0) - } - } - - fn remap(&self, _ptr: *mut u8, _oldsize: usize, _newsize: usize, _can_move: bool) -> *mut u8 { - ptr::null_mut() - } - - fn free_part(&self, ptr: *mut u8, oldsize: usize, newsize: usize) -> bool { - unsafe { libc::munmap(ptr.offset(newsize as isize) as *mut _, oldsize - newsize) == 0 } - } - - fn free(&self, ptr: *mut u8, size: usize) -> bool { - unsafe { libc::munmap(ptr as *mut _, size) == 0 } - } - - fn can_release_part(&self, _flags: u32) -> bool { - true - } - - fn allocates_zeros(&self) -> bool { - true - } - - fn page_size(&self) -> usize { - 4096 - } -} - -#[cfg(feature = "global")] -pub fn acquire_global_lock() { - unsafe { assert_eq!(libc::pthread_mutex_lock(&mut LOCK), 0) } -} - -#[cfg(feature = "global")] -pub fn release_global_lock() { - unsafe { assert_eq!(libc::pthread_mutex_unlock(&mut LOCK), 0) } -} - -#[cfg(feature = "global")] -/// allows the allocator to remain unsable in the child process, -/// after a call to `fork(2)` -/// -/// #Safety -/// -/// if used, this function must be called, -/// before any allocations are made with the global allocator. -pub unsafe fn enable_alloc_after_fork() { - // atfork must only be called once, to avoid a deadlock, - // where the handler attempts to acquire the global lock twice - static mut FORK_PROTECTED: bool = false; - - unsafe extern "C" fn _acquire_global_lock() { - acquire_global_lock() - } - - unsafe extern "C" fn _release_global_lock() { - release_global_lock() - } - - acquire_global_lock(); - // if a process forks, - // it will acquire the lock before any other thread, - // protecting it from deadlock, - // due to the child being created with only the calling thread. - if !FORK_PROTECTED { - libc::pthread_atfork( - Some(_acquire_global_lock), - Some(_release_global_lock), - Some(_release_global_lock), - ); - FORK_PROTECTED = true; - } - release_global_lock(); -} diff --git a/src/linux.rs b/src/unix.rs similarity index 86% rename from src/linux.rs rename to src/unix.rs index cd61ac4..a4f9914 100644 --- a/src/linux.rs +++ b/src/unix.rs @@ -24,7 +24,7 @@ unsafe impl Allocator for System { 0 as *mut _, size, libc::PROT_WRITE | libc::PROT_READ, - libc::MAP_ANONYMOUS | libc::MAP_PRIVATE, + libc::MAP_ANON | libc::MAP_PRIVATE, -1, 0, ) @@ -36,6 +36,7 @@ unsafe impl Allocator for System { } } + #[cfg(target_os = "linux")] fn remap(&self, ptr: *mut u8, oldsize: usize, newsize: usize, can_move: bool) -> *mut u8 { let flags = if can_move { libc::MREMAP_MAYMOVE } else { 0 }; let ptr = unsafe { libc::mremap(ptr as *mut _, oldsize, newsize, flags) }; @@ -46,6 +47,12 @@ unsafe impl Allocator for System { } } + #[cfg(target_os = "macos")] + fn remap(&self, _ptr: *mut u8, _oldsize: usize, _newsize: usize, _can_move: bool) -> *mut u8 { + ptr::null_mut() + } + + #[cfg(target_os = "linux")] fn free_part(&self, ptr: *mut u8, oldsize: usize, newsize: usize) -> bool { unsafe { let rc = libc::mremap(ptr as *mut _, oldsize, newsize, 0); @@ -56,6 +63,11 @@ unsafe impl Allocator for System { } } + #[cfg(target_os = "macos")] + fn free_part(&self, ptr: *mut u8, oldsize: usize, newsize: usize) -> bool { + unsafe { libc::munmap(ptr.offset(newsize as isize) as *mut _, oldsize - newsize) == 0 } + } + fn free(&self, ptr: *mut u8, size: usize) -> bool { unsafe { libc::munmap(ptr as *mut _, size) == 0 } } From b510fa90d2a22d72c7e059618379e76ec9014ee0 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 1 Nov 2021 11:16:00 -0700 Subject: [PATCH 6/7] Add support for wasm64 Tweak some cfgs and some code to ensure that this crate compiles and works on wasm64 in the same manner as wasm32 --- src/lib.rs | 4 ++-- src/wasm.rs | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ce59652..92e8af7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,7 +72,7 @@ pub unsafe trait Allocator: Send { /// lingering memory back to the OS. That may happen eventually though! pub struct Dlmalloc(dlmalloc::Dlmalloc); -#[cfg(target_arch = "wasm32")] +#[cfg(target_family = "wasm")] #[path = "wasm.rs"] mod sys; @@ -80,7 +80,7 @@ mod sys; #[path = "unix.rs"] mod sys; -#[cfg(not(any(target_os = "linux", target_os = "macos", target_arch = "wasm32")))] +#[cfg(not(any(target_os = "linux", target_os = "macos", target_family = "wasm")))] #[path = "dummy.rs"] mod sys; diff --git a/src/wasm.rs b/src/wasm.rs index e47d19a..216fe43 100644 --- a/src/wasm.rs +++ b/src/wasm.rs @@ -1,4 +1,7 @@ -use core::arch::wasm32; +#[cfg(target_arch = "wasm32")] +use core::arch::wasm32 as wasm; +#[cfg(target_arch = "wasm64")] +use core::arch::wasm64 as wasm; use core::ptr; use Allocator; @@ -16,7 +19,7 @@ impl System { unsafe impl Allocator for System { fn alloc(&self, size: usize) -> (*mut u8, usize, u32) { let pages = size / self.page_size(); - let prev = wasm32::memory_grow(0, pages); + let prev = wasm::memory_grow(0, pages); if prev == usize::max_value() { return (ptr::null_mut(), 0, 0); } From d9ed983e2d3e07a5fa5a85f5994a645900026700 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 1 Nov 2021 13:12:00 -0700 Subject: [PATCH 7/7] Bump to 0.2.2 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 56fe4a8..4169f9a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "dlmalloc" -version = "0.2.1" +version = "0.2.2" authors = ["Alex Crichton "] license = "MIT/Apache-2.0" readme = "README.md"