From b06731355205f0a4ffa1464137b1d55992f08b48 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 16:56:16 +0200 Subject: [PATCH 1/7] use intptrcast for heap_allocator test; then it should work on Windows --- tests/run-pass/heap_allocator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run-pass/heap_allocator.rs b/tests/run-pass/heap_allocator.rs index b201f24e25..457734cb60 100644 --- a/tests/run-pass/heap_allocator.rs +++ b/tests/run-pass/heap_allocator.rs @@ -1,3 +1,4 @@ +// compile-flag: -Zmiri-seed= #![feature(allocator_api)] use std::ptr::NonNull; @@ -75,7 +76,6 @@ fn box_to_global() { fn main() { check_alloc(System); check_alloc(Global); - #[cfg(not(target_os = "windows"))] // TODO: Inspects allocation base address on Windows; needs intptrcast model check_overalign_requests(System); check_overalign_requests(Global); global_to_box(); From 0ea4b50025ea71e11aa8b326508b4d40e998727a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 17:02:20 +0200 Subject: [PATCH 2/7] Miri is not deterministic any more --- tests/run-pass/heap_allocator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/run-pass/heap_allocator.rs b/tests/run-pass/heap_allocator.rs index 457734cb60..3eb0f70fd6 100644 --- a/tests/run-pass/heap_allocator.rs +++ b/tests/run-pass/heap_allocator.rs @@ -33,8 +33,8 @@ fn check_overalign_requests(mut allocator: T) { let size = 8; // Greater than `size`. let align = 16; - // Miri is deterministic; no need to try many times. - let iterations = 1; + + let iterations = 5; unsafe { let pointers: Vec<_> = (0..iterations).map(|_| { allocator.alloc(Layout::from_size_align(size, align).unwrap()).unwrap() From 78261b788d0107efed65b4fb24e18c1a7ff5840b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 19:10:09 +0200 Subject: [PATCH 3/7] fix setting rustc flags --- tests/run-pass/heap_allocator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run-pass/heap_allocator.rs b/tests/run-pass/heap_allocator.rs index 3eb0f70fd6..57db5407b7 100644 --- a/tests/run-pass/heap_allocator.rs +++ b/tests/run-pass/heap_allocator.rs @@ -1,4 +1,4 @@ -// compile-flag: -Zmiri-seed= +// compile-flags: -Zmiri-seed= #![feature(allocator_api)] use std::ptr::NonNull; From a04890795db44682f4afd276dbb7277eee3bcc11 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 21:03:52 +0200 Subject: [PATCH 4/7] move appveyor env var settings to more appropriate section --- .appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index f7d3f990ac..4e8b8cd4b8 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -29,8 +29,6 @@ install: - rustc --version build_script: - - set RUST_TEST_NOCAPTURE=1 - - set RUST_BACKTRACE=1 - set RUSTFLAGS=-C debug-assertions # Build and install miri - cargo build --release --all-features --all-targets @@ -40,6 +38,8 @@ build_script: - set MIRI_SYSROOT=%USERPROFILE%\AppData\Local\rust-lang\miri\cache\HOST test_script: + - set RUST_TEST_NOCAPTURE=1 + - set RUST_BACKTRACE=1 # Test miri - cargo test --release --all-features # Test cargo integration From e960270662e74aa19beee319641e12d67e45aa07 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 21:06:32 +0200 Subject: [PATCH 5/7] add some tracing to intptrcast --- src/intptrcast.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/intptrcast.rs b/src/intptrcast.rs index f8102642bd..5480700005 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -90,6 +90,10 @@ impl<'mir, 'tcx> GlobalState { // From next_base_addr + slack, round up to adjust for alignment. let base_addr = Self::align_addr(global_state.next_base_addr + slack, align.bytes()); entry.insert(base_addr); + trace!( + "Assigning base address {:#x} to allocation {:?} (slack: {}, align: {})", + base_addr, ptr.alloc_id, slack, align.bytes(), + ); // Remember next base address. If this allocation is zero-sized, leave a gap // of at least 1 to avoid two allocations having the same base address. From 709b4748599881184f4f03137d1b81ea160d5dd1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 21:08:17 +0200 Subject: [PATCH 6/7] fix minimal alignment for system allocation functions --- src/shims/foreign_items.rs | 16 ++++++++-- tests/run-pass/heap_allocator.rs | 50 ++++++++++++++++++++------------ 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 9c9e77abfe..2e2a9062fb 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -51,6 +51,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(Some(this.load_mir(instance.def)?)) } + /// Returns the minimum alignment for the target architecture. + fn min_align(&self) -> Align { + let this = self.eval_context_ref(); + // List taken from `libstd/sys_common/alloc.rs`. + let min_align = match this.tcx.tcx.sess.target.target.arch.as_str() { + "x86" | "arm" | "mips" | "powerpc" | "powerpc64" | "asmjs" | "wasm32" => 8, + "x86_64" | "aarch64" | "mips64" | "s390x" | "sparc64" => 16, + arch => bug!("Unsupported target architecture: {}", arch), + }; + Align::from_bytes(min_align).unwrap() + } + fn malloc( &mut self, size: u64, @@ -61,7 +73,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx if size == 0 { Scalar::from_int(0, this.pointer_size()) } else { - let align = this.tcx.data_layout.pointer_align.abi; + let align = this.min_align(); let ptr = this.memory_mut().allocate(Size::from_bytes(size), align, MiriMemoryKind::C.into()); if zero_init { // We just allocated this, the access cannot fail @@ -94,7 +106,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx new_size: u64, ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let align = this.tcx.data_layout.pointer_align.abi; + let align = this.min_align(); if old_ptr.is_null_ptr(this) { if new_size == 0 { Ok(Scalar::from_int(0, this.pointer_size())) diff --git a/tests/run-pass/heap_allocator.rs b/tests/run-pass/heap_allocator.rs index 57db5407b7..1ea04ec467 100644 --- a/tests/run-pass/heap_allocator.rs +++ b/tests/run-pass/heap_allocator.rs @@ -6,35 +6,47 @@ use std::alloc::{Global, Alloc, Layout, System}; use std::slice; fn check_alloc(mut allocator: T) { unsafe { - let layout = Layout::from_size_align(20, 4).unwrap(); - let a = allocator.alloc(layout).unwrap(); - allocator.dealloc(a, layout); + for &align in &[4, 8, 16, 32] { + let layout = Layout::from_size_align(20, align).unwrap(); - let p1 = allocator.alloc_zeroed(layout).unwrap(); + for _ in 0..32 { + let a = allocator.alloc(layout).unwrap(); + assert_eq!(a.as_ptr() as usize % align, 0, "pointer is incorrectly aligned"); + allocator.dealloc(a, layout); + } + + let p1 = allocator.alloc_zeroed(layout).unwrap(); + assert_eq!(p1.as_ptr() as usize % align, 0, "pointer is incorrectly aligned"); - let p2 = allocator.realloc(p1, Layout::from_size_align(20, 4).unwrap(), 40).unwrap(); - let slice = slice::from_raw_parts(p2.as_ptr(), 20); - assert_eq!(&slice, &[0_u8; 20]); + let p2 = allocator.realloc(p1, layout, 40).unwrap(); + let layout = Layout::from_size_align(40, align).unwrap(); + assert_eq!(p2.as_ptr() as usize % align, 0, "pointer is incorrectly aligned"); + let slice = slice::from_raw_parts(p2.as_ptr(), 20); + assert_eq!(&slice, &[0_u8; 20]); - // old size == new size - let p3 = allocator.realloc(p2, Layout::from_size_align(40, 4).unwrap(), 40).unwrap(); - let slice = slice::from_raw_parts(p3.as_ptr(), 20); - assert_eq!(&slice, &[0_u8; 20]); + // old size == new size + let p3 = allocator.realloc(p2, layout, 40).unwrap(); + assert_eq!(p3.as_ptr() as usize % align, 0, "pointer is incorrectly aligned"); + let slice = slice::from_raw_parts(p3.as_ptr(), 20); + assert_eq!(&slice, &[0_u8; 20]); - // old size > new size - let p4 = allocator.realloc(p3, Layout::from_size_align(40, 4).unwrap(), 10).unwrap(); - let slice = slice::from_raw_parts(p4.as_ptr(), 10); - assert_eq!(&slice, &[0_u8; 10]); + // old size > new size + let p4 = allocator.realloc(p3, layout, 10).unwrap(); + let layout = Layout::from_size_align(10, align).unwrap(); + assert_eq!(p4.as_ptr() as usize % align, 0, "pointer is incorrectly aligned"); + let slice = slice::from_raw_parts(p4.as_ptr(), 10); + assert_eq!(&slice, &[0_u8; 10]); - allocator.dealloc(p4, Layout::from_size_align(10, 4).unwrap()); + allocator.dealloc(p4, layout); + } } } fn check_overalign_requests(mut allocator: T) { let size = 8; - // Greater than `size`. - let align = 16; + // Greater than `size`, and also greater than `MIN_ALIGN`. + let align = 32; - let iterations = 5; + let iterations = 32; unsafe { let pointers: Vec<_> = (0..iterations).map(|_| { allocator.alloc(Layout::from_size_align(size, align).unwrap()).unwrap() From cb6d4f0c9ab95125af236abea72661efd094f699 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 21:23:48 +0200 Subject: [PATCH 7/7] test even more size-alignment combinations. found a bug in libstd! --- README.md | 1 + src/shims/foreign_items.rs | 4 ++++ tests/run-pass/heap_allocator.rs | 32 ++++++++++++++++---------------- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 91dcfd7cf8..be104ed0b2 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,7 @@ Definite bugs found: * [Futures turning a shared reference into a mutable one](https://github.com/rust-lang/rust/pull/56319) * [`str` turning a shared reference into a mutable one](https://github.com/rust-lang/rust/pull/58200) * [`rand` performing unaligned reads](https://github.com/rust-random/rand/issues/779) +* [The Unix allocator calling `posix_memalign` in an invalid way](https://github.com/rust-lang/rust/issues/62251) Violations of Stacked Borrows found that are likely bugs (but Stacked Borrows is currently just an experiment): diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 2e2a9062fb..1a39df9cce 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -203,12 +203,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx if !align.is_power_of_two() { return err!(HeapAllocNonPowerOfTwoAlignment(align)); } + /* + FIXME: This check is disabled because rustc violates it. + See . if align < this.pointer_size().bytes() { return err!(MachineError(format!( "posix_memalign: alignment must be at least the size of a pointer, but is {}", align, ))); } + */ if size == 0 { this.write_null(ret.into())?; } else { diff --git a/tests/run-pass/heap_allocator.rs b/tests/run-pass/heap_allocator.rs index 1ea04ec467..2f3a48f535 100644 --- a/tests/run-pass/heap_allocator.rs +++ b/tests/run-pass/heap_allocator.rs @@ -42,23 +42,23 @@ fn check_alloc(mut allocator: T) { unsafe { } } fn check_overalign_requests(mut allocator: T) { - let size = 8; - // Greater than `size`, and also greater than `MIN_ALIGN`. - let align = 32; + for &size in &[2, 8, 64] { // size less than and bigger than alignment + for &align in &[4, 8, 16, 32] { // Be sure to cover less than and bigger than `MIN_ALIGN` for all architectures + let iterations = 32; + unsafe { + let pointers: Vec<_> = (0..iterations).map(|_| { + allocator.alloc(Layout::from_size_align(size, align).unwrap()).unwrap() + }).collect(); + for &ptr in &pointers { + assert_eq!((ptr.as_ptr() as usize) % align, 0, + "Got a pointer less aligned than requested") + } - let iterations = 32; - unsafe { - let pointers: Vec<_> = (0..iterations).map(|_| { - allocator.alloc(Layout::from_size_align(size, align).unwrap()).unwrap() - }).collect(); - for &ptr in &pointers { - assert_eq!((ptr.as_ptr() as usize) % align, 0, - "Got a pointer less aligned than requested") - } - - // Clean up. - for &ptr in &pointers { - allocator.dealloc(ptr, Layout::from_size_align(size, align).unwrap()) + // Clean up. + for &ptr in &pointers { + allocator.dealloc(ptr, Layout::from_size_align(size, align).unwrap()) + } + } } } }