Merge branch 'master' into rustup

Author: RalfJung

.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

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):
 

src/intptrcast.rs

@@ -1,37 +1,37 @@
-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::stacked_borrows::Tag;
-use crate::Evaluator;
+use crate::{Evaluator, Tag, STACK_ADDR};
 
 pub type MemoryExtra = RefCell<GlobalState>;
 
-#[derive(Clone, Debug, Default)]
-pub struct AllocExtra {
-    base_addr: Cell<Option<u64>>
-}
-
 #[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<AllocId, u64>,
     /// 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,
 }
 
 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)
+            base_addr: HashMap::default(),
+            next_base_addr: STACK_ADDR,
         }
     }
 }
@@ -73,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 = {
@@ -88,11 +88,16 @@ 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));
-
-                // Remember next base address.
-                global_state.next_base_addr = base_addr + alloc.bytes.len() as u64;
+                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.
+                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)); 
@@ -101,7 +106,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())
     }
 

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,15 +28,18 @@ 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};
 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};
 
 /// Insert rustc arguments at the beginning of the argument list that Miri wants to be

src/machine.rs

@@ -16,6 +16,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 {
@@ -40,7 +45,6 @@ impl Into<MemoryKind<MiriMemoryKind>> for MiriMemoryKind {
 #[derive(Debug, Clone)]
 pub struct AllocExtra {
     pub stacked_borrows: stacked_borrows::AllocExtra,
-    pub intptrcast: intptrcast::AllocExtra,
 }
 
 /// Extra global memory data
@@ -275,7 +279,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)

src/fn_call.rs renamed to 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<Tag>> {
         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()))
@@ -191,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 <https://github.com/rust-lang/rust/issues/62251>.
                 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 {
@@ -622,11 +638,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 +664,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 +743,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 +859,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,
                     )?;
             }

src/intrinsic.rs renamed to src/shims/intrinsics.rs

@@ -0,0 +1,2 @@
+pub mod foreign_items;
+pub mod intrinsics;

src/shims/mod.rs

@@ -9,3 +9,4 @@ byteorder = "1.0"
 
 [dev-dependencies]
 rand = { version = "0.7", features = ["small_rng"] }
+num_cpus = "1.10.1"