From d654b7396322bcaa8b26020c355f0e349106122d Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 7 May 2024 23:11:48 +0000 Subject: [PATCH 1/2] support aligned_alloc for unixes support. --- src/shims/alloc.rs | 38 ++++++++++++++++++ src/shims/unix/foreign_items.rs | 11 +++++ src/shims/wasi/foreign_items.rs | 9 +++++ .../libc/aligned_alloc_size_zero_leak.rs | 15 +++++++ .../libc/aligned_alloc_size_zero_leak.stderr | 15 +++++++ tests/pass-dep/libc/libc-mem.rs | 40 +++++++++++++++++++ 6 files changed, 128 insertions(+) create mode 100644 tests/fail-dep/libc/aligned_alloc_size_zero_leak.rs create mode 100644 tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr diff --git a/src/shims/alloc.rs b/src/shims/alloc.rs index d0f36bd475..6c18989caa 100644 --- a/src/shims/alloc.rs +++ b/src/shims/alloc.rs @@ -172,4 +172,42 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } } + + fn aligned_alloc( + &mut self, + align: u64, + size: u64, + ) -> InterpResult<'tcx, Pointer>> { + let this = self.eval_context_mut(); + // Alignment must be a power of 2, and "supported by the implementation". + // We decide that "supported by the implementation" means that the + // size must be a multiple of the alignment. (This restriction seems common + // enough that it is stated on + // as a general rule, but the actual standard has no such rule.) + // If any of these are violated, we have to return NULL. + // All fundamental alignments must be supported. + // + // macOS and Illumos are buggy in that they require the alignment + // to be at least the size of a pointer, so they do not support all fundamental + // alignments. We do not emulate those platform bugs. + // + // Linux also sets errno to EINVAL, but that's non-standard behavior that we do not + // emulate. + // FreeBSD says some of these cases are UB but that's violating the C standard. + // http://en.cppreference.com/w/cpp/memory/c/aligned_alloc + // Linux: https://linux.die.net/man/3/aligned_alloc + // FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=aligned_alloc&apropos=0&sektion=3&manpath=FreeBSD+9-current&format=html + match size.checked_rem(align) { + Some(0) if align.is_power_of_two() => { + let align = align.max(this.malloc_align(size).bytes()); + let ptr = this.allocate_ptr( + Size::from_bytes(size), + Align::from_bytes(align).unwrap(), + MiriMemoryKind::C.into(), + )?; + Ok(ptr.into()) + } + _ => Ok(Pointer::null()), + } + } } diff --git a/src/shims/unix/foreign_items.rs b/src/shims/unix/foreign_items.rs index 78d297d4b0..ca59e6e650 100644 --- a/src/shims/unix/foreign_items.rs +++ b/src/shims/unix/foreign_items.rs @@ -295,6 +295,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } } + "aligned_alloc" => { + // This is a C11 function, we assume all Unixes have it. + // (MSVC explicitly does not support this.) + let [align, size] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let align = this.read_target_usize(align)?; + let size = this.read_target_usize(size)?; + + let res = this.aligned_alloc(align, size)?; + this.write_pointer(res, dest)?; + } // Dynamic symbol loading "dlsym" => { diff --git a/src/shims/wasi/foreign_items.rs b/src/shims/wasi/foreign_items.rs index 12bf049093..9c0a8e6639 100644 --- a/src/shims/wasi/foreign_items.rs +++ b/src/shims/wasi/foreign_items.rs @@ -26,6 +26,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let result = this.posix_memalign(memptr, align, size)?; this.write_scalar(result, dest)?; } + "aligned_alloc" => { + let [align, size] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let align = this.read_target_usize(align)?; + let size = this.read_target_usize(size)?; + + let res = this.aligned_alloc(align, size)?; + this.write_pointer(res, dest)?; + } _ => return Ok(EmulateItemResult::NotSupported), } diff --git a/tests/fail-dep/libc/aligned_alloc_size_zero_leak.rs b/tests/fail-dep/libc/aligned_alloc_size_zero_leak.rs new file mode 100644 index 0000000000..9a33cdccd2 --- /dev/null +++ b/tests/fail-dep/libc/aligned_alloc_size_zero_leak.rs @@ -0,0 +1,15 @@ +//@ignore-target-windows: Windows does not support the standard C11 aligned_alloc. + +fn main() { + // libc doesn't have this function (https://github.com/rust-lang/libc/issues/3689), + // so we declare it ourselves. + extern "C" { + fn aligned_alloc(alignment: libc::size_t, size: libc::size_t) -> *mut libc::c_void; + } + + // Make sure even zero-sized allocations need to be freed. + + unsafe { + aligned_alloc(2, 0); //~ERROR: memory leaked + } +} diff --git a/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr b/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr new file mode 100644 index 0000000000..b0756d5721 --- /dev/null +++ b/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr @@ -0,0 +1,15 @@ +error: memory leaked: ALLOC (C heap, size: 0, align: 2), allocated here: + --> $DIR/aligned_alloc_size_zero_leak.rs:LL:CC + | +LL | aligned_alloc(2, 0); + | ^^^^^^^^^^^^^^^^^^^ + | + = note: BACKTRACE: + = note: inside `main` at $DIR/aligned_alloc_size_zero_leak.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check + +error: aborting due to 1 previous error + diff --git a/tests/pass-dep/libc/libc-mem.rs b/tests/pass-dep/libc/libc-mem.rs index 5bd205dd08..25aef0a183 100644 --- a/tests/pass-dep/libc/libc-mem.rs +++ b/tests/pass-dep/libc/libc-mem.rs @@ -241,6 +241,44 @@ fn test_reallocarray() { } } +#[cfg(not(target_os = "windows"))] +fn test_aligned_alloc() { + // libc doesn't have this function (https://github.com/rust-lang/libc/issues/3689), + // so we declare it ourselves. + extern "C" { + fn aligned_alloc(alignment: libc::size_t, size: libc::size_t) -> *mut libc::c_void; + } + // size not a multiple of the alignment + unsafe { + let p = aligned_alloc(16, 3); + assert_eq!(p, ptr::null_mut()); + } + + // alignment not power of 2 + unsafe { + let p = aligned_alloc(63, 8); + assert_eq!(p, ptr::null_mut()); + } + + // alignment lesser than a word but still a successful allocation + unsafe { + let p = aligned_alloc(1, 4); + assert!(!p.is_null()); + assert!(p.is_aligned_to(4)); + libc::free(p); + } + + // repeated tests on correct alignment/size + for _ in 0..16 { + unsafe { + let p = aligned_alloc(16, 16); + assert!(!p.is_null()); + assert!(p.is_aligned_to(16)); + libc::free(p); + } + } +} + fn main() { test_malloc(); test_calloc(); @@ -254,6 +292,8 @@ fn main() { target_os = "wasi", )))] test_reallocarray(); + #[cfg(not(target_os = "windows"))] + test_aligned_alloc(); test_memcpy(); test_strcpy(); From 78ed7ac2188ac41f9c9c0dd5bbd0a04095ae319f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 11:48:51 +0200 Subject: [PATCH 2/2] a bit of refactoring and tweak the aligned-allocation tests --- src/shims/alloc.rs | 7 +- src/shims/unix/foreign_items.rs | 3 - src/shims/wasi/foreign_items.rs | 3 - tests/pass-dep/libc/libc-mem.rs | 110 ++++++++++++++++---------------- 4 files changed, 61 insertions(+), 62 deletions(-) diff --git a/src/shims/alloc.rs b/src/shims/alloc.rs index 6c18989caa..bd84de81e6 100644 --- a/src/shims/alloc.rs +++ b/src/shims/alloc.rs @@ -175,10 +175,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn aligned_alloc( &mut self, - align: u64, - size: u64, + align: &OpTy<'tcx, Provenance>, + size: &OpTy<'tcx, Provenance>, ) -> InterpResult<'tcx, Pointer>> { let this = self.eval_context_mut(); + let align = this.read_target_usize(align)?; + let size = this.read_target_usize(size)?; + // Alignment must be a power of 2, and "supported by the implementation". // We decide that "supported by the implementation" means that the // size must be a multiple of the alignment. (This restriction seems common diff --git a/src/shims/unix/foreign_items.rs b/src/shims/unix/foreign_items.rs index ca59e6e650..74fb2fb4b2 100644 --- a/src/shims/unix/foreign_items.rs +++ b/src/shims/unix/foreign_items.rs @@ -300,9 +300,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // (MSVC explicitly does not support this.) let [align, size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let align = this.read_target_usize(align)?; - let size = this.read_target_usize(size)?; - let res = this.aligned_alloc(align, size)?; this.write_pointer(res, dest)?; } diff --git a/src/shims/wasi/foreign_items.rs b/src/shims/wasi/foreign_items.rs index 9c0a8e6639..d911f43eb6 100644 --- a/src/shims/wasi/foreign_items.rs +++ b/src/shims/wasi/foreign_items.rs @@ -29,9 +29,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { "aligned_alloc" => { let [align, size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let align = this.read_target_usize(align)?; - let size = this.read_target_usize(size)?; - let res = this.aligned_alloc(align, size)?; this.write_pointer(res, dest)?; } diff --git a/tests/pass-dep/libc/libc-mem.rs b/tests/pass-dep/libc/libc-mem.rs index 25aef0a183..aa383a99bc 100644 --- a/tests/pass-dep/libc/libc-mem.rs +++ b/tests/pass-dep/libc/libc-mem.rs @@ -148,53 +148,55 @@ fn test_calloc() { #[cfg(not(target_os = "windows"))] fn test_memalign() { - // A normal allocation. - unsafe { - let mut ptr: *mut libc::c_void = ptr::null_mut(); - let align = 8; - let size = 64; - assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - assert!(!ptr.is_null()); - assert!(ptr.is_aligned_to(align)); - ptr.cast::().write_bytes(1, size); - libc::free(ptr); - } + for _ in 0..16 { + // A normal allocation. + unsafe { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 8; + let size = 64; + assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); + assert!(!ptr.is_null()); + assert!(ptr.is_aligned_to(align)); + ptr.cast::().write_bytes(1, size); + libc::free(ptr); + } - // Align > size. - unsafe { - let mut ptr: *mut libc::c_void = ptr::null_mut(); - let align = 64; - let size = 8; - assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - assert!(!ptr.is_null()); - assert!(ptr.is_aligned_to(align)); - ptr.cast::().write_bytes(1, size); - libc::free(ptr); - } + // Align > size. + unsafe { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 64; + let size = 8; + assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); + assert!(!ptr.is_null()); + assert!(ptr.is_aligned_to(align)); + ptr.cast::().write_bytes(1, size); + libc::free(ptr); + } - // Size not multiple of align - unsafe { - let mut ptr: *mut libc::c_void = ptr::null_mut(); - let align = 16; - let size = 31; - assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - assert!(!ptr.is_null()); - assert!(ptr.is_aligned_to(align)); - ptr.cast::().write_bytes(1, size); - libc::free(ptr); - } + // Size not multiple of align + unsafe { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 16; + let size = 31; + assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); + assert!(!ptr.is_null()); + assert!(ptr.is_aligned_to(align)); + ptr.cast::().write_bytes(1, size); + libc::free(ptr); + } - // Size == 0 - unsafe { - let mut ptr: *mut libc::c_void = ptr::null_mut(); - let align = 64; - let size = 0; - assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - // Non-null pointer is returned if size == 0. - // (This is not a guarantee, it just reflects our current behavior.) - assert!(!ptr.is_null()); - assert!(ptr.is_aligned_to(align)); - libc::free(ptr); + // Size == 0 + unsafe { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 64; + let size = 0; + assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); + // Non-null pointer is returned if size == 0. + // (This is not a guarantee, it just reflects our current behavior.) + assert!(!ptr.is_null()); + assert!(ptr.is_aligned_to(align)); + libc::free(ptr); + } } // Non-power of 2 align @@ -260,20 +262,20 @@ fn test_aligned_alloc() { assert_eq!(p, ptr::null_mut()); } - // alignment lesser than a word but still a successful allocation - unsafe { - let p = aligned_alloc(1, 4); - assert!(!p.is_null()); - assert!(p.is_aligned_to(4)); - libc::free(p); - } - // repeated tests on correct alignment/size for _ in 0..16 { + // alignment 1, size 4 should succeed and actually must align to 4 (because C says so...) + unsafe { + let p = aligned_alloc(1, 4); + assert!(!p.is_null()); + assert!(p.is_aligned_to(4)); + libc::free(p); + } + unsafe { - let p = aligned_alloc(16, 16); + let p = aligned_alloc(64, 64); assert!(!p.is_null()); - assert!(p.is_aligned_to(16)); + assert!(p.is_aligned_to(64)); libc::free(p); } }