From 0fac868685020a78b056facee7fa321a02457d5f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Jun 2019 13:33:47 +0200 Subject: [PATCH 01/15] support num_cpus and test that --- src/fn_call.rs | 19 ++++++++++++------- src/intptrcast.rs | 6 ++---- src/lib.rs | 5 +++++ test-cargo-miri/Cargo.lock | 10 ++++++++++ test-cargo-miri/Cargo.toml | 1 + test-cargo-miri/run-test.py | 6 +++--- test-cargo-miri/src/main.rs | 3 +++ test-cargo-miri/test.stdout.ref | 8 +++++--- test-cargo-miri/test.stdout.ref2 | 4 ++-- test-cargo-miri/tests/test.rs | 26 +++++++++++++++++++------- 10 files changed, 62 insertions(+), 26 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index e2a4daa6f38..559524b2fe0 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -622,11 +622,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let name = this.read_scalar(args[0])?.to_i32()?; trace!("sysconf() called with name {}", name); - // Cache the sysconf integers via Miri's global cache. + // TODO: Cache the sysconf integers via Miri's global cache. let paths = &[ - (&["libc", "_SC_PAGESIZE"], Scalar::from_int(4096, dest.layout.size)), + (&["libc", "_SC_PAGESIZE"], Scalar::from_int(PAGE_SIZE, dest.layout.size)), (&["libc", "_SC_GETPW_R_SIZE_MAX"], Scalar::from_int(-1, dest.layout.size)), - (&["libc", "_SC_NPROCESSORS_ONLN"], Scalar::from_int(1, dest.layout.size)), + (&["libc", "_SC_NPROCESSORS_ONLN"], Scalar::from_int(NUM_CPUS, dest.layout.size)), ]; let mut result = None; for &(path, path_value) in paths { @@ -648,6 +648,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } + "sched_getaffinity" => { + // Return an error; `num_cpus` then falls back to `sysconf`. + this.write_scalar(Scalar::from_int(-1, dest.layout.size), dest)?; + } + "isatty" => { this.write_null(dest)?; } @@ -722,14 +727,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Second argument is where we are supposed to write the stack size. let ptr = this.deref_operand(args[1])?; // Just any address. - let stack_addr = Scalar::from_int(0x80000, args[1].layout.size); + let stack_addr = Scalar::from_uint(STACK_ADDR, args[1].layout.size); this.write_scalar(stack_addr, ptr.into())?; // Return success (`0`). this.write_null(dest)?; } "pthread_get_stackaddr_np" => { // Just any address. - let stack_addr = Scalar::from_int(0x80000, dest.layout.size); + let stack_addr = Scalar::from_uint(STACK_ADDR, dest.layout.size); this.write_scalar(stack_addr, dest)?; } @@ -838,14 +843,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Initialize with `0`. this.memory_mut().get_mut(system_info_ptr.alloc_id)? .write_repeat(tcx, system_info_ptr, 0, system_info.layout.size)?; - // Set number of processors to `1`. + // Set number of processors. let dword_size = Size::from_bytes(4); let offset = 2*dword_size + 3*tcx.pointer_size(); this.memory_mut().get_mut(system_info_ptr.alloc_id)? .write_scalar( tcx, system_info_ptr.offset(offset, tcx)?, - Scalar::from_int(1, dword_size).into(), + Scalar::from_int(NUM_CPUS, dword_size).into(), dword_size, )?; } diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 176e1bc591c..83e500ea32d 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -4,8 +4,7 @@ use rustc::mir::interpret::{AllocId, Pointer, InterpResult}; use rustc_mir::interpret::Memory; use rustc_target::abi::Size; -use crate::stacked_borrows::Tag; -use crate::Evaluator; +use crate::{Evaluator, Tag, STACK_ADDR}; pub type MemoryExtra = RefCell; @@ -25,11 +24,10 @@ pub struct GlobalState { } impl Default for GlobalState { - // FIXME: Query the page size in the future fn default() -> Self { GlobalState { int_to_ptr_map: Vec::default(), - next_base_addr: 2u64.pow(16) + next_base_addr: STACK_ADDR, } } } diff --git a/src/lib.rs b/src/lib.rs index ab9e9854c34..567fdae53b4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,6 +40,11 @@ pub use crate::stacked_borrows::{EvalContextExt as StackedBorEvalContextExt, Tag pub use crate::machine::{MemoryExtra, AllocExtra, MiriMemoryKind, Evaluator, MiriEvalContext, MiriEvalContextExt}; pub use crate::eval::{eval_main, create_ecx, MiriConfig}; +// Some global facts about the emulated machine. +pub const PAGE_SIZE: u64 = 4*1024; +pub const STACK_ADDR: u64 = 16*PAGE_SIZE; +pub const NUM_CPUS: u64 = 1; + /// Insert rustc arguments at the beginning of the argument list that Miri wants to be /// set per default, for maximal validation power. pub fn miri_default_args() -> &'static [&'static str] { diff --git a/test-cargo-miri/Cargo.lock b/test-cargo-miri/Cargo.lock index d404872f9db..8343832886a 100644 --- a/test-cargo-miri/Cargo.lock +++ b/test-cargo-miri/Cargo.lock @@ -29,6 +29,7 @@ name = "cargo-miri-test" version = "0.1.0" dependencies = [ "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", + "num_cpus 1.10.1 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -66,6 +67,14 @@ name = "libc" version = "0.2.58" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "num_cpus" +version = "1.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "libc 0.2.58 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "ppv-lite86" version = "0.2.5" @@ -148,6 +157,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum getrandom 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8d1dffef07351aafe6ef177e4dd2b8dcf503e6bc765dea3b0de9ed149a3db1ec" "checksum lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "bc5729f27f159ddd61f4df6228e827e86643d4d3e7c32183cb30a1c08f604a14" "checksum libc 0.2.58 (registry+https://github.com/rust-lang/crates.io-index)" = "6281b86796ba5e4366000be6e9e18bf35580adf9e63fbe2294aadb587613a319" +"checksum num_cpus 1.10.1 (registry+https://github.com/rust-lang/crates.io-index)" = "bcef43580c035376c0705c42792c294b66974abbfd2789b511784023f71f3273" "checksum ppv-lite86 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)" = "e3cbf9f658cdb5000fcf6f362b8ea2ba154b9f146a61c7a20d647034c6b6561b" "checksum rand 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d47eab0e83d9693d40f825f86948aa16eff6750ead4bdffc4ab95b8b3a7f052c" "checksum rand_chacha 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e193067942ef6f485a349a113329140d0ab9e2168ce92274499bb0e9a4190d9d" diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index c7fc62b79e8..3abb437049f 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -9,3 +9,4 @@ byteorder = "1.0" [dev-dependencies] rand = { version = "0.7", features = ["small_rng"] } +num_cpus = "1.10.1" diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 33664737709..73515c74e40 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -8,7 +8,7 @@ and the working directory to contain the cargo-miri-test project. import sys, subprocess, os def fail(msg): - print("TEST FAIL: {}".format(msg)) + print("\nTEST FAIL: {}".format(msg)) sys.exit(1) def cargo_miri(cmd): @@ -57,7 +57,7 @@ def test_cargo_miri_test(): "test.stdout.ref", "test.stderr.ref" ) test("cargo miri test (with filter)", - cargo_miri("test") + ["--", "--", "impl"], + cargo_miri("test") + ["--", "--", "le1"], "test.stdout.ref2", "test.stderr.ref" ) @@ -66,5 +66,5 @@ os.chdir(os.path.dirname(os.path.realpath(__file__))) test_cargo_miri_run() test_cargo_miri_test() -print("TEST SUCCESSFUL!") +print("\nTEST SUCCESSFUL!") sys.exit(0) diff --git a/test-cargo-miri/src/main.rs b/test-cargo-miri/src/main.rs index c21547c29ff..d3663ec849d 100644 --- a/test-cargo-miri/src/main.rs +++ b/test-cargo-miri/src/main.rs @@ -1,10 +1,13 @@ use byteorder::{BigEndian, ByteOrder}; fn main() { + // Exercise external crate, printing to stdout. let buf = &[1,2,3,4]; let n = ::read_u32(buf); assert_eq!(n, 0x01020304); println!("{:#010x}", n); + + // Access program arguments, printing to stderr. for arg in std::env::args() { eprintln!("{}", arg); } diff --git a/test-cargo-miri/test.stdout.ref b/test-cargo-miri/test.stdout.ref index 58b9b4794b9..c2257e68e25 100644 --- a/test-cargo-miri/test.stdout.ref +++ b/test-cargo-miri/test.stdout.ref @@ -5,9 +5,11 @@ test test::rng ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out -running 2 tests +running 4 tests test entropy_rng ... ok -test simple ... ok +test num_cpus ... ok +test simple1 ... ok +test simple2 ... ok -test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out +test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out diff --git a/test-cargo-miri/test.stdout.ref2 b/test-cargo-miri/test.stdout.ref2 index ce3506709d5..a6f6e915e0e 100644 --- a/test-cargo-miri/test.stdout.ref2 +++ b/test-cargo-miri/test.stdout.ref2 @@ -5,7 +5,7 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out running 1 test -test simple ... ok +test simple1 ... ok -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 3 filtered out diff --git a/test-cargo-miri/tests/test.rs b/test-cargo-miri/tests/test.rs index ac0db2778ca..cfbe3f6d7fb 100644 --- a/test-cargo-miri/tests/test.rs +++ b/test-cargo-miri/tests/test.rs @@ -3,13 +3,28 @@ use rand::{SeedableRng, Rng, rngs::SmallRng}; // Having more than 1 test does seem to make a difference // (i.e., this calls ptr::swap which having just one test does not). #[test] -fn simple() { +fn simple1() { assert_eq!(4, 4); } +#[test] +fn simple2() { + assert_ne!(42, 24); +} + +// A test that won't work on miri (tests disabling tests) +#[cfg(not(miri))] +#[test] +fn does_not_work_on_miri() { + let x = 0u8; + assert!(&x as *const _ as usize % 4 < 4); +} + +// We also use this to test some external crates, that we cannot depend on in the compiletest suite. + #[test] fn entropy_rng() { - // Use this opportunity to test querying the RNG (needs an external crate, hence tested here and not in the compiletest suite) + // Try seeding with "real" entropy. let mut rng = SmallRng::from_entropy(); let _val = rng.gen::(); let _val = rng.gen::(); @@ -22,10 +37,7 @@ fn entropy_rng() { let _val = rng.gen::(); } -// A test that won't work on miri -#[cfg(not(miri))] #[test] -fn does_not_work_on_miri() { - let x = 0u8; - assert!(&x as *const _ as usize % 4 < 4); +fn num_cpus() { + assert_eq!(num_cpus::get(), 1); } From c7bf9064f7028a9e43feaf699e23f1443704e448 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Jun 2019 13:37:38 +0200 Subject: [PATCH 02/15] comment on STACK_ADDR --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 567fdae53b4..66b6afde331 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -42,7 +42,7 @@ pub use crate::eval::{eval_main, create_ecx, MiriConfig}; // Some global facts about the emulated machine. pub const PAGE_SIZE: u64 = 4*1024; -pub const STACK_ADDR: u64 = 16*PAGE_SIZE; +pub const STACK_ADDR: u64 = 16*PAGE_SIZE; // not really about the "stack", but where we start assigning integer addresses to allocations pub const NUM_CPUS: u64 = 1; /// Insert rustc arguments at the beginning of the argument list that Miri wants to be From 019ad4bab456bf107cba54dc8c396fbdd6d2bae0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Jun 2019 14:37:41 +0200 Subject: [PATCH 03/15] move constants to machine.rs --- src/lib.rs | 10 ++++------ src/machine.rs | 5 +++++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 66b6afde331..0e4a9c4ccc3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -37,14 +37,12 @@ pub use crate::range_map::RangeMap; pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt}; pub use crate::mono_hash_map::MonoHashMap; pub use crate::stacked_borrows::{EvalContextExt as StackedBorEvalContextExt, Tag, Permission, Stack, Stacks, Item}; -pub use crate::machine::{MemoryExtra, AllocExtra, MiriMemoryKind, Evaluator, MiriEvalContext, MiriEvalContextExt}; +pub use crate::machine::{ + PAGE_SIZE, STACK_ADDR, NUM_CPUS, + MemoryExtra, AllocExtra, MiriMemoryKind, Evaluator, MiriEvalContext, MiriEvalContextExt, +}; pub use crate::eval::{eval_main, create_ecx, MiriConfig}; -// Some global facts about the emulated machine. -pub const PAGE_SIZE: u64 = 4*1024; -pub const STACK_ADDR: u64 = 16*PAGE_SIZE; // not really about the "stack", but where we start assigning integer addresses to allocations -pub const NUM_CPUS: u64 = 1; - /// Insert rustc arguments at the beginning of the argument list that Miri wants to be /// set per default, for maximal validation power. pub fn miri_default_args() -> &'static [&'static str] { diff --git a/src/machine.rs b/src/machine.rs index eb177fa2a18..383b6aac631 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -12,6 +12,11 @@ use rustc::mir; use crate::*; +// Some global facts about the emulated machine. +pub const PAGE_SIZE: u64 = 4*1024; // FIXME: adjust to target architecture +pub const STACK_ADDR: u64 = 16*PAGE_SIZE; // not really about the "stack", but where we start assigning integer addresses to allocations +pub const NUM_CPUS: u64 = 1; + /// Extra memory kinds #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum MiriMemoryKind { From 11457a4ad942ec35952682f474556f6eae2d4db3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 10:59:42 +0200 Subject: [PATCH 04/15] fix comparing function pointers with intptrcast --- src/intptrcast.rs | 39 +++++++++++++------------ src/machine.rs | 2 -- tests/run-pass/intptrcast_format.rs | 6 ++++ tests/run-pass/intptrcast_format.stdout | 2 ++ 4 files changed, 29 insertions(+), 20 deletions(-) create mode 100644 tests/run-pass/intptrcast_format.rs create mode 100644 tests/run-pass/intptrcast_format.stdout diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 7fd48defda1..f8102642bdb 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -1,25 +1,26 @@ -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; +use std::collections::{HashMap, hash_map::Entry}; +use std::cmp::max; use rand::Rng; -use rustc::mir::interpret::{AllocId, Pointer, InterpResult}; -use rustc_mir::interpret::Memory; +use rustc_mir::interpret::{AllocId, Pointer, InterpResult, Memory, AllocCheck}; use rustc_target::abi::Size; use crate::{Evaluator, Tag, STACK_ADDR}; pub type MemoryExtra = RefCell; -#[derive(Clone, Debug, Default)] -pub struct AllocExtra { - base_addr: Cell> -} - #[derive(Clone, Debug)] pub struct GlobalState { /// This is used as a map between the address of each allocation and its `AllocId`. /// It is always sorted pub int_to_ptr_map: Vec<(u64, AllocId)>, + /// The base address for each allocation. We cannot put that into + /// `AllocExtra` because function pointers also have a base address, and + /// they do not have an `AllocExtra`. + /// This is the inverse of `int_to_ptr_map`. + pub base_addr: HashMap, /// This is used as a memory address when a new pointer is casted to an integer. It /// is always larger than any address that was previously made part of a block. pub next_base_addr: u64, @@ -29,6 +30,7 @@ impl Default for GlobalState { fn default() -> Self { GlobalState { int_to_ptr_map: Vec::default(), + base_addr: HashMap::default(), next_base_addr: STACK_ADDR, } } @@ -71,13 +73,13 @@ impl<'mir, 'tcx> GlobalState { memory: &Memory<'mir, 'tcx, Evaluator<'tcx>>, ) -> InterpResult<'tcx, u64> { let mut global_state = memory.extra.intptrcast.borrow_mut(); + let global_state = &mut *global_state; - let alloc = memory.get(ptr.alloc_id)?; - let align = alloc.align.bytes(); + let (size, align) = memory.get_size_and_align(ptr.alloc_id, AllocCheck::Live)?; - let base_addr = match alloc.extra.intptrcast.base_addr.get() { - Some(base_addr) => base_addr, - None => { + let base_addr = match global_state.base_addr.entry(ptr.alloc_id) { + Entry::Occupied(entry) => *entry.get(), + Entry::Vacant(entry) => { // This allocation does not have a base address yet, pick one. // Leave some space to the previous allocation, to give it some chance to be less aligned. let slack = { @@ -86,11 +88,12 @@ impl<'mir, 'tcx> GlobalState { rng.gen_range(0, 16) }; // From next_base_addr + slack, round up to adjust for alignment. - let base_addr = Self::align_addr(global_state.next_base_addr + slack, align); - alloc.extra.intptrcast.base_addr.set(Some(base_addr)); + let base_addr = Self::align_addr(global_state.next_base_addr + slack, align.bytes()); + entry.insert(base_addr); - // Remember next base address. - global_state.next_base_addr = base_addr + alloc.bytes.len() as u64; + // 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. + global_state.next_base_addr = base_addr + max(size.bytes(), 1); // Given that `next_base_addr` increases in each allocation, pushing the // corresponding tuple keeps `int_to_ptr_map` sorted global_state.int_to_ptr_map.push((base_addr, ptr.alloc_id)); @@ -99,7 +102,7 @@ impl<'mir, 'tcx> GlobalState { } }; - debug_assert_eq!(base_addr % align, 0); // sanity check + debug_assert_eq!(base_addr % align.bytes(), 0); // sanity check Ok(base_addr + ptr.offset.bytes()) } diff --git a/src/machine.rs b/src/machine.rs index 485286ea400..20fe2e055fb 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -42,7 +42,6 @@ impl Into> for MiriMemoryKind { #[derive(Debug, Clone)] pub struct AllocExtra { pub stacked_borrows: stacked_borrows::AllocExtra, - pub intptrcast: intptrcast::AllocExtra, } /// Extra global memory data @@ -277,7 +276,6 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { mutability: alloc.mutability, extra: AllocExtra { stacked_borrows: stacks, - intptrcast: Default::default(), }, }; (Cow::Owned(alloc), base_tag) diff --git a/tests/run-pass/intptrcast_format.rs b/tests/run-pass/intptrcast_format.rs new file mode 100644 index 00000000000..c0d3e9398dc --- /dev/null +++ b/tests/run-pass/intptrcast_format.rs @@ -0,0 +1,6 @@ +// compile-flags: -Zmiri-seed= + +fn main() { + println!("Hello {}", 13); + println!("{:0 Date: Sun, 30 Jun 2019 15:35:28 +0200 Subject: [PATCH 05/15] move shims (foreign items and intrinsics) into submodule --- src/lib.rs | 7 +++---- src/{fn_call.rs => shims/foreign_items.rs} | 0 src/{intrinsic.rs => shims/intrinsics.rs} | 0 src/shims/mod.rs | 2 ++ 4 files changed, 5 insertions(+), 4 deletions(-) rename src/{fn_call.rs => shims/foreign_items.rs} (100%) rename src/{intrinsic.rs => shims/intrinsics.rs} (100%) create mode 100644 src/shims/mod.rs diff --git a/src/lib.rs b/src/lib.rs index 0e4a9c4ccc3..6b2de4ac08b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,9 +12,8 @@ extern crate rustc_data_structures; extern crate rustc_mir; extern crate rustc_target; -mod fn_call; +mod shims; mod operator; -mod intrinsic; mod helpers; mod tls; mod range_map; @@ -29,9 +28,9 @@ pub use rustc_mir::interpret::*; // Resolve ambiguity. pub use rustc_mir::interpret::{self, AllocMap, PlaceTy}; -pub use crate::fn_call::EvalContextExt as MissingFnsEvalContextExt; +pub use crate::shims::foreign_items::EvalContextExt as ForeignItemsEvalContextExt; +pub use crate::shims::intrinsics::EvalContextExt as IntrinsicsEvalContextExt; pub use crate::operator::EvalContextExt as OperatorEvalContextExt; -pub use crate::intrinsic::EvalContextExt as IntrinsicEvalContextExt; pub use crate::tls::{EvalContextExt as TlsEvalContextExt, TlsData}; pub use crate::range_map::RangeMap; pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt}; diff --git a/src/fn_call.rs b/src/shims/foreign_items.rs similarity index 100% rename from src/fn_call.rs rename to src/shims/foreign_items.rs diff --git a/src/intrinsic.rs b/src/shims/intrinsics.rs similarity index 100% rename from src/intrinsic.rs rename to src/shims/intrinsics.rs diff --git a/src/shims/mod.rs b/src/shims/mod.rs new file mode 100644 index 00000000000..cadfc05681d --- /dev/null +++ b/src/shims/mod.rs @@ -0,0 +1,2 @@ +pub mod foreign_items; +pub mod intrinsics; From 5d4aae8c05c6389c41b0cf5585c9dd1a645bfbd3 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 30 Jun 2019 15:31:14 +0100 Subject: [PATCH 06/15] Fix `unused_must_use` inside `Box` After https://github.com/rust-lang/rust/pull/62228, this will be linted against (and causes the test to fail). --- tests/run-pass/issue-30530.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/run-pass/issue-30530.rs b/tests/run-pass/issue-30530.rs index 10dec30c64c..86c2d9184e0 100644 --- a/tests/run-pass/issue-30530.rs +++ b/tests/run-pass/issue-30530.rs @@ -21,7 +21,9 @@ pub enum Handler { } fn main() { - take(Handler::Default, Box::new(main)); + #[allow(unused_must_use)] { + take(Handler::Default, Box::new(main)); + } } #[inline(never)] From db6283b88447577f11e13b6eca99dfb7d0215b1b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 16:44:25 +0200 Subject: [PATCH 07/15] better name for a test: threads -> sync --- tests/run-pass/{threads.rs => sync.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/run-pass/{threads.rs => sync.rs} (100%) diff --git a/tests/run-pass/threads.rs b/tests/run-pass/sync.rs similarity index 100% rename from tests/run-pass/threads.rs rename to tests/run-pass/sync.rs From e44d38e0511376b73e1a17588780b21c359789f6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 16:45:41 +0200 Subject: [PATCH 08/15] improve comment --- tests/run-pass/sync.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/run-pass/sync.rs b/tests/run-pass/sync.rs index dad47d85a24..54d79566eae 100644 --- a/tests/run-pass/sync.rs +++ b/tests/run-pass/sync.rs @@ -8,8 +8,7 @@ fn main() { drop(m.lock()); drop(m); - // We don't provide RwLock on Windows - #[cfg(not(target_os = "windows"))] + #[cfg(not(target_os = "windows"))] // TODO: implement RwLock on Windows { let rw = sync::RwLock::new(0); drop(rw.read()); From b06731355205f0a4ffa1464137b1d55992f08b48 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 16:56:16 +0200 Subject: [PATCH 09/15] 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 b201f24e256..457734cb602 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 10/15] 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 457734cb602..3eb0f70fd66 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 11/15] 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 3eb0f70fd66..57db5407b7a 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 12/15] 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 f7d3f990ac9..4e8b8cd4b89 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 13/15] 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 f8102642bdb..54807000050 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 14/15] 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 9c9e77abfed..2e2a9062fba 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 57db5407b7a..1ea04ec467f 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 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 p1 = allocator.alloc_zeroed(layout).unwrap(); + assert_eq!(p1.as_ptr() as usize % align, 0, "pointer is incorrectly aligned"); - // 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]); + 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 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 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]); - allocator.dealloc(p4, Layout::from_size_align(10, 4).unwrap()); + // 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); + } } } 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 15/15] 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 91dcfd7cf83..be104ed0b2b 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 2e2a9062fba..1a39df9cce1 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 1ea04ec467f..2f3a48f535d 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()) + } + } } } }