diff --git a/src/shims/intrinsics.rs b/src/shims/intrinsics.rs index 5c3ff139c026..920dc564e154 100644 --- a/src/shims/intrinsics.rs +++ b/src/shims/intrinsics.rs @@ -1,7 +1,7 @@ use rustc_apfloat::Float; use rustc::mir; use rustc::mir::interpret::{InterpResult, PointerArithmetic}; -use rustc::ty::layout::{self, LayoutOf, Size}; +use rustc::ty::layout::{self, LayoutOf, Size, Align}; use rustc::ty; use crate::{ @@ -48,30 +48,44 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } + "volatile_load" => { + let place = this.deref_operand(args[0])?; + this.copy_op(place.into(), dest)?; + } + + "volatile_store" => { + let place = this.deref_operand(args[0])?; + this.copy_op(args[1], place.into())?; + } + "atomic_load" | "atomic_load_relaxed" | "atomic_load_acq" => { - let ptr = this.deref_operand(args[0])?; - let val = this.read_scalar(ptr.into())?; // make sure it fits into a scalar; otherwise it cannot be atomic - this.write_scalar(val, dest)?; - } + let place = this.deref_operand(args[0])?; + let val = this.read_scalar(place.into())?; // make sure it fits into a scalar; otherwise it cannot be atomic - "volatile_load" => { - let ptr = this.deref_operand(args[0])?; - this.copy_op(ptr.into(), dest)?; + // Check alignment requirements. Atomics must always be aligned to their size, + // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must + // be 8-aligned). + let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); + this.memory().check_ptr_access(place.ptr, place.layout.size, align)?; + + this.write_scalar(val, dest)?; } "atomic_store" | "atomic_store_relaxed" | "atomic_store_rel" => { - let ptr = this.deref_operand(args[0])?; + let place = this.deref_operand(args[0])?; let val = this.read_scalar(args[1])?; // make sure it fits into a scalar; otherwise it cannot be atomic - this.write_scalar(val, ptr.into())?; - } - "volatile_store" => { - let ptr = this.deref_operand(args[0])?; - this.copy_op(args[1], ptr.into())?; + // Check alignment requirements. Atomics must always be aligned to their size, + // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must + // be 8-aligned). + let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); + this.memory().check_ptr_access(place.ptr, place.layout.size, align)?; + + this.write_scalar(val, place.into())?; } "atomic_fence_acq" => { @@ -79,25 +93,39 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } _ if intrinsic_name.starts_with("atomic_xchg") => { - let ptr = this.deref_operand(args[0])?; + let place = this.deref_operand(args[0])?; let new = this.read_scalar(args[1])?; - let old = this.read_scalar(ptr.into())?; + let old = this.read_scalar(place.into())?; + + // Check alignment requirements. Atomics must always be aligned to their size, + // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must + // be 8-aligned). + let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); + this.memory().check_ptr_access(place.ptr, place.layout.size, align)?; + this.write_scalar(old, dest)?; // old value is returned - this.write_scalar(new, ptr.into())?; + this.write_scalar(new, place.into())?; } _ if intrinsic_name.starts_with("atomic_cxchg") => { - let ptr = this.deref_operand(args[0])?; + let place = this.deref_operand(args[0])?; let expect_old = this.read_immediate(args[1])?; // read as immediate for the sake of `binary_op()` let new = this.read_scalar(args[2])?; - let old = this.read_immediate(ptr.into())?; // read as immediate for the sake of `binary_op()` + let old = this.read_immediate(place.into())?; // read as immediate for the sake of `binary_op()` + + // Check alignment requirements. Atomics must always be aligned to their size, + // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must + // be 8-aligned). + let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); + this.memory().check_ptr_access(place.ptr, place.layout.size, align)?; + // binary_op will bail if either of them is not a scalar let (eq, _) = this.binary_op(mir::BinOp::Eq, old, expect_old)?; let res = Immediate::ScalarPair(old.to_scalar_or_undef(), eq.into()); this.write_immediate(res, dest)?; // old value is returned // update ptr depending on comparison if eq.to_bool()? { - this.write_scalar(new, ptr.into())?; + this.write_scalar(new, place.into())?; } } @@ -131,12 +159,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx "atomic_xsub_rel" | "atomic_xsub_acqrel" | "atomic_xsub_relaxed" => { - let ptr = this.deref_operand(args[0])?; - if !ptr.layout.ty.is_integral() { + let place = this.deref_operand(args[0])?; + if !place.layout.ty.is_integral() { bug!("Atomic arithmetic operations only work on integer types"); } let rhs = this.read_immediate(args[1])?; - let old = this.read_immediate(ptr.into())?; + let old = this.read_immediate(place.into())?; + + // Check alignment requirements. Atomics must always be aligned to their size, + // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must + // be 8-aligned). + let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); + this.memory().check_ptr_access(place.ptr, place.layout.size, align)?; + this.write_immediate(*old, dest)?; // old value is returned let (op, neg) = match intrinsic_name.split('_').nth(1).unwrap() { "or" => (mir::BinOp::BitOr, false), @@ -154,7 +189,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } else { val }; - this.write_scalar(val, ptr.into())?; + this.write_scalar(val, place.into())?; } "breakpoint" => unimplemented!(), // halt miri @@ -335,8 +370,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } "move_val_init" => { - let ptr = this.deref_operand(args[0])?; - this.copy_op(args[1], ptr.into())?; + let place = this.deref_operand(args[0])?; + this.copy_op(args[1], place.into())?; } "offset" => { diff --git a/tests/compile-fail/atomic_unaligned.rs b/tests/compile-fail/atomic_unaligned.rs new file mode 100644 index 000000000000..5d2347c03ffe --- /dev/null +++ b/tests/compile-fail/atomic_unaligned.rs @@ -0,0 +1,12 @@ +#![feature(core_intrinsics)] + +fn main() { + // Do a 4-aligned u64 atomic access. That should be UB on all platforms, + // even if u64 only has alignment 4. + let z = [0u32; 2]; + let zptr = &z as *const _ as *const u64; + unsafe { + ::std::intrinsics::atomic_load(zptr); + //~^ ERROR tried to access memory with alignment 4, but alignment 8 is required + } +} diff --git a/tests/run-pass/atomic-access-bool.rs b/tests/run-pass/atomic-access-bool.rs deleted file mode 100644 index 68a5d7295f17..000000000000 --- a/tests/run-pass/atomic-access-bool.rs +++ /dev/null @@ -1,19 +0,0 @@ -use std::sync::atomic::{AtomicBool, Ordering::*}; - -static mut ATOMIC: AtomicBool = AtomicBool::new(false); - -fn main() { - unsafe { - assert_eq!(*ATOMIC.get_mut(), false); - ATOMIC.store(true, SeqCst); - assert_eq!(*ATOMIC.get_mut(), true); - ATOMIC.fetch_or(false, SeqCst); - assert_eq!(*ATOMIC.get_mut(), true); - ATOMIC.fetch_and(false, SeqCst); - assert_eq!(*ATOMIC.get_mut(), false); - ATOMIC.fetch_nand(true, SeqCst); - assert_eq!(*ATOMIC.get_mut(), true); - ATOMIC.fetch_xor(true, SeqCst); - assert_eq!(*ATOMIC.get_mut(), false); - } -} diff --git a/tests/run-pass/atomic-compare_exchange.rs b/tests/run-pass/atomic.rs similarity index 55% rename from tests/run-pass/atomic-compare_exchange.rs rename to tests/run-pass/atomic.rs index 575b53fb44b0..f0b8ec06b905 100644 --- a/tests/run-pass/atomic-compare_exchange.rs +++ b/tests/run-pass/atomic.rs @@ -1,8 +1,32 @@ -use std::sync::atomic::{AtomicIsize, Ordering::*}; - -static ATOMIC: AtomicIsize = AtomicIsize::new(0); +use std::sync::atomic::{AtomicBool, AtomicIsize, AtomicU64, Ordering::*}; fn main() { + atomic_bool(); + atomic_isize(); + atomic_u64(); +} + +fn atomic_bool() { + static mut ATOMIC: AtomicBool = AtomicBool::new(false); + + unsafe { + assert_eq!(*ATOMIC.get_mut(), false); + ATOMIC.store(true, SeqCst); + assert_eq!(*ATOMIC.get_mut(), true); + ATOMIC.fetch_or(false, SeqCst); + assert_eq!(*ATOMIC.get_mut(), true); + ATOMIC.fetch_and(false, SeqCst); + assert_eq!(*ATOMIC.get_mut(), false); + ATOMIC.fetch_nand(true, SeqCst); + assert_eq!(*ATOMIC.get_mut(), true); + ATOMIC.fetch_xor(true, SeqCst); + assert_eq!(*ATOMIC.get_mut(), false); + } +} + +fn atomic_isize() { + static ATOMIC: AtomicIsize = AtomicIsize::new(0); + // Make sure trans can emit all the intrinsics correctly assert_eq!(ATOMIC.compare_exchange(0, 1, Relaxed, Relaxed), Ok(0)); assert_eq!(ATOMIC.compare_exchange(0, 2, Acquire, Relaxed), Err(1)); @@ -27,3 +51,12 @@ fn main() { ATOMIC.compare_exchange_weak(0, 1, SeqCst, Acquire).ok(); ATOMIC.compare_exchange_weak(0, 1, SeqCst, SeqCst).ok(); } + +fn atomic_u64() { + static ATOMIC: AtomicU64 = AtomicU64::new(0); + + ATOMIC.store(1, SeqCst); + assert_eq!(ATOMIC.compare_exchange(0, 0x100, AcqRel, Acquire), Err(1)); + assert_eq!(ATOMIC.compare_exchange_weak(1, 0x100, AcqRel, Acquire), Ok(1)); + assert_eq!(ATOMIC.load(Relaxed), 0x100); +}